-
Notifications
You must be signed in to change notification settings - Fork 29.8k
[a11y] Re-land Make sure RenderFractionalTranslation updates its semantics after the translation field is set #49708
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[a11y] Re-land Make sure RenderFractionalTranslation updates its semantics after the translation field is set #49708
Conversation
jonahwilliams
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
This change had a dramatic impact on the frame build times as can be seen in the |
|
This change fixed a critical a11y bug. Do you have any recommendations on how to preserve this behavior while remaining performant? |
|
It looks like this change basically just enables some aspect of the semantics code for the ProxyBox so the performance hit is likely due to the performance of the semantics code. I don't know enough about the semantics code to make an off-hand suggestion as to how to reduce its impact. Someone more familiar with semantics should look at why the performance hit was so dramatic. |
|
Unfortunately the bug this change fixed was fairly severe, so the benchmark was baselined under incorrect conditions. Similar to a rendering benchmark that didn't do any rendering, the semantics benchmark didn't do any semantics updating :( |
|
That said, it is generally known that our semantics updates are computationally intensive. |
Well that explains the change. The issue should probably be closed then and the benchmark re-baselined? |
Description
Re-land #48985
This PR ensures that RenderFractionalTranslation calls markNeedsSemanticsUpdate after its translation is set/changed, so that semantics are properly updated (for instance if a FractionalTranslation is animating).
Related Issues
closes #20449
Tests
I added the following tests:
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.