Changeset 272201 in webkit


Ignore:
Timestamp:
Feb 2, 2021 2:59:00 AM (18 months ago)
Author:
graouts@webkit.org
Message:

Animation of "rotate" or "scale" property does not correctly account for static "translate" property
​https://bugs.webkit.org/show_bug.cgi?id=219894
<rdar://problem/72342798>

Reviewed by Dean Jackson.

Source/WebCore:

The CSS transform-related properties are designed to be applied in a specific order, guaranteeing that
"translate" is applied prior to both "scale" and "rotate". Since Core Animation has no concept of these
individual transform-related CSS properties, we use additive Core Animation animations to apply the value
of each CSS property, using non-interpolating animations set to start at the earliest time in the Core
Animation timeline and lasting forever to set the value of any underlying, non-animated value.

As such, in an example where an element would have a static "translate" property set as well as a "rotate"
or "scale" animation, we would yield the following animations, added in this order:

  1. non-interpolating animation beginning at 1s setting the identity transform (the "clean slate" animation)
  2. interpolating animation beginning at a time > 1s for the "scale" or "rotate" animation
  3. non-interpolating animation beginning at 1s setting the "translate" value

Note that animations 2 and 3 are additive and thus added in the inverse order that we expect animations to be
applied. Due to a peculiarity of Core Animation (introduced in macOS 10.15), additive animations are applied
in an inverse order, hence the build-time flag CA_WHERE_ADDITIVE_TRANSFORMS_ARE_REVERSED.

However, Core Animation will first sort all animations based on their begin time, only respecting the order
in which animations are added when their begin time is equal. This means that in practice, our animations were
applied in the order 1, 3, 2, and thus the "translate" property was applied after the "rotate" or "scale" animation.

In order to address this, we now create a CAAnimationGroup for each set of animations created for a given CSS
property. Each of these groups shares the same begin time, 1s, to allow for "forever" non-interpolating animations
to be applied, but also to set a common base time for animations to be applied in the expected order.

Tests: webanimations/relative-ordering-of-translate-and-rotate-properties-accelerated.html

webanimations/relative-ordering-of-translate-and-scale-properties-accelerated.html

  • platform/graphics/ca/GraphicsLayerCA.cpp:

(WebCore::GraphicsLayerCA::updateAnimations):

  • platform/graphics/ca/GraphicsLayerCA.h:

(WebCore::GraphicsLayerCA::LayerPropertyAnimation::computedBeginTime const):

LayoutTests:

Add two new tests that ensure that translate is indeed applied before rotate and scale.

  • webanimations/relative-ordering-of-translate-and-rotate-properties-accelerated-expected.html: Added.
  • webanimations/relative-ordering-of-translate-and-rotate-properties-accelerated.html: Added.
  • webanimations/relative-ordering-of-translate-and-scale-properties-accelerated-expected.html: Added.
  • webanimations/relative-ordering-of-translate-and-scale-properties-accelerated.html: Added.
Location:
trunk
Files:
4 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r272198 r272201  
     12021-02-01  Antoine Quint  <graouts@webkit.org>
     2
     3        Animation of "rotate" or "scale" property does not correctly account for static "translate" property
     4        https://bugs.webkit.org/show_bug.cgi?id=219894
     5        <rdar://problem/72342798>
     6
     7        Reviewed by Dean Jackson.
     8
     9        Add two new tests that ensure that translate is indeed applied before rotate and scale.
     10
     11        * webanimations/relative-ordering-of-translate-and-rotate-properties-accelerated-expected.html: Added.
     12        * webanimations/relative-ordering-of-translate-and-rotate-properties-accelerated.html: Added.
     13        * webanimations/relative-ordering-of-translate-and-scale-properties-accelerated-expected.html: Added.
     14        * webanimations/relative-ordering-of-translate-and-scale-properties-accelerated.html: Added.
     15
    1162021-02-02  Rob Buis  <rbuis@igalia.com>
    217

  • trunk/Source/WebCore/ChangeLog

    r272200 r272201  
     12021-02-01  Antoine Quint  <graouts@webkit.org>
     2
     3        Animation of "rotate" or "scale" property does not correctly account for static "translate" property
     4        https://bugs.webkit.org/show_bug.cgi?id=219894
     5        <rdar://problem/72342798>
     6
     7        Reviewed by Dean Jackson.
     8
     9        The CSS transform-related properties are designed to be applied in a specific order, guaranteeing that
     10        "translate" is applied prior to both "scale" and "rotate". Since Core Animation has no concept of these
     11        individual transform-related CSS properties, we use additive Core Animation animations to apply the value
     12        of each CSS property, using non-interpolating animations set to start at the earliest time in the Core
     13        Animation timeline and lasting forever to set the value of any underlying, non-animated value.
     14
     15        As such, in an example where an element would have a static "translate" property set as well as a "rotate"
     16        or "scale" animation, we would yield the following animations, added in this order:
     17       
     18            1. non-interpolating animation beginning at 1s setting the identity transform (the "clean slate" animation)
     19            2. interpolating animation beginning at a time > 1s for the "scale" or "rotate" animation
     20            3. non-interpolating animation beginning at 1s setting the "translate" value
     21
     22        Note that animations 2 and 3 are additive and thus added in the inverse order that we expect animations to be
     23        applied. Due to a peculiarity of Core Animation (introduced in macOS 10.15), additive animations are applied
     24        in an inverse order, hence the build-time flag CA_WHERE_ADDITIVE_TRANSFORMS_ARE_REVERSED.
     25
     26        However, Core Animation will first sort all animations based on their begin time, only respecting the order
     27        in which animations are added when their begin time is equal. This means that in practice, our animations were
     28        applied in the order 1, 3, 2, and thus the "translate" property was applied after the "rotate" or "scale" animation.
     29
     30        In order to address this, we now create a CAAnimationGroup for each set of animations created for a given CSS
     31        property. Each of these groups shares the same begin time, 1s, to allow for "forever" non-interpolating animations
     32        to be applied, but also to set a common base time for animations to be applied in the expected order.
     33
     34        Tests: webanimations/relative-ordering-of-translate-and-rotate-properties-accelerated.html
     35               webanimations/relative-ordering-of-translate-and-scale-properties-accelerated.html
     36
     37        * platform/graphics/ca/GraphicsLayerCA.cpp:
     38        (WebCore::GraphicsLayerCA::updateAnimations):
     39        * platform/graphics/ca/GraphicsLayerCA.h:
     40        (WebCore::GraphicsLayerCA::LayerPropertyAnimation::computedBeginTime const):
     41
    1422021-02-02  Youenn Fablet  <youenn@apple.com>
    243

  • trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp

    r272004 r272201  
    28892889void GraphicsLayerCA::updateAnimations()
    28902890{
    2891     auto baseTransformAnimationBeginTime = 1_s;
     2891    // In order to guarantee that transform animations are applied in the expected order (translate, rotate, scale and transform),
     2892    // we need to have them wrapped individually in an animation group because Core Animation sorts animations first by their begin
     2893    // time, and then by the order in which they were added (for those with the same begin time). Since a rotate animation can have
     2894    // an earlier begin time than a translate animation, we cannot rely on adding the animations in the correct order.
     2895    //
     2896    // Having an animation group wrapping each animation means that we can guarantee the order in which animations are applied by
     2897    // ensuring they each have the same begin time. We set this begin time to be the smallest value possible, ensuring that base
     2898    // transform animations are applied continuously. We'll then set the begin time of interpolating animations to be local to the
     2899    // animation group, which means subtracting the group's begin time.
     2900
     2901    // We use 1_s here because 0_s would have special meaning in Core Animation, meaning that the animation would have its begin
     2902    // time set to the current time when it's committed.
     2903    auto animationGroupBeginTime = 1_s;
     2904    auto infiniteDuration = std::numeric_limits<double>::max();
    28922905    auto currentTime = Seconds(CACurrentMediaTime());
    2893     auto updateBeginTimes = [&](LayerPropertyAnimation& animation)
    2894     {
    2895         if (animation.m_pendingRemoval)
    2896             return;
    2897 
    2898         // In case we have an offset, and we haven't set an explicit begin time previously,
    2899         // we need to record the beginTime now.
    2900         if (animation.m_timeOffset && !animation.m_beginTime)
    2901             animation.m_beginTime = currentTime;
    2902 
    2903         // Now check if we have a resolved begin time and ensure the begin time we'll use for
    2904         // base transform animations matches the smallest known begin time to guarantee that
    2905         // such animations can combine with other explicit transform animations correctly.
    2906         if (auto computedBeginTime = animation.computedBeginTime())
    2907             baseTransformAnimationBeginTime = std::min(baseTransformAnimationBeginTime, *computedBeginTime);
     2906
     2907    auto addAnimationGroup = [&](AnimatedPropertyID property, const Vector<RefPtr<PlatformCAAnimation>>& animations) {
     2908        auto caAnimationGroup = createPlatformCAAnimation(PlatformCAAnimation::Group, "");
     2909        caAnimationGroup->setDuration(infiniteDuration);
     2910        caAnimationGroup->setAnimations(animations);
     2911
     2912        auto animationGroup = LayerPropertyAnimation(WTFMove(caAnimationGroup), "group-" + createCanonicalUUIDString(), property, 0, 0, 0_s);
     2913        animationGroup.m_beginTime = animationGroupBeginTime;
     2914
     2915        setAnimationOnLayer(animationGroup);
     2916        m_animationGroups.append(WTFMove(animationGroup));
    29082917    };
    29092918
    29102919    enum class Additive { Yes, No };
    2911     auto addAnimation = [&](LayerPropertyAnimation& animation, Additive additive = Additive::Yes) {
    2912         animation.m_animation->setAdditive(additive == Additive::Yes);
    2913         setAnimationOnLayer(animation);
     2920    auto prepareAnimationForAddition = [&](LayerPropertyAnimation& animation, Additive additive = Additive::Yes) {
     2921        auto caAnim = animation.m_animation;
     2922        caAnim->setAdditive(additive == Additive::Yes);
     2923        if (auto beginTime = animation.computedBeginTime())
     2924            caAnim->setBeginTime(beginTime->seconds());
     2925
    29142926        if (animation.m_playState == PlayState::PausePending || animation.m_playState == PlayState::Paused) {
    2915             pauseCAAnimationOnLayer(animation);
     2927            caAnim->setSpeed(0);
     2928            caAnim->setTimeOffset(animation.m_timeOffset.seconds());
    29162929            animation.m_playState = PlayState::Paused;
    29172930        } else
    … …  
    29192932    };
    29202933
     2934    auto addAnimation = [&](LayerPropertyAnimation& animation, Additive additive = Additive::Yes) {
     2935        prepareAnimationForAddition(animation, additive);
     2936        addAnimationGroup(animation.m_property, { animation.m_animation });
     2937    };
     2938
    29212939    enum class TransformationMatrixSource { UseIdentityMatrix, AskClient };
    2922     auto addBaseValueTransformAnimation = [&](AnimatedPropertyID property, TransformationMatrixSource matrixSource = TransformationMatrixSource::AskClient, Seconds beginTimeOfEarliestPropertyAnimation = 0_s) {
     2940    auto makeBaseValueTransformAnimation = [&](AnimatedPropertyID property, TransformationMatrixSource matrixSource = TransformationMatrixSource::AskClient, Seconds beginTimeOfEarliestPropertyAnimation = 0_s) -> LayerPropertyAnimation* {
    29232941        // A base value transform animation can either be set to the identity matrix or to read the underlying
    29242942        // value from the GraphicsLayerClient. If we didn't explicitly ask for an identity matrix, we can skip
    … …  
    29262944        auto matrix = matrixSource == TransformationMatrixSource::UseIdentityMatrix ? TransformationMatrix() : client().transformMatrixForProperty(property);
    29272945        if (matrixSource == TransformationMatrixSource::AskClient && matrix.isIdentity())
    2928             return;
     2946            return nullptr;
    29292947
    29302948        auto delay = beginTimeOfEarliestPropertyAnimation > currentTime ? beginTimeOfEarliestPropertyAnimation - currentTime : 0_s;
    … …  
    29342952        // of the delay until that animation.
    29352953        auto caAnimation = createPlatformCAAnimation(PlatformCAAnimation::Basic, propertyIdToString(property));
    2936         caAnimation->setDuration((delay ? delay : Seconds::infinity()).seconds());
     2954        caAnimation->setDuration(delay ? delay.seconds() : infiniteDuration);
    29372955        caAnimation->setFromValue(matrix);
    29382956        caAnimation->setToValue(matrix);
    29392957
    29402958        auto animation = LayerPropertyAnimation(WTFMove(caAnimation), "base-transform-" + createCanonicalUUIDString(), property, 0, 0, 0_s);
    2941         // To ensure the base value transform is applied along with all the interpolating animations, we set it to have started
    2942         // as early as possible, which combined with the infinite duration ensures it's current for any given CA media time,
    2943         // unless we're just filling until an animation for this property starts, in which case it must start now.
    2944         animation.m_beginTime = delay ? currentTime : baseTransformAnimationBeginTime;
    2945 
     2959        if (delay)
     2960            animation.m_beginTime = currentTime - animationGroupBeginTime;
     2961
     2962        m_baseValueTransformAnimations.append(WTFMove(animation));
     2963        return &m_baseValueTransformAnimations.last();
     2964    };
     2965
     2966    auto addBaseValueTransformAnimation = [&](AnimatedPropertyID property, TransformationMatrixSource matrixSource = TransformationMatrixSource::AskClient, Seconds beginTimeOfEarliestPropertyAnimation = 0_s) {
    29462967        // Additivity will depend on the source of the matrix, if it was explicitly provided as an identity matrix, it
    29472968        // is the initial base value transform animation and must override the current transform value for this layer.
    29482969        // Otherwise, it is meant to apply the underlying value for one specific transform-related property and be additive
    29492970        // to be combined with the other base value transform animations and interpolating animations.
    2950         addAnimation(animation, matrixSource == TransformationMatrixSource::AskClient ? Additive::Yes : Additive::No);
    2951         m_baseValueTransformAnimations.append(WTFMove(animation));
     2971        if (auto* animation = makeBaseValueTransformAnimation(property, matrixSource, beginTimeOfEarliestPropertyAnimation))
     2972            addAnimation(*animation, matrixSource == TransformationMatrixSource::AskClient ? Additive::Yes : Additive::No);
    29522973    };
    29532974
    2954     // Iterate through all animations to update each animation's begin time, if necessary,
    2955     // compute the base transform animation begin times and remove all running CA animations.
     2975    // Iterate through all animations to set the begin time of any new animations.
    29562976    for (auto& animation : m_animations) {
    2957         updateBeginTimes(animation);
    2958         if (animation.m_playState == PlayState::Playing || animation.m_playState == PlayState::Paused)
    2959             removeCAAnimationFromLayer(animation);
    2960     }
    2961 
    2962     // Also remove all the base value transform CA animations.
    2963     for (auto& animation : m_baseValueTransformAnimations)
    2964         removeCAAnimationFromLayer(animation);
    2965 
    2966     // Now remove all the animations marked as pending removal and all base value transform animations.
     2977        if (!animation.m_pendingRemoval && !animation.m_beginTime)
     2978            animation.m_beginTime = currentTime - animationGroupBeginTime;
     2979    }
     2980
     2981    // Now, remove all animation groups from the layer so that we no longer have any layer animations.
     2982    for (auto& animationGroup : m_animationGroups)
     2983        removeCAAnimationFromLayer(animationGroup);
     2984
     2985    // We can remove all previously-created base value transform animations and animation groups.
     2986    m_baseValueTransformAnimations.clear();
     2987    m_animationGroups.clear();
     2988
     2989    // Now remove all the animations marked as pending removal.
    29672990    m_animations.removeAllMatching([&](LayerPropertyAnimation animation) {
    29682991        return animation.m_pendingRemoval;
    29692992    });
    2970     m_baseValueTransformAnimations.clear();
    29712993
    29722994    // Now that our list of animations is current, we can separate animations by property so that
    … …  
    30503072            }
    30513073
    3052             if (earliestBeginTime > currentTime)
    3053                 addBaseValueTransformAnimation(property, TransformationMatrixSource::AskClient, earliestBeginTime);
    3054 
    3055             for (auto* animation : animations)
    3056                 addAnimation(*animation);
     3074            if (earliestBeginTime)
     3075                earliestBeginTime += animationGroupBeginTime;
     3076
     3077            if (earliestBeginTime > currentTime) {
     3078                if (auto* baseValueTransformAnimation = makeBaseValueTransformAnimation(property, TransformationMatrixSource::AskClient, earliestBeginTime)) {
     3079                    prepareAnimationForAddition(*baseValueTransformAnimation);
     3080                    caAnimations.append(baseValueTransformAnimation->m_animation);
     3081                }
     3082            }
     3083
     3084            for (auto* animation : animations) {
     3085                prepareAnimationForAddition(*animation);
     3086                caAnimations.append(animation->m_animation);
     3087            }
     3088
     3089            addAnimationGroup(property, caAnimations);
    30573090        };
    30583091
    … …  
    30693102
    30703103            Seconds earliestBeginTime = 0_s;
     3104            Vector<RefPtr<PlatformCAAnimation>> caAnimations;
    30713105            for (auto* animation : WTF::makeReversedRange(animations)) {
    30723106                if (auto beginTime = animation->computedBeginTime()) {
    … …  
    30743108                        earliestBeginTime = *beginTime;
    30753109                }
    3076                 addAnimation(*animation);
     3110                prepareAnimationForAddition(*animation);
     3111                caAnimations.append(animation->m_animation);
    30773112            }
    30783113
    3079             if (earliestBeginTime > currentTime)
    3080                 addBaseValueTransformAnimation(property, TransformationMatrixSource::AskClient, earliestBeginTime);
     3114            if (earliestBeginTime)
     3115                earliestBeginTime += animationGroupBeginTime;
     3116
     3117            if (earliestBeginTime > currentTime) {
     3118                if (auto* baseValueTransformAnimation = makeBaseValueTransformAnimation(property, TransformationMatrixSource::AskClient, earliestBeginTime)) {
     3119                    prepareAnimationForAddition(*baseValueTransformAnimation);
     3120                    caAnimations.append(baseValueTransformAnimation->m_animation);
     3121                }
     3122            }
     3123
     3124            addAnimationGroup(property, caAnimations);
    30813125        };
    30823126
    … …  
    44414485
    44424486    for (auto& animation : m_animations) {
    4443         auto caAnimation = animatedLayer(animation.m_property)->animationForKey(animation.animationIdentifier());
    4444         animations.append({ animatedPropertyIDAsString(animation.m_property), caAnimation->speed() });
     4487        if (auto caAnimation = animatedLayer(animation.m_property)->animationForKey(animation.animationIdentifier()))
     4488            animations.append({ animatedPropertyIDAsString(animation.m_property), caAnimation->speed() });
     4489        else
     4490            animations.append({ animatedPropertyIDAsString(animation.m_property), (animation.m_playState == PlayState::Playing || animation.m_playState == PlayState::PlayPending) ? 1 : 0 });
    44454491    }
    44464492

  • trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h

    r271699 r272201  
    475475        {
    476476            if (m_beginTime)
    477                 return m_beginTime - m_timeOffset;
     477                return *m_beginTime - m_timeOffset;
    478478            return WTF::nullopt;
    479479        }
    … …  
    485485        int m_subIndex;
    486486        Seconds m_timeOffset { 0_s };
    487         Seconds m_beginTime { 0_s };
     487        Optional<Seconds> m_beginTime;
    488488        PlayState m_playState { PlayState::PlayPending };
    489489        bool m_pendingRemoval { false };
    … …  
    617617    Vector<LayerPropertyAnimation> m_animations;
    618618    Vector<LayerPropertyAnimation> m_baseValueTransformAnimations;
     619    Vector<LayerPropertyAnimation> m_animationGroups;
    619620
    620621    Vector<FloatRect> m_dirtyRects;
Note: See TracChangeset for help on using the changeset viewer.