[mac] ImageBuffer should create accelerated buffers for small canvases, but we shouldn't force them to create compositing layers

• Component: Canvas
• Status: RESOLVED FIXED

[Tim Horton]

<rdar://problem/11752381>

Mac WebKit is slower than it needs to be on http://www.mikechambers.com/html5/javascript/QuadTree/examples/collision.html because it makes lots of tiny canvases and draws them into a big one. The tiny ones fall below ImageBufferCG's "minIOSurfaceSize", so they are created in main memory. The big one is big, so it's an IOSurface. Painting between these is incredibly slow.

The reason we cap ImageBuffer acceleration by size is so that small canvases (for example, http://typeface.neocracy.org/examples.html) don't force compositing (and then potentially cause overlap, and huge memory cost). But, we can still make IOSurfaces and still not pop them into compositing layers. So, let's do that!

[Tim Horton]

Created attachment 184441 [details]
patch

name of the constant is a little wonky

[Tim Horton]

Comment on attachment 184441 [details]
patch

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

> Source/WebCore/rendering/RenderLayerCompositor.cpp:1907
> +        return canvas->renderingContext() && canvas->renderingContext()->isAccelerated() && (canvas->size().area() > canvasAreaThresholdRequiringCompositing);

This is shared code, I wonder if this is reasonable for everyone...

[Build Bot]

Comment on attachment 184441 [details]
patch

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

New failing tests:
fast/canvas/webgl/gl-pixelstorei.html
fast/canvas/webgl/tex-image-webgl.html
compositing/layer-creation/spanOverlapsCanvas.html

[Simon Fraser (smfr)]

Comment on attachment 184441 [details]
patch

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

>> Source/WebCore/rendering/RenderLayerCompositor.cpp:1907
>> +        return canvas->renderingContext() && canvas->renderingContext()->isAccelerated() && (canvas->size().area() > canvasAreaThresholdRequiringCompositing);
> 
> This is shared code, I wonder if this is reasonable for everyone...

We should probably #ifdef it. Maybe USE_COMPOSITING_FOR_SMALL_CANVASES or something, disabled for PLATFORM(MAC) and PLATFORM(IOS).

[Tim Horton]

(In reply to comment #3)
> (From update of attachment 184441 [details])
> Attachment 184441 [details] did not pass mac-ews (mac):
> Output: http://queues.webkit.org/results/16082655
> 
> New failing tests:
> fast/canvas/webgl/gl-pixelstorei.html
> fast/canvas/webgl/tex-image-webgl.html

... not a clue, but I can reproduce, so I'll look.

> compositing/layer-creation/spanOverlapsCanvas.html

This one uses a canvas at exactly the threshold and demonstrates that I should have used >= instead of > to match the < from before.

[Tim Horton]

Created attachment 185580 [details]
patch

[Tim Horton]

Needs a WTF changelog.

[Tim Horton]

Created attachment 185582 [details]
patch

[Simon Fraser (smfr)]

Comment on attachment 185582 [details]
patch

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

> Source/WTF/wtf/Platform.h:1218
> +#if !PLATFORM(MAC) && !PLATFORM(IOS)
> +#define WTF_USE_COMPOSITING_FOR_SMALL_CANVASES 1
> +#endif

Please don't cause megawatts of energy use in making us all rebuild because of this #ifdef that's only used in one place.

[Tim Horton]

Created attachment 185589 [details]
Simon really doesn't like it when you touch Platform.h

[Simon Fraser (smfr)]

Comment on attachment 185589 [details]
Simon really doesn't like it when you touch Platform.h

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

> Source/WebCore/rendering/RenderLayerCompositor.cpp:1921
> +        bool isCanvasLargeEnoughToForceCompositing = canvas->size().area() >= canvasAreaThresholdRequiringCompositing;

Hmm, this looks at element size. Maybe we should composite based on the renderer dimensions? You might have a small canvas scaled up via CSS.

[Simon Fraser (smfr)]

Comment on attachment 185589 [details]
Simon really doesn't like it when you touch Platform.h

I think this is fine.

[Tim Horton]

Thanks!

http://trac.webkit.org/changeset/141333