Incorrect damage rect when a layer switches from owning a render surface to not owning one. |
|||
Issue descriptionWhile computing damage rect for a render surface(A), we check if a layer contributes as itself or as a surface and do the damage rect computation accordingly. So, if a layer contributes as a surface (B), any damage to contributing layers of B will also damage A. But, this leaves out one case. Suppose a layer contributed as a surface in the last frame, but contributes as itself in the current frame and if the layer itself has no damage from the last frame, then we miss out on tracking the damage caused to the old contributing surface (B). One possible fix for this is to make the damage tracker track if a layer contributes as a layer or surface and mark the entire layer as damaged when it switches its contribution from a surface to layer.
,
May 11 2016
The example I was describing is :
Frame 1 :
[A]
/
[B]>
\
C
Frame 2 :
[A]
/
B
In frame 1, A's layer list will be {A, B}, where B is contributing as a surface.
In frame 2, A's layer list will still be {A, B} but B no longer creates a render surface.
The deletion of C in frame 2 should contribute to damage of A but it won't because A only checks if B has a damage.
https://codereview.chromium.org/1973503004 repros the bug
,
May 11 2016
Ah because at the time of deleting C, there was a surface on B? I went to read the code and I expected to find that B::RemoveChild(C) would mark B as damaged but B::RemoveChild does not exist. Can you explain to me how this would work if we weren't changing render surfaces? Only A has a surface for both frames, but we delete C on frame 2.
,
May 11 2016
In that case A's layer list for frame 1 will be {A, B, C} and in frame 2 it will be {A, B} and DamageTracker::TrackDamageFromLeftoverRects will notice that C is missing.
,
May 11 2016
Ah thanks. > One possible fix for this is to make the damage tracker track if a layer > contributes as a layer or surface and mark the entire layer as damaged when it > switches its contribution from a surface to layer. To clarify, you'd have to mark the entire render surface A as dirty. You don't know that C was contained within B or any idea about it's rect then. Or when B drops its render surface it makes its dirty rect be the dirty rect from the damage tracker or something? Or we move ownership of DamageTracker to the layer instead of the RenderSurfaceImpl?
,
May 13 2016
,
May 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cf22376f77a6c31d4f144f00826b687864031d46 commit cf22376f77a6c31d4f144f00826b687864031d46 Author: jaydasika <jaydasika@chromium.org> Date: Mon May 16 23:02:09 2016 cc : Fix damage rect bug caused by deletion of render surface The damage tracker tracks if a layer contributed as a layer or surface and when a surface is removed, it is added to the damage rect. BUG= 611230 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Review-Url: https://codereview.chromium.org/1976413002 Cr-Commit-Position: refs/heads/master@{#393966} [modify] https://crrev.com/cf22376f77a6c31d4f144f00826b687864031d46/cc/layers/texture_layer_unittest.cc [modify] https://crrev.com/cf22376f77a6c31d4f144f00826b687864031d46/cc/trees/damage_tracker.cc [modify] https://crrev.com/cf22376f77a6c31d4f144f00826b687864031d46/cc/trees/damage_tracker.h [modify] https://crrev.com/cf22376f77a6c31d4f144f00826b687864031d46/cc/trees/damage_tracker_unittest.cc
,
May 16 2016
|
|||
►
Sign in to add a comment |
|||
Comment 1 by danakj@chromium.org
, May 11 2016Let us draw a layer tree. C / B< / \ A< D \ E IIUC. You are saying in frame *1* we have render surfaces as follows: C / [B]< / \ [A]< D \ E And then in frame *2* as: C / B< / \ [A]< D \ E Or I might be misunderstanding, can you confirm? I'm not sure what you mean by "contributed as itself" in frame *2*. It could still have a render surface on B and it both contributes to it, and contributes that surface to A, or it could not have one any more and just contribute itself to A's.