Bug 236397 - [css-transforms] transform animation on SVG element jumps as it ends
Summary: [css-transforms] transform animation on SVG element jumps as it ends
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Animations (show other bugs)
Version: Safari 15
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-02-09 12:26 PST by halifirien
Modified: 2022-02-10 12:29 PST (History)
7 users (show)

See Also:


Attachments
matrix-transform-jump-demo (2.31 KB, text/html)
2022-02-09 12:26 PST, halifirien
no flags Details
Slight reduced test (1.31 KB, text/html)
2022-02-10 09:57 PST, Antoine Quint
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description halifirien 2022-02-09 12:26:45 PST
Created attachment 451425 [details]
matrix-transform-jump-demo

using animate on svg <g> with matrix transform keyframes. It completes the animation to the wrong position - and then instantaneously jumps to the correct position.
Comment 1 Radar WebKit Bug Importer 2022-02-10 09:11:32 PST
<rdar://problem/88758676>
Comment 2 Antoine Quint 2022-02-10 09:57:24 PST
Created attachment 451558 [details]
Slight reduced test

I've reduced the test a bit to not visibly interpolate.
Comment 3 Antoine Quint 2022-02-10 11:42:00 PST
RenderStyle::applyTransform() gets called once at the beginning of the animation and once at the end. Logging transformOperations shows that it differs between the two calls while originTranslate is a zero point.
Comment 4 Antoine Quint 2022-02-10 11:52:34 PST
The first set of transformOperations doesn't match what is being provided to the animate() call.
Comment 5 Antoine Quint 2022-02-10 12:09:42 PST
The call to CSSPropertyAnimation::blendProperties() in KeyframeEffect::setAnimatedPropertiesInStyle() takes two identical transforms which are correct and generates an incorrect one.
Comment 6 Antoine Quint 2022-02-10 12:20:42 PST
This simple diff fixes the bug:

diff --git a/Source/WebCore/animation/CSSPropertyAnimation.cpp b/Source/WebCore/animation/CSSPropertyAnimation.cpp
index 217e768c131d..f7338e6ccc40 100644
--- a/Source/WebCore/animation/CSSPropertyAnimation.cpp
+++ b/Source/WebCore/animation/CSSPropertyAnimation.cpp
@@ -171,6 +171,9 @@ static inline TransformOperations blendFunc(const TransformOperations& from, con
         return resultOperations;
     }
 
+    if (from == to)
+        return from;
+
     if (context.client->transformFunctionListsMatch())
         return to.blendByMatchingOperations(from, context);
     return to.blendByUsingMatrixInterpolation(from, context, is<RenderBox>(context.client->renderer()) ? downcast<RenderBox>(*context.client->renderer()).borderBoxRect().size() : LayoutSize());

But I'm not sure if this is the best fix. On the one hand, this is probably a good thing to do in general, but it also feels like this might be hiding an issue further down in the blending code where we would also yield an incorrect result with non-equal matrices.
Comment 7 Antoine Quint 2022-02-10 12:27:36 PST
Also, this doesn't fix the original test.