[webkit-reviews] review granted: [Bug 112824] Provide 2D Matrix decomposition for animation : [Attachment 212817] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Dean Jackson
<dino at apple.com>'s request for review:
Bug 112824: Provide 2D Matrix decomposition for animation
https://bugs.webkit.org/show_bug.cgi?id=112824

Attachment 212817: Patch
https://bugs.webkit.org/attachment.cgi?id=212817&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
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)"