⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 268802 in webkit


Ignore:
Timestamp:
Oct 21, 2020, 9:54:53 AM (6 years ago)
Author:
Alan Coon
Message:

Cherry-pick r268771. rdar://problem/70532973

REGRESSION (r268483): Map jumps around while zooming on windy.com, strava.com
​https://bugs.webkit.org/show_bug.cgi?id=217987
<rdar://problem/70418548>

Reviewed by Simon Fraser.

Source/WebCore:

When several animations targetting the same property and the same layer are overlapping, we used to
always override the previous animations. With r268483 we started maintaining all active animations
and let them run, potentially with additivity if the animation could be broken into several animations
each targeting a given transform operation.

On top of that, with r268615 and the support for accelerated animation of individual CSS transform
properties (translate, scale and rotate), all transform-related animations were made additive.

This meant that we would always run active animations targeting "transform" in a way where they would be
additive rather than being replaced.

Any animation targeting "transform" will yield one or several accelerated animations, and the first of this
animation set will always have a 0 index. So now, when we compile a list of transform animations in
GraphicsLayerCA::updateAnimations(), we reset that list any time we encounted an animation with a 0 index,
ensuring only the top-most transform animation is applied.

We also fix an issue where we didn't account for the possibility that a single KeyframeEffect could yield
several transform animations with the same name in pauseAnimation() and removeAnimation(). We now pause or
remove all animations with the provided name.

Test: webanimations/accelerated-overlapping-transform-animations.html

  • platform/graphics/ca/GraphicsLayerCA.cpp: (WebCore::GraphicsLayerCA::pauseAnimation): (WebCore::GraphicsLayerCA::removeAnimation): (WebCore::GraphicsLayerCA::updateAnimations):

LayoutTests:

Add a new test that checks that only the last of two overlapping "transform" animations is applied.

  • platform/mac-wk1/TestExpectations:
  • webanimations/accelerated-overlapping-transform-animations-expected.html: Added.
  • webanimations/accelerated-overlapping-transform-animations.html: Added.

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@268771 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:
branches/safari-611.1.4-branch
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-611.1.4-branch/LayoutTests/ChangeLog

    r268754 r268802  
     12020-10-21  Alan Coon  <alancoon@apple.com>
     2
     3        Cherry-pick r268771. rdar://problem/70532973
     4
     5    REGRESSION (r268483): Map jumps around while zooming on windy.com, strava.com
     6    https://bugs.webkit.org/show_bug.cgi?id=217987
     7    <rdar://problem/70418548>
     8   
     9    Reviewed by Simon Fraser.
     10   
     11    Source/WebCore:
     12   
     13    When several animations targetting the same property and the same layer are overlapping, we used to
     14    always override the previous animations. With r268483 we started maintaining all active animations
     15    and let them run, potentially with additivity if the animation could be broken into several animations
     16    each targeting a given transform operation.
     17   
     18    On top of that, with r268615 and the support for accelerated animation of individual CSS transform
     19    properties (translate, scale and rotate), all transform-related animations were made additive.
     20   
     21    This meant that we would always run active animations targeting "transform" in a way where they would be
     22    additive rather than being replaced.
     23   
     24    Any animation targeting "transform" will yield one or several accelerated animations, and the first of this
     25    animation set will always have a 0 index. So now, when we compile a list of transform animations in
     26    GraphicsLayerCA::updateAnimations(), we reset that list any time we encounted an animation with a 0 index,
     27    ensuring only the top-most transform animation is applied.
     28   
     29    We also fix an issue where we didn't account for the possibility that a single KeyframeEffect could yield
     30    several transform animations with the same name in pauseAnimation() and removeAnimation(). We now pause or
     31    remove all animations with the provided name.
     32   
     33    Test: webanimations/accelerated-overlapping-transform-animations.html
     34   
     35    * platform/graphics/ca/GraphicsLayerCA.cpp:
     36    (WebCore::GraphicsLayerCA::pauseAnimation):
     37    (WebCore::GraphicsLayerCA::removeAnimation):
     38    (WebCore::GraphicsLayerCA::updateAnimations):
     39   
     40    LayoutTests:
     41   
     42    Add a new test that checks that only the last of two overlapping "transform" animations is applied.
     43   
     44    * platform/mac-wk1/TestExpectations:
     45    * webanimations/accelerated-overlapping-transform-animations-expected.html: Added.
     46    * webanimations/accelerated-overlapping-transform-animations.html: Added.
     47   
     48    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@268771 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     49
     50    2020-10-20  Antoine Quint  <graouts@webkit.org>
     51
     52            REGRESSION (r268483): Map jumps around while zooming on windy.com, strava.com
     53            https://bugs.webkit.org/show_bug.cgi?id=217987
     54            <rdar://problem/70418548>
     55
     56            Reviewed by Simon Fraser.
     57
     58            Add a new test that checks that only the last of two overlapping "transform" animations is applied.
     59
     60            * platform/mac-wk1/TestExpectations:
     61            * webanimations/accelerated-overlapping-transform-animations-expected.html: Added.
     62            * webanimations/accelerated-overlapping-transform-animations.html: Added.
     63
    1642020-10-20  Alan Coon  <alancoon@apple.com>
    265

  • branches/safari-611.1.4-branch/LayoutTests/platform/mac-wk1/TestExpectations

    r268527 r268802  
    11941194
    11951195webkit.org/b/217761 [ Mojave+ Debug ] webgl/2.0.0/conformance/extensions/webgl-compressed-texture-s3tc-srgb.html [ Pass Crash ]
     1196
     1197webkit.org/b/217997 webanimations/accelerated-overlapping-transform-animations.html [ Pass Failure ]

  • branches/safari-611.1.4-branch/Source/WebCore/ChangeLog

    r268756 r268802  
     12020-10-21  Alan Coon  <alancoon@apple.com>
     2
     3        Cherry-pick r268771. rdar://problem/70532973
     4
     5    REGRESSION (r268483): Map jumps around while zooming on windy.com, strava.com
     6    https://bugs.webkit.org/show_bug.cgi?id=217987
     7    <rdar://problem/70418548>
     8   
     9    Reviewed by Simon Fraser.
     10   
     11    Source/WebCore:
     12   
     13    When several animations targetting the same property and the same layer are overlapping, we used to
     14    always override the previous animations. With r268483 we started maintaining all active animations
     15    and let them run, potentially with additivity if the animation could be broken into several animations
     16    each targeting a given transform operation.
     17   
     18    On top of that, with r268615 and the support for accelerated animation of individual CSS transform
     19    properties (translate, scale and rotate), all transform-related animations were made additive.
     20   
     21    This meant that we would always run active animations targeting "transform" in a way where they would be
     22    additive rather than being replaced.
     23   
     24    Any animation targeting "transform" will yield one or several accelerated animations, and the first of this
     25    animation set will always have a 0 index. So now, when we compile a list of transform animations in
     26    GraphicsLayerCA::updateAnimations(), we reset that list any time we encounted an animation with a 0 index,
     27    ensuring only the top-most transform animation is applied.
     28   
     29    We also fix an issue where we didn't account for the possibility that a single KeyframeEffect could yield
     30    several transform animations with the same name in pauseAnimation() and removeAnimation(). We now pause or
     31    remove all animations with the provided name.
     32   
     33    Test: webanimations/accelerated-overlapping-transform-animations.html
     34   
     35    * platform/graphics/ca/GraphicsLayerCA.cpp:
     36    (WebCore::GraphicsLayerCA::pauseAnimation):
     37    (WebCore::GraphicsLayerCA::removeAnimation):
     38    (WebCore::GraphicsLayerCA::updateAnimations):
     39   
     40    LayoutTests:
     41   
     42    Add a new test that checks that only the last of two overlapping "transform" animations is applied.
     43   
     44    * platform/mac-wk1/TestExpectations:
     45    * webanimations/accelerated-overlapping-transform-animations-expected.html: Added.
     46    * webanimations/accelerated-overlapping-transform-animations.html: Added.
     47   
     48    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@268771 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     49
     50    2020-10-20  Antoine Quint  <graouts@webkit.org>
     51
     52            REGRESSION (r268483): Map jumps around while zooming on windy.com, strava.com
     53            https://bugs.webkit.org/show_bug.cgi?id=217987
     54            <rdar://problem/70418548>
     55
     56            Reviewed by Simon Fraser.
     57
     58            When several animations targetting the same property and the same layer are overlapping, we used to
     59            always override the previous animations. With r268483 we started maintaining all active animations
     60            and let them run, potentially with additivity if the animation could be broken into several animations
     61            each targeting a given transform operation.
     62
     63            On top of that, with r268615 and the support for accelerated animation of individual CSS transform
     64            properties (translate, scale and rotate), all transform-related animations were made additive.
     65
     66            This meant that we would always run active animations targeting "transform" in a way where they would be
     67            additive rather than being replaced.
     68
     69            Any animation targeting "transform" will yield one or several accelerated animations, and the first of this
     70            animation set will always have a 0 index. So now, when we compile a list of transform animations in
     71            GraphicsLayerCA::updateAnimations(), we reset that list any time we encounted an animation with a 0 index,
     72            ensuring only the top-most transform animation is applied.
     73
     74            We also fix an issue where we didn't account for the possibility that a single KeyframeEffect could yield
     75            several transform animations with the same name in pauseAnimation() and removeAnimation(). We now pause or
     76            remove all animations with the provided name.
     77
     78            Test: webanimations/accelerated-overlapping-transform-animations.html
     79
     80            * platform/graphics/ca/GraphicsLayerCA.cpp:
     81            (WebCore::GraphicsLayerCA::pauseAnimation):
     82            (WebCore::GraphicsLayerCA::removeAnimation):
     83            (WebCore::GraphicsLayerCA::updateAnimations):
     84
    1852020-10-20  Alan Coon  <alancoon@apple.com>
    286

  • branches/safari-611.1.4-branch/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp

    r268754 r268802  
    10801080    LOG_WITH_STREAM(Animations, stream << "GraphicsLayerCA " << this << " id " << primaryLayerID() << " pauseAnimation " << animationName << " (is running " << animationIsRunning(animationName) << ")");
    10811081
    1082     auto index = m_animations.findMatching([&](LayerPropertyAnimation animation) {
    1083         return animation.m_name == animationName && !animation.m_pendingRemoval;
    1084     });
    1085 
    1086     if (index == notFound)
    1087         return;
    1088 
    1089     auto& animation = m_animations[index];
    1090     animation.m_playState = PlayState::PausePending;
    1091     animation.m_timeOffset = Seconds { timeOffset };
    1092 
    1093     noteLayerPropertyChanged(AnimationChanged);
     1082    for (auto& animation : m_animations) {
     1083        // There may be several animations with the same name in the case of transform animations
     1084        // animating multiple components as individual animations.
     1085        if (animation.m_name == animationName && !animation.m_pendingRemoval) {
     1086            animation.m_playState = PlayState::PausePending;
     1087            animation.m_timeOffset = Seconds { timeOffset };
     1088
     1089            noteLayerPropertyChanged(AnimationChanged);
     1090        }
     1091    }
    10941092}
    10951093
    … …  
    10981096    LOG_WITH_STREAM(Animations, stream << "GraphicsLayerCA " << this << " id " << primaryLayerID() << " removeAnimation " << animationName << " (is running " << animationIsRunning(animationName) << ")");
    10991097
    1100     auto index = m_animations.findMatching([&](LayerPropertyAnimation animation) {
    1101         return animation.m_name == animationName && !animation.m_pendingRemoval;
    1102     });
    1103 
    1104     if (index == notFound)
    1105         return;
    1106 
    1107     m_animations[index].m_pendingRemoval = true;
    1108 
    1109     noteLayerPropertyChanged(AnimationChanged | CoverageRectChanged);
     1098    for (auto& animation : m_animations) {
     1099        // There may be several animations with the same name in the case of transform animations
     1100        // animating multiple components as individual animations.
     1101        if (animation.m_name == animationName && !animation.m_pendingRemoval) {
     1102            animation.m_pendingRemoval = true;
     1103            noteLayerPropertyChanged(AnimationChanged | CoverageRectChanged);
     1104        }
     1105    }
    11101106}
    11111107
    … …  
    29612957            break;
    29622958        case AnimatedPropertyTransform:
     2959            // In the case of animations targeting the "transform" CSS property, there may be several
     2960            // animations created for a single KeyframeEffect, one for each transform component. In that
     2961            // case the animation index starts at 0 and increases for each component. If we encounter an
     2962            // index of 0 this means this animation establishes a new group of animation belonging to a
     2963            // single KeyframeEffect. As such, since the top-most KeyframeEffect replaces the previous
     2964            // ones, we can remove all the previously-added "transform" animations.
     2965            if (!animation.m_index)
     2966                transformAnimations.clear();
    29632967            transformAnimations.append(&animation);
    29642968            break;
Note: See TracChangeset for help on using the changeset viewer.