New issue
Advanced search Search tips

Issue 761298 link

Starred by 6 users
Status: Fixed
Owner:
schenney@chromium.org
Closed: Jan 2018
Cc:
flackr@chromium.org
ajha@chromium.org
wangxianzhu@chromium.org
schenney@chromium.org
chrishtr@chromium.org
vmp...@chromium.org
Components:
EstimatedDays: ----
NextAction: 2017-11-20
OS: Windows , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Applying a translateZ(0) transform to any element breaks border-radius clipping.

Reported by jason.gi...@omnis.net, Sep 1 2017 
UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.113 Safari/537.36

Steps to reproduce the problem:
1. Create a container element with a border-radius applied, and overflow = "hidden", and position = "absolute".
2. Create a child element of this container which fills the container.
3. Apply a "translateZ(0)" transform to an element BEFORE the container in the DOM.

See https://codepen.io/jasongiss/pen/YxBpEd for test case.

What is the expected behavior?
The child element should be clipped by the rounded corners.

What went wrong?
The child element renders into the rounded corners.

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 60.0.3112.113  Channel: stable
OS Version: OS X 10.12.6
Flash Version: 

Does not occur in Safari or Firefox on macOS.

My use case is that I use iscroll (https://github.com/cubiq/iscroll) and this applies such a "translateZ(0)" transform to elements to which it is attached.

Comment 1 by phistuck@chromium.org, Sep 1 2017

Components: Internals>Compositing Blink>Compositing Blink>Layout Blink>Paint
Labels: OS-Windows
Owner: schenney@chromium.org
Status: Assigned (was: Unconfirmed)
Reproduced using Chrome 61 on Windows 10 (with and without hardware acceleration) and Chrome 60 on Windows 7.
I thought it was  issue 157218  or  issue 672561 , which were fixed in Chrome 57 and in the upcoming Chrome 61, respectively, but your test case still reproduces. :(

1. I tried changing transform: translateZ(0) to will-change: transform but it did not reproduce.
2. I tried disabling hardware acceleration but it still reproduced.

So while it looks related to  issue 672561 , it might have a different root cause, I guess.
Assigning to @schenney for the slight chance his fix is some sort of a foundation for fixing this one.

When an engineer investigates this, it would be great if you could try and come up with other ways for something to get composited/its own layer/accelerated/whatever it is and test clipping in conjunction with all of those ways.

Comment 2 by chrishtr@chromium.org, Sep 1 2017 

Perhaps this has to to with not being about to apply border-radius clipping to squashed
layers.

Perhaps we should add a condition to disable squashing for such situations.

Comment 3 by schenney@chromium.org, Sep 5 2017 

The problem only arises when the transform is inline, so it's another situation where we fail to apply some check in the inlined transform special case.
cr761298.html
689 bytes View Download

Comment 4 by chrishtr@chromium.org, Sep 26 2017 

Any update on this bug? Need help?

Comment 5 by schenney@chromium.org, Sep 27 2017 

Just no time. Someone else could pick it up for sure.

Comment 6 by schenney@chromium.org, Oct 3 2017 

So removing the optimization for inline transform compositing update fixes the bug, as one would expect. Detecting the condition that causes this bug, siblings with border radius, might be tricky though not impossible. I'll investigate.
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 12 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8ff2371257ac4f24d77da4a62569840a53c49b47

commit 8ff2371257ac4f24d77da4a62569840a53c49b47
Author: Stephen Chenney <schenney@chromium.org>
Date: Thu Oct 12 13:11:52 2017

Prevent squashing of border-radius layers

Inline transforms cause overlap to be assumed, which in turn
results in sibling layers being squashed. But when that is
done, the border radius mask needed for descendants of the
siblings is not applied and border radius is missing.

This patch prevents squashing of layers with border radius.

R=chrishtr@chromium.org
BUG= 761298 

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Idc321a376634b0fe0b2f3cda92415d014904fda7
Reviewed-on: https://chromium-review.googlesource.com/699170
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508329}
[modify] https://crrev.com/8ff2371257ac4f24d77da4a62569840a53c49b47/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[add] https://crrev.com/8ff2371257ac4f24d77da4a62569840a53c49b47/third_party/WebKit/LayoutTests/compositing/overflow/border-radius-prevents-squashing-with-inline-transform-expected.txt
[add] https://crrev.com/8ff2371257ac4f24d77da4a62569840a53c49b47/third_party/WebKit/LayoutTests/compositing/overflow/border-radius-prevents-squashing-with-inline-transform.html
[add] https://crrev.com/8ff2371257ac4f24d77da4a62569840a53c49b47/third_party/WebKit/LayoutTests/platform/mac/compositing/overflow/border-radius-prevents-squashing-with-inline-transform-expected.png
[add] https://crrev.com/8ff2371257ac4f24d77da4a62569840a53c49b47/third_party/WebKit/LayoutTests/platform/mac/virtual/prefer_compositing_to_lcd_text/compositing/overflow/border-radius-prevents-squashing-with-inline-transform-expected.png
[add] https://crrev.com/8ff2371257ac4f24d77da4a62569840a53c49b47/third_party/WebKit/LayoutTests/platform/win/compositing/overflow/border-radius-prevents-squashing-with-inline-transform-expected.png
[add] https://crrev.com/8ff2371257ac4f24d77da4a62569840a53c49b47/third_party/WebKit/LayoutTests/platform/win/virtual/prefer_compositing_to_lcd_text/compositing/overflow/border-radius-prevents-squashing-with-inline-transform-expected.png
[modify] https://crrev.com/8ff2371257ac4f24d77da4a62569840a53c49b47/third_party/WebKit/Source/core/paint/compositing/CompositedLayerMappingTest.cpp
[modify] https://crrev.com/8ff2371257ac4f24d77da4a62569840a53c49b47/third_party/WebKit/Source/core/paint/compositing/CompositingLayerAssigner.cpp
[modify] https://crrev.com/8ff2371257ac4f24d77da4a62569840a53c49b47/third_party/WebKit/Source/platform/graphics/SquashingDisallowedReasons.cpp
[modify] https://crrev.com/8ff2371257ac4f24d77da4a62569840a53c49b47/third_party/WebKit/Source/platform/graphics/SquashingDisallowedReasons.h

Comment 8 by schenney@chromium.org, Oct 12 2017

Labels: Merge-Request-62

Comment 9 by schenney@chromium.org, Oct 12 2017

Cc: flackr@chromium.org schenney@chromium.org ajha@chromium.org
 Issue 773628  has been merged into this issue.
Project Member

Comment 10 by sheriffbot@chromium.org, Oct 12 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: We are only 4 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 11 by abdulsyed@chromium.org, Oct 12 2017 

Thanks for the fix - can you please explain why this is critical for M62 and what are the implications if we wait until M63? And we should also wait until this has baked in canary.

Comment 12 by abdulsyed@chromium.org, Oct 13 2017

Labels: -Merge-Review-62 Merge-Rejected-63
Confirmed with schenney@ offline and it's okay to punt this to M63.

Comment 13 by schenney@chromium.org, Oct 18 2017

Status: Fixed (was: Assigned)

Comment 14 by schenney@chromium.org, Nov 15 2017

Cc: chrishtr@chromium.org wangxianzhu@chromium.org
 Issue 781801  has been merged into this issue.

Comment 15 by schenney@chromium.org, Nov 15 2017

Status: Assigned (was: Fixed)
Project Member

Comment 16 by bugdroid1@chromium.org, Nov 16 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ad129f43d6ef7b47ef3845bfc43636bbcc2b5b92

commit ad129f43d6ef7b47ef3845bfc43636bbcc2b5b92
Author: Stephen Chenney <schenney@chromium.org>
Date: Thu Nov 16 01:58:24 2017

Revert "Prevent squashing of border-radius layers"

This reverts commit 8ff2371257ac4f24d77da4a62569840a53c49b47.

Reason for revert: Bad for performance and not correct

Original change's description:
> Prevent squashing of border-radius layers
>
> Inline transforms cause overlap to be assumed, which in turn
> results in sibling layers being squashed. But when that is
> done, the border radius mask needed for descendants of the
> siblings is not applied and border radius is missing.
>
> This patch prevents squashing of layers with border radius.
>
> R=​chrishtr@chromium.org
> BUG= 761298 
>
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> Change-Id: Idc321a376634b0fe0b2f3cda92415d014904fda7
> Reviewed-on: https://chromium-review.googlesource.com/699170
> Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> Commit-Queue: Stephen Chenney <schenney@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#508329}

TBR=chrishtr@chromium.org,schenney@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  761298 
Change-Id: Ia46c645c2baa74e5b3d19634b4739b97bab0812b
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Reviewed-on: https://chromium-review.googlesource.com/772758
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516951}
[modify] https://crrev.com/ad129f43d6ef7b47ef3845bfc43636bbcc2b5b92/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[modify] https://crrev.com/ad129f43d6ef7b47ef3845bfc43636bbcc2b5b92/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[modify] https://crrev.com/ad129f43d6ef7b47ef3845bfc43636bbcc2b5b92/third_party/WebKit/LayoutTests/TestExpectations
[delete] https://crrev.com/2fabfffa6a748c6eca7125ea5cf656b43536c262/third_party/WebKit/LayoutTests/compositing/overflow/border-radius-prevents-squashing-with-inline-transform-expected.txt
[delete] https://crrev.com/2fabfffa6a748c6eca7125ea5cf656b43536c262/third_party/WebKit/LayoutTests/compositing/overflow/border-radius-prevents-squashing-with-inline-transform.html
[delete] https://crrev.com/2fabfffa6a748c6eca7125ea5cf656b43536c262/third_party/WebKit/LayoutTests/platform/mac/compositing/overflow/border-radius-prevents-squashing-with-inline-transform-expected.png
[delete] https://crrev.com/2fabfffa6a748c6eca7125ea5cf656b43536c262/third_party/WebKit/LayoutTests/platform/mac/virtual/prefer_compositing_to_lcd_text/compositing/overflow/border-radius-prevents-squashing-with-inline-transform-expected.png
[delete] https://crrev.com/2fabfffa6a748c6eca7125ea5cf656b43536c262/third_party/WebKit/LayoutTests/platform/win/compositing/overflow/border-radius-prevents-squashing-with-inline-transform-expected.png
[delete] https://crrev.com/2fabfffa6a748c6eca7125ea5cf656b43536c262/third_party/WebKit/LayoutTests/platform/win/virtual/prefer_compositing_to_lcd_text/compositing/overflow/border-radius-prevents-squashing-with-inline-transform-expected.png
[modify] https://crrev.com/ad129f43d6ef7b47ef3845bfc43636bbcc2b5b92/third_party/WebKit/Source/core/paint/compositing/CompositedLayerMappingTest.cpp
[modify] https://crrev.com/ad129f43d6ef7b47ef3845bfc43636bbcc2b5b92/third_party/WebKit/Source/core/paint/compositing/CompositingLayerAssigner.cpp
[modify] https://crrev.com/ad129f43d6ef7b47ef3845bfc43636bbcc2b5b92/third_party/WebKit/Source/platform/graphics/SquashingDisallowedReasons.cpp
[modify] https://crrev.com/ad129f43d6ef7b47ef3845bfc43636bbcc2b5b92/third_party/WebKit/Source/platform/graphics/SquashingDisallowedReasons.h

Comment 17 by schenney@chromium.org, Nov 16 2017

Labels: -Hotlist-Merge-Review -Merge-Rejected-63 Merge-Request-63
M-63 Merge request for the revert of this patch, in comment #16, for performance reasons. We seriously regressed painting performance on some major sites. See https://bugs.chromium.org/p/chromium/issues/detail?id=782353

Note the earlier merge request for M-62, which is not relevant at this time.

Comment 18 by gov...@chromium.org, Nov 16 2017 

Thank you schenney@.

Before we approve merge to M63 for revert listed at #16, could you pls confirm the revert is well baked/verified in canary, having automation tests coverage and safe to merge?

Comment 19 by gov...@chromium.org, Nov 16 2017

NextAction: 2017-11-17
schenney@ to update bug with canary result tomorrow afternoon.
Project Member

Comment 20 by sheriffbot@chromium.org, Nov 17 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 21 by monor...@bugs.chromium.org, Nov 17 2017 

The NextAction date has arrived: 2017-11-17

Comment 22 by gov...@chromium.org, Nov 17 2017 

How is the change looking so far in canary?

Comment 23 by schenney@chromium.org, Nov 17 2017 

There have been no reports of issues with Canary, and this just removes added code with no impact beyond the code itself. So I'm very confident of the merge.

The perf regression really is unacceptable without the merge.

I'll do the work Monday morning East Coast time.

Comment 24 by gov...@chromium.org, Nov 17 2017

Labels: -Merge-Review-63 Merge-Approved-63
NextAction: 2017-11-20
Approving merge to M63 based on comment #23. 
schenney@ will merge on Monday morning if canary result continue to look good.

Comment 25 by monor...@bugs.chromium.org, Nov 20 2017 

The NextAction date has arrived: 2017-11-20
Project Member

Comment 26 by bugdroid1@chromium.org, Nov 20 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a4c6f49cbbb3210910b787b13703b8ceb9ac03ad

commit a4c6f49cbbb3210910b787b13703b8ceb9ac03ad
Author: Stephen Chenney <schenney@chromium.org>
Date: Mon Nov 20 15:17:28 2017

Revert "Prevent squashing of border-radius layers"

This reverts commit 8ff2371257ac4f24d77da4a62569840a53c49b47.

Reason for revert: Bad for performance and not correct

M-63 revert

Original change's description:
> Prevent squashing of border-radius layers
>
> Inline transforms cause overlap to be assumed, which in turn
> results in sibling layers being squashed. But when that is
> done, the border radius mask needed for descendants of the
> siblings is not applied and border radius is missing.
>
> This patch prevents squashing of layers with border radius.
>
> R=​chrishtr@chromium.org
> BUG= 761298 
>
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> Change-Id: Idc321a376634b0fe0b2f3cda92415d014904fda7
> Reviewed-on: https://chromium-review.googlesource.com/699170
> Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> Commit-Queue: Stephen Chenney <schenney@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#508329}

TBR=chrishtr@chromium.org, schenney@chromium.org


(cherry picked from commit ad129f43d6ef7b47ef3845bfc43636bbcc2b5b92)

Bug:  761298 
Change-Id: Ia46c645c2baa74e5b3d19634b4739b97bab0812b
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Reviewed-on: https://chromium-review.googlesource.com/772758
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#516951}
Reviewed-on: https://chromium-review.googlesource.com/779439
Cr-Commit-Position: refs/branch-heads/3239@{#540}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/a4c6f49cbbb3210910b787b13703b8ceb9ac03ad/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[modify] https://crrev.com/a4c6f49cbbb3210910b787b13703b8ceb9ac03ad/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[modify] https://crrev.com/a4c6f49cbbb3210910b787b13703b8ceb9ac03ad/third_party/WebKit/LayoutTests/TestExpectations
[delete] https://crrev.com/f0688e1096e73f17f894b5d25c563f340af87cb8/third_party/WebKit/LayoutTests/compositing/overflow/border-radius-prevents-squashing-with-inline-transform-expected.txt
[delete] https://crrev.com/f0688e1096e73f17f894b5d25c563f340af87cb8/third_party/WebKit/LayoutTests/compositing/overflow/border-radius-prevents-squashing-with-inline-transform.html
[delete] https://crrev.com/f0688e1096e73f17f894b5d25c563f340af87cb8/third_party/WebKit/LayoutTests/platform/mac/compositing/overflow/border-radius-prevents-squashing-with-inline-transform-expected.png
[delete] https://crrev.com/f0688e1096e73f17f894b5d25c563f340af87cb8/third_party/WebKit/LayoutTests/platform/mac/virtual/prefer_compositing_to_lcd_text/compositing/overflow/border-radius-prevents-squashing-with-inline-transform-expected.png
[delete] https://crrev.com/f0688e1096e73f17f894b5d25c563f340af87cb8/third_party/WebKit/LayoutTests/platform/win/compositing/overflow/border-radius-prevents-squashing-with-inline-transform-expected.png
[delete] https://crrev.com/f0688e1096e73f17f894b5d25c563f340af87cb8/third_party/WebKit/LayoutTests/platform/win/virtual/prefer_compositing_to_lcd_text/compositing/overflow/border-radius-prevents-squashing-with-inline-transform-expected.png
[modify] https://crrev.com/a4c6f49cbbb3210910b787b13703b8ceb9ac03ad/third_party/WebKit/Source/core/paint/compositing/CompositedLayerMappingTest.cpp
[modify] https://crrev.com/a4c6f49cbbb3210910b787b13703b8ceb9ac03ad/third_party/WebKit/Source/core/paint/compositing/CompositingLayerAssigner.cpp
[modify] https://crrev.com/a4c6f49cbbb3210910b787b13703b8ceb9ac03ad/third_party/WebKit/Source/platform/graphics/SquashingDisallowedReasons.cpp
[modify] https://crrev.com/a4c6f49cbbb3210910b787b13703b8ceb9ac03ad/third_party/WebKit/Source/platform/graphics/SquashingDisallowedReasons.h
Project Member

Comment 27 by bugdroid1@chromium.org, Dec 4 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a63dd55906d02fcac050c36b289e29d3f7a95453

commit a63dd55906d02fcac050c36b289e29d3f7a95453
Author: Stephen Chenney <schenney@chromium.org>
Date: Mon Dec 04 19:22:00 2017

[PE] Apply border-radius clips to squashed layers that need them.

When layers are squashed they currently lose their border radius clips.
Fix that by using ClipRects instead of IntRects in the sqaushed layer
clipping code, and gathering the clips appropriately.

Based on https://chromium-review.googlesource.com/c/chromium/src/+/772081 by chrishtr@.

R=chrishtr@chromium.org
Bug:  761298 

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I58a4c8bc785a5cea562a7de52ba24ee1600c9cff
Reviewed-on: https://chromium-review.googlesource.com/797337
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521403}
[modify] https://crrev.com/a63dd55906d02fcac050c36b289e29d3f7a95453/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[add] https://crrev.com/a63dd55906d02fcac050c36b289e29d3f7a95453/third_party/WebKit/LayoutTests/compositing/overflow/border-radius-on-squashed-layers-expected.png
[add] https://crrev.com/a63dd55906d02fcac050c36b289e29d3f7a95453/third_party/WebKit/LayoutTests/compositing/overflow/border-radius-on-squashed-layers-expected.txt
[add] https://crrev.com/a63dd55906d02fcac050c36b289e29d3f7a95453/third_party/WebKit/LayoutTests/compositing/overflow/border-radius-on-squashed-layers.html
[modify] https://crrev.com/a63dd55906d02fcac050c36b289e29d3f7a95453/third_party/WebKit/Source/core/paint/compositing/CompositedLayerMapping.cpp
[modify] https://crrev.com/a63dd55906d02fcac050c36b289e29d3f7a95453/third_party/WebKit/Source/core/paint/compositing/CompositedLayerMapping.h

Comment 28 by schenney@chromium.org, Jan 24 2018

Status: Fixed (was: Assigned)
Fixed in M-65.
►

Sign in to add a comment