-
Notifications
You must be signed in to change notification settings - Fork 6k
Reland: Only provide frame damage to rasterizer if partial repaint is enabled #30704
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. 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. |
| framebuffer_info.existing_damage = i->second; | ||
| } | ||
|
|
||
| framebuffer_info.supports_partial_repaint = true; |
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.
Do we need to ensure that this is an iOS device and not macOS for example? I realize that we typically go through the compositor api for macOS, but AFAIK there is a boolean flag we can flip to enable root surface rendering on macOS as well. I was wondering if there was a way to delegate this to the iOS specific impl.
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.
Right now this is only set in GPUSurfaceMetal::AcquireFrameFromCAMetalLayer, which is iOS specific. On MacOS we use AcquireFrameFromMTLTexture.
iskakaushik
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
aa91d44 to
81af568
Compare
Fixes flutter/flutter#95681
The original PR had an issue where
FramebufferInfo::existing_damage != std::nulloptwas used to determine whether target supports partial repaint. This was wrong, because emptyexisting_damageis a valid state even if partial repaint is supported. It simply means that the target doesn't know anything about the state of the framebuffer and thus entire frame needs to be repainted. When partial repaint is supported, even with emptyexisting_damage, we still need to perform tree diffing, so that we have valid paint regions for layers in current frame (that will be used when diffing next frame).Instead this PR introduces a specific flag to determine if target supports partial repaint (
supports_partial_repaint). Setting this flag tofalse(default) will avoid executing any tree diffing code whatsoever.If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.