[Webkit-unassigned] [Bug 48031] AffineTransform operator* reverses order of operands.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=48031





--- Comment #14 from Nikolas Zimmermann <zimmermann at kde.org>  2010-12-26 03:33:15 PST ---
(In reply to comment #12)
> Yes, this test is failing locally and it's probably failing as a result of 48215.  Unfortunately the bug I fixed in 48215 introduced another bug that's now evident in LayoutTests/svg/W3C-SVG-1.1/animate-elem-24-t.svg :(
Okay.

> 
> The messy details:
> 
> SVGStyledTransformableElement instances have a TransformList and a supplementalTransform.  While AnimateTransform elements update values in the TransformList, AnimateMotion elements update the supplementalTransform.  Crucially, transform values provided in the transform attribute of an element also appear in the TransformList.
Yes, in my opinion the supplementalTransform stuff is a huge hack anyway, as it forces each element transformable element to store another AffineTransform object, even when not animated. It was a crude hack, when animation was implemented in the beginning, to get Acid3 going...

> 
> The order in which these matrices are accumulated matters - for example, a rotate followed by a translate does not produce the same result as a translate followed by a rotate.
> 
> The problem that I fixed in 48215 was caused by the AnimateMotion's transform being applied first, before anything in the TransformList; whereas the specification indicates that this transform should be applied after values in the transform attribute.
Right.

> 
> Unfortunately, what I did was move the application of AnimateMotion to the end - i.e. after everything in the TransformList.  From the animate-elem-24-t.svg test, it appears that this is also wrong.  In fact, it's possible that the supplementalTransform needs to apply after some TransformList values but before others, in which case it might make sense to remove the supplementalTransform altogether and use the TransformList for AnimateMotion as well.  I'll read the specification carefully and seek guidance on the svg-wg list if necessary to determine what's appropriate.
Great. I think you should investigate to use TransformList directly. When we have animVal support, this is the way to go anyways. (transformList.animVal needs to reflect any animation done by animateTransform and/or animateMotion).

> 
> This current bug, however, is unrelated in that it doesn't (or at least, shouldn't) change the behaviour of any of the code - it's simply about fixing the order of operands in matrix multiplication.  So I'd like to propose filing the fact that animate-elem-24-t.svg fails as a separate bug. 
Fine with me! I'll review this patch soon.

> While we're at it, it seems like we should be modifying or copying any animation tests in this directory so that we have versions that actually work with run-webkit-tests.  If this test was actually testing when I was working on 48215 I probably would have picked up the issue then.
That would be great! You'd need to copy the tests from svg/W3C-SVG-1.1/animate* to svg/animations, and extend them to use the SVG Animation snapshot API.
But it will take a huge amount of time to convert them all to use the DRT snapshot API, as you have to write JS code for each sampling point, checking the values, whether they are correct etc etc.
If you want to work on that, it would be highly appreciated!

Cheers,
Niko

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.