Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 5a8f69dc67ce414cca0c9575080c73d858a603bc
https://github.com/WebKit/WebKit/commit/5a8f69dc67ce414cca0c9575080c73d858a603bc
Author: Nikolas Zimmermann <[email protected]>
Date: 2023-10-06 (Fri, 06 Oct 2023)
Changed paths:
M LayoutTests/platform/mac-ventura-wk2-lbse-text/TestExpectations
M Source/WebCore/svg/properties/SVGAnimatedPropertyAnimator.h
Log Message:
-----------
Fix reset to baseVal after animation end for SVGTransformList
https://bugs.webkit.org/show_bug.cgi?id=249140
Reviewed by Rob Buis.
Partly revert the fix from webkit.org/b/198576 (REGRESSION (r243121): Load event
should not be fired while animating the 'externalResourcesRequired' attribute),
that landed in https://commits.webkit.org/212621@main.
First of all, externalResourcesRequired support is gone from WebKit, therefore
SMIL animations of that property are no longer possible: the workaround is
obsolete.
More importantly, the applied workaround is harmful for SVGTransformList
animations,
which go through the same SVGAnimatedPropertyAnimator code paths to apply
changes,
when <animateTransform> animations are active.
The now-reverted patch changed the order of two calls, when applying property
animations. Before the patch, stopAnimation() was called first, then
applyAnimatedPropertyChange() (which calls svgAttributeChanged()).
SVGAnimatedPropertyList::stopAnimation() first calls
SVGAnimatedProperty::stopAnimation(),
and then resets m_animVal to m_baseVal (#). The
SVGAnimatedProperty::stopAnimation() code
removes the 'SVGAttributeAnimator' object from the m_animators hash set. If
there was
only one animation running, isAnimating() would now return false. Thus when
calling
stopAnimation() first and applyAnimatedPropertyChanged() afterwards, one cannot
query
anymore if the svgAttributeChanged() origin is a SMIL animation or not (that
kind of
information was needed by SVGExternalResourcesRequired support, and nowhere
else).
However it is guaranteed that the "m_animVal" is reset to the "m_baseVal"
_BEFORE_
calling svgAttributeChanged(), which potentially triggers repaints / relayouts
etc.
For the legacy SVG engine this imposes no problem: svgAttributeChanged() _marks_
the renderer for a transform update (renderer()->setNeedsTransformUpdate()) and
triggers
an async relayout. Next time layout is updated, the correct values are used to
update
and render the scene (property now reflect 'baseVal' visually, animVal was
reset).
For LBSE we only want to update the layer transform and repaint, avoiding any
relayout.
Therefore the current behaviour in ToT, first calling
applyAnimatedPropertyChange()
(which calls svgAttributeChanged()) and then stopAnimation() is harmful.
When svgAttributeChanged() is called, the SVGTransformList state reflects the
_LAST_
animation progress value that was calculated. That state is used to update the
layer
transform and trigger a repaint. Just afterwards stopAnimation() reset the
animVal
to the baseVal (e.g. back to identity matrix, ...) -- but since we no longer
trigger
any async relayout from svgAttributeChanged(), nobody is going to pick up that
change
and redraw the scene.
The fix is simple: revert the order again, and things are fine.
Solves the recently induced regression in
svg/animations/list-wrapper-assertion.svg.
* LayoutTests/platform/mac-ventura-wk2-lbse-text/TestExpectations: Remove
svg/animations/list-wrapper-assertion.svg failure expectation.
* Source/WebCore/svg/properties/SVGAnimatedPropertyAnimator.h: Call
stopAnimation() before applyAnimatedPropertyChange().
Canonical link: https://commits.webkit.org/268987@main
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
