[Blink] Non-composited clips are not applied correctly when an element escapes clips |
||||
Issue descriptionChrome Version: ToT OS: All What steps will reproduce the problem? (1) blink/tools/run_layout_tests.sh compositing/overflow/fixed-pos-escape-clip-and-apply-noncomposited-clip.html What is the expected result? Test should pass. What happens instead? Test failed. The aforementioned test is not submitted at time of reporting. See attachment for the test. The bug is due to that an element has to escape some clip, but then its true clip parent was not composited thus its composited layer clip parent points to the true clip parent's enclosing composited layer. In this case an ancestor clipping layer should be created. This bug was discovered during a code review (https://chromium-review.googlesource.com/c/609321/6/third_party/WebKit/Source/core/paint/compositing/CompositedLayerMapping.cpp#2462), and also from investigating another clip-escaping bug ( crbug.com/752182 ). Note: SPv2 is immune to this bug!
,
Aug 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a7482a88d73040a616ef53294eeef75ca8b6c5a2 commit a7482a88d73040a616ef53294eeef75ca8b6c5a2 Author: Tien-Ren Chen <trchen@chromium.org> Date: Tue Aug 15 23:58:04 2017 [Blink] Escape clip even if true clip container is not stacking context CompositingInputsUpdater.cpp:HasClippedStackingAncestor reported false negative when the true clipping container is not stacking context, because the layer->CompositingContainer() chain will never hit a non-stacking context, thus fell through and returned false. This CL made it return early as soon as an escaped clip was found. As a side change, also early return when compositing container chain already ran past the true clipping container, because no possible escaped clip can be found at this point. BUG= 752182 , 754927 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I3cb1aca53e38aa4e4cd9f6c7c0a1587a11eb7c75 Reviewed-on: https://chromium-review.googlesource.com/612584 Commit-Queue: Tien-Ren Chen <trchen@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#494615} [modify] https://crrev.com/a7482a88d73040a616ef53294eeef75ca8b6c5a2/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2 [modify] https://crrev.com/a7482a88d73040a616ef53294eeef75ca8b6c5a2/third_party/WebKit/LayoutTests/TestExpectations [add] https://crrev.com/a7482a88d73040a616ef53294eeef75ca8b6c5a2/third_party/WebKit/LayoutTests/compositing/overflow/fixed-pos-escape-clip-and-apply-noncomposited-clip-expected.html [add] https://crrev.com/a7482a88d73040a616ef53294eeef75ca8b6c5a2/third_party/WebKit/LayoutTests/compositing/overflow/fixed-pos-escape-clip-and-apply-noncomposited-clip.html [add] https://crrev.com/a7482a88d73040a616ef53294eeef75ca8b6c5a2/third_party/WebKit/LayoutTests/compositing/overflow/fixed-pos-escape-clip-having-non-stacking-context-clipping-container-expected.html [add] https://crrev.com/a7482a88d73040a616ef53294eeef75ca8b6c5a2/third_party/WebKit/LayoutTests/compositing/overflow/fixed-pos-escape-clip-having-non-stacking-context-clipping-container.html [modify] https://crrev.com/a7482a88d73040a616ef53294eeef75ca8b6c5a2/third_party/WebKit/Source/core/paint/compositing/CompositingInputsUpdater.cpp
,
Aug 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7f3d3dc4e5fdb09d8e53af63d97cb91d35fd4f32 commit 7f3d3dc4e5fdb09d8e53af63d97cb91d35fd4f32 Author: Tien-Ren Chen <trchen@chromium.org> Date: Wed Aug 16 23:13:20 2017 Revert "[Blink] Escape clip even if true clip container is not stacking context" This reverts commit a7482a88d73040a616ef53294eeef75ca8b6c5a2. Reason for revert: https://crbug.com/755850 Original change's description: > [Blink] Escape clip even if true clip container is not stacking context > > CompositingInputsUpdater.cpp:HasClippedStackingAncestor reported false > negative when the true clipping container is not stacking context, > because the layer->CompositingContainer() chain will never hit a > non-stacking context, thus fell through and returned false. > > This CL made it return early as soon as an escaped clip was found. As a > side change, also early return when compositing container chain already > ran past the true clipping container, because no possible escaped clip > can be found at this point. > > BUG= 752182 , 754927 > > Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 > Change-Id: I3cb1aca53e38aa4e4cd9f6c7c0a1587a11eb7c75 > Reviewed-on: https://chromium-review.googlesource.com/612584 > Commit-Queue: Tien-Ren Chen <trchen@chromium.org> > Reviewed-by: Chris Harrelson <chrishtr@chromium.org> > Cr-Commit-Position: refs/heads/master@{#494615} TBR=trchen@chromium.org,chrishtr@chromium.org Change-Id: Ibf42da5bdbbeec47ab75692f846e8c5e2a11e5d0 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 752182 , 754927 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Reviewed-on: https://chromium-review.googlesource.com/618102 Reviewed-by: Tien-Ren Chen <trchen@chromium.org> Commit-Queue: Tien-Ren Chen <trchen@chromium.org> Cr-Commit-Position: refs/heads/master@{#494992} [modify] https://crrev.com/7f3d3dc4e5fdb09d8e53af63d97cb91d35fd4f32/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2 [modify] https://crrev.com/7f3d3dc4e5fdb09d8e53af63d97cb91d35fd4f32/third_party/WebKit/LayoutTests/TestExpectations [delete] https://crrev.com/ed801e3b65bd6fb79017603ecd720c7c1413fef4/third_party/WebKit/LayoutTests/compositing/overflow/fixed-pos-escape-clip-and-apply-noncomposited-clip-expected.html [delete] https://crrev.com/ed801e3b65bd6fb79017603ecd720c7c1413fef4/third_party/WebKit/LayoutTests/compositing/overflow/fixed-pos-escape-clip-and-apply-noncomposited-clip.html [delete] https://crrev.com/ed801e3b65bd6fb79017603ecd720c7c1413fef4/third_party/WebKit/LayoutTests/compositing/overflow/fixed-pos-escape-clip-having-non-stacking-context-clipping-container-expected.html [delete] https://crrev.com/ed801e3b65bd6fb79017603ecd720c7c1413fef4/third_party/WebKit/LayoutTests/compositing/overflow/fixed-pos-escape-clip-having-non-stacking-context-clipping-container.html [modify] https://crrev.com/7f3d3dc4e5fdb09d8e53af63d97cb91d35fd4f32/third_party/WebKit/Source/core/paint/compositing/CompositingInputsUpdater.cpp
,
Aug 16 2017
Issue 755850 has been merged into this issue.
,
Aug 21 2017
trchen: do you know what the problem in your CL was that caused the revert?
,
Aug 21 2017
Yes, it is a series of bugs related to clip parent. Hunting them down was a traumatizing experience, but I think I fixed all of them. The CL is growing a bit big though. Let's go over it today.
,
Aug 24 2017
,
Aug 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/09dd9e06d4c19f4702597e8563d9b956e4dac9ae commit 09dd9e06d4c19f4702597e8563d9b956e4dac9ae Author: Tien-Ren Chen <trchen@chromium.org> Date: Sat Aug 26 00:43:03 2017 [Blink] Fixing a series of composited clip bug This CL fixes a number of issues around composited clip escaping. 1. Layers wouldn't to set clip_parent correctly if its clipping container is not a stacking context. For example: <div style="position:relative; overflow:hidden;"> <div style="will-change:opacity; overflow:hidden;"> <div style="position:absolute;"></div> </div> </div> The innermost element should be only affected by the outer clip, but not the clip in the middle. This was due to a bug in CompositingInputsUpdater.cpp:HasClippedStackingAncestor that the loop iterates over CompositingContainer would fall through if the clipping container is not in the chain. 2. Layers with clip parent do not have ancestor clipping layer computed correctly. Using the same example above, if the innermost element has clip_parent correctly set to the outer clip, it would still not have the correct clip applied. It is because the outer clip does not get composited, thus its enclosing composited layer (root layer in this case) will be used as clip_parent instead. The other clip therefore is a non-composited clip, and must be applied by ancestor clipping layer. Previously CompositedLayerMapping::OwningLayerClippedOrMaskedByLayerNotAboveCompositedAncestor() and CompositedLayerMapping::UpdateAncestorClippingLayerGeometry() computes only the clips between the compositing ancestor and the layer. The clips between the clip parent and the layer should be computed instead. 3. With both above issues fixed, layers might sometimes need to compute ancestor clip across transforms. This caused problem because PaintLayerClipper doesn't treat transforms as fixed-pos container, nor handle coordinate conversion in any way. For example: <div style="overflow:hidden;"> <div style="transform:rotate(30deg);"> <div style="will-change:opacity; overflow:hidden;"> <div style="position:absolute;"></div> </div> </div> </div> The fixed-pos element set its clip_parent to the outer clip. Again, the clip was not composited, thus the root layer is used as the clip parent and the clip will be applied by ancestor clipping layer. However PaintLayerClipper doesn't compute clip correctly in this case. It is impossible to, because the clip is not even in a space axis-aligned to the compositing container. This can be fixed by setting clip_parent to the nearest containing block parent (the transformed element in this case) instead. In this example, the transformed element will be composited for descendant reason, and the innermost element can inherit its clip using clip parent. 4. Out-of-flow layers escape clip unnecessarily. For example: <div style="will-change:opacity; overflow:hidden;"> <div style="position:absolute; z-index:0;"> <div style="position:absolute;"></div> </div> </div> The innermost element will have clip_parent set, but is not necessary because it inherits clip state from its parent stacking context. This is due to CompositingInputsUpdater.cpp:HasClippedStackingAncestor looping over all ancestor stacking context to see if any stacking context clip needs to be escaped. Instead, we only need to check if we need to escape clip applied to the direct parent stacking context, because by induction if our parent is in its target clip state and we don't have to escape anything in it, we would have inherited the correct state from the parent. BUG= 752182 , 754927 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: Ib36dfb7e45a494fd609df4dd3a239233ff763d8e Reviewed-on: https://chromium-review.googlesource.com/624615 Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Commit-Queue: Tien-Ren Chen <trchen@chromium.org> Cr-Commit-Position: refs/heads/master@{#497613} [modify] https://crrev.com/09dd9e06d4c19f4702597e8563d9b956e4dac9ae/cc/layers/layer.h [modify] https://crrev.com/09dd9e06d4c19f4702597e8563d9b956e4dac9ae/cc/trees/property_tree_builder.cc [modify] https://crrev.com/09dd9e06d4c19f4702597e8563d9b956e4dac9ae/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2 [add] https://crrev.com/09dd9e06d4c19f4702597e8563d9b956e4dac9ae/third_party/WebKit/LayoutTests/compositing/overflow/clip-parent-across-transform-boundary-expected.html [add] https://crrev.com/09dd9e06d4c19f4702597e8563d9b956e4dac9ae/third_party/WebKit/LayoutTests/compositing/overflow/clip-parent-across-transform-boundary.html [add] https://crrev.com/09dd9e06d4c19f4702597e8563d9b956e4dac9ae/third_party/WebKit/LayoutTests/compositing/overflow/escape-clip-and-paint-before-clip-parent-expected.html [add] https://crrev.com/09dd9e06d4c19f4702597e8563d9b956e4dac9ae/third_party/WebKit/LayoutTests/compositing/overflow/escape-clip-and-paint-before-clip-parent.html [add] https://crrev.com/09dd9e06d4c19f4702597e8563d9b956e4dac9ae/third_party/WebKit/LayoutTests/compositing/overflow/fixed-pos-escape-clip-and-apply-noncomposited-clip-expected.html [add] https://crrev.com/09dd9e06d4c19f4702597e8563d9b956e4dac9ae/third_party/WebKit/LayoutTests/compositing/overflow/fixed-pos-escape-clip-and-apply-noncomposited-clip.html [add] https://crrev.com/09dd9e06d4c19f4702597e8563d9b956e4dac9ae/third_party/WebKit/LayoutTests/compositing/overflow/fixed-pos-escape-clip-having-non-stacking-context-clipping-container-expected.html [add] https://crrev.com/09dd9e06d4c19f4702597e8563d9b956e4dac9ae/third_party/WebKit/LayoutTests/compositing/overflow/fixed-pos-escape-clip-having-non-stacking-context-clipping-container.html [add] https://crrev.com/09dd9e06d4c19f4702597e8563d9b956e4dac9ae/third_party/WebKit/LayoutTests/compositing/overflow/no-excessive-clip-parent-if-parent-escaped-expected.png [add] https://crrev.com/09dd9e06d4c19f4702597e8563d9b956e4dac9ae/third_party/WebKit/LayoutTests/compositing/overflow/no-excessive-clip-parent-if-parent-escaped-expected.txt [add] https://crrev.com/09dd9e06d4c19f4702597e8563d9b956e4dac9ae/third_party/WebKit/LayoutTests/compositing/overflow/no-excessive-clip-parent-if-parent-escaped.html [modify] https://crrev.com/09dd9e06d4c19f4702597e8563d9b956e4dac9ae/third_party/WebKit/LayoutTests/compositing/overflow/scroll-parent-absolute-with-backdrop-filter-expected.txt [modify] https://crrev.com/09dd9e06d4c19f4702597e8563d9b956e4dac9ae/third_party/WebKit/LayoutTests/virtual/prefer_compositing_to_lcd_text/compositing/overflow/scroll-parent-absolute-with-backdrop-filter-expected.txt [modify] https://crrev.com/09dd9e06d4c19f4702597e8563d9b956e4dac9ae/third_party/WebKit/Source/core/paint/compositing/CompositedLayerMapping.cpp [modify] https://crrev.com/09dd9e06d4c19f4702597e8563d9b956e4dac9ae/third_party/WebKit/Source/core/paint/compositing/CompositedLayerMapping.h [modify] https://crrev.com/09dd9e06d4c19f4702597e8563d9b956e4dac9ae/third_party/WebKit/Source/core/paint/compositing/CompositingInputsUpdater.cpp
,
Aug 26 2017
|
||||
►
Sign in to add a comment |
||||
Comment 1 by schenney@chromium.org
, Aug 14 2017Status: Available (was: Untriaged)