Bug 105758

Summary:

[TexMap] A Minor optimization of GraphicsLayerTransform.

Product:

WebKit

Reporter:

Dongseong Hwang <dongseong.hwang>

Component:

Layout and Rendering

Assignee:

Dongseong Hwang <dongseong.hwang>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

noam, simon.fraser, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	dongseong.hwang: review+, dongseong.hwang: commit-queue-
Patch	none

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	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 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.