WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
35520
[Qt] GraphicsLayer: matrix interpolations in transform-animations don't behave correctly
https://bugs.webkit.org/show_bug.cgi?id=35520
Summary
[Qt] GraphicsLayer: matrix interpolations in transform-animations don't behav...
Noam Rosenthal
Reported
2010-03-01 03:53:27 PST
GraphicsLayer.h has an API that helps realizing whether AC should avoid taking control of the animation. The Mac implementation, for example, avoids > 180 deg. rotations. Right now Qt tries to do those rotations anyway, and it doesn't look right.
Attachments
Test page for testing transitions and animations with empty source list
(1.50 KB, text/html)
2010-03-03 01:56 PST
,
Kim Grönholm
no flags
Details
Test page for testing animations and transitions with same sized but mismatching transform lists
(1.62 KB, text/html)
2010-03-03 01:58 PST
,
Kim Grönholm
no flags
Details
Fixes animations behavior with AC to match non-AC in some special cases
(2.55 KB, patch)
2010-03-03 02:10 PST
,
Kim Grönholm
eric
: review-
Details
Formatted Diff
Diff
Fixes the transform animations by correctly deciding whether to do matrix interpolation or not
(3.03 KB, patch)
2010-03-18 00:04 PDT
,
Kim Grönholm
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Kim Grönholm
Comment 1
2010-03-03 01:56:41 PST
Created
attachment 49889
[details]
Test page for testing transitions and animations with empty source list The >180deg rotation animations don't work properly (meaning that they "take the shortest way") if the rotation is not interpolated as an angle value but as a transformation matrix. This also happens without AC. However, there were actually two bugs in determining whether to fall back to matrix interpolation in case of AC: 1. When the source transformation is empty, AC still falls to matrix interpolation even though it should not. 2. If the source and target transformationlists are of same size but mismatch, AC doesn't fall to matrix interpolation even though it should Then there also was a bug in the actual interpolation of matrices (target matrix was not taken as is but first combined with source matrix). I'll attach test cases for both situations and a patch to fix all the mentioned issues.
Kim Grönholm
Comment 2
2010-03-03 01:58:14 PST
Created
attachment 49890
[details]
Test page for testing animations and transitions with same sized but mismatching transform lists
Kim Grönholm
Comment 3
2010-03-03 02:10:02 PST
Created
attachment 49891
[details]
Fixes animations behavior with AC to match non-AC in some special cases Submitted a patch that solves the issues making the test cases behave similarly with AC than without AC.
Tor Arne Vestbø
Comment 4
2010-03-05 09:39:49 PST
Please follow the QtWebKit bug reporting guidelines when reporting bugs. See
http://trac.webkit.org/wiki/QtWebKitBugs
Specifically: - The 'QtWebKit' component should be used for bugs/features in the public QtWebKit API layer, not to signify that the bug is specific to the Qt port of WebKit
http://trac.webkit.org/wiki/QtWebKitBugs#Component
Eric Seidel (no email)
Comment 5
2010-03-05 15:14:47 PST
Comment on
attachment 49891
[details]
Fixes animations behavior with AC to match non-AC in some special cases Needs a test.
Noam Rosenthal
Comment 6
2010-03-16 13:58:34 PDT
This is a good test for >180 degree rotations:
http://qt.gitorious.org/qt-labs/scxml/blobs/raw/statechartz/statechartz/demo.html
We should test if the patch fixes this site, and doesn't regress anything else.
Kim Grönholm
Comment 7
2010-03-18 00:04:59 PDT
Created
attachment 51010
[details]
Fixes the transform animations by correctly deciding whether to do matrix interpolation or not
Noam Rosenthal
Comment 8
2010-03-18 00:49:24 PDT
Signed of by /me :)
Simon Hausmann
Comment 9
2010-03-21 15:13:26 PDT
Comment on
attachment 51010
[details]
Fixes the transform animations by correctly deciding whether to do matrix interpolation or not You might consider using WebCore/manual-tests or manual-tests/qt for manual tests. But we _really_ need layout tests for this code.
WebKit Commit Bot
Comment 10
2010-03-21 15:53:02 PDT
Comment on
attachment 51010
[details]
Fixes the transform animations by correctly deciding whether to do matrix interpolation or not Clearing flags on attachment: 51010 Committed
r56321
: <
http://trac.webkit.org/changeset/56321
>
WebKit Commit Bot
Comment 11
2010-03-21 15:53:06 PDT
All reviewed patches have been landed. Closing bug.
Kenneth Rohde Christiansen
Comment 12
2010-03-21 17:13:19 PDT
Comment on
attachment 51010
[details]
Fixes the transform animations by correctly deciding whether to do matrix interpolation or not
> + else > + for (size_t j = 0; j < sourceOperationCount && validTransformLists; ++j) > + if (!sourceOperations.operations()[j]->isSameType(*targetOperations.operations()[j])) > + validTransformLists = false;
Simon the patch you reviewed has wrong coding style. The else and for has contents that spans multiple real lines in total. Both the else and the for should use braces. The if doesn't not need braces, thus it is correct.
Noam Rosenthal
Comment 13
2010-03-21 19:49:39 PDT
ah, right Kenneth. Should I submit a style-fix patch for that and a few other style issues?
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug