Bug 83295 – [chromium] Drawing an accelerated canvas onto itself is slow.

• Status: RESOLVED
• Resolution: FIXED
• Component: Canvas

[Stephen White]

[chromium] Drawing an accelerated canvas onto itself is slow.

[Stephen White]

Created attachment 135860 [details]
Test case

from http://code.google.com/p/chromium/issues/detail?id=106646

[Stephen White]

Created attachment 135861 [details]
Patch

[James Robinson]

Comment on attachment 135861 [details]
Patch

Interesting.  What if this is a gpu->cpu or cpu->gpu copy?  Should we be checking the return value?

[Stephen White]

(In reply to comment #3)
> (From update of attachment 135861 [details])
> Interesting.  What if this is a gpu->cpu or cpu->gpu copy?  Should we be checking the return value?

Yeah, thanks for reminding me.  Mike made me remove the copyTo() fallback from the internal implementation.

[Stephen White]

Note that it should always be GPU->GPU or CPU->CPU, since it's the same canvas.

[Stephen White]

Created attachment 135881 [details]
Patch

[James Robinson]

Comment on attachment 135881 [details]
Patch

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

> Source/WebCore/platform/graphics/skia/ImageSkia.cpp:483
> +        if (!bitmap.deepCopyTo(&temp, bitmap.config()))
> +            bitmap.copyTo(&temp, bitmap.config());

gotcha. so given that the configs match, when would deepCopyTo fail?

[Stephen White]

(In reply to comment #7)
> (From update of attachment 135881 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=135881&action=review
> 
> > Source/WebCore/platform/graphics/skia/ImageSkia.cpp:483
> > +        if (!bitmap.deepCopyTo(&temp, bitmap.config()))
> > +            bitmap.copyTo(&temp, bitmap.config());
> 
> gotcha. so given that the configs match, when would deepCopyTo fail?

Actually, it turns out that there is an internal copyTo fallback in the software case; it's only the texture-backed case that can fail.

If the target has a texture which isn't a renderable format (eg., compressed texture), deepCopy() would fail, since the copy works by rendering via FBO.  But given that this texture is a backing store, it should always succeed.  So, never in this case.  :)

[Stephen White]

Committed r113373: <http://trac.webkit.org/changeset/113373>