Bug 52780 - TransformationMatrix multiply operations apply operands in wrong order.
Summary: TransformationMatrix multiply operations apply operands in wrong order.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Shane Stephens
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-19 20:45 PST by Shane Stephens
Modified: 2011-01-24 11:42 PST (History)
6 users (show)

See Also:


Attachments
Patch (14.45 KB, patch)
2011-01-19 20:48 PST, Shane Stephens
no flags Details | Formatted Diff | Diff
Patch (24.01 KB, patch)
2011-01-20 19:51 PST, Shane Stephens
no flags Details | Formatted Diff | Diff
Patch (24.59 KB, patch)
2011-01-23 16:18 PST, Shane Stephens
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Shane Stephens 2011-01-19 20:45:56 PST
TransformationMatrix multiply operations apply operands in wrong order.
Comment 1 Shane Stephens 2011-01-19 20:48:13 PST
Created attachment 79547 [details]
Patch
Comment 2 WebKit Review Bot 2011-01-19 21:15:04 PST
Attachment 79547 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7578232
Comment 3 Early Warning System Bot 2011-01-19 21:18:08 PST
Attachment 79547 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7515234
Comment 4 Shane Stephens 2011-01-20 19:51:14 PST
Created attachment 79686 [details]
Patch
Comment 5 Chris Marrin 2011-01-21 10:32:18 PST
Comment on attachment 79686 [details]
Patch

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"
Comment 6 Shane Stephens 2011-01-23 16:18:52 PST
Created attachment 79882 [details]
Patch
Comment 7 WebKit Commit Bot 2011-01-24 11:41:30 PST
The commit-queue encountered the following flaky tests while processing attachment 79882 [details]:

animations/dynamic-stylesheet-loading.html bug 52669 (author: cmarrin@apple.com)
The commit-queue is continuing to process your patch.
Comment 8 WebKit Commit Bot 2011-01-24 11:42:15 PST
Comment on attachment 79882 [details]
Patch

Clearing flags on attachment: 79882

Committed r76537: <http://trac.webkit.org/changeset/76537>
Comment 9 WebKit Commit Bot 2011-01-24 11:42:21 PST
All reviewed patches have been landed.  Closing bug.