There is an animation jerk when animating box-shadow
Reported by
pratik_b...@gridle.io,
Apr 20 2017
|
|||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.81 Safari/537.36 Steps to reproduce the problem: 1. Go to this pen "https://codepen.io/anon/pen/gWrbZg" 2. Hover over the card What is the expected behavior? Animation should work smoothly. What went wrong? Animation is jerking. Did this work before? Yes 57 Does this work in other browsers? Yes Chrome version: 58.0.3029.81 Channel: stable OS Version: Ubuntu 17.04 Flash Version: It is introduced in version 58. Before version 58 it was working fine.
,
Apr 21 2017
I suspect this is in the way we interpolate different numbers of box-shadow values. During the interpolation it grows to the lowest common multiple (in this case 6) so there would be 6 shadows stacked on top of each other with blurring making it much darker than 2 or 3. I'll be interested in which patch the bisect points out for this.
,
Apr 21 2017
,
Apr 21 2017
Able to reproduce the issue on windows 7, Ubuntu 14.04 and Mac 10.12.4 using chrome version 58.0.3029.81 and canary 60.0.3073.0. This is regression issue broken in M58.Please find the bisect information as below Narrow bisect:: Good :: 58.0.3018.0 -- (build revision 451537) Bad:: 58.0.3019.0 -- (build revision 451676) Change Log:: https://chromium.googlesource.com/chromium/src/+log/2090c9f72c7eebd76726965cfc7d15cb8d1cc12d..6b6a6c64cdccdb95c20fa6c704cd44d5685f9be9 Possible suspect:: https://chromium.googlesource.com/chromium/src/+/6b6a6c64cdccdb95c20fa6c704cd44d5685f9be9 alancutter @ Could you please look into this issue if it is related to your change,else please help us in finding the appropriate owner for this issue. Thanks,
,
Apr 21 2017
,
Apr 21 2017
Guessing this is applicable for Android also, but needs triage. alancutter @ Please take a look at the culprit, also confirm whether this bug blocks Stable auto update.
,
Apr 23 2017
,
Apr 24 2017
The old transition code was using AnimatableShadow: https://chromium.googlesource.com/chromium/src/+/ce6b6a0d684b1ba63accead22156e32d53d169cc/third_party/WebKit/Source/core/animation/animatable/AnimatableShadow.cpp That in turn used ShadowList::Blend: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/style/ShadowList.cpp?type=cs&q=shadowlist+blend&sq=package:chromium&l=52 Looks like the new code path for shadow interpolation was not following the spec correctly: https://www.w3.org/TR/css3-transitions/#animatable-types "shadow list: Each shadow in the list is interpolated via the color (as color) component, and x, y, blur, and (when appropriate) spread (as length) components. For each shadow, if one input shadow is ‘inset’ and the other is not, then the result for that shadow matches the inputs; otherwise the entire list is not interpolable. If the lists of shadows have different lengths, then the shorter list is padded at the end with shadows whose color is ‘transparent’, all lengths are ‘0’, and whose ‘inset’ (or not) matches the longer list." Instead it treats them like repeatable lists which expand to the lowest common multiple.
,
Apr 24 2017
We are planning a stable release soon and RC cut Monday April 24, 5.00 PM PST. alancutter@ do you think this a blocker for M58? Can it wait until M59 hits stable? Please confirm. Thanks.
,
Apr 24 2017
I don't think this needs to block that release. The scenario is relatively corner casey and I haven't seen reports about it other than this one.
,
Apr 24 2017
It doesn't need to block the release, but this affects a production site for Stripe and we're watching the issue closely (rather than reporting again). Thanks!
,
Apr 24 2017
#11: Thanks for letting us know, it's important for us to understand the impact of this issue. FYI a workaround for the bug is to ensure that the box-shadows undergoing the transition have the same number of (comma separated) shadows in them. The shorter one can be padded to the same length with a no-op "0 0 0 0 transparent" shadow. Here's the workaround applied to the original test case: https://codepen.io/anon/pen/XRjbVG
,
Apr 24 2017
,
Apr 24 2017
For any external folks, note that when we state this won't block the M58 release, this means we'll roll the affected version out to 100% of users shortly and won't deploy the fix until M59 (currently slated to roll out ~early June). I'll assume you have workarounds in place or this isn't a major regression for you if that's the case; as alancutter@ explained we really appreciate your input. Since as of right now this won't block M58 stable, we shouldn't block M59 beta on it either. Adjusting tags so that this is marked as blocking M59's stable release so this is fixed before then, though.
,
Apr 28 2017
Tested this issue on Android 7.0.99; Build/MRA20 using chrome version #58.0.3029.83 by following steps mentioned in comment #0. Able to reproduce the issue, Observed the jerking on hovering over the card. Thanks,
,
May 5 2017
,
May 8 2017
This is breaking Google Keep in M-58.
,
May 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8 commit 4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8 Author: alancutter <alancutter@chromium.org> Date: Tue May 09 04:29:17 2017 Fix behaviour of shadow interpolation with mismatched list lengths For shadow properties like box-shadow and text-shadow we were incorrectly interpolating shadow lists with mismatched lengths. The old behaviour would repeat each list to the lowest common multiple, the correct behaviour is to pad the shorted list with "zero" shadows. Spec: https://www.w3.org/TR/css3-transitions/#animatable-types This change adds the "padding" behaviour to list interpolation and composition and updates our shadow animation test cases to match spec behaviour. BUG= 713674 Review-Url: https://codereview.chromium.org/2844213002 Cr-Commit-Position: refs/heads/master@{#470205} [modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/LayoutTests/animations/composition/box-shadow-composition.html [modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/LayoutTests/animations/composition/text-shadow-composition.html [modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/LayoutTests/animations/interpolation/box-shadow-interpolation.html [modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/LayoutTests/animations/interpolation/text-shadow-interpolation.html [modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/LayoutTests/animations/responsive/box-shadow-responsive.html [modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/LayoutTests/transitions/multiple-shadow-transitions-expected.txt [modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/LayoutTests/transitions/multiple-shadow-transitions.html [modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/Source/core/animation/CSSImageListInterpolationType.cpp [modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/Source/core/animation/CSSLengthListInterpolationType.cpp [modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/Source/core/animation/CSSShadowListInterpolationType.cpp [modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/Source/core/animation/CSSSizeListInterpolationType.cpp [modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/Source/core/animation/ListInterpolationFunctions.cpp [modify] https://crrev.com/4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8/third_party/WebKit/Source/core/animation/ListInterpolationFunctions.h
,
May 10 2017
Google Keep has implemented the workaround for this bug. Will request merge to Beta 59 once the fix has baked in Canary for a bit.
,
May 11 2017
Verified that this is fixed in Mac Canary, requesting merge to Beta.
,
May 11 2017
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6 commit ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6 Author: Alan Cutter <alancutter@chromium.org> Date: Thu May 11 01:49:21 2017 Fix behaviour of shadow interpolation with mismatched list lengths For shadow properties like box-shadow and text-shadow we were incorrectly interpolating shadow lists with mismatched lengths. The old behaviour would repeat each list to the lowest common multiple, the correct behaviour is to pad the shorted list with "zero" shadows. Spec: https://www.w3.org/TR/css3-transitions/#animatable-types This change adds the "padding" behaviour to list interpolation and composition and updates our shadow animation test cases to match spec behaviour. BUG= 713674 Review-Url: https://codereview.chromium.org/2844213002 Cr-Commit-Position: refs/heads/master@{#470205} (cherry picked from commit 4aaaf95a33ef49f61476ca1f3699700fa7ea3fe8) Review-Url: https://codereview.chromium.org/2876723002 . Cr-Commit-Position: refs/branch-heads/3071@{#506} Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641} [modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/LayoutTests/animations/composition/box-shadow-composition.html [modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/LayoutTests/animations/composition/text-shadow-composition.html [modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/LayoutTests/animations/interpolation/box-shadow-interpolation.html [modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/LayoutTests/animations/interpolation/text-shadow-interpolation.html [modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/LayoutTests/animations/responsive/box-shadow-responsive.html [modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/LayoutTests/transitions/multiple-shadow-transitions-expected.txt [modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/LayoutTests/transitions/multiple-shadow-transitions.html [modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/Source/core/animation/CSSImageListInterpolationType.cpp [modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/Source/core/animation/CSSLengthListInterpolationType.cpp [modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/Source/core/animation/CSSShadowListInterpolationType.cpp [modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/Source/core/animation/CSSSizeListInterpolationType.cpp [modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/Source/core/animation/ListInterpolationFunctions.cpp [modify] https://crrev.com/ff1b1926fea47a35bfc0c9729a020ec20c8fa5f6/third_party/WebKit/Source/core/animation/ListInterpolationFunctions.h
,
May 12 2017
,
May 15 2017
Issue 722102 has been merged into this issue.
,
May 17 2017
Rechecked this issue on chrome version 59.0.3071.61 on Windows 10, MAC 10.12.4, Ubuntu 14.04. Fix is working as intended. On mouse hover, animation appears to be smooth. Adding TE-verified labels. Thanks.! |
|||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||
Comment 1 by ligim...@chromium.org
, Apr 20 2017Components: Blink>Animation
Labels: Prestable-58.0.3029.81 Needs-Triage-M58 Needs-Bisect