-
Notifications
You must be signed in to change notification settings - Fork 6k
simplify the logic around computing subtree opacity inheritance #32698
simplify the logic around computing subtree opacity inheritance #32698
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
| // method will then reset the field to false if it detects any | ||
| // incompatible children. | ||
| // | ||
| // 2. If you need to do change any logic depending on the answer |
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.
nit: "neet to do change" is confusingly phrased.
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.
D'oh! Fixed in my local repo pending writing some tests and going over the logic layer by layer.
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
|
Eeek, I was busy adding tests and fixing things. I should have marked this a WIP. |
Fixes flutter/flutter#101957
This PR fixes a flaw in how the opacity inheritance flags are computed between layers of the tree. It is still somewhat of a work in progress as I want to revisit all of the layers and check their logic, but at least for the combination of a FadeTransition with a ScaleTransition child, the layer cache no longer re-populates the cache on every frame and the rasterizer times are way down with a complex child.
This PR also adds the inheritance logic to ImageFilterLayer which used to not participate in opacity inheritance, so that is an additional win with this fix, but each part of this PR (both the part that fixes the logic and the new inheritance in ImageFilterLayer) are important to the example above. Fixing either still ends up with problems.
Note that there is one more problem I've discovered before this is a total solution for the Fade/Scale pair and that is that we fumble the coordinates a bit for an animated scale with the filterQuality. The reasons for that are in our cache rendering logic and I'm still investigating those.