[Blink] Border-radius clip omitted incorrectly for composited descendants. |
|||||
Issue descriptionChrome Version: ToT OS: All What steps will reproduce the problem? (1) Go https://output.jsbin.com/keqekuyeto/quiet What is the expected result? The lower right corner of the green box should be clipped. What happens instead? The lower right corner of the green box leaked. Discovered this bug when I worked a fix for https://crbug.com/754927 . This is due to ancestor clipping mask layer being omitted because current layer bound is strictly enclosed by all rounded clip that needs to be applied. However composited in-flow descendants still expect to inherit clip from the current layer and they can overflow. https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/paint/compositing/CompositedLayerMapping.cpp?rcl=07f24fc82c276cf195cadc119fef28be1185c151&l=590 FloatRect bounds_in_ancestor_space = GetLayoutObject() .LocalToAncestorQuad(FloatRect(composited_bounds_), ^^^^^^^^^^^^^^^^^^ Bug: Probably should use full visual overflow rect instead. &compositing_ancestor->GetLayoutObject(), kUseTransforms) .BoundingBox();
,
Aug 17 2017
No bounds that I can find associated with the layer or it's paint layer or it layout object seem to contain the child in this situation. Do we need to accumulate all the subtree composited bounds with offsets?
,
Aug 21 2017
Yes we would need to accumulate that. :( That is expensive though, and might also have bugs because you have to manage the dirty bits correctly when those children move around. How about just giving up on this optimization if the current CLM's owning PaintLayer has composited descendants? (PaintLayer::HasCompositingDescendant()). @schenney, assigning to you for next step. The proposed change if it works is quite simple.
,
Aug 21 2017
+1 it is good enough to just give up optimization when there is compositing descendants. Considering the descendants could be animating... My head started to ache already.
,
Aug 22 2017
,
Aug 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/82bf86466e224dec6cf609dc8128e64b4e96db5d commit 82bf86466e224dec6cf609dc8128e64b4e96db5d Author: Stephen Chenney <schenney@chromium.org> Date: Fri Aug 25 00:48:07 2017 Fix border-radius masks for composited descendants We were failing to use a border-radius mask layer for cases where a descendant of a composited layer needs clipping but the layer itself does not. This change forces the mask layer whenever there is a composited descendant. That will cause more masks than we strictly need, but we have no efficient way to detect that the mask is unnecessary. R=trchen@chromium.org BUG= 756265 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I3aaccf95b17d8c3266d12c291b27ed6cf2af4ab2 Reviewed-on: https://chromium-review.googlesource.com/627396 Commit-Queue: Tien-Ren Chen <trchen@chromium.org> Reviewed-by: Tien-Ren Chen <trchen@chromium.org> Cr-Commit-Position: refs/heads/master@{#497271} [modify] https://crrev.com/82bf86466e224dec6cf609dc8128e64b4e96db5d/third_party/WebKit/LayoutTests/TestExpectations [add] https://crrev.com/82bf86466e224dec6cf609dc8128e64b4e96db5d/third_party/WebKit/LayoutTests/compositing/composited-descendant-grandparent-border-radius-mask-expected.html [add] https://crrev.com/82bf86466e224dec6cf609dc8128e64b4e96db5d/third_party/WebKit/LayoutTests/compositing/composited-descendant-grandparent-border-radius-mask.html [add] https://crrev.com/82bf86466e224dec6cf609dc8128e64b4e96db5d/third_party/WebKit/LayoutTests/compositing/composited-descendant-requiring-border-radius-mask-expected.html [add] https://crrev.com/82bf86466e224dec6cf609dc8128e64b4e96db5d/third_party/WebKit/LayoutTests/compositing/composited-descendant-requiring-border-radius-mask.html [modify] https://crrev.com/82bf86466e224dec6cf609dc8128e64b4e96db5d/third_party/WebKit/LayoutTests/fast/borders/overflow-hidden-border-radius-force-backing-store-expected.txt [modify] https://crrev.com/82bf86466e224dec6cf609dc8128e64b4e96db5d/third_party/WebKit/Source/core/paint/compositing/CompositedLayerMapping.cpp [modify] https://crrev.com/82bf86466e224dec6cf609dc8128e64b4e96db5d/third_party/WebKit/Source/core/paint/compositing/CompositedLayerMappingTest.cpp
,
Aug 25 2017
,
Apr 11 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ea0de299bdeb794285d623942566cd586eac9700 commit ea0de299bdeb794285d623942566cd586eac9700 Author: Tien-Ren Chen <trchen@chromium.org> Date: Wed Apr 11 19:44:30 2018 [Blink] Remove lingering TODO( crbug.com/756265 ) which no longer applies BUG= 756265 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I92274be4c9ed75922d675c7c027c2fcce54709a2 Reviewed-on: https://chromium-review.googlesource.com/1006398 Reviewed-by: Stephen Chenney <schenney@chromium.org> Commit-Queue: Tien-Ren Chen <trchen@chromium.org> Cr-Commit-Position: refs/heads/master@{#549943} [modify] https://crrev.com/ea0de299bdeb794285d623942566cd586eac9700/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by schenney@chromium.org
, Aug 17 2017Owner: schenney@chromium.org
Status: Started (was: Untriaged)