Summary: | Provide 2D Matrix decomposition for animation | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Rik Cabanier <cabanier> | ||||||||||||
Component: | CSS | Assignee: | Dean Jackson <dino> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | cmarrin, commit-queue, dino, esprehn+autocc, glenn, kondapallykalyan, krit, simon.fraser, webkit-bug-importer | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Mac | ||||||||||||||
OS: | OS X 10.8 | ||||||||||||||
URL: | http://jsfiddle.net/56T5V/light/ | ||||||||||||||
Attachments: |
|
Description
Rik Cabanier
2013-03-20 11:10:38 PDT
Created attachment 194089 [details]
test file
Created attachment 196277 [details]
Not for review.
Have to add test file.
Comment on attachment 196277 [details]
Not for review.
Have to add test file.
The patch looks really interesting. Do you think you can add more descriptions what you changed and how it affected the behavior? Can you add some tests please?
(In reply to comment #3) > (From update of attachment 196277 [details]) > The patch looks really interesting. Do you think you can add more descriptions what you changed and how it affected the behavior? Can you add some tests please? Yeah. It's not ready for review yet. I have to think a bit and maybe simplify the code. I also need to check if z-animations match Created attachment 212654 [details]
Nearly ready patch
Patch mostly works. I think there is a typo somewhere.
Not for review.
Basically, swap to a 2d decomposition if we know we're 2d.
Created attachment 212777 [details]
Patch
Created attachment 212817 [details]
Patch
Comment on attachment 212817 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=212817&action=review > Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp:298 > +static bool decompose2(const TransformationMatrix::Matrix4& mat, TransformationMatrix::Decomposed2Type& result) Nit: is there any reason not to spell out "matrix" completely? > Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp:308 > + Nit: you can lose the extra blank line. > Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp:324 > + // Renormalize matrix to remove scale. > + > + if (result.scaleX) { Nit: you don't need the extra blank line. > Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp:335 > + // Compute rotation and renormalize matrix. > + > + result.angle = atan2(row0y, row0x); Ditto. > Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp:342 > + // Thanks to the normalization above. > + > + double sn = -row0y; Ditto. > Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp:1528 > + if (!WebCore::decompose2(m_matrix, decomp)) > + return false; > + return true; Nit: this can be just "return WebCore::decompose2(m_matrix, decomp)" > Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp:1543 > + if (!WebCore::decompose4(m_matrix, decomp)) > return false; > return true; And this can be "return WebCore::decompose4(m_matrix, decomp)" Comment on attachment 212817 [details]
Patch
Fixed all comments. Thanks.
Committed r156553: <http://trac.webkit.org/changeset/156553> |