RESOLVED FIXED Bug 105758
[TexMap] A Minor optimization of GraphicsLayerTransform. 
https://bugs.webkit.org/show_bug.cgi?id=105758
Summary [TexMap] A Minor optimization of GraphicsLayerTransform.
Dongseong Hwang
Reported 2012-12-26 00:45:24 PST
Don't combine transforms if GraphicsLayerTransform isn't changed actually.
Attachments
Patch (2.63 KB, patch)
2012-12-26 00:48 PST, Dongseong Hwang 		no flags
DetailsFormatted DiffDiff
Patch (2.76 KB, patch)
2012-12-26 15:52 PST, Dongseong Hwang 		dongseong.hwang: review+
dongseong.hwang: commit-queue-
DetailsFormatted DiffDiff
Patch (2.78 KB, patch)
2012-12-26 16:22 PST, Dongseong Hwang 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Dongseong Hwang
Comment 1 2012-12-26 00:48:03 PST
Created attachment 180733 [details] Patch
Noam Rosenthal
Comment 2 2012-12-26 15:09:42 PST
Comment on attachment 180733 [details] Patch This is redundant; We already perform those checks in GraphicsLayerTextureMapper.
Dongseong Hwang
Comment 3 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.
Noam Rosenthal
Comment 4 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.
Dongseong Hwang
Comment 5 2012-12-26 15:52:49 PST
Created attachment 180764 [details] Patch
Dongseong Hwang
Comment 6 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.
Noam Rosenthal
Comment 7 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().
Dongseong Hwang
Comment 8 2012-12-26 16:22:33 PST
Created attachment 180765 [details] Patch
Dongseong Hwang
Comment 9 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!
WebKit Review Bot
Comment 10 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>
WebKit Review Bot
Comment 11 2012-12-26 16:40:02 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.