Repository navigation
[Impeller] Reserve content depth in the shared backdrop fast path - #193176
Conversation
When several backdrop filters share a backdrop id and all of their filters
are equal, `Canvas::SaveLayer` reuses one shared snapshot: it renders the
backdrop entity using the layer's own depth and then calls `Save(0)`.
That left the layer's contents with no depth budget, since the new stack
entry's `clip_depth` was set to the current depth. The first draw inside the
layer then landed one past `clip_depth` and tripped the depth check in
`AddRenderEntityToCurrentPass` on debug engines:
Check failed: current_depth_ <= transform_stack_.back().clip_depth.
Reserve `total_content_depth` for the contents instead, matching the regular
saveLayer path. The backdrop entity keeps using the layer's own depth slot,
and the matching `Restore()` pops a plain save without drawing, so no depth
is counted twice.
Fixes flutter#193124
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Canvas::SaveLayer method in Impeller to pass total_content_depth instead of 0 to the Save call when rendering a backdrop entity. It also adds a corresponding unit test, BackdropGroupSharedSnapshotReservesContentDepth, to verify that the backdrop group shared snapshot correctly reserves content depth. There are no review comments, and I have no feedback to provide.
github-actions
Bot
added
engine
gaaclarke
left a comment
gaaclarke
left a comment
There was a problem hiding this comment.
LGTM, verified the test fails without the fix locally. Thanks!
|
the ci.yaml validation check failure likely just means you need a sync |
gaaclarke
added
CICD
auto-submit
Bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/193176, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label. |
gaaclarke
added
the
autosubmit
auto-submit
Bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/193176, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label. |
|
The The other customer tests in the same run passed, and this one never got as far as running any test code. Could someone kick off a rerun of that task once the tree is green? |
Already kicked, yea it's not something you should have to worry about. It's some sort of flake unfortunately. |
|
All the checks have passed on 5f57206. @gaaclarke could you re-apply autosubmit? Thanks for the reviews. |
andywolff
added
the
autosubmit
flutter-dashboard
Bot
removed
the
autosubmit
The grouped backdrop fast path in
Canvas::SaveLayerleaves the layer's contents with no depth budget, which trips the depth check inAddRenderEntityToCurrentPasson debug engines.When several backdrop filters share a backdrop id and all of their filters are equal, the layer is processed once and the shared snapshot is reused. That path renders the backdrop entity with the layer's own depth (
++current_depth_) and then callsSave(0), so the new stack entry'sclip_depthends up equal to the current depth. The first draw inside the layer then lands one past it:This reserves
total_content_depthfor the contents instead, which is what the regular saveLayer path does. The backdrop entity keeps using the layer's own depth slot, and the matchingRestore()pops a plain save without drawing anything, so no depth is counted twice.Fixes #193124
Testing
Added
BackdropGroupSharedSnapshotReservesContentDepthtoimpeller/display_list/canvas_unittests.cc. It is modelled on the neighbouringBackdropGroupIdUsesCoverageUnionForSnapshot, with the difference that each of the two grouped layers draws something inside, which is what exposes the missing depth budget.Verified against an unoptimized host engine build (
host_debug_unopt) on Windows:2 <=? 1) on both OpenGLES and Vulkan.*Backdrop*and*ClipDepth*pass with no failures. The remaining instances are skipped on this machine: Metal is unavailable on Windows, and the framebuffer-fetch tests only run on Vulkan, which I ran with SwiftShader and validation layers enabled.Pre-launch Checklist
///).