Summary: | [Qt] GraphicsLayer: matrix interpolations in transform-animations don't behave correctly | ||
---|---|---|---|
Product: | WebKit | Reporter: | Noam Rosenthal <noam> |
Component: | New Bugs | Assignee: | Noam Rosenthal <noam> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | commit-queue, kent.hansen |
Priority: | P2 | Keywords: | Qt |
Version: | 528+ (Nightly build) | ||
Hardware: | PC | ||
OS: | All | ||
Attachments: |
Description
Noam Rosenthal
2010-03-01 03:53:27 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.
Created attachment 49890 [details]
Test page for testing animations and transitions with same sized but mismatching transform lists
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.
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 Comment on attachment 49891 [details]
Fixes animations behavior with AC to match non-AC in some special cases
Needs a test.
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. Created attachment 51010 [details]
Fixes the transform animations by correctly deciding whether to do matrix interpolation or not
Signed of by /me :) 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.
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> All reviewed patches have been landed. Closing bug. 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. ah, right Kenneth. Should I submit a style-fix patch for that and a few other style issues? |