Bug 112824 – Provide 2D Matrix decomposition for animation

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Rik Cabanier]

When you get the matrix of an object that is animated with matrices, it sometimes doesn't match what Core Animation calculated,
The attached test file shows the problem. Open the HTML, and hover over any element in row 2 or 3. The matrix of the hovered elememt is applied to the bottom object.
You will see that it doesn't match the rendering.

This also affects hit testing. If you hover over an object, the hit testing is done using the internal animation. Since the internal one scale to a point, you sometimes don't get a hit.

[Rik Cabanier]

Created attachment 194089 [details]
test file

[Rik Cabanier]

Created attachment 196277 [details]
Not for review.
Have to add test file.

[Dirk Schulze]

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?

[Rik Cabanier]

(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

[Dean Jackson]

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.

[Radar WebKit Bug Importer]

<rdar://problem/15091882>

[Dean Jackson]

Created attachment 212777 [details]
Patch

[Dean Jackson]

Created attachment 212817 [details]
Patch

[Eric Carlson]

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)"

[Dean Jackson]

Comment on attachment 212817 [details]
Patch

Fixed all comments. Thanks.

[Dean Jackson]

Committed r156553: <http://trac.webkit.org/changeset/156553>