[webkit-reviews] review granted: [Bug 52780] TransformationMatrix multiply operations apply operands in wrong order. : [Attachment 79686] Patch

[bugzilla-daemon at webkit.org]

Chris Marrin <cmarrin at apple.com> has granted Shane Stephens
<shanestephens at google.com>'s request for review:
Bug 52780: TransformationMatrix multiply operations apply operands in wrong
order.
https://bugs.webkit.org/show_bug.cgi?id=52780

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

------- Additional Comments from Chris Marrin <cmarrin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=79686&action=review

r=me with a bit more detail in the Changelog.

> Source/WebCore/ChangeLog:10
> +

Could you be a bit more specific about the change? You're fixing a very
confusing part of the code and think you're doing two things. One is flipping
the order used to combine matrix, the other is changing multLeft to multiply.
This is really doing 2 things: it fixes the incorrect naming of the function
being called implicitly flipping the order of the operation. All that gets
buried in the name change, so if you could add another sentence or two here, it
would make it more clear to future coders.

> Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp:61
> +// the array refers to the column that the element lies in; the second the
row.

This last sentence is awkward. either say "the second, the row", or better "the
second index refers to the row"