Bug 20667

Summary: Animation of -webkit-transform matrix() function should not do linear interpolation
Product: WebKit Reporter: Chris Marrin <cmarrin>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
Patch to fix bug
sam: review-
New patch fixing nits sam: review+

Description Chris Marrin 2008-09-05 08:59:05 PDT
Currently, the 6 components of the matrix() function are linearly interpolated. This causes simple rotations to grow and shrink as they animate and other problems. We should do decomposition of the matrix into scale, rotation, skew and translation components, then linearly interpolated, then recomposed into an affine transform.
Comment 1 Chris Marrin 2008-09-05 09:02:33 PDT
Created attachment 23195 [details]
Patch to fix bug
Comment 2 Sam Weinig 2008-09-05 10:33:49 PDT
Comment on attachment 23195 [details]
Patch to fix bug

Just style nits.

+    double sx, sy;
+    double angle;
We don't usually pre declare variables.

+    /* Compute scaling factors. */
We tend to use C++ style comments.  This issue is in a few 

+    sx = sqrt (m.a() * m.a() + m.b() * m.b());
+    sy = sqrt (m.c() * m.c() + m.d() * m.d());
Space after sqrt.

+    angle = atan2 (m.b(), m.a());
Space after atan2.

+    int i;
No need to pre declare this.

+        srA[2] += srA[2] < 0 ? M_PI : -M_PI;
+    }
+
+    // Don't rotate the long way around.
+    srA[2] = fmod(srA[2], 2.0 * M_PI);
+    srB[2] = fmod(srB[2], 2.0 * M_PI);
+
+    if (fabs (srA[2] - srB[2]) > M_PI) {
+        if (srA[2] > srB[2])
+            srA[2] -= M_PI * 2.0;
+        else
+            srB[2] -= M_PI * 2.0;
+    }
We use piDouble/piFloat instead of M_PI
Comment 3 Chris Marrin 2008-09-05 16:36:52 PDT
Created attachment 23206 [details]
New patch fixing nits
Comment 4 Sam Weinig 2008-09-07 12:59:49 PDT
Comment on attachment 23206 [details]
New patch fixing nits

Still one space after function name.
+    if (fabs (srA[2] - srB[2]) > piDouble) {

Please fix that before landing. r=me
Comment 5 Dean Jackson 2008-09-08 15:47:24 PDT
	M	LayoutTests/ChangeLog
	A	LayoutTests/animations/matrix-anim-expected.txt
	A	LayoutTests/animations/matrix-anim.html
	M	WebCore/ChangeLog
	M	WebCore/platform/graphics/AffineTransform.cpp
	M	WebCore/platform/graphics/AffineTransform.h
	M	WebCore/rendering/style/RenderStyle.cpp
Committed r36274

Comment 6 Dean Jackson 2008-09-08 15:52:34 PDT
forgot to fix sam's comment.

	M	WebCore/ChangeLog
	M	WebCore/platform/graphics/AffineTransform.cpp
Committed r36275