Bug 105758 - [TexMap] A Minor optimization of GraphicsLayerTransform.
Summary: [TexMap] A Minor optimization of GraphicsLayerTransform.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dongseong Hwang
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-26 00:45 PST by Dongseong Hwang
Modified: 2012-12-26 16:40 PST (History)
3 users (show)

See Also:


Attachments
Patch (2.63 KB, patch)
2012-12-26 00:48 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch (2.76 KB, patch)
2012-12-26 15:52 PST, Dongseong Hwang
dongseong.hwang: review+
dongseong.hwang: commit-queue-
Details | Formatted Diff | Diff
Patch (2.78 KB, patch)
2012-12-26 16:22 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dongseong Hwang 2012-12-26 00:45:24 PST
Don't combine transforms if GraphicsLayerTransform isn't changed actually.
Comment 1 Dongseong Hwang 2012-12-26 00:48:03 PST
Created attachment 180733 [details]
Patch
Comment 2 Noam Rosenthal 2012-12-26 15:09:42 PST
Comment on attachment 180733 [details]
Patch

This is redundant; We already perform those checks in GraphicsLayerTextureMapper.
Comment 3 Dongseong Hwang 2012-12-26 15:22:36 PST
(In reply to comment #2)
> (From update of attachment 180733 [details])
> This is redundant; We already perform those checks in GraphicsLayerTextureMapper.

But TextureMapperLayer::flushCompositingStateForThisLayerOnly can change GraphicsLayerTransform every flush.

Furthermore, animations can call setAnimatedTransform(const TransformationMatrix& matrix) with the same matrix.
Comment 4 Noam Rosenthal 2012-12-26 15:44:14 PST
(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 180733 [details] [details])
> > This is redundant; We already perform those checks in GraphicsLayerTextureMapper.
> 
> But TextureMapperLayer::flushCompositingStateForThisLayerOnly can change GraphicsLayerTransform every flush.
> 
> Furthermore, animations can call setAnimatedTransform(const TransformationMatrix& matrix) with the same matrix.

OK, you should describe in the changelog what use cases this actually optimizes.
Comment 5 Dongseong Hwang 2012-12-26 15:52:49 PST
Created attachment 180764 [details]
Patch
Comment 6 Dongseong Hwang 2012-12-26 15:53:17 PST
(In reply to comment #4)
> OK, you should describe in the changelog what use cases this actually optimizes.

Absolutely! I did.
Comment 7 Noam Rosenthal 2012-12-26 15:55:41 PST
Comment on attachment 180764 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        Don't combine transforms if GraphicsLayerTransform isn't changed actually.
> +
> +        Now this patch optimizes flushCompositingStateForThisLayerOnly() and
> +        setAnimatedTransform() in TextureMapperLayer.

Only multiply the transformation matrices if the paramaters are actually changed.

This optimizes the code path called from flushCompositingStateForThisLayerOnly(), and potentially setAnimatedTransform().
Comment 8 Dongseong Hwang 2012-12-26 16:22:33 PST
Created attachment 180765 [details]
Patch
Comment 9 Dongseong Hwang 2012-12-26 16:25:05 PST
(In reply to comment #7)
> Only multiply the transformation matrices if the paramaters are actually changed.
> 
> This optimizes the code path called from flushCompositingStateForThisLayerOnly(), and potentially setAnimatedTransform().

I don't think it is not natural for you to do favor to me. Thank you very much!
Comment 10 WebKit Review Bot 2012-12-26 16:39:58 PST
Comment on attachment 180765 [details]
Patch

Clearing flags on attachment: 180765

Committed r138487: <http://trac.webkit.org/changeset/138487>
Comment 11 WebKit Review Bot 2012-12-26 16:40:02 PST
All reviewed patches have been landed.  Closing bug.