[Impeller] Don't cull readbacks outside the damage rect. #46705
Conversation
|
I think the idea of readback region that doesn't overlap filter drawn area hasn't occurred to me when implementing this. However the issue here is that readback regions are added unconditionally. That means with the fix readback area of any backdrop filter, even if the filter is culled out, will be repainted. For blurred header / footer this would pretty much invalidate partial repaint. I'm wondering if we could store redbacks as a map |
|
I mean something like knopp@23147d7 might work, need to test it though. |
Buffer damage is just frame_damage + accumulated buffer damage and there's no point intersecting against accumulated_buffer_damage.
gaaclarke
left a comment
gaaclarke
left a comment
There was a problem hiding this comment.
The code looks good. The test is very difficult to follow. This is a classic case of a test should assert one thing. Also the number literals took some time to understand. It would be easier to follow if they were broken down to how they are calculated.
It sounds like there are pathological cases where this is very inefficient as noted by knopp. I think it probably makes sense to prefer slower but correct rendering and we can file an issue about the pathological cases.
|
That test could have been split into multiple tests. @gaaclarke, I added PR for this PR that should fix the magnifier rendering without regressing in bdero#1. If this gets merged like this I'll follow-up with a separate PR. |
Awesome, I just gave it a look and added a few notes. I'll let you and bdero decide how to best land it. I really appreciate the work both of you put into this one. |
|
Thanks, @knopp! |
…136294) flutter/engine@bdeb534...4a4d9de 2023-10-10 [email protected] Roll Fuchsia Linux SDK from dCjN58uZQBmAFWSxN... to FX5YzwP_ZEnPP0b3v... (flutter/engine#46734) 2023-10-10 [email protected] [Impeller] Don't cull readbacks outside the damage rect. (flutter/engine#46705) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from dCjN58uZQBmA to FX5YzwP_ZEnP If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Resolves flutter/flutter#136058. The damage rect properly expands to include the backdrop filter readback area to a certain extent, but it suddenly gets excluded once it no longer partially overlaps with the damage rect. This caused the illusion of EntityPass improperly culling the backdrop entity, leading me into a bit of a wild goose chase. Before: https://github.com/flutter/engine/assets/919017/94b8c077-0945-4a2c-96e0-27230d980c38 After: https://github.com/flutter/engine/assets/919017/f1c78365-6e9b-46cb-9e69-33472d488831
3 participants
Resolves flutter/flutter#136058.
The damage rect properly expands to include the backdrop filter readback area to a certain extent, but it suddenly gets excluded once it no longer partially overlaps with the damage rect. This caused the illusion of EntityPass improperly culling the backdrop entity, leading me into a bit of a wild goose chase.
Before:
before.mov
After:
after.mov