Bug 73830 – optimize TransformationMatrix::scale by not calling through to generic multiply

• Component: New Bugs
• Status: RESOLVED FIXED

[Mike Reed]

optimize TransformationMatrix::scale by not calling through to generic multiply

[Mike Reed]

Created attachment 117880 [details]
Patch

[WebKit Review Bot]

Comment on attachment 117880 [details]
Patch

Attachment 117880 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10731720

New failing tests:
svg/custom/linking-uri-01-b.svg

[Kenneth Russell]

Comment on attachment 117880 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=117880&action=review

Is the new SVG failure spurious? It's hard to believe that it could be caused by this patch.

> Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp:695
> +    m_matrix[0][0] *= sx;
> +    m_matrix[0][1] *= sx;
> +    m_matrix[0][2] *= sx;
> +    m_matrix[0][3] *= sx;
> +    
> +    m_matrix[1][0] *= sy;
> +    m_matrix[1][1] *= sy;
> +    m_matrix[1][2] *= sy;
> +    m_matrix[1][3] *= sy;

Could this one at least call scaleNonUniform to reduce code duplication? Would that eliminate the benefit of the optimization?

[Mike Reed]

I'll run drt locally and see if I can repro the failure. *possibly* we are getting different precision with this version, and therefore a slightly different matrix?

I think we can have the scale3 function call the 2D one, and then scale its column. I'll try that in the next patch.

[Mike Reed]

Created attachment 118031 [details]
Patch

[Mike Reed]

previous failure must have been a flake, as the new patch is the same (modulo sharing code between the scaleNonUniform and scale3D)

[Kenneth Russell]

Comment on attachment 118031 [details]
Patch

Looks good. r=me

[WebKit Review Bot]

Comment on attachment 118031 [details]
Patch

Clearing flags on attachment: 118031

Committed r102157: <http://trac.webkit.org/changeset/102157>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

Why were none of the main authors of TransformationMatrix.cpp cc'd on this bug?

[Kenneth Russell]

(In reply to comment #10)
> Why were none of the main authors of TransformationMatrix.cpp cc'd on this bug?

Sorry, that was a simple oversight on my part...

Please let us know if you find any issue with this change. I'm happy to roll it out and help rework it if necessary.

[Mike Reed]

My bad too. Is there a generic way to know who the interested authors are for a given directory/file?

[Mike Reed]

using 'svn blame' I see cmarrin and simon. Will remember to do that next time.