Branch: refs/heads/main Home: https://github.com/WebKit/WebKit Commit: 5a8f69dc67ce414cca0c9575080c73d858a603bc https://github.com/WebKit/WebKit/commit/5a8f69dc67ce414cca0c9575080c73d858a603bc Author: Nikolas Zimmermann <nzimmerm...@igalia.com> 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 webkit-changes@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-changes