Bug 66251 – Canvas resizing can be slow

• Component: Canvas
• Status: RESOLVED FIXED

[Stephen White]

Currently, resizing canvas elements can be slow.  This could be improved by:

1)  Deferring all ImageBuffer creation until the first actual draw call.
2)  [chromium] Moving ownership of the DrawingBuffer into ImageBuffer, so it can benefit from Ganesh's texure cache reuse for its FBOs.
3)  [chromium] Avoiding use of texImage2DResourceSafe() when allocating FBOs for canvas.

[Stephen White]

Created attachment 103962 [details]
Patch

[James Robinson]

Comment on attachment 103962 [details]
Patch

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

I like the way this is going, although I'm not sure I understand the complete picture yet.  Left some comments inline.

> Source/WebCore/platform/graphics/gpu/DrawingBuffer.cpp:273
> +        m_context->texImage2D(GraphicsContext3D::TEXTURE_2D, 0, internalColorFormat, m_size.width(), m_size.height(), 0, colorFormat, GraphicsContext3D::UNSIGNED_BYTE, 0);

this seems bad - in the command buffer, this'll cause a client-side buffer to get allocated, transferred, and uploaded.

> Source/WebKit/chromium/ChangeLog:9
> +        Allow the hostWindow param (and m_webViewImpl) to be NULL.  This
> +        makes it much easier to enable GPU acceleration at a lower level
> +        in WebKit's platform/graphics layer, without needing access to the
> +        Page* or HostWindow*.

this is a really good change but probably should be a separate patch

[Stephen White]

(In reply to comment #2)
> (From update of attachment 103962 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=103962&action=review
> 
> I like the way this is going, although I'm not sure I understand the complete picture yet.  Left some comments inline.
> 
> > Source/WebCore/platform/graphics/gpu/DrawingBuffer.cpp:273
> > +        m_context->texImage2D(GraphicsContext3D::TEXTURE_2D, 0, internalColorFormat, m_size.width(), m_size.height(), 0, colorFormat, GraphicsContext3D::UNSIGNED_BYTE, 0);
> 
> this seems bad - in the command buffer, this'll cause a client-side buffer to get allocated, transferred, and uploaded.

Really?  Hmm. I thought that it didn't.  Still, considering that GraphicsContext3D::texImage2DResourceSafe() was doing the same thing, I don't think this is any net change for the command buffer impl.  At least this'll prevent it in the in-process version from doing it.

We really need some kind of "resourceSafety" creation attribute, or a flavour of glTexImage2D() that doesn't do that braindead memset() and upload.  Only WebGL needs it, AFAIK.

> > Source/WebKit/chromium/ChangeLog:9
> > +        Allow the hostWindow param (and m_webViewImpl) to be NULL.  This
> > +        makes it much easier to enable GPU acceleration at a lower level
> > +        in WebKit's platform/graphics layer, without needing access to the
> > +        Page* or HostWindow*.
> 
> this is a really good change but probably should be a separate patch

OK, if you insist.  :)

[Stephen White]

Created attachment 104047 [details]
Patch

[Kenneth Russell]

Comment on attachment 104047 [details]
Patch

Looks good to me.

[Stephen White]

Committed r93157: <http://trac.webkit.org/changeset/93157>