New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 823362 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug-Regression



Sign in to add a comment

clip-path inset top value causes offset of child during transform transition

Reported by josero...@gmail.com, Mar 19 2018

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.162 Safari/537.36

Steps to reproduce the problem:
0. Better look at the codepen... https://codepen.io/ncodefun/pen/qoRdpj
1. create a div 200x100px, add clip-path: inset(20px 0 0 0);  (use any non-zero top value)
2. put inside this div another div 100x100px styled to look as a circle and with a transition: transform .5s ease;
3. create a class 'fx' with a transform translateX(100px)
4. toggle this class on and off on the child to trigger the transition

What is the expected behavior?
child should not change vertical position when animated

What went wrong?
The child is offset by the amount of the inset top value of the clip-path property.

Did this work before? Yes This works in stable 65.0.3325.162 

Does this work in other browsers? Yes

Chrome version: 67.0.3375.0  Channel: canary
OS Version: 10.0
Flash Version: 

There's been a fix for a problem in stable chrome with this exact situation which is probably the cause of this bug, but I don't know which one... 

You can see both issues with the same test case, depending on your chrome version: https://codepen.io/ncodefun/pen/qoRdpj
 
Labels: Needs-Bisect Needs-Triage-M67
Cc: vamshi.kommuri@chromium.org
Labels: Triaged-ET Needs-Feedback
Thanks for filing the issue!

Able to see the clip during transform transition on reported chrome version 67.0.3375.0 and on stable 65.0.3325.162 using Windows 10, Ubuntu 14.04 and Mac 10.13.1. 

As mentioned in comment#0 i.e., it worked in "stable 65.0.3325.162" but we are able to see the clip while transform transition in that version too. We have even checked it on version 60.0.3072.0, similar behaviour is seen. Attaching the screen casts from M60 and M65 for reference.

@Reporter: Could you please have a look at those screen casts and let us know if anything missed from our end. It would be highly helpful if clarified/confirmed on the M60 screencast if the issue is present in 60.0.3072.0.

Any further inputs may help..!
823362 M60.mp4
1.5 MB View Download
823362 M65.mp4
1.6 MB View Download

Comment 3 by woxxom@gmail.com, Mar 20 2018

Chrome never animated it properly according to my bisect:

* ancient Chrome didn't apply clipping at all as it wasn't implemented
* r437635 (57.0.2947.0) introduced the bottom clipping bug
* r532650 (66.0.3335.0) introduced the bouncing circles bug
* r536392 (66.0.3347.0) or r536386 fixed the bottom clipping bug introduced in r437635

So Chrome 58-65 has the bottom clipping bug, Chrome 66 and newer still has the bouncing circles bug.
Owner: trchen@chromium.org
Status: Assigned (was: Unconfirmed)
trchen@, most likely due to changes you have made for clipping.
Labels: -Needs-Feedback -Needs-Bisect -Needs-Triage-M67 FoundIn-66 FoundIn-67
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 14 2018

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

commit 10a4e34cc9207c2f6045ae5154cded6fed51f995
Author: Tien-Ren Chen <trchen@chromium.org>
Date: Sat Apr 14 01:06:34 2018

[Blink] Simplify CompositedLayerMapping::ComputeGraphicsLayerParentLocation()

This function computes the parent layer location relative to the
parent's snapped border box. Not only this duplicated code from the layer
geometry update functions, the result is incorrect, and the correct
values are already there on the layers.

This CL scrapped all the nonsense and simply use the cached values instead.

BUG= 823362 

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I36dfa12e7791fe49ce1b20573341a11922c80e09
Reviewed-on: https://chromium-review.googlesource.com/1006395
Commit-Queue: Tien-Ren Chen <trchen@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550843}
[modify] https://crrev.com/10a4e34cc9207c2f6045ae5154cded6fed51f995/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[add] https://crrev.com/10a4e34cc9207c2f6045ae5154cded6fed51f995/third_party/WebKit/LayoutTests/compositing/geometry/child-layer-position-with-clip-path-overflow-expected.html
[add] https://crrev.com/10a4e34cc9207c2f6045ae5154cded6fed51f995/third_party/WebKit/LayoutTests/compositing/geometry/child-layer-position-with-clip-path-overflow.html
[modify] https://crrev.com/10a4e34cc9207c2f6045ae5154cded6fed51f995/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc
[modify] https://crrev.com/10a4e34cc9207c2f6045ae5154cded6fed51f995/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.h

Project Member

Comment 7 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/10a4e34cc9207c2f6045ae5154cded6fed51f995

commit 10a4e34cc9207c2f6045ae5154cded6fed51f995
Author: Tien-Ren Chen <trchen@chromium.org>
Date: Sat Apr 14 01:06:34 2018

[Blink] Simplify CompositedLayerMapping::ComputeGraphicsLayerParentLocation()

This function computes the parent layer location relative to the
parent's snapped border box. Not only this duplicated code from the layer
geometry update functions, the result is incorrect, and the correct
values are already there on the layers.

This CL scrapped all the nonsense and simply use the cached values instead.

BUG= 823362 

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I36dfa12e7791fe49ce1b20573341a11922c80e09
Reviewed-on: https://chromium-review.googlesource.com/1006395
Commit-Queue: Tien-Ren Chen <trchen@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550843}
[modify] https://crrev.com/10a4e34cc9207c2f6045ae5154cded6fed51f995/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[add] https://crrev.com/10a4e34cc9207c2f6045ae5154cded6fed51f995/third_party/WebKit/LayoutTests/compositing/geometry/child-layer-position-with-clip-path-overflow-expected.html
[add] https://crrev.com/10a4e34cc9207c2f6045ae5154cded6fed51f995/third_party/WebKit/LayoutTests/compositing/geometry/child-layer-position-with-clip-path-overflow.html
[modify] https://crrev.com/10a4e34cc9207c2f6045ae5154cded6fed51f995/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc
[modify] https://crrev.com/10a4e34cc9207c2f6045ae5154cded6fed51f995/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.h

Comment 8 by trchen@chromium.org, Apr 23 2018

Cc: trchen@chromium.org sindhu.chelamcherla@chromium.org
 Issue 835612  has been merged into this issue.

Comment 9 by trchen@chromium.org, Apr 23 2018

Labels: Merge-Request-67 Merge-Request-66
Project Member

Comment 10 by sheriffbot@chromium.org, Apr 23 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Can you please confirm why this is critical for M66? M66 has been out for a week now at 25%. Have there been user reports? Or do you think we're fine waiting until M67 for this fix?
Project Member

Comment 12 by sheriffbot@chromium.org, Apr 24 2018

Labels: -Merge-Request-67 Merge-Approved-67 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Pls merge your change to M67 branch 3396 ASAP so we can pick it up for next M67 Beta release. Thank you.
Labels: -Hotlist-Merge-Review -Merge-Review-66
Re #11: Yea, I'm also thinking this may not be critical enough to bring to M66. This bug is a correctness issue around a rarely used CSS feature, not a stability issue. I say it is probably less risky to sit on it unless some important real-world sites are affected by it.
Project Member

Comment 15 by bugdroid1@chromium.org, Apr 24 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/27f437a45d2f8e20101579360cef2fee7d64cdb0

commit 27f437a45d2f8e20101579360cef2fee7d64cdb0
Author: Tien-Ren Chen <trchen@chromium.org>
Date: Tue Apr 24 20:58:32 2018

[Blink] Simplify CompositedLayerMapping::ComputeGraphicsLayerParentLocation()

This function computes the parent layer location relative to the
parent's snapped border box. Not only this duplicated code from the layer
geometry update functions, the result is incorrect, and the correct
values are already there on the layers.

This CL scrapped all the nonsense and simply use the cached values instead.

BUG= 823362 

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I36dfa12e7791fe49ce1b20573341a11922c80e09
Reviewed-on: https://chromium-review.googlesource.com/1006395
Commit-Queue: Tien-Ren Chen <trchen@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#550843}(cherry picked from commit 10a4e34cc9207c2f6045ae5154cded6fed51f995)
Reviewed-on: https://chromium-review.googlesource.com/1026611
Reviewed-by: Tien-Ren Chen <trchen@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#266}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/27f437a45d2f8e20101579360cef2fee7d64cdb0/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[add] https://crrev.com/27f437a45d2f8e20101579360cef2fee7d64cdb0/third_party/WebKit/LayoutTests/compositing/geometry/child-layer-position-with-clip-path-overflow-expected.html
[add] https://crrev.com/27f437a45d2f8e20101579360cef2fee7d64cdb0/third_party/WebKit/LayoutTests/compositing/geometry/child-layer-position-with-clip-path-overflow.html
[modify] https://crrev.com/27f437a45d2f8e20101579360cef2fee7d64cdb0/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc
[modify] https://crrev.com/27f437a45d2f8e20101579360cef2fee7d64cdb0/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.h

Status: Fixed (was: Assigned)
Fixed in M67 and trunk. M66 won't be fixed unless too much real world usage is observed.
Cc: chrishtr@chromium.org
 Issue 838826  has been merged into this issue.
Labels: Merge-Request-66
Over on 838826 we have some evidence of more widespread impact. I'm requesting a merge consistent with our need to be more aggressive with relatively simple merges once we start to see real-world impact.
How safe is this merge overall? Seems like it's been out in beta for a while now. Will this be a clean merge to 66?
Note we have another report over on https://bugs.chromium.org/p/chromium/issues/detail?id=838826

So I think we do need to merge if at all possible.
Yes I'm working on it right now. As there was a mass renaming in M67, it will require a manual merge, but the code itself should be pretty safe otherwise. I'm checking out a full M66 tree to verify my merge will build properly.
Labels: -Merge-Request-66 Merge-Approved-66
Confirmed with trchen@, safe merge - approved for m66. Branch:3359
Project Member

Comment 24 by bugdroid1@chromium.org, May 8 2018

Labels: -merge-approved-66 merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8b5041c6c8557565d1df36fbfbce4062e2968a36

commit 8b5041c6c8557565d1df36fbfbce4062e2968a36
Author: Tien-Ren Chen <trchen@chromium.org>
Date: Tue May 08 21:45:52 2018

[Blink] Simplify CompositedLayerMapping::ComputeGraphicsLayerParentLocation()

This function computes the parent layer location relative to the
parent's snapped border box. Not only this duplicated code from the layer
geometry update functions, the result is incorrect, and the correct
values are already there on the layers.

This CL scrapped all the nonsense and simply use the cached values
instead.

BUG= 823362 
TBR=trchen@chromium.org

(cherry picked from commit 10a4e34cc9207c2f6045ae5154cded6fed51f995)

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I36dfa12e7791fe49ce1b20573341a11922c80e09
Reviewed-on: https://chromium-review.googlesource.com/1006395
Commit-Queue: Tien-Ren Chen <trchen@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#550843}
Reviewed-on: https://chromium-review.googlesource.com/1050421
Reviewed-by: Tien-Ren Chen <trchen@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#810}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/8b5041c6c8557565d1df36fbfbce4062e2968a36/third_party/WebKit/Source/core/paint/compositing/CompositedLayerMapping.cpp
[modify] https://crrev.com/8b5041c6c8557565d1df36fbfbce4062e2968a36/third_party/WebKit/Source/core/paint/compositing/CompositedLayerMapping.h

Labels: TE-Verified-M66 TE-Verified-66.0.3359.170
Verified the fix on Windows-10 using Chrome version #66.0.3359.170 as per the comment #0.
Attaching screen cast for reference.
The offset of child during transform transition is not seen.
Hence, the fix is working as expected. 
Adding the verified labels.
Note: Able to reproduce the issue on chrome version with out fix.

Thanks...!!
823362 CL.mp4
682 KB View Download
 Issue 840721  has been merged into this issue.

Sign in to add a comment