Modified: trunk/Source/WebCore/ChangeLog (272200 => 272201)
--- trunk/Source/WebCore/ChangeLog 2021-02-02 10:25:52 UTC (rev 272200)
+++ trunk/Source/WebCore/ChangeLog 2021-02-02 10:59:00 UTC (rev 272201)
@@ -1,3 +1,44 @@
+2021-02-01 Antoine Quint <[email protected]>
+
+ 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.
+
+ 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):
+
2021-02-02 Youenn Fablet <[email protected]>
imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-stop.html is flaky when GPUProcess is enabled
Modified: trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp (272200 => 272201)
--- trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2021-02-02 10:25:52 UTC (rev 272200)
+++ trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2021-02-02 10:59:00 UTC (rev 272201)
@@ -2888,44 +2888,62 @@
void GraphicsLayerCA::updateAnimations()
{
- auto baseTransformAnimationBeginTime = 1_s;
+ // In order to guarantee that transform animations are applied in the expected order (translate, rotate, scale and transform),
+ // we need to have them wrapped individually in an animation group because Core Animation sorts animations first by their begin
+ // time, and then by the order in which they were added (for those with the same begin time). Since a rotate animation can have
+ // an earlier begin time than a translate animation, we cannot rely on adding the animations in the correct order.
+ //
+ // Having an animation group wrapping each animation means that we can guarantee the order in which animations are applied by
+ // ensuring they each have the same begin time. We set this begin time to be the smallest value possible, ensuring that base
+ // transform animations are applied continuously. We'll then set the begin time of interpolating animations to be local to the
+ // animation group, which means subtracting the group's begin time.
+
+ // We use 1_s here because 0_s would have special meaning in Core Animation, meaning that the animation would have its begin
+ // time set to the current time when it's committed.
+ auto animationGroupBeginTime = 1_s;
+ auto infiniteDuration = std::numeric_limits<double>::max();
auto currentTime = Seconds(CACurrentMediaTime());
- auto updateBeginTimes = [&](LayerPropertyAnimation& animation)
- {
- if (animation.m_pendingRemoval)
- return;
- // In case we have an offset, and we haven't set an explicit begin time previously,
- // we need to record the beginTime now.
- if (animation.m_timeOffset && !animation.m_beginTime)
- animation.m_beginTime = currentTime;
+ auto addAnimationGroup = [&](AnimatedPropertyID property, const Vector<RefPtr<PlatformCAAnimation>>& animations) {
+ auto caAnimationGroup = createPlatformCAAnimation(PlatformCAAnimation::Group, "");
+ caAnimationGroup->setDuration(infiniteDuration);
+ caAnimationGroup->setAnimations(animations);
- // Now check if we have a resolved begin time and ensure the begin time we'll use for
- // base transform animations matches the smallest known begin time to guarantee that
- // such animations can combine with other explicit transform animations correctly.
- if (auto computedBeginTime = animation.computedBeginTime())
- baseTransformAnimationBeginTime = std::min(baseTransformAnimationBeginTime, *computedBeginTime);
+ auto animationGroup = LayerPropertyAnimation(WTFMove(caAnimationGroup), "group-" + createCanonicalUUIDString(), property, 0, 0, 0_s);
+ animationGroup.m_beginTime = animationGroupBeginTime;
+
+ setAnimationOnLayer(animationGroup);
+ m_animationGroups.append(WTFMove(animationGroup));
};
enum class Additive { Yes, No };
- auto addAnimation = [&](LayerPropertyAnimation& animation, Additive additive = Additive::Yes) {
- animation.m_animation->setAdditive(additive == Additive::Yes);
- setAnimationOnLayer(animation);
+ auto prepareAnimationForAddition = [&](LayerPropertyAnimation& animation, Additive additive = Additive::Yes) {
+ auto caAnim = animation.m_animation;
+ caAnim->setAdditive(additive == Additive::Yes);
+ if (auto beginTime = animation.computedBeginTime())
+ caAnim->setBeginTime(beginTime->seconds());
+
if (animation.m_playState == PlayState::PausePending || animation.m_playState == PlayState::Paused) {
- pauseCAAnimationOnLayer(animation);
+ caAnim->setSpeed(0);
+ caAnim->setTimeOffset(animation.m_timeOffset.seconds());
animation.m_playState = PlayState::Paused;
} else
animation.m_playState = PlayState::Playing;
};
+ auto addAnimation = [&](LayerPropertyAnimation& animation, Additive additive = Additive::Yes) {
+ prepareAnimationForAddition(animation, additive);
+ addAnimationGroup(animation.m_property, { animation.m_animation });
+ };
+
enum class TransformationMatrixSource { UseIdentityMatrix, AskClient };
- auto addBaseValueTransformAnimation = [&](AnimatedPropertyID property, TransformationMatrixSource matrixSource = TransformationMatrixSource::AskClient, Seconds beginTimeOfEarliestPropertyAnimation = 0_s) {
+ auto makeBaseValueTransformAnimation = [&](AnimatedPropertyID property, TransformationMatrixSource matrixSource = TransformationMatrixSource::AskClient, Seconds beginTimeOfEarliestPropertyAnimation = 0_s) -> LayerPropertyAnimation* {
// A base value transform animation can either be set to the identity matrix or to read the underlying
// value from the GraphicsLayerClient. If we didn't explicitly ask for an identity matrix, we can skip
// the addition of this base value transform animation since it will be a no-op.
auto matrix = matrixSource == TransformationMatrixSource::UseIdentityMatrix ? TransformationMatrix() : client().transformMatrixForProperty(property);
if (matrixSource == TransformationMatrixSource::AskClient && matrix.isIdentity())
- return;
+ return nullptr;
auto delay = beginTimeOfEarliestPropertyAnimation > currentTime ? beginTimeOfEarliestPropertyAnimation - currentTime : 0_s;
@@ -2933,41 +2951,45 @@
// unless we're just filling until an animation for this property starts, in which case it must last for duration
// of the delay until that animation.
auto caAnimation = createPlatformCAAnimation(PlatformCAAnimation::Basic, propertyIdToString(property));
- caAnimation->setDuration((delay ? delay : Seconds::infinity()).seconds());
+ caAnimation->setDuration(delay ? delay.seconds() : infiniteDuration);
caAnimation->setFromValue(matrix);
caAnimation->setToValue(matrix);
auto animation = LayerPropertyAnimation(WTFMove(caAnimation), "base-transform-" + createCanonicalUUIDString(), property, 0, 0, 0_s);
- // To ensure the base value transform is applied along with all the interpolating animations, we set it to have started
- // as early as possible, which combined with the infinite duration ensures it's current for any given CA media time,
- // unless we're just filling until an animation for this property starts, in which case it must start now.
- animation.m_beginTime = delay ? currentTime : baseTransformAnimationBeginTime;
+ if (delay)
+ animation.m_beginTime = currentTime - animationGroupBeginTime;
+ m_baseValueTransformAnimations.append(WTFMove(animation));
+ return &m_baseValueTransformAnimations.last();
+ };
+
+ auto addBaseValueTransformAnimation = [&](AnimatedPropertyID property, TransformationMatrixSource matrixSource = TransformationMatrixSource::AskClient, Seconds beginTimeOfEarliestPropertyAnimation = 0_s) {
// Additivity will depend on the source of the matrix, if it was explicitly provided as an identity matrix, it
// is the initial base value transform animation and must override the current transform value for this layer.
// Otherwise, it is meant to apply the underlying value for one specific transform-related property and be additive
// to be combined with the other base value transform animations and interpolating animations.
- addAnimation(animation, matrixSource == TransformationMatrixSource::AskClient ? Additive::Yes : Additive::No);
- m_baseValueTransformAnimations.append(WTFMove(animation));
+ if (auto* animation = makeBaseValueTransformAnimation(property, matrixSource, beginTimeOfEarliestPropertyAnimation))
+ addAnimation(*animation, matrixSource == TransformationMatrixSource::AskClient ? Additive::Yes : Additive::No);
};
- // Iterate through all animations to update each animation's begin time, if necessary,
- // compute the base transform animation begin times and remove all running CA animations.
+ // Iterate through all animations to set the begin time of any new animations.
for (auto& animation : m_animations) {
- updateBeginTimes(animation);
- if (animation.m_playState == PlayState::Playing || animation.m_playState == PlayState::Paused)
- removeCAAnimationFromLayer(animation);
+ if (!animation.m_pendingRemoval && !animation.m_beginTime)
+ animation.m_beginTime = currentTime - animationGroupBeginTime;
}
- // Also remove all the base value transform CA animations.
- for (auto& animation : m_baseValueTransformAnimations)
- removeCAAnimationFromLayer(animation);
+ // Now, remove all animation groups from the layer so that we no longer have any layer animations.
+ for (auto& animationGroup : m_animationGroups)
+ removeCAAnimationFromLayer(animationGroup);
- // Now remove all the animations marked as pending removal and all base value transform animations.
+ // We can remove all previously-created base value transform animations and animation groups.
+ m_baseValueTransformAnimations.clear();
+ m_animationGroups.clear();
+
+ // Now remove all the animations marked as pending removal.
m_animations.removeAllMatching([&](LayerPropertyAnimation animation) {
return animation.m_pendingRemoval;
});
- m_baseValueTransformAnimations.clear();
// Now that our list of animations is current, we can separate animations by property so that
// we can apply them in order. We only need to apply the last animation applied for a given
@@ -3049,11 +3071,22 @@
}
}
- if (earliestBeginTime > currentTime)
- addBaseValueTransformAnimation(property, TransformationMatrixSource::AskClient, earliestBeginTime);
+ if (earliestBeginTime)
+ earliestBeginTime += animationGroupBeginTime;
- for (auto* animation : animations)
- addAnimation(*animation);
+ if (earliestBeginTime > currentTime) {
+ if (auto* baseValueTransformAnimation = makeBaseValueTransformAnimation(property, TransformationMatrixSource::AskClient, earliestBeginTime)) {
+ prepareAnimationForAddition(*baseValueTransformAnimation);
+ caAnimations.append(baseValueTransformAnimation->m_animation);
+ }
+ }
+
+ for (auto* animation : animations) {
+ prepareAnimationForAddition(*animation);
+ caAnimations.append(animation->m_animation);
+ }
+
+ addAnimationGroup(property, caAnimations);
};
addAnimationsForProperty(translateAnimations, AnimatedPropertyTranslate);
@@ -3068,16 +3101,27 @@
}
Seconds earliestBeginTime = 0_s;
+ Vector<RefPtr<PlatformCAAnimation>> caAnimations;
for (auto* animation : WTF::makeReversedRange(animations)) {
if (auto beginTime = animation->computedBeginTime()) {
if (!earliestBeginTime || earliestBeginTime > *beginTime)
earliestBeginTime = *beginTime;
}
- addAnimation(*animation);
+ prepareAnimationForAddition(*animation);
+ caAnimations.append(animation->m_animation);
}
- if (earliestBeginTime > currentTime)
- addBaseValueTransformAnimation(property, TransformationMatrixSource::AskClient, earliestBeginTime);
+ if (earliestBeginTime)
+ earliestBeginTime += animationGroupBeginTime;
+
+ if (earliestBeginTime > currentTime) {
+ if (auto* baseValueTransformAnimation = makeBaseValueTransformAnimation(property, TransformationMatrixSource::AskClient, earliestBeginTime)) {
+ prepareAnimationForAddition(*baseValueTransformAnimation);
+ caAnimations.append(baseValueTransformAnimation->m_animation);
+ }
+ }
+
+ addAnimationGroup(property, caAnimations);
};
addAnimationsForProperty(transformAnimations, AnimatedPropertyTransform);
@@ -4440,8 +4484,10 @@
Vector<std::pair<String, double>> animations;
for (auto& animation : m_animations) {
- auto caAnimation = animatedLayer(animation.m_property)->animationForKey(animation.animationIdentifier());
- animations.append({ animatedPropertyIDAsString(animation.m_property), caAnimation->speed() });
+ if (auto caAnimation = animatedLayer(animation.m_property)->animationForKey(animation.animationIdentifier()))
+ animations.append({ animatedPropertyIDAsString(animation.m_property), caAnimation->speed() });
+ else
+ animations.append({ animatedPropertyIDAsString(animation.m_property), (animation.m_playState == PlayState::Playing || animation.m_playState == PlayState::PlayPending) ? 1 : 0 });
}
return animations;
