Bug 20667 - Animation of -webkit-transform matrix() function should not do linear interpolation
Summary: Animation of -webkit-transform matrix() function should not do linear interpo...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-09-05 08:59 PDT by Chris Marrin
Modified: 2008-09-08 15:52 PDT (History)
2 users (show)

See Also:


Attachments
Patch to fix bug (9.86 KB, patch)
2008-09-05 09:02 PDT, Chris Marrin
sam: review-
Details | Formatted Diff | Diff
New patch fixing nits (9.84 KB, patch)
2008-09-05 16:36 PDT, Chris Marrin
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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