Fix invalidation during removing the layer#3146
Fix invalidation during removing the layer#3146Ivan Matkov (MatkovIvan) wants to merge 3 commits into
Conversation
| invokeInvalidationCallbacks() | ||
| // Removing a layer changes the scene's composited output (e.g. it removes a dialog scrim) | ||
| // without dirtying any remaining owner, so force a draw pass to repaint without it. | ||
| invokeInvalidationCallbacks(forceDraw = true) |
There was a problem hiding this comment.
This uses knowledge of the Dialog implementation details.
There are also questions arise - why not layout, why not on attach too.
Also, we violate the contract of hasPendingDraw — it should be true when there is a draw pending.
I believe that instead of forcing the redraw, we should add the hiding scrim into the invalidation. Where is it located?
There was a problem hiding this comment.
This uses knowledge of the Dialog implementation details.
It doesn't. The dialog scrim is just one example — this affects graphics of any kind graphics by any layer. CanvasLayersComposeScene composites every layer onto its own single canvas, so removing any layer changes what the scene paints, regardless of what that layer happened to draw.
why not layout
Because there's nothing to recompute on detach — neither in Compose terms (no node needs re-measure/relayout) nor in platform-view terms. Only the composited output changes, so only a draw pass is needed.
why not on attach too
Because a layer is added to the layers list already in an invalidated state, so the regular invokeInvalidationCallbacks() already covers attach. On detach the layer is gone from the list and no remaining owner is dirty.
Also, we violate the contract of hasPendingDraw — it should be true when there is a draw pending.
Agreed.
I believe that instead of forcing the redraw, we should add the hiding scrim into the invalidation. Where is it located?
The scrim lives inside the dismissed layer and invalidating there doesn't work - it's already removed from the list and also we shouldn't rely on use-after-close pattern. The invalidation has to happen in the scene implementation: CanvasLayersComposeScene mixes multiple levels onto one surface, so it's the thing responsible for reporting its layers' invalidations and its combination as its own.
There was a problem hiding this comment.
Also, we violate the contract of hasPendingDraw - it should be true when there is a draw pending.
Fixed
There was a problem hiding this comment.
This uses knowledge of the Dialog implementation details.
Rewrote the comment in a more generic way to avoid such confusion
2 participants
CMP-10360 Dialog hide animation hangs on the last frame
Regression after #3096, not released yet
Release Notes
N/A