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.
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?