Bug 64640 – Add optimizations to TransformationMatrix for matrices that are identity or only have translation values

• Status: NEW
• Component: New Bugs

[Chris Marrin]

Add optimizations to TransformationMatrix for matrices that are identity or only have translation values

[Chris Marrin]

Created attachment 101071 [details]
Patch

[WebKit Review Bot]

Attachment 101071 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/platform/graphics/transforms/TransformationMatrix.h:353:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/platform/graphics/transforms/TransformationMatrix.h:357:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 2 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Fraser (smfr)]

Comment on attachment 101071 [details]
Patch

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

I think we could get more optimizations here.

> Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp:650
> +    m_identityType = IdentityUnknown;

If the angle is 0, the type shouldn't change, right?

> Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp:739
> +    m_identityType = IdentityUnknown;

Optimize for no-op rotation?

> Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp:814
> +    m_identityType = IdentityUnknown;

Seems like you should be able to deduce the new identity type easily.

> Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp:825
> +    m_identityType = IdentityUnknown;

Ditto.

> Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp:836
> +    m_identityType = IdentityUnknown;

Ditto.

> Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp:907
> +    m_identityType = IdentityUnknown;

Test the identity type of the incoming matrix?

> Source/WebCore/platform/graphics/transforms/TransformationMatrix.h:68
> +    enum IdentityType { IdentityUnknown, IdentityYes, IdentityTranslation, IdentityNo };

I'm not sure "Identity" is the right prefix. IdentityTranslation is nonsensical, and IdentityNo is hard to parse. Maybe

MatrixType { Unknown, Identity, Translation, Other }

> Source/WebCore/platform/graphics/transforms/TransformationMatrix.h:133
> +        return isIdentity() || m_identityType == IdentityTranslation;

It's slightly subtle that the call to isIdentity() calls determineIdentityType(). Maybe have a determineIdentityTypeIfNecessary() method?

> Source/WebCore/platform/graphics/transforms/TransformationMatrix.h:376
> +    IdentityType m_identityType;

I think this could be mutable to avoid the nasty const casts.