Bug 103237 – Performance gain for Animations using CSS 2D transforms while leveraging GPU accelerated compositing

• Component: CSS
• Status: RESOLVED CONFIGURATION CHANGED

This issue is ported from Chromuim Issue 154762 (https://code.google.com/p/chromium/issues/detail?id=154762) and 135260 (https://code.google.com/p/chromium/issues/detail?id=135260). Chromium's performance for this case is far behind IE. You may see more details from the above links.
I am wondering if it is possible to introduce new compositing layers and leverage GPU accelerated compositing for those typical cases?

[Simon Fraser (smfr)]

What are these cases? (I should not have to read the Chromiium bugs to find out.)

Created attachment 176131 [details]
The PerfMarks benchmark for CSS transformation test

Here I attached the benchmark for this case and we got poor scores for CSS 2D Scale and Rotate categories under Chrome and the scores is far behind the scores under IE.
This benchmark simply calls CSS3 2D transform methods continually to make the animations and WebKit seems only use software rendering for all the CSS3 2D transforms. I found out the performance may gain much if we introduce new compositing layers and leverage GPU accelerated compositing for these typical cases. So is it available to introduce compositing layers for these cases?

Created attachment 176133 [details]
PerfMarks benchmark zip

The benchmark zip is also attached.

[Dean Jackson]

(In reply to comment #2)
> Created an attachment (id=176131) [details]
> The PerfMarks benchmark for CSS transformation test
> 
> Here I attached the benchmark for this case and we got poor scores for CSS 2D Scale and Rotate categories under Chrome and the scores is far behind the scores under IE.
> This benchmark simply calls CSS3 2D transform methods continually to make the animations and WebKit seems only use software rendering for all the CSS3 2D transforms. I found out the performance may gain much if we introduce new compositing layers and leverage GPU accelerated compositing for these typical cases. So is it available to introduce compositing layers for these cases?

Sure, this could be done. In fact, our original implementation did. The problem is that it has a negative impact on memory use. That's why we moved to the non-composited mode.

If the benchmark is updating 2D transforms continually using script, then we could possibly optimize for this case. I wouldn't want to always have 2d transforms create compositing layers - it's a waste of memory and GPU resources for static content. We actually have a nice medium at the moment which, like it or not, the community has adopted: if you're going to update 2d transforms by script a lot, then force the element into compositing mode, usually by making the 2d look as if it is a 3d transform.

I know there is some code that puts images on a fast path if they are being resized continually. It's possible that we could do something similar. Detect this type of use for 2d transforms, and jump into composited mode for a bit. Note however, that moving an element between composited and non-composited is not free.

To repeat, I don't think we should always composite 2d transforms.

(In reply to comment #4)
> (In reply to comment #2)
> > Created an attachment (id=176131) [details] [details]
> > The PerfMarks benchmark for CSS transformation test
> > 
> > Here I attached the benchmark for this case and we got poor scores for CSS 2D Scale and Rotate categories under Chrome and the scores is far behind the scores under IE.
> > This benchmark simply calls CSS3 2D transform methods continually to make the animations and WebKit seems only use software rendering for all the CSS3 2D transforms. I found out the performance may gain much if we introduce new compositing layers and leverage GPU accelerated compositing for these typical cases. So is it available to introduce compositing layers for these cases?
> 
> Sure, this could be done. In fact, our original implementation did. The problem is that it has a negative impact on memory use. That's why we moved to the non-composited mode.
> 
> If the benchmark is updating 2D transforms continually using script, then we could possibly optimize for this case. I wouldn't want to always have 2d transforms create compositing layers - it's a waste of memory and GPU resources for static content. We actually have a nice medium at the moment which, like it or not, the community has adopted: if you're going to update 2d transforms by script a lot, then force the element into compositing mode, usually by making the 2d look as if it is a 3d transform.
> 
> I know there is some code that puts images on a fast path if they are being resized continually. It's possible that we could do something similar. Detect this type of use for 2d transforms, and jump into composited mode for a bit. Note however, that moving an element between composited and non-composited is not free.
> 
> To repeat, I don't think we should always composite 2d transforms.

Thanks a lot Jackson! Your idea is actually aligned with our's. In your reply you mentioned that you already have a nice medium now and I am wondering if it has been implemented or still WIP? I may try to provide a patch or do some assistance if it is available. :)

Created attachment 178669 [details]
Patch

[WebKit Review Bot]

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

Source/WebCore/platform/graphics/transforms/TransformOperations.h:61:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/graphics/transforms/TransformOperations.h:63:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperations.h:63:  Missing space before {  [whitespace/braces] [5]
Source/WebCore/platform/graphics/transforms/TransformOperations.h:64:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperations.h:65:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperations.h:66:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperations.h:67:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperations.h:68:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/rendering/RenderLayerCompositor.cpp:1808:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:37:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:37:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:38:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:38:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:39:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:86:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:87:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:87:  Missing space before {  [whitespace/braces] [5]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:88:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:89:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:90:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:90:  This { should be at the end of the previous line  [whitespace/braces] [4]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:91:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:91:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:92:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:92:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:93:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:93:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:94:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:94:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:95:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:96:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:97:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:97:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:97:  An else should appear on the same line as the preceding }  [whitespace/newline] [4]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:98:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:98:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:99:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:99:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:100:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:100:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:101:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:102:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:103:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:103:  Extra space after ( in if  [whitespace/parens] [5]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:104:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:104:  This { should be at the end of the previous line  [whitespace/braces] [4]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:105:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:106:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:107:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:108:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:109:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:110:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:111:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:112:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:112:  An else should appear on the same line as the preceding }  [whitespace/newline] [4]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:113:  Tab found; better to use sFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
paces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:113:  This { should be at the end of the previous line  [whitespace/braces] [4]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:114:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:115:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:116:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/transforms/TransformOperation.h:117:  Tab found; better to use spaces  [whitespace/tab] [1]
Total errors found: 62 in 4 files


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

[Early Warning System Bot]

Comment on attachment 178669 [details]
Patch

Attachment 178669 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15230829

[Early Warning System Bot]

Comment on attachment 178669 [details]
Patch

Attachment 178669 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15231724

[Build Bot]

Comment on attachment 178669 [details]
Patch

Attachment 178669 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15238773

[Simon Fraser (smfr)]

Comment on attachment 178669 [details]
Patch

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

We already accelerated animating 2D transforms. I don't understand what this patch is trying to do.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:1807
> +#if (CPU(X86) || CPU(X86_64))

We don't use #ifdefs like this. Reading the code, I have no idea what platforms these are, and why they need to be different.

(In reply to comment #11)
> (From update of attachment 178669 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=178669&action=review
> 
> We already accelerated animating 2D transforms. I don't understand what this patch is trying to do.
> 
> > Source/WebCore/rendering/RenderLayerCompositor.cpp:1807
> > +#if (CPU(X86) || CPU(X86_64))
> 
> We don't use #ifdefs like this. Reading the code, I have no idea what platforms these are, and why they need to be different.

(In reply to comment #11)
> (From update of attachment 178669 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=178669&action=review
> 
> We already accelerated animating 2D transforms. I don't understand what this patch is trying to do.
> 
> > Source/WebCore/rendering/RenderLayerCompositor.cpp:1807
> > +#if (CPU(X86) || CPU(X86_64))
> 
> We don't use #ifdefs like this. Reading the code, I have no idea what platforms these are, and why they need to be different.

Thanks smfr! 
This patch is proposed to introduce new compositing layers and leverage GPU acceleration for those typical cases as I mentioned above.
Could you please kindly help to provide any clues for the implementation of existing accelerated animating 2D transforms since I am not familiar with this?
Thanks a lot!

[Dean Jackson]

(In reply to comment #12)

> This patch is proposed to introduce new compositing layers and leverage GPU acceleration for those typical cases as I mentioned above.
> Could you please kindly help to provide any clues for the implementation of existing accelerated animating 2D transforms since I am not familiar with this?

See the existing code that toggles compositing when a transform starts/stops animating. Simon is asking whether that's what your patch is trying to do. If so, it's already done. If it's trying to turn on compositing everywhere for 2d transforms, then we don't really want to do that for the reasons we've already mentioned. Is this patch trying to do something else?

(In reply to comment #13)
> (In reply to comment #12)
> 
> > This patch is proposed to introduce new compositing layers and leverage GPU acceleration for those typical cases as I mentioned above.
> > Could you please kindly help to provide any clues for the implementation of existing accelerated animating 2D transforms since I am not familiar with this?
> 
> See the existing code that toggles compositing when a transform starts/stops animating. Simon is asking whether that's what your patch is trying to do. If so, it's already done. If it's trying to turn on compositing everywhere for 2d transforms, then we don't really want to do that for the reasons we've already mentioned. Is this patch trying to do something else?

Yes Jackson, I checked the code and found that the existed compositing seems only be toggled by the '-webkit-animation' property. However, some cases use only '-webkit-transform' plus 'setTimeout' to make animations (e.g. the attached benchmark). This patch was trying to introduce compositing layers for those typical cases. I will try to refine the patch and update a new one soon.

Created attachment 180620 [details]
Patch

[WebKit Review Bot]

Attachment 180620 [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/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 4 files


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

[Early Warning System Bot]

Comment on attachment 180620 [details]
Patch

Attachment 180620 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15490216

[Early Warning System Bot]

Comment on attachment 180620 [details]
Patch

Attachment 180620 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15493163

[EFL EWS Bot]

Comment on attachment 180620 [details]
Patch

Attachment 180620 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15507111

[Build Bot]

Comment on attachment 180620 [details]
Patch

Attachment 180620 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15464947

[WebKit Review Bot]

Comment on attachment 180620 [details]
Patch

Attachment 180620 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15497109

[Peter Beverloo (cr-android ews)]

Comment on attachment 180620 [details]
Patch

Attachment 180620 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15498107

[Build Bot]

Comment on attachment 180620 [details]
Patch

Attachment 180620 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15627609

[Eric Seidel (no email)]

Comment on attachment 180620 [details]
Patch

This makes all the bots red.  Also, the ChangeLog is unwrapped and has a stray tab.

[Simon Fraser (smfr)]

Comment on attachment 180620 [details]
Patch

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

> Source/WebCore/rendering/RenderLayerCompositor.cpp:1812
> +    return renderer->hasTransform() && (style->transform().has3DOperation() || style->transform().hasCSS2DAnimations());

This will cause compositing for all elements with any transform, which we don't want.

A possible approach here would be to make an element composited if it has a 2D transform which is being changed frequently. What "frequently" means would be a heuristic.

[Brent Fulgham]

Given current benchmarking, I don't believe there is further action needed here. Please open a new bug if you believe there are specific optimizations that could be made.