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
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

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.