Bug 44926 – Multiple accelerated 2D canvases should be able to use the same GraphicsContext3D

• Component: New Bugs
• Status: RESOLVED FIXED

[James Robinson]

It's wasteful to create a new GraphicsContext3D for every 2d canvas since each GraphicsContext3D implies a potentially heavyweight backing OpenGL context.  Additionally having each canvas in its own GraphicsContext3D makes sharing resources between canvases very difficult, since there is no notion of resource sharing in the GraphicsContext3D APIs (although OpenGL has such a concept).  There are also a set of resources (shaders, etc) that we will want to use for every canvas.

Instead, we should share a single GraphicsContext3D across all 2d canvases that we wish to accelerate that exist in a single compositor context.

[James Robinson]

Created attachment 66011 [details]
Patch

[James Robinson]

Patch for review/discussion.  This is a larger patch than I would prefer (108kb), but since it changes interfaces and ownership models it's very difficult to break up into independent pieces.

The intended use of SharedContext3D is that callers can use it as if it were a GraphicsContext3D, being careful to re-bind everything they need on every draw, and that SharedContext3D would have caching to avoid issuing redundant calls to the underlying GraphicsContext3D.  I've left the caching out of this patch as it was already getting pretty big.

[Eric Seidel (no email)]

Attachment 66011 [details] did not build on mac:
Build output: http://queues.webkit.org/results/3882178

[James Robinson]

Mac failure is due to SVG+EmptyClients nonsense.  Will fix.

[WebKit Review Bot]

Attachment 66011 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/3859191

[James Robinson]

Created attachment 66013 [details]
Patch

[Eric Seidel (no email)]

Attachment 66013 [details] did not build on mac:
Build output: http://queues.webkit.org/results/3840174

[WebKit Review Bot]

Attachment 66013 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/3825140

[Early Warning System Bot]

Attachment 66013 [details] did not build on qt:
Build output: http://queues.webkit.org/results/3886164

[WebKit Review Bot]

Attachment 66013 [details] did not build on win:
Build output: http://queues.webkit.org/results/3889168

[James Robinson]

Created attachment 66074 [details]
compile fix for gtk/win/mac

[Stephen White]

> WebCore/html/canvas/CanvasRenderingContext2D.cpp:1283
>      sourceCanvas->makeRenderingResultsAvailable();
Are you sure it isn't necessary to do something here?  Obviously, reading back to the buffer isn't right in this case, but we need some way to make sure that the sourceCanvas is up-to-date (e.g., last software draw is uploaded to framebuffer) before we draw.

And actually, I think this will break the software path, since it needs to make this call.  If the GPU truly doesn't need this, it should probably be doing an !isAccelerated() check instead.

(Reading on a bit further, it seems you're doing the GPU equivalent of this call in drawImageBuffer().  Fair enough, although it would be kind of cool to unify them in the same makeRenderingResultsAvailable() function.  And you'll still need to fix the software path).

> WebCore/platform/graphics/chromium/Canvas2DLayerChromium.cpp:38
> +#include "PlatformContextSkia.h"
Is this #include really needed?

> WebCore/platform/graphics/chromium/Canvas2DLayerChromium.cpp:63
> +        glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
Should be glTexParameteri (someone really ought to fix the original too).

> WebCore/platform/graphics/chromium/GLES2Canvas.cpp:130
> +    m_context->useFillSolidProgram(color, matrix);
Please use matrix, color order -- this matches the order in the shader code.

> WebCore/platform/graphics/chromium/GLES2Canvas.cpp:232
> +    drawQuad(IntSize(tileBoundsWithBorder.width(), tileBoundsWithBorder.height()), srcRectClippedInTileSpace, dstRectIntersected, transform, alpha);
I think tileBoundsWithBorder.size() should work here, rather than constructing your own IntSize().

> WebCore/platform/graphics/chromium/GLES2Canvas.cpp:249
> +    m_context->useTextureProgram(alpha, matrix, texMatrix);
Should be matrix, texMatrix, alpha (to be consistent w/shader).

> WebCore/platform/graphics/chromium/GLES2Canvas.cpp:261
> +    return m_context->createTexture(ptr, format, width, height);
OT:  Now that uploaded textures are shared between all contexts, I think we could actually get rid of the hash table entirely, and put a RefPtr<Texture> in the NativeImageSkia itself, if we wanted to.  This should help tie its lifetime to that of the image cache.

> WebCore/platform/graphics/chromium/GLES2Canvas.cpp:266
> +    return m_context->getTexture(ptr);
Does this function really need to transfer ownership?  If not, it should be passing a bare ptr, as it was before (see http://webkit.org/coding/RefPtr.html).

> WebCore/platform/graphics/chromium/GLES2Canvas.h:79
> +    PassRefPtr<Texture> getTexture(NativeImagePtr);
Again, since the ownership is not transferred, these should be bare pointers.

> WebCore/platform/graphics/chromium/GLES2Canvas.h:87
> +    unsigned textureIdForCanvas() const;
This will all have to change once the compositor supports texture tiling, but I'm sure you knew that.

> WebCore/platform/graphics/chromium/WebGLLayerChromium.cpp:63
> +        glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
glTexParameteri (not your fault, just hoping someone finally fixes this).

> WebCore/platform/graphics/gpu/CanvasFramebuffer.cpp:67
> +    m_context->copyTextureToCompositor(m_offscreenColorTexture, m_offscreenParentColorTexture);
As discussed, I would have preferred that this end up in the same code as WebGL on the GPU process side, but that can be refactored later.

> WebCore/platform/graphics/gpu/SharedContext3D.cpp:61
> +}
This looks like it's unused.

> WebCore/platform/graphics/gpu/SharedContext3D.cpp:278
> +        m_context->bindTexture(target, texture);
This looks superfluous -- same code as below.

> WebCore/platform/graphics/gpu/SharedContext3D.cpp:285
> +void SharedContext3D::useFillSolidProgram(const Color& color, const AffineTransform& transform)
Please switch these back so they match the shader.

> WebCore/platform/graphics/gpu/SharedContext3D.cpp:290
> +void SharedContext3D::useTextureProgram(float alpha, const AffineTransform& transform, const AffineTransform& texTransform)
Ibid.

> WebCore/platform/graphics/gpu/SharedContext3D.h:87
> +    bool supportsBGRA();
Is this needed?  It looks like Texture is unchanged, so it'll still call GraphicsContext3D::supportsBGRA() (although I could be wrong).

> WebCore/platform/graphics/skia/PlatformContextSkia.cpp:791
> +        m_uploadTexture = context->createTexture(0, Texture::BGRA8, bitmap.width(), bitmap.height());
Hmm.. hashing on the NULL ptr (besides making it "magic" and fragile, in case anyone else tries the same trick) will share a single upload texture for all canvases.  This means we might hang onto an 8K texture long after a big canvas is gone, and we only need to upload little 150x150 canvases, e.g.

> WebCore/platform/graphics/skia/PlatformContextSkia.h:184
> +    void setSharedContext3D(SharedContext3D*, CanvasFramebuffer*, const IntSize&);
I'd rather the CanvasFramebuffer be owned by the GLES2Canvas, if possible.  But we can talk about that later.

> WebKit/chromium/src/WebViewImpl.cpp:2220
> +        attr.stencil = true;
Nit:  probably not much point in requesting stencil, since it looks like your FBO implementation doesn't support it.

[Kenneth Russell]

Comment on attachment 66013 [details]
Patch

This looks very good overall. There are a couple of relatively minor issues where PassRefPtr is used in a return value where it shouldn't be (see "Function results" in http://webkit.org/coding/RefPtr.html ) for which I'm marking this patch r-, but after these are cleaned up I'll be happy to r+ it.


> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index ddcd52e9676ba66bb03c410afff59e2261f7d830..94147a54b8123231a1af74267031e26ad174b08f 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,173 @@
> [phone]  James Robinson  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Multiple accelerated 2D canvases should be able to use the same GraphicsContext3D
> +        https://bugs.webkit.org/show_bug.cgi?id=44926
> +
> +        This allows many accelerated 2d canvases to render using a single underlying GraphicsContext3D.
> +        It introduces a new class SharedContext3D that manages several callers.  This class could
> +        also cache the current state to avoid issuing redundant calls, although in this first cut it doesn't.
> +        The SharedContext3D is provided through the ChromeClient so that its lifetime can be tied to that
> +        of the platform-specific compositor infrastructure.
> +
> +        Accelerated 2d canvases maintain a reference to a SharedContext3D and have ownership of a CanvasFramebuffer,
> +        which represents the canvas's rendering target.  The compositing layer for an accelerated 2d canvas is
> +        aware only of the canvas's CanvasFramebuffer.  This means that WebGL and 2d canvases are no longer treated
> +        as the same time of layer by the compositor.

time -> type

> +        Covered by existing canvas tests.
> +
> +        I've divided the per file/function changes into three sections:
> +        - Platform-independent application logic outside WebCore/platform
> +        - Platform-independent infrastructure in WebCore/platform
> +        - Platform-specific changes
> +
> +        * html/canvas/CanvasRenderingContext.cpp:
> +        * html/canvas/CanvasRenderingContext.h:
> +        (WebCore::CanvasRenderingContext::paintsIntoCanvasBuffer):
> +        * html/canvas/CanvasRenderingContext2D.cpp:
> +        (WebCore::CanvasRenderingContext2D::CanvasRenderingContext2D):
> +        (WebCore::CanvasRenderingContext2D::paintsIntoCanvasBuffer):
> +        (WebCore::CanvasRenderingContext2D::reset):
> +        (WebCore::CanvasRenderingContext2D::drawImage):
> +        (WebCore::CanvasRenderingContext2D::didDraw):
> +        * html/canvas/CanvasRenderingContext2D.h:
> +        (WebCore::CanvasRenderingContext2D::framebuffer):
> +        * html/canvas/WebGLRenderingContext.cpp:
> +        (WebCore::WebGLRenderingContext::paintsIntoCanvasBuffer):
> +        * html/canvas/WebGLRenderingContext.h:
> +        (WebCore::WebGLRenderingContext::graphicsContext3D):
> +        * loader/EmptyClients.h:
> +        (WebCore::EmptyChromeClient::attachRootGraphicsLayer):
> +        (WebCore::EmptyChromeClient::setNeedsOneShotDrawingSynchronization):
> +        (WebCore::EmptyChromeClient::scheduleCompositingLayerSync):
> +        (WebCore::EmptyChromeClient::getSharedContext3D):
> +        * page/ChromeClient.h:
> +        * rendering/RenderLayerBacking.cpp:
> +        (WebCore::RenderLayerBacking::updateGraphicsLayerConfiguration):
> +          Cross-platform infrastructure in WebCore/platform:

Could you put some sort of prefix on these separator lines to distinguish them from the autogenerated output?

> +        * WebCore.gypi:
> +        * platform/graphics/GraphicsContext.h:
> +        * platform/graphics/GraphicsContext3D.h:
> +        * platform/graphics/GraphicsLayer.h:
> +        (WebCore::GraphicsLayer::setContentsToWebGL):
> +        (WebCore::GraphicsLayer::setContentsToCanvas2D):
> +        * platform/graphics/gpu/CanvasFramebuffer.cpp: Added.
> +        (WebCore::CanvasFramebuffer::CanvasFramebuffer):
> +        (WebCore::CanvasFramebuffer::~CanvasFramebuffer):
> +        (WebCore::CanvasFramebuffer::bind):
> +        (WebCore::CanvasFramebuffer::swapBuffers):
> +        (WebCore::CanvasFramebuffer::setWillSwapBuffersCallback):
> +        (WebCore::CanvasFramebuffer::create):
> +        * platform/graphics/gpu/CanvasFramebuffer.h: Added.
> +        (WebCore::CanvasFramebuffer::textureIdForCompositor):
> +        (WebCore::CanvasFramebuffer::textureIdForCanvas):
> +        * platform/graphics/gpu/SharedContext3D.cpp: Added.
> +        (WebCore::affineTo3x3):
> +        (WebCore::SharedContext3D::create):
> +        (WebCore::SharedContext3D::SharedContext3D):
> +        (WebCore::SharedContext3D::~SharedContext3D):
> +        (WebCore::SharedContext3D::scissor):
> +        (WebCore::SharedContext3D::enable):
> +        (WebCore::SharedContext3D::disable):
> +        (WebCore::SharedContext3D::clearColor):
> +        (WebCore::SharedContext3D::clear):
> +        (WebCore::SharedContext3D::drawArrays):
> +        (WebCore::SharedContext3D::getError):
> +        (WebCore::SharedContext3D::getIntegerv):
> +        (WebCore::SharedContext3D::createFramebuffer):
> +        (WebCore::SharedContext3D::createTexture):
> +        (WebCore::SharedContext3D::deleteFramebuffer):
> +        (WebCore::SharedContext3D::deleteTexture):
> +        (WebCore::SharedContext3D::deleteCompositorTexture):
> +        (WebCore::SharedContext3D::framebufferTexture2D):
> +        (WebCore::SharedContext3D::texParameteri):
> +        (WebCore::SharedContext3D::texImage2D):
> +        (WebCore::SharedContext3D::texSubImage2D):
> +        (WebCore::SharedContext3D::readPixels):
> +        (WebCore::SharedContext3D::getTexture):
> +        (WebCore::SharedContext3D::applyCompositeOperator):
> +        (WebCore::SharedContext3D::useQuadVertices):
> +        (WebCore::SharedContext3D::setActiveTexture):
> +        (WebCore::SharedContext3D::bindTexture):
> +        (WebCore::SharedContext3D::useFillSolidProgram):
> +        (WebCore::SharedContext3D::useTextureProgram):
> +        (WebCore::SharedContext3D::bindFramebuffer):
> +        (WebCore::SharedContext3D::setViewport):
> +        (WebCore::SharedContext3D::supportsBGRA):
> +        (WebCore::SharedContext3D::createCompositorTexture):
> +        (WebCore::SharedContext3D::copyTextureToCompositor):
> +        (WebCore::SharedContext3D::paintsIntoCanvasBuffer):
> +        * platform/graphics/gpu/SharedContext3D.h: Added.
> +        * platform/graphics/gpu/Texture.cpp:
> +          Mac-specific changes:
> +        * platform/graphics/mac/GraphicsLayerCA.h:
> +        * platform/graphics/mac/GraphicsLayerCA.mm:
> +        (WebCore::GraphicsLayerCA::setContentsToWebGL):
> +          Chromium-specific changes:
> +        * platform/graphics/chromium/Canvas2DLayerChromium.cpp: Added.
> +        (WebCore::Canvas2DLayerChromium::create):
> +        (WebCore::Canvas2DLayerChromium::Canvas2DLayerChromium):
> +        (WebCore::Canvas2DLayerChromium::updateContents):
> +        (WebCore::Canvas2DLayerChromium::setCanvasFramebuffer):
> +        (WebCore::Canvas2DLayerChromium::textureId):
> +        * platform/graphics/chromium/Canvas2DLayerChromium.h: Added.
> +        (WebCore::Canvas2DLayerChromium::drawsContent):
> +        * platform/graphics/chromium/CanvasLayerChromium.cpp:
> +          CanvasLayerChromium is now a base class for WebGLLayerChromium and Canvas2DLayerChromium.
> +        (WebCore::CanvasLayerChromium::CanvasLayerChromium):
> +        (WebCore::CanvasLayerChromium::~CanvasLayerChromium):
> +        (WebCore::CanvasLayerChromium::draw):
> +        * platform/graphics/chromium/CanvasLayerChromium.h:
> +        * platform/graphics/chromium/GLES2Canvas.cpp:
> +        (WebCore::GLES2Canvas::GLES2Canvas):
> +        (WebCore::GLES2Canvas::~GLES2Canvas):
> +        (WebCore::GLES2Canvas::swapBuffers):
> +        (WebCore::GLES2Canvas::bindFramebuffer):
> +        (WebCore::GLES2Canvas::textureIdForCompositor):
> +        (WebCore::GLES2Canvas::textureIdForCanvas):
> +        (WebCore::GLES2Canvas::clearRect):
> +        (WebCore::GLES2Canvas::fillRect):
> +        (WebCore::GLES2Canvas::drawTexturedRect):
> +        (WebCore::GLES2Canvas::drawTexturedRectTile):
> +        (WebCore::GLES2Canvas::drawQuad):
> +        (WebCore::GLES2Canvas::createTexture):
> +        (WebCore::GLES2Canvas::getTexture):
> +        * platform/graphics/chromium/GLES2Canvas.h:
> +        (WebCore::GLES2Canvas::context):
> +        * platform/graphics/chromium/GraphicsLayerChromium.cpp:
> +        (WebCore::GraphicsLayerChromium::setContentsToWebGL):
> +        (WebCore::GraphicsLayerChromium::setContentsToCanvas2D):
> +        * platform/graphics/chromium/GraphicsLayerChromium.h:
> +        * platform/graphics/chromium/LayerRendererChromium.cpp:
> +        * platform/graphics/chromium/WebGLLayerChromium.cpp: Added.
> +        (WebCore::WebGLLayerChromium::create):
> +        (WebCore::WebGLLayerChromium::WebGLLayerChromium):
> +        (WebCore::WebGLLayerChromium::updateContents):
> +        (WebCore::WebGLLayerChromium::setContext):
> +        * platform/graphics/chromium/WebGLLayerChromium.h: Added.
> +        (WebCore::WebGLLayerChromium::drawsContent):
> +        (WebCore::WebGLLayerChromium::textureId):
> +        * platform/graphics/skia/GraphicsContextSkia.cpp:
> +        (WebCore::GraphicsContext::setSharedContext3D):
> +        (WebCore::GraphicsContext::textureIdForCompositor):
> +        (WebCore::GraphicsContext::textureIdForCanvas):
> +        * platform/graphics/skia/ImageBufferSkia.cpp:
> +        (WebCore::ImageBuffer::draw):
> +        * platform/graphics/skia/ImageSkia.cpp:
> +        (WebCore::drawBitmapGLES2):
> +        * platform/graphics/skia/PlatformContextSkia.cpp:
> +        (WebCore::PlatformContextSkia::~PlatformContextSkia):
> +        (WebCore::SwapBuffersCallbackImpl::create):
> +        (WebCore::SwapBuffersCallbackImpl::willSwapBuffers):
> +        (WebCore::SwapBuffersCallbackImpl::SwapBuffersCallbackImpl):
> +        (WebCore::PlatformContextSkia::setSharedContext3D):
> +        (WebCore::PlatformContextSkia::swapBuffers):
> +        (WebCore::PlatformContextSkia::uploadSoftwareToHardware):
> +        (WebCore::PlatformContextSkia::readbackHardwareToSoftware):
> +        * platform/graphics/skia/PlatformContextSkia.h:
> +
>  2010-08-30  Chris Rogers  <[e-mail]>
>  
>          Reviewed by Kenneth Russell.
> diff --git a/WebCore/loader/EmptyClients.h b/WebCore/loader/EmptyClients.h
> index 270752dd7f8f01fa94f7c293d3cfe7fa95ed291f..eda55ba756bcd55507f6d4e79269d8eb0c5ae2d3 100644
> --- a/WebCore/loader/EmptyClients.h
> +++ b/WebCore/loader/EmptyClients.h
> @@ -46,6 +46,10 @@
>  #include "ResourceError.h"
>  #include "SearchPopupMenu.h"
>  
> +#if ENABLE(ACCELERATED_2D_CANVAS)
> +#include "SharedContext3D.h"

Is it possible to use a forward declaration instead?

> +#endif
> +
>  /*
>   This file holds empty Client stubs for use by WebCore.
>   Viewless element needs to create a dummy Page->Frame->FrameView tree for use in parsing or executing JavaScript.
> @@ -190,9 +194,13 @@ public:
>      virtual void cancelGeolocationPermissionRequestForFrame(Frame*, Geolocation*) {}
>  
>  #if USE(ACCELERATED_COMPOSITING)
> -    virtual void attachRootGraphicsLayer(Frame*, GraphicsLayer*) {};
> -    virtual void setNeedsOneShotDrawingSynchronization() {};
> -    virtual void scheduleCompositingLayerSync() {};
> +    virtual void attachRootGraphicsLayer(Frame*, GraphicsLayer*) {}
> +    virtual void setNeedsOneShotDrawingSynchronization() {}
> +    virtual void scheduleCompositingLayerSync() {}
> +#endif
> +
> +#if ENABLE(ACCELERATED_2D_CANVAS)
> +    virtual PassRefPtr<SharedContext3D> getSharedContext3D() { return 0; }
>  #endif
>  
>  #if PLATFORM(WIN)
> diff --git a/WebCore/platform/graphics/GraphicsContext.h b/WebCore/platform/graphics/GraphicsContext.h
> index 93ef890062eda52899e4d33dd1091a2d32f4d08c..80d1e0201461515ae9050febc4cddbb67898ad11 100644
> --- a/WebCore/platform/graphics/GraphicsContext.h
> +++ b/WebCore/platform/graphics/GraphicsContext.h
> @@ -122,16 +122,17 @@ namespace WebCore {
>      const int cMisspellingLinePatternGapWidth = 1;
>  
>      class AffineTransform;
> +    class CanvasFramebuffer;
>      class Font;
>      class Generator;
>      class Gradient;
> -    class GraphicsContext3D;
>      class GraphicsContextPlatformPrivate;
>      class GraphicsContextPrivate;
>      class ImageBuffer;
>      class KURL;
>      class Path;
>      class Pattern;
> +    class SharedContext3D;
>      class TextRun;
>  
>      // These bits can be ORed together for a total of 8 possible text drawing modes.
> @@ -412,8 +413,12 @@ namespace WebCore {
>          pattern getHaikuStrokeStyle();
>  #endif
>  
> -        void setGraphicsContext3D(GraphicsContext3D*, const IntSize&);
> +#if PLATFORM(SKIA)
> +        void setSharedContext3D(SharedContext3D*, CanvasFramebuffer*, const IntSize&);
>          void syncSoftwareCanvas();
> +        unsigned textureIdForCompositor() const;
> +        unsigned textureIdForCanvas() const;
> +#endif

How about a comment before this block of methods indicating that they're for accelerated 2D canvas support?

>  
>      private:
>          void savePlatformState();
> diff --git a/WebCore/platform/graphics/GraphicsContext3D.h b/WebCore/platform/graphics/GraphicsContext3D.h
> index b583813886698ecb5605531be7f56b0a39d65069..009efe5d4baecba641c496abe100c70318d58f20 100644
> --- a/WebCore/platform/graphics/GraphicsContext3D.h
> +++ b/WebCore/platform/graphics/GraphicsContext3D.h
> @@ -753,6 +753,10 @@ public:
>  
>      bool supportsBGRA();
>  
> +    unsigned createCompositorTexture(unsigned width, unsigned height);
> +    void deleteCompositorTexture(unsigned compositorTexture);
> +    void copyTextureToCompositor(unsigned sourceTexture, unsigned compositorTexture);

A comment above these few methods indicating they're currently for accelerated 2d canvas support would be useful. Not sure whether it's worth #ifdef'ing them under that flag to avoid the need to add them to all ports currently supporting WebGL.

> +
>    private:
>      GraphicsContext3D(Attributes attrs, HostWindow* hostWindow);
>  
> diff --git a/WebCore/platform/graphics/chromium/Canvas2DLayerChromium.cpp b/WebCore/platform/graphics/chromium/Canvas2DLayerChromium.cpp
> new file mode 100644
> index 0000000000000000000000000000000000000000..d90b6f8b13f93eeb4e03fcfc545dd961a69e1a4a
> --- /dev/null
> +++ b/WebCore/platform/graphics/chromium/Canvas2DLayerChromium.cpp
> @@ -0,0 +1,87 @@
> +/*
> + * Copyright (C) 2010 Google Inc. All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are
> + * met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above
> + * copyright notice, this list of conditions and the following disclaimer
> + * in the documentation and/or other materials provided with the
> + * distribution.
> + *     * Neither the name of Google Inc. nor the names of its
> + * contributors may be used to endorse or promote products derived from
> + * this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include "config.h"
> +
> +#if USE(ACCELERATED_COMPOSITING)
> +
> +#include "Canvas2DLayerChromium.h"
> +
> +#include "CanvasFramebuffer.h"
> +#include "PlatformContextSkia.h"
> +
> +#include <GLES2/gl2.h>
> +
> +namespace WebCore {
> +
> +PassRefPtr<Canvas2DLayerChromium> Canvas2DLayerChromium::create(CanvasFramebuffer* framebuffer, GraphicsLayerChromium* owner)
> +{
> +    return adoptRef(new Canvas2DLayerChromium(framebuffer, owner));
> +}
> +
> +Canvas2DLayerChromium::Canvas2DLayerChromium(CanvasFramebuffer* framebuffer, GraphicsLayerChromium* owner)
> +    : CanvasLayerChromium(owner)
> +    , m_framebuffer(framebuffer)
> +    , m_textureChanged(true)
> +{
> +}
> +
> +void Canvas2DLayerChromium::updateContents()
> +{
> +    unsigned textureId = m_framebuffer->textureIdForCompositor();
> +    if (m_textureChanged) {
> +        glBindTexture(GL_TEXTURE_2D, textureId);
> +        // Set the min-mag filters to linear and wrap modes to GL_CLAMP_TO_EDGE
> +        // to get around NPOT texture limitations of GLES.
> +        glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
> +        glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
> +        glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
> +        glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
> +        m_textureChanged = false;
> +    }
> +    // Update the contents of the texture used by the compositor.
> +    if (m_contentsDirty) {
> +        m_framebuffer->swapBuffers();
> +        m_contentsDirty = false;
> +    }
> +}
> +
> +void Canvas2DLayerChromium::setCanvasFramebuffer(CanvasFramebuffer* framebuffer)
> +{
> +    m_framebuffer = framebuffer;

Do you need m_textureChanged = true here?

> +}
> +
> +unsigned Canvas2DLayerChromium::textureId()
> +{
> +    return m_framebuffer->textureIdForCompositor();
> +}
> +
> +}
> +#endif // USE(ACCELERATED_COMPOSITING)
> diff --git a/WebCore/platform/graphics/chromium/GLES2Canvas.cpp b/WebCore/platform/graphics/chromium/GLES2Canvas.cpp
> index 82d4c0b033a14f91e566f7b4eea08b06692ebbd3..e1d1967bf4c02871c4f8e71caa2a40de5468f653 100644
> --- a/WebCore/platform/graphics/chromium/GLES2Canvas.cpp
> +++ b/WebCore/platform/graphics/chromium/GLES2Canvas.cpp
> @@ -32,10 +32,12 @@
>  
>  #include "GLES2Canvas.h"
>  
> +#include "CanvasFramebuffer.h"
>  #include "FloatRect.h"
>  #include "GraphicsContext3D.h"
>  #include "IntRect.h"
>  #include "PlatformString.h"
> +#include "SharedContext3D.h"
>  #include "SolidFillShader.h"
>  #include "TexShader.h"
>  #include "Texture.h"
> @@ -61,37 +63,50 @@ struct GLES2Canvas::State {
>      AffineTransform m_ctm;
>  };
>  
> -GLES2Canvas::GLES2Canvas(GraphicsContext3D* context, const IntSize& size)
> -    : m_context(context)
> +GLES2Canvas::GLES2Canvas(SharedContext3D* context, CanvasFramebuffer* framebuffer, const IntSize& size)
> +    : m_size(size)
> +    , m_context(context)
> +    , m_framebuffer(framebuffer)
>      , m_state(0)
> -    , m_quadVertices(0)
> -    , m_solidFillShader(SolidFillShader::create(context))
> -    , m_texShader(TexShader::create(context))
>  {
>      m_flipMatrix.translate(-1.0f, 1.0f);
>      m_flipMatrix.scale(2.0f / size.width(), -2.0f / size.height());
>  
> -    m_context->reshape(size.width(), size.height());
> -    m_context->viewport(0, 0, size.width(), size.height());
> -
>      m_stateStack.append(State());
>      m_state = &m_stateStack.last();
> -
> -    // Force the source over composite mode to be applied.
> -    m_lastCompositeOp = CompositeClear;
> -    applyCompositeOperator(CompositeSourceOver);
>  }
>  
>  GLES2Canvas::~GLES2Canvas()
>  {
> -    m_context->deleteBuffer(m_quadVertices);
> +}
> +
> +void GLES2Canvas::swapBuffers()
> +{
> +    m_framebuffer->swapBuffers();
> +}
> +
> +void GLES2Canvas::bindFramebuffer()
> +{
> +    m_framebuffer->bind();
> +}
> +
> +unsigned GLES2Canvas::textureIdForCompositor() const
> +{
> +    return m_framebuffer->textureIdForCompositor();
> +}
> +
> +unsigned GLES2Canvas::textureIdForCanvas() const
> +{
> +    return m_framebuffer->textureIdForCanvas();
>  }
>  
>  void GLES2Canvas::clearRect(const FloatRect& rect)
>  {
> +    bindFramebuffer();
>      if (m_state->m_ctm.isIdentity()) {
> -        m_context->scissor(rect.x(), rect.y(), rect.width(), rect.height());
> +        m_context->scissor(rect);
>          m_context->enable(GraphicsContext3D::SCISSOR_TEST);
> +        m_context->clearColor(Color(RGBA32(0)));
>          m_context->clear(GraphicsContext3D::COLOR_BUFFER_BIT);
>          m_context->disable(GraphicsContext3D::SCISSOR_TEST);
>      } else {
> @@ -104,16 +119,17 @@ void GLES2Canvas::clearRect(const FloatRect& rect)
>  
>  void GLES2Canvas::fillRect(const FloatRect& rect, const Color& color, ColorSpace colorSpace)
>  {
> -    applyCompositeOperator(m_state->m_compositeOp);
> -
> -    m_context->bindBuffer(GraphicsContext3D::ARRAY_BUFFER, getQuadVertices());
> +    m_context->applyCompositeOperator(m_state->m_compositeOp);
> +    m_context->useQuadVertices();
>  
>      AffineTransform matrix(m_flipMatrix);
>      matrix.multLeft(m_state->m_ctm);
>      matrix.translate(rect.x(), rect.y());
>      matrix.scale(rect.width(), rect.height());
> -    m_solidFillShader->use(matrix, color);
>  
> +    m_context->useFillSolidProgram(color, matrix);
> +
> +    bindFramebuffer();
>      m_context->drawArrays(GraphicsContext3D::TRIANGLE_STRIP, 0, 4);
>  }
>  
> @@ -165,21 +181,33 @@ void GLES2Canvas::restore()
>      m_state = &m_stateStack.last();
>  }
>  
> +void GLES2Canvas::drawTexturedRect(unsigned texture, const IntSize& textureSize, const FloatRect& srcRect, const FloatRect& dstRect, ColorSpace colorSpace, CompositeOperator compositeOp)
> +{
> +    m_context->applyCompositeOperator(compositeOp);
> +
> +    m_context->useQuadVertices();
> +    m_context->setActiveTexture(GraphicsContext3D::TEXTURE0);
> +
> +    m_context->bindTexture(GraphicsContext3D::TEXTURE_2D, texture);
> +
> +    drawQuad(textureSize, srcRect, dstRect, m_state->m_ctm, m_state->m_alpha);
> +}
> +
>  void GLES2Canvas::drawTexturedRect(Texture* texture, const FloatRect& srcRect, const FloatRect& dstRect, ColorSpace colorSpace, CompositeOperator compositeOp)
>  {
>      drawTexturedRect(texture, srcRect, dstRect, m_state->m_ctm, m_state->m_alpha, colorSpace, compositeOp);
>  }
>  
> +
>  void GLES2Canvas::drawTexturedRect(Texture* texture, const FloatRect& srcRect, const FloatRect& dstRect, const AffineTransform& transform, float alpha, ColorSpace colorSpace, CompositeOperator compositeOp)
>  {
> -    applyCompositeOperator(compositeOp);
> -
> -    m_context->bindBuffer(GraphicsContext3D::ARRAY_BUFFER, getQuadVertices());
> -    checkGLError("glBindBuffer");
> -
> +    m_context->applyCompositeOperator(compositeOp);
>      const TilingData& tiles = texture->tiles();
>      IntRect tileIdxRect = tiles.overlappedTileIndices(srcRect);
>  
> +    m_context->useQuadVertices();
> +    m_context->setActiveTexture(GraphicsContext3D::TEXTURE0);
> +
>      for (int y = tileIdxRect.y(); y <= tileIdxRect.bottom(); y++) {
>          for (int x = tileIdxRect.x(); x <= tileIdxRect.right(); x++)
>              drawTexturedRectTile(texture, tiles.tileIndex(x, y), srcRect, dstRect, transform, alpha);
> @@ -193,7 +221,6 @@ void GLES2Canvas::drawTexturedRectTile(Texture* texture, int tile, const FloatRe
>  
>      const TilingData& tiles = texture->tiles();
>  
> -    m_context->activeTexture(GraphicsContext3D::TEXTURE0);
>      texture->bindTile(tile);
>  
>      FloatRect srcRectClippedInTileSpace;
> @@ -202,18 +229,24 @@ void GLES2Canvas::drawTexturedRectTile(Texture* texture, int tile, const FloatRe
>  
>      IntRect tileBoundsWithBorder = tiles.tileBoundsWithBorder(tile);
>  
> +    drawQuad(IntSize(tileBoundsWithBorder.width(), tileBoundsWithBorder.height()), srcRectClippedInTileSpace, dstRectIntersected, transform, alpha);
> +}
> +
> +void GLES2Canvas::drawQuad(const IntSize& textureSize, const FloatRect& srcRect, const FloatRect& dstRect, const AffineTransform& transform, float alpha)
> +{
>      AffineTransform matrix(m_flipMatrix);
>      matrix.multLeft(transform);
> -    matrix.translate(dstRectIntersected.x(), dstRectIntersected.y());
> -    matrix.scale(dstRectIntersected.width(), dstRectIntersected.height());
> +    matrix.translate(dstRect.x(), dstRect.y());
> +    matrix.scale(dstRect.width(), dstRect.height());
>  
>      AffineTransform texMatrix;
> -    texMatrix.scale(1.0f / tileBoundsWithBorder.width(), 1.0f / tileBoundsWithBorder.height());
> -    texMatrix.translate(srcRectClippedInTileSpace.x(), srcRectClippedInTileSpace.y());
> -    texMatrix.scale(srcRectClippedInTileSpace.width(), srcRectClippedInTileSpace.height());
> +    texMatrix.scale(1.0f / textureSize.width(), 1.0f / textureSize.height());
> +    texMatrix.translate(srcRect.x(), srcRect.y());
> +    texMatrix.scale(srcRect.width(), srcRect.height());

Have these changes to the math been verified?

> -    m_texShader->use(matrix, texMatrix, 0, alpha);
> +    bindFramebuffer();
>  
> +    m_context->useTextureProgram(alpha, matrix, texMatrix);
>      m_context->drawArrays(GraphicsContext3D::TRIANGLE_STRIP, 0, 4);
>      checkGLError("glDrawArrays");
>  }
> @@ -223,98 +256,14 @@ void GLES2Canvas::setCompositeOperation(CompositeOperator op)
>      m_state->m_compositeOp = op;
>  }
>  
> -void GLES2Canvas::applyCompositeOperator(CompositeOperator op)
> -{
> -    if (op == m_lastCompositeOp)
> -        return;
> -
> -    switch (op) {
> -    case CompositeClear:
> -        m_context->enable(GraphicsContext3D::BLEND);
> -        m_context->blendFunc(GraphicsContext3D::ZERO, GraphicsContext3D::ZERO);
> -        break;
> -    case CompositeCopy:
> -        m_context->disable(GraphicsContext3D::BLEND);
> -        break;
> -    case CompositeSourceOver:
> -        m_context->enable(GraphicsContext3D::BLEND);
> -        m_context->blendFunc(GraphicsContext3D::ONE, GraphicsContext3D::ONE_MINUS_SRC_ALPHA);
> -        break;
> -    case CompositeSourceIn:
> -        m_context->enable(GraphicsContext3D::BLEND);
> -        m_context->blendFunc(GraphicsContext3D::DST_ALPHA, GraphicsContext3D::ZERO);
> -        break;
> -    case CompositeSourceOut:
> -        m_context->enable(GraphicsContext3D::BLEND);
> -        m_context->blendFunc(GraphicsContext3D::ONE_MINUS_DST_ALPHA, GraphicsContext3D::ZERO);
> -        break;
> -    case CompositeSourceAtop:
> -        m_context->enable(GraphicsContext3D::BLEND);
> -        m_context->blendFunc(GraphicsContext3D::DST_ALPHA, GraphicsContext3D::ONE_MINUS_SRC_ALPHA);
> -        break;
> -    case CompositeDestinationOver:
> -        m_context->enable(GraphicsContext3D::BLEND);
> -        m_context->blendFunc(GraphicsContext3D::ONE_MINUS_DST_ALPHA, GraphicsContext3D::ONE);
> -        break;
> -    case CompositeDestinationIn:
> -        m_context->enable(GraphicsContext3D::BLEND);
> -        m_context->blendFunc(GraphicsContext3D::ZERO, GraphicsContext3D::SRC_ALPHA);
> -        break;
> -    case CompositeDestinationOut:
> -        m_context->enable(GraphicsContext3D::BLEND);
> -        m_context->blendFunc(GraphicsContext3D::ZERO, GraphicsContext3D::ONE_MINUS_SRC_ALPHA);
> -        break;
> -    case CompositeDestinationAtop:
> -        m_context->enable(GraphicsContext3D::BLEND);
> -        m_context->blendFunc(GraphicsContext3D::ONE_MINUS_DST_ALPHA, GraphicsContext3D::SRC_ALPHA);
> -        break;
> -    case CompositeXOR:
> -        m_context->enable(GraphicsContext3D::BLEND);
> -        m_context->blendFunc(GraphicsContext3D::ONE_MINUS_DST_ALPHA, GraphicsContext3D::ONE_MINUS_SRC_ALPHA);
> -        break;
> -    case CompositePlusDarker:
> -    case CompositeHighlight:
> -        // unsupported
> -        m_context->disable(GraphicsContext3D::BLEND);
> -        break;
> -    case CompositePlusLighter:
> -        m_context->enable(GraphicsContext3D::BLEND);
> -        m_context->blendFunc(GraphicsContext3D::ONE, GraphicsContext3D::ONE);
> -        break;
> -    }
> -    m_lastCompositeOp = op;
> -}
> -
> -unsigned GLES2Canvas::getQuadVertices()
> -{
> -    if (!m_quadVertices) {
> -        float vertices[] = { 0.0f, 0.0f, 1.0f,
> -                             1.0f, 0.0f, 1.0f,
> -                             0.0f, 1.0f, 1.0f,
> -                             1.0f, 1.0f, 1.0f };
> -        m_quadVertices = m_context->createBuffer();
> -        m_context->bindBuffer(GraphicsContext3D::ARRAY_BUFFER, m_quadVertices);
> -        m_context->bufferData(GraphicsContext3D::ARRAY_BUFFER, sizeof(vertices), vertices, GraphicsContext3D::STATIC_DRAW);
> -    }
> -    return m_quadVertices;
> -}
> -
> -Texture* GLES2Canvas::createTexture(NativeImagePtr ptr, Texture::Format format, int width, int height)
> +PassRefPtr<Texture> GLES2Canvas::createTexture(NativeImagePtr ptr, Texture::Format format, int width, int height)

If the ownership of the Texture object is not being transferred -- and it doesn't look like it is, since the SharedContext3D conceptually owns the created Texture -- then this should return Texture*, not PassRefPtr<Texture>.

>  {
> -    PassRefPtr<Texture> texture = m_textures.get(ptr);
> -    if (texture)
> -        return texture.get();
> -
> -    texture = Texture::create(m_context, format, width, height);
> -    Texture* t = texture.get();
> -    m_textures.set(ptr, texture);
> -    return t;
> +    return m_context->createTexture(ptr, format, width, height);
>  }
>  
> -Texture* GLES2Canvas::getTexture(NativeImagePtr ptr)
> +PassRefPtr<Texture> GLES2Canvas::getTexture(NativeImagePtr ptr)

Similar ownership issue here -- this should return Texture*.

>  {
> -    PassRefPtr<Texture> texture = m_textures.get(ptr);
> -    return texture ? texture.get() : 0;
> +    return m_context->getTexture(ptr);
>  }
>  
>  void GLES2Canvas::checkGLError(const char* header)
> diff --git a/WebCore/platform/graphics/chromium/GLES2Canvas.h b/WebCore/platform/graphics/chromium/GLES2Canvas.h
> index f49ac8ba18d1d34234b8d06278e7028bfe9697e1..af3c0538f05f4072e20d173fe023b03926f6a69c 100644
> --- a/WebCore/platform/graphics/chromium/GLES2Canvas.h
> +++ b/WebCore/platform/graphics/chromium/GLES2Canvas.h
> @@ -44,17 +44,15 @@
>  
>  namespace WebCore {
>  
> +class CanvasFramebuffer;
> +class SharedContext3D;
>  class Color;
>  class FloatRect;
>  class GraphicsContext3D;
> -class SolidFillShader;
> -class TexShader;
> -
> -typedef HashMap<NativeImagePtr, RefPtr<Texture> > TextureHashMap;
>  
>  class GLES2Canvas : public Noncopyable {
>  public:
> -    GLES2Canvas(GraphicsContext3D*, const IntSize&);
> +    GLES2Canvas(SharedContext3D*, CanvasFramebuffer*, const IntSize&);
>      ~GLES2Canvas();
>  
>      void fillRect(const FloatRect&, const Color&, ColorSpace);
> @@ -74,28 +72,35 @@ public:
>      // non-standard functions
>      // These are not standard GraphicsContext functions, and should be pushed
>      // down into a PlatformContextGLES2 at some point.
> +    void drawTexturedRect(unsigned texture, const IntSize& textureSize, const FloatRect& srcRect, const FloatRect& dstRect, ColorSpace, CompositeOperator);
>      void drawTexturedRect(Texture*, const FloatRect& srcRect, const FloatRect& dstRect, const AffineTransform&, float alpha, ColorSpace, CompositeOperator);
>      void drawTexturedRect(Texture*, const FloatRect& srcRect, const FloatRect& dstRect, ColorSpace, CompositeOperator);
> -    GraphicsContext3D* context() { return m_context; }
> -    Texture* createTexture(NativeImagePtr, Texture::Format, int width, int height);
> -    Texture* getTexture(NativeImagePtr);
> +    PassRefPtr<Texture> createTexture(NativeImagePtr, Texture::Format, int width, int height);
> +    PassRefPtr<Texture> getTexture(NativeImagePtr);

These should return Texture*.

> +
> +    SharedContext3D* context() const { return m_context; }
> +
> +    void swapBuffers();
> +    void bindFramebuffer();
> +
> +    unsigned textureIdForCompositor() const;
> +    unsigned textureIdForCanvas() const;
>  
>  private:
>      void drawTexturedRectTile(Texture* texture, int tile, const FloatRect& srcRect, const FloatRect& dstRect, const AffineTransform&, float alpha);
> +    void drawQuad(const IntSize& textureSize, const FloatRect& srcRect, const FloatRect& dstRect, const AffineTransform&, float alpha);
>      void applyCompositeOperator(CompositeOperator);
>      void checkGLError(const char* header);
> -    unsigned getQuadVertices();
>  
> -    GraphicsContext3D* m_context;
> +    IntSize m_size;
> +
> +    SharedContext3D* m_context;
> +    CanvasFramebuffer* m_framebuffer;
> +
>      struct State;
>      WTF::Vector<State> m_stateStack;
>      State* m_state;
> -    unsigned m_quadVertices;
> -    OwnPtr<SolidFillShader> m_solidFillShader;
> -    OwnPtr<TexShader> m_texShader;
>      AffineTransform m_flipMatrix;
> -    TextureHashMap m_textures;
> -    CompositeOperator m_lastCompositeOp; // This is the one last set, not necessarily the one in the state stack.
>  };
>  
>  }
> diff --git a/WebCore/platform/graphics/gpu/CanvasFramebuffer.cpp b/WebCore/platform/graphics/gpu/CanvasFramebuffer.cpp
> new file mode 100644
> index 0000000000000000000000000000000000000000..fe4ddb471e222f853566bfc456f1e183adb7e969
> --- /dev/null
> +++ b/WebCore/platform/graphics/gpu/CanvasFramebuffer.cpp
> @@ -0,0 +1,110 @@
> +/*
> + * Copyright (c) 2010, Google Inc. All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are
> + * met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above
> + * copyright notice, this list of conditions and the following disclaimer
> + * in the documentation and/or other materials provided with the
> + * distribution.
> + *     * Neither the name of Google Inc. nor the names of its
> + * contributors may be used to endorse or promote products derived from
> + * this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include "config.h"
> +
> +#include "CanvasFramebuffer.h"
> +
> +#include "GraphicsContext3D.h"
> +#include "SharedContext3D.h"
> +
> +namespace WebCore {
> +
> +CanvasFramebuffer::CanvasFramebuffer(SharedContext3D* context, const IntSize& size, unsigned framebuffer, unsigned offscreenColorTexture, unsigned offscreenParentColorTexture)
> +    : m_context(context)
> +    , m_size(size)
> +    , m_framebuffer(framebuffer)
> +    , m_offscreenColorTexture(offscreenColorTexture)
> +    , m_offscreenParentColorTexture(offscreenParentColorTexture)
> +{
> +}
> +
> +CanvasFramebuffer::~CanvasFramebuffer()
> +{
> +    m_context->deleteTexture(m_offscreenColorTexture);
> +    m_context->deleteCompositorTexture(m_offscreenParentColorTexture);
> +    m_context->deleteFramebuffer(m_framebuffer);
> +}
> +
> +void CanvasFramebuffer::bind()
> +{
> +    m_context->bindFramebuffer(m_framebuffer);
> +    m_context->setViewport(m_size);
> +}
> +
> +void CanvasFramebuffer::swapBuffers()
> +{
> +    if (m_willSwapBuffersCallback)
> +        m_willSwapBuffersCallback->willSwapBuffers();
> +    m_context->setViewport(m_size);
> +    m_context->copyTextureToCompositor(m_offscreenColorTexture, m_offscreenParentColorTexture);
> +}
> +
> +void CanvasFramebuffer::setWillSwapBuffersCallback(PassOwnPtr<SwapBuffersCallback> callback)
> +{
> +    m_willSwapBuffersCallback = callback;
> +}
> +
> +PassOwnPtr<CanvasFramebuffer> CanvasFramebuffer::create(SharedContext3D* context, const IntSize& size)
> +{
> +    unsigned framebuffer = context->createFramebuffer();
> +    ASSERT(framebuffer);
> +    if (!framebuffer)
> +        return 0;
> +    context->bindFramebuffer(framebuffer);
> +
> +    // Color texture
> +    unsigned offscreenColorTexture = context->createTexture();
> +    ASSERT(offscreenColorTexture);
> +    if (!offscreenColorTexture) {
> +        context->deleteFramebuffer(framebuffer);
> +        return 0;
> +    }
> +    context->bindTexture(GraphicsContext3D::TEXTURE_2D, offscreenColorTexture);
> +    context->texParameteri(GraphicsContext3D::TEXTURE_2D, GraphicsContext3D::TEXTURE_MAG_FILTER, GraphicsContext3D::NEAREST);
> +    context->texParameteri(GraphicsContext3D::TEXTURE_2D, GraphicsContext3D::TEXTURE_MIN_FILTER, GraphicsContext3D::NEAREST);
> +    context->texParameteri(GraphicsContext3D::TEXTURE_2D, GraphicsContext3D::TEXTURE_WRAP_S, GraphicsContext3D::CLAMP_TO_EDGE);
> +    context->texParameteri(GraphicsContext3D::TEXTURE_2D, GraphicsContext3D::TEXTURE_WRAP_T, GraphicsContext3D::CLAMP_TO_EDGE);
> +    context->texImage2D(GraphicsContext3D::TEXTURE_2D, 0, GraphicsContext3D::RGBA, size.width(), size.height(), 0, GraphicsContext3D::RGBA, GraphicsContext3D::UNSIGNED_BYTE, 0);
> +
> +    unsigned offscreenParentColorTexture = context->createCompositorTexture(size);
> +    ASSERT(offscreenParentColorTexture);
> +    if (!offscreenParentColorTexture) {
> +        context->deleteTexture(offscreenColorTexture);
> +        context->deleteFramebuffer(framebuffer);
> +        return 0;
> +    }
> +
> +    context->framebufferTexture2D(GraphicsContext3D::FRAMEBUFFER, GraphicsContext3D::COLOR_ATTACHMENT0, GraphicsContext3D::TEXTURE_2D, offscreenColorTexture, 0);
> +
> +    return new CanvasFramebuffer(context, size, framebuffer, offscreenColorTexture, offscreenParentColorTexture);

return adoptPtr(new CanvasFrameBuffer(...));

> +}
> +
> +} // namespace WebCore
> diff --git a/WebCore/platform/graphics/gpu/SharedContext3D.cpp b/WebCore/platform/graphics/gpu/SharedContext3D.cpp
> new file mode 100644
> index 0000000000000000000000000000000000000000..3b738f01f7281392937fed5e0df257cc9cfb92f4
> --- /dev/null
> +++ b/WebCore/platform/graphics/gpu/SharedContext3D.cpp
> @@ -0,0 +1,325 @@
> +/*
> + * Copyright (c) 2010, Google Inc. All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are
> + * met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above
> + * copyright notice, this list of conditions and the following disclaimer
> + * in the documentation and/or other materials provided with the
> + * distribution.
> + *     * Neither the name of Google Inc. nor the names of its
> + * contributors may be used to endorse or promote products derived from
> + * this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include "config.h"
> +
> +#include "SharedContext3D.h"
> +
> +#include "AffineTransform.h"
> +#include "CanvasFramebuffer.h"
> +#include "Color.h"
> +#include "FloatRect.h"
> +#include "GraphicsContext3D.h"
> +#include "GraphicsTypes.h"
> +#include "IntSize.h"
> +#include "SolidFillShader.h"
> +#include "TexShader.h"
> +
> +#include <wtf/text/CString.h>
> +#include <wtf/text/WTFString.h>
> +
> +namespace WebCore {
> +
> +static inline void affineTo3x3(const AffineTransform& transform, float mat[9])
> +{
> +    mat[0] = transform.a();
> +    mat[1] = transform.b();
> +    mat[2] = 0.0f;
> +    mat[3] = transform.c();
> +    mat[4] = transform.d();
> +    mat[5] = 0.0f;
> +    mat[6] = transform.e();
> +    mat[7] = transform.f();
> +    mat[8] = 1.0f;
> +}
> +
> +// static
> +PassRefPtr<SharedContext3D> SharedContext3D::create(PassOwnPtr<GraphicsContext3D> context)
> +{
> +    return adoptRef(new SharedContext3D(context));
> +}
> +
> +SharedContext3D::SharedContext3D(PassOwnPtr<GraphicsContext3D> context)
> +    : m_context(context)
> +    , m_quadVertices(0)
> +    , m_solidFillShader(SolidFillShader::create(m_context.get()))
> +    , m_texShader(TexShader::create(m_context.get()))
> +
> +{
> +}
> +
> +SharedContext3D::~SharedContext3D()
> +{
> +    m_context->deleteBuffer(m_quadVertices);
> +}
> +
> +void SharedContext3D::scissor(const FloatRect& rect)
> +{
> +    m_context->scissor(rect.x(), rect.y(), rect.width(), rect.height());
> +}
> +
> +void SharedContext3D::enable(unsigned capacity)
> +{
> +    m_context->enable(capacity);
> +}
> +
> +void SharedContext3D::disable(unsigned capacity)
> +{
> +    m_context->disable(capacity);
> +}
> +
> +void SharedContext3D::clearColor(const Color& color)
> +{
> +    float rgba[4];
> +    color.getRGBA(rgba[0], rgba[1], rgba[2], rgba[3]);
> +    m_context->clearColor(rgba[0], rgba[1], rgba[2], rgba[3]);
> +}
> +
> +void SharedContext3D::clear(unsigned mask)
> +{
> +    m_context->clear(mask);
> +}
> +
> +void SharedContext3D::drawArrays(unsigned long mode, long first, long count)
> +{
> +    m_context->drawArrays(mode, first, count);
> +}
> +
> +unsigned long SharedContext3D::getError()
> +{
> +    return m_context->getError();
> +}
> +
> +void SharedContext3D::getIntegerv(unsigned long pname, int* value)
> +{
> +    m_context->getIntegerv(pname, value);
> +}
> +
> +unsigned SharedContext3D::createFramebuffer()
> +{
> +    return m_context->createFramebuffer();
> +}
> +
> +unsigned SharedContext3D::createTexture()
> +{
> +    return m_context->createTexture();
> +}
> +
> +void SharedContext3D::deleteFramebuffer(unsigned framebuffer)
> +{
> +    m_context->deleteFramebuffer(framebuffer);
> +}
> +
> +void SharedContext3D::deleteTexture(unsigned texture)
> +{
> +    m_context->deleteTexture(texture);
> +}
> +
> +void SharedContext3D::deleteCompositorTexture(unsigned texture)
> +{
> +    m_context->deleteCompositorTexture(texture);
> +}
> +
> +void SharedContext3D::framebufferTexture2D(unsigned long target, unsigned long attachment, unsigned long textarget, unsigned texture, long level)
> +{
> +    m_context->framebufferTexture2D(target, attachment, textarget, texture, level);
> +}
> +
> +void SharedContext3D::texParameteri(unsigned target, unsigned pname, int param)
> +{
> +    m_context->texParameteri(target, pname, param);
> +}
> +
> +int SharedContext3D::texImage2D(unsigned target, unsigned level, unsigned internalformat, unsigned width, unsigned height, unsigned border, unsigned format, unsigned type, void* pixels)
> +{
> +    m_context->texImage2D(target, level, internalformat, width, height, border, format, type, pixels);
> +}
> +
> +int SharedContext3D::texSubImage2D(unsigned target, unsigned level, unsigned xoffset, unsigned yoffset, unsigned width, unsigned height, unsigned format, unsigned type, void* pixels)
> +{
> +    m_context->texSubImage2D(target, level, xoffset, yoffset, width, height, format, type, pixels);
> +}
> +
> +void SharedContext3D::readPixels(long x, long y, unsigned long width, unsigned long height, unsigned long format, unsigned long type, void* data)
> +{
> +    m_context->readPixels(x, y, width, height, format, type, data);
> +}
> +
> +PassRefPtr<Texture> SharedContext3D::createTexture(NativeImagePtr ptr, Texture::Format format, int width, int height)

Since this class owns the Texture objects (ownership is not being transferred), this should return Texture*, not PassRefPtr<Texture>.

> +{
> +    RefPtr<Texture> texture;
> +    if (ptr)
> +        texture = m_textures.get(ptr);
> +    if (texture)
> +        return texture.get();
> +
> +    texture = Texture::create(m_context.get(), format, width, height);
> +    Texture* t = texture.get();
> +    if (ptr)
> +        m_textures.set(ptr, texture);
> +    return t;
> +}
> +
> +PassRefPtr<Texture> SharedContext3D::getTexture(NativeImagePtr ptr)

Should return Texture*.

> +{
> +    RefPtr<Texture> texture = m_textures.get(ptr);
> +    return texture ? texture.get() : 0;
> +}
> +
> +
> +void SharedContext3D::applyCompositeOperator(CompositeOperator op)
> +{
> +    switch (op) {
> +    case CompositeClear:
> +        m_context->enable(GraphicsContext3D::BLEND);
> +        m_context->blendFunc(GraphicsContext3D::ZERO, GraphicsContext3D::ZERO);
> +        break;
> +    case CompositeCopy:
> +        m_context->disable(GraphicsContext3D::BLEND);
> +        break;
> +    case CompositeSourceOver:
> +        m_context->enable(GraphicsContext3D::BLEND);
> +        m_context->blendFunc(GraphicsContext3D::ONE, GraphicsContext3D::ONE_MINUS_SRC_ALPHA);
> +        break;
> +    case CompositeSourceIn:
> +        m_context->enable(GraphicsContext3D::BLEND);
> +        m_context->blendFunc(GraphicsContext3D::DST_ALPHA, GraphicsContext3D::ZERO);
> +        break;
> +    case CompositeSourceOut:
> +        m_context->enable(GraphicsContext3D::BLEND);
> +        m_context->blendFunc(GraphicsContext3D::ONE_MINUS_DST_ALPHA, GraphicsContext3D::ZERO);
> +        break;
> +    case CompositeSourceAtop:
> +        m_context->enable(GraphicsContext3D::BLEND);
> +        m_context->blendFunc(GraphicsContext3D::DST_ALPHA, GraphicsContext3D::ONE_MINUS_SRC_ALPHA);
> +        break;
> +    case CompositeDestinationOver:
> +        m_context->enable(GraphicsContext3D::BLEND);
> +        m_context->blendFunc(GraphicsContext3D::ONE_MINUS_DST_ALPHA, GraphicsContext3D::ONE);
> +        break;
> +    case CompositeDestinationIn:
> +        m_context->enable(GraphicsContext3D::BLEND);
> +        m_context->blendFunc(GraphicsContext3D::ZERO, GraphicsContext3D::SRC_ALPHA);
> +        break;
> +    case CompositeDestinationOut:
> +        m_context->enable(GraphicsContext3D::BLEND);
> +        m_context->blendFunc(GraphicsContext3D::ZERO, GraphicsContext3D::ONE_MINUS_SRC_ALPHA);
> +        break;
> +    case CompositeDestinationAtop:
> +        m_context->enable(GraphicsContext3D::BLEND);
> +        m_context->blendFunc(GraphicsContext3D::ONE_MINUS_DST_ALPHA, GraphicsContext3D::SRC_ALPHA);
> +        break;
> +    case CompositeXOR:
> +        m_context->enable(GraphicsContext3D::BLEND);
> +        m_context->blendFunc(GraphicsContext3D::ONE_MINUS_DST_ALPHA, GraphicsContext3D::ONE_MINUS_SRC_ALPHA);
> +        break;
> +    case CompositePlusDarker:
> +    case CompositeHighlight:
> +        // unsupported
> +        m_context->disable(GraphicsContext3D::BLEND);
> +        break;
> +    case CompositePlusLighter:
> +        m_context->enable(GraphicsContext3D::BLEND);
> +        m_context->blendFunc(GraphicsContext3D::ONE, GraphicsContext3D::ONE);
> +        break;
> +    }
> +}
> +
> +void SharedContext3D::useQuadVertices()
> +{
> +    if (!m_quadVertices) {
> +        float vertices[] = { 0.0f, 0.0f, 1.0f,
> +                             1.0f, 0.0f, 1.0f,
> +                             0.0f, 1.0f, 1.0f,
> +                             1.0f, 1.0f, 1.0f };
> +        m_quadVertices = m_context->createBuffer();
> +        m_context->bindBuffer(GraphicsContext3D::ARRAY_BUFFER, m_quadVertices);
> +        m_context->bufferData(GraphicsContext3D::ARRAY_BUFFER, sizeof(vertices), vertices, GraphicsContext3D::STATIC_DRAW);
> +    } else {
> +        m_context->bindBuffer(GraphicsContext3D::ARRAY_BUFFER, m_quadVertices);
> +    }
> +}
> +
> +void SharedContext3D::setActiveTexture(unsigned textureUnit)
> +{
> +    m_context->activeTexture(textureUnit);
> +}
> +
> +void SharedContext3D::bindTexture(unsigned target, unsigned texture)
> +{
> +    if (target != GraphicsContext3D::TEXTURE_2D) {
> +        m_context->bindTexture(target, texture);
> +        return;
> +    }
> +
> +    m_context->bindTexture(target, texture);
> +}
> +
> +void SharedContext3D::useFillSolidProgram(const Color& color, const AffineTransform& transform)
> +{
> +    m_solidFillShader->use(transform, color);
> +}
> +
> +void SharedContext3D::useTextureProgram(float alpha, const AffineTransform& transform, const AffineTransform& texTransform)
> +{
> +    m_texShader->use(transform, texTransform, 0, alpha);
> +}
> +
> +void SharedContext3D::bindFramebuffer(unsigned framebuffer)
> +{
> +    m_context->bindFramebuffer(GraphicsContext3D::FRAMEBUFFER, framebuffer);
> +}
> +
> +void SharedContext3D::setViewport(const IntSize& size)
> +{
> +    m_context->viewport(0, 0, size.width(), size.height());
> +}
> +
> +bool SharedContext3D::supportsBGRA()
> +{
> +    return m_context->supportsBGRA();
> +}
> +
> +unsigned SharedContext3D::createCompositorTexture(const IntSize& size)
> +{
> +    return m_context->createCompositorTexture(size.width(), size.height());
> +}
> +
> +void SharedContext3D::copyTextureToCompositor(unsigned source, unsigned destination)
> +{
> +    m_context->copyTextureToCompositor(source, destination);
> +}
> +
> +bool SharedContext3D::paintsIntoCanvasBuffer() const
> +{
> +    return m_context->paintsIntoCanvasBuffer();
> +}
> +
> +} // namespace WebCore
> diff --git a/WebCore/platform/graphics/gpu/SharedContext3D.h b/WebCore/platform/graphics/gpu/SharedContext3D.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..8abcb316dd27b98e7a4684adec208587adcbbf8f
> --- /dev/null
> +++ b/WebCore/platform/graphics/gpu/SharedContext3D.h
> @@ -0,0 +1,121 @@
> +/*
> + * Copyright (c) 2010, Google Inc. All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are
> + * met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above
> + * copyright notice, this list of conditions and the following disclaimer
> + * in the documentation and/or other materials provided with the
> + * distribution.
> + *     * Neither the name of Google Inc. nor the names of its
> + * contributors may be used to endorse or promote products derived from
> + * this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef SharedContext3D_h
> +#define SharedContext3D_h
> +
> +#include "GraphicsTypes.h"
> +#include "ImageSource.h"
> +#include "Texture.h"
> +
> +#include <wtf/HashMap.h>
> +#include <wtf/OwnPtr.h>
> +#include <wtf/RefCounted.h>
> +#include <wtf/RefPtr.h>
> +
> +namespace WebCore {
> +
> +class AffineTransform;
> +class CanvasFramebuffer;
> +class Color;
> +class GraphicsContext3D;
> +class FloatRect;
> +class IntSize;
> +class SolidFillShader;
> +class TexShader;
> +
> +typedef HashMap<NativeImagePtr, RefPtr<Texture> > TextureHashMap;
> +
> +class SharedContext3D : public RefCounted<SharedContext3D> {
> +public:
> +    static PassRefPtr<SharedContext3D> create(PassOwnPtr<GraphicsContext3D>);
> +    ~SharedContext3D();
> +
> +    // Functions that delegate directly to GraphicsContext3D, with caching
> +    void bindFramebuffer(unsigned framebuffer);
> +    void setViewport(const IntSize&);
> +    void scissor(const FloatRect&);
> +    void enable(unsigned capacity);
> +    void disable(unsigned capacity);
> +    void clearColor(const Color&);
> +    void clear(unsigned mask);
> +    void drawArrays(unsigned long mode, long first, long count);
> +    unsigned long getError();
> +    void getIntegerv(unsigned long pname, int* value);
> +
> +    unsigned createFramebuffer();
> +    unsigned createTexture();
> +
> +    void deleteFramebuffer(unsigned framebuffer);
> +    void deleteTexture(unsigned texture);
> +    void deleteCompositorTexture(unsigned texture);
> +
> +    void framebufferTexture2D(unsigned long target, unsigned long attachment, unsigned long textarget, unsigned, long level);
> +    void texParameteri(unsigned target, unsigned pname, int param);
> +    int texImage2D(unsigned target, unsigned level, unsigned internalformat, unsigned width, unsigned height, unsigned border, unsigned format, unsigned type, void* pixels);
> +    int texSubImage2D(unsigned target, unsigned level, unsigned xoffset, unsigned yoffset, unsigned width, unsigned height, unsigned format, unsigned type, void* pixels);
> +
> +    void readPixels(long x, long y, unsigned long width, unsigned long height, unsigned long format, unsigned long type, void* data);
> +
> +    bool supportsBGRA();
> +    unsigned createCompositorTexture(const IntSize&);
> +    void copyTextureToCompositor(unsigned, unsigned);
> +
> +    bool paintsIntoCanvasBuffer() const;
> +
> +    // Shared logic for canvas 2d
> +    void applyCompositeOperator(CompositeOperator);
> +    void useQuadVertices();
> +
> +    void useFillSolidProgram(const Color&, const AffineTransform&);
> +    void useTextureProgram(float alpha, const AffineTransform&, const AffineTransform&);
> +
> +    void setActiveTexture(unsigned textureUnit);
> +    void bindTexture(unsigned target, unsigned texture);
> +
> +    PassRefPtr<Texture> createTexture(NativeImagePtr, Texture::Format, int width, int height);
> +    PassRefPtr<Texture> getTexture(NativeImagePtr);

Should return Texture*, not PassRefPtr<Texture>.

> +
> +private:
> +    SharedContext3D(PassOwnPtr<GraphicsContext3D> context);
> +
> +    OwnPtr<GraphicsContext3D> m_context;
> +
> +    unsigned m_quadVertices;
> +
> +    OwnPtr<SolidFillShader> m_solidFillShader;
> +    OwnPtr<TexShader> m_texShader;
> +
> +    TextureHashMap m_textures;
> +};
> +
> +} // namespace WebCore
> +
> +#endif // SharedContext3D_h
> diff --git a/WebCore/platform/graphics/skia/ImageSkia.cpp b/WebCore/platform/graphics/skia/ImageSkia.cpp
> index aed289fe3c155aa5d3596d494b60893e4c016bb6..c517a8a808ba9f6efb0b821ef158dc4ddb99e73f 100644
> --- a/WebCore/platform/graphics/skia/ImageSkia.cpp
> +++ b/WebCore/platform/graphics/skia/ImageSkia.cpp
> @@ -47,6 +47,7 @@
>  #include "SkRect.h"
>  #include "SkShader.h"
>  #include "SkiaUtils.h"
> +#include "Texture.h"
>  
>  #include "skia/ext/image_operations.h"
>  #include "skia/ext/platform_canvas.h"
> @@ -411,7 +412,7 @@ static void drawBitmapGLES2(GraphicsContext* ctxt, NativeImageSkia* bitmap, cons
>  {
>      ctxt->platformContext()->prepareForHardwareDraw();
>      GLES2Canvas* gpuCanvas = ctxt->platformContext()->gpuCanvas();
> -    Texture* texture = gpuCanvas->getTexture(bitmap);
> +    RefPtr<Texture> texture = gpuCanvas->getTexture(bitmap);

Because getTexture() will no longer return PassRefPtr, this doesn't need to change.

>      if (!texture) {
>          ASSERT(bitmap->config() == SkBitmap::kARGB_8888_Config);
>          ASSERT(bitmap->rowBytes() == bitmap->width() * 4);
> @@ -420,7 +421,7 @@ static void drawBitmapGLES2(GraphicsContext* ctxt, NativeImageSkia* bitmap, cons
>          ASSERT(bitmap->getPixels());
>          texture->load(bitmap->getPixels());
>      }
> -    gpuCanvas->drawTexturedRect(texture, srcRect, dstRect, styleColorSpace, compositeOp);
> +    gpuCanvas->drawTexturedRect(texture.get(), srcRect, dstRect, styleColorSpace, compositeOp);

Same here.

>  }
>  
>  // ================================================
> diff --git a/WebKit/chromium/src/ChromeClientImpl.cpp b/WebKit/chromium/src/ChromeClientImpl.cpp
> index e6f14007180d8d3bf6128b130dc28abaaec1f90a..42424453c34691358b8371d2d7f0d3a49c2834bb 100644
> --- a/WebKit/chromium/src/ChromeClientImpl.cpp
> +++ b/WebKit/chromium/src/ChromeClientImpl.cpp
> @@ -59,6 +59,7 @@
>  #include "SearchPopupMenuChromium.h"
>  #include "ScriptController.h"
>  #include "SecurityOrigin.h"
> +#include "SharedContext3D.h"
>  #include "WebGeolocationService.h"
>  #if USE(V8)
>  #include "V8Proxy.h"
> @@ -751,6 +752,11 @@ void ChromeClientImpl::scheduleCompositingLayerSync()
>  }
>  #endif
>  
> +PassRefPtr<WebCore::SharedContext3D> ChromeClientImpl::getSharedContext3D()

See below about lack of ownership transfer.

> +{
> +    return m_webView->getSharedContext3D();
> +}
> +
>  bool ChromeClientImpl::supportsFullscreenForNode(const WebCore::Node* node)
>  {
>      if (m_webView->client() && node->hasTagName(WebCore::HTMLNames::videoTag))
> diff --git a/WebKit/chromium/src/ChromeClientImpl.h b/WebKit/chromium/src/ChromeClientImpl.h
> index bff9f9047fa7c7f3abd704d94785d9c1facf7740..53db94f79af605fc0c9c09a21fb329375dc5f97e 100644
> --- a/WebKit/chromium/src/ChromeClientImpl.h
> +++ b/WebKit/chromium/src/ChromeClientImpl.h
> @@ -152,6 +152,8 @@ public:
>      virtual void scheduleCompositingLayerSync();
>  #endif
>  
> +    virtual PassRefPtr<WebCore::SharedContext3D> getSharedContext3D();

Because ownership isn't being transferred, this should return WebCore::SharedContext3D* rather than PassRefPtr<...>.

> +
>      virtual bool supportsFullscreenForNode(const WebCore::Node*);
>      virtual void enterFullscreenForNode(WebCore::Node*);
>      virtual void exitFullscreenForNode(WebCore::Node*);
> diff --git a/WebKit/chromium/src/WebViewImpl.cpp b/WebKit/chromium/src/WebViewImpl.cpp
> index 5065f5eb9b64083fc3b1e174f42815c5737b4839..d2331a3f3e5ab62e3a7d554cd7dd848c2090ce6d 100644
> --- a/WebKit/chromium/src/WebViewImpl.cpp
> +++ b/WebKit/chromium/src/WebViewImpl.cpp
> @@ -58,6 +58,7 @@
>  #include "GLES2Context.h"
>  #include "GLES2ContextInternal.h"
>  #include "GraphicsContext.h"
> +#include "GraphicsContext3D.h"
>  #include "HTMLInputElement.h"
>  #include "HTMLMediaElement.h"
>  #include "HitTestResult.h"
> @@ -86,6 +87,7 @@
>  #include "SecurityOrigin.h"
>  #include "SelectionController.h"
>  #include "Settings.h"
> +#include "SharedContext3D.h"
>  #include "Timer.h"
>  #include "TypingCommand.h"
>  #include "UserGestureIndicator.h"
> @@ -2211,14 +2213,16 @@ PassOwnPtr<GLES2Context> WebViewImpl::getOnscreenGLES2Context()
>      return GLES2Context::create(GLES2ContextInternal::create(gles2Context(), false));
>  }
>  
> -PassOwnPtr<GLES2Context> WebViewImpl::getOffscreenGLES2Context()
> +PassRefPtr<SharedContext3D> WebViewImpl::getSharedContext3D()

Should return SharedContext3D* since ownership is not being transferred.

>  {
> -    WebGLES2Context* context = webKitClient()->createGLES2Context();
> -    if (!context)
> -        return 0;
> -    if (!context->initialize(0, gles2Context()))
> -        return 0;
> -    return GLES2Context::create(GLES2ContextInternal::create(context, true));
> +    if (!m_sharedContext3D) {
> +        GraphicsContext3D::Attributes attr;
> +        attr.stencil = true;
> +        OwnPtr<GraphicsContext3D> context = GraphicsContext3D::create(attr, m_page->chrome());
> +        m_sharedContext3D = SharedContext3D::create(context.release());
> +    }
> +
> +    return m_sharedContext3D;
>  }
>  
>  // Returns the GLES2 context associated with this View. If one doesn't exist
> diff --git a/WebKit/chromium/src/WebViewImpl.h b/WebKit/chromium/src/WebViewImpl.h
> index c29612123f4514c94dd7d970c73114a4beeec355..4ad74fb12da3ff55f8c82ed22a0e3a89ff1ba2fd 100644
> --- a/WebKit/chromium/src/WebViewImpl.h
> +++ b/WebKit/chromium/src/WebViewImpl.h
> @@ -329,10 +329,10 @@ public:
>      // Offscreen contexts render offscreen but can share resources with the
>      // onscreen context and thus can be composited.
>      PassOwnPtr<WebCore::GLES2Context> getOnscreenGLES2Context();
> -    PassOwnPtr<WebCore::GLES2Context> getOffscreenGLES2Context();
>  
>      // Returns an onscreen context
>      virtual WebGLES2Context* gles2Context();
> +    virtual PassRefPtr<WebCore::SharedContext3D> getSharedContext3D();

SharedContext3D*.

>  
>      WebCore::PopupContainer* selectPopup() const { return m_selectPopup.get(); }
>  
> @@ -521,6 +521,8 @@ private:
>  
>      OwnPtr<WebGLES2Context> m_gles2Context;
>  
> +    RefPtr<WebCore::SharedContext3D> m_sharedContext3D;
> +
>      OwnPtr<DeviceOrientationClientProxy> m_deviceOrientationClientProxy;
>  };
>

[Vangelis Kokkevis]

Comment on attachment 66074 [details]
compile fix for gtk/win/mac

Looks good!  Just a couple of comments, inlined below:

> +void Canvas2DLayerChromium::updateContents()
> +{
> +    unsigned textureId = m_framebuffer->textureIdForCompositor();

m_textureChanged isn't set for this layer type.  There are two options here:

1. Make sure the proper modes are set when GraphicsContext3D creates the compositor texture so you don't worry about it here
2. Move this logic up to CanvasLayerChromium::draw() and set the modes before using the texture if m_textureChanged == true

I prefer #1 if possible.

> +    if (m_textureChanged) {
> +        glBindTexture(GL_TEXTURE_2D, textureId);
> +        // Set the min-mag filters to linear and wrap modes to GL_CLAMP_TO_EDGE
> +        // to get around NPOT texture limitations of GLES.
> +        glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
> +        glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
> +        glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
> +        glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
> +        m_textureChanged = false;
> +    }
> +    // Update the contents of the texture used by the compositor.
> +    if (m_contentsDirty) {
> +        m_framebuffer->swapBuffers();
> +        m_contentsDirty = false;
> +    }
> +}
> +
> diff --git a/WebCore/platform/graphics/chromium/Canvas2DLayerChromium.h b/WebCore/platform/graphics/chromium/Canvas2DLayerChromium.h
> +
> +// A layer containing an accelerated 2d canvas
> +class Canvas2DLayerChromium : public CanvasLayerChromium {
> +public:
> +    static PassRefPtr<Canvas2DLayerChromium> create(CanvasFramebuffer*, GraphicsLayerChromium* owner);
> +    virtual bool drawsContent() { return true; }
> +    virtual void updateContents();
> +
> +    void setCanvasFramebuffer(CanvasFramebuffer*);
> +
> +protected:
> +    virtual unsigned textureId();
> +
> +private:
> +    explicit Canvas2DLayerChromium(CanvasFramebuffer*, GraphicsLayerChromium* owner);
> +    CanvasFramebuffer* m_framebuffer;
> +    bool m_textureChanged;

m_textureChanged is already defined in CanvasLayerChromium so no need to re-define it here. Please see comment above.

> +
> +    static unsigned m_shaderProgramId;
> +};
> +
> +}
> diff --git a/WebCore/platform/graphics/chromium/CanvasLayerChromium.h b/WebCore/platform/graphics/chromium/CanvasLayerChromium.h
> index 053efffa01e142838121453c1a4b8a194b6fa4c1..d6da6ac44e9107af100e54d3ecae365562de0d2f 100644
> --- a/WebCore/platform/graphics/chromium/CanvasLayerChromium.h
> +++ b/WebCore/platform/graphics/chromium/CanvasLayerChromium.h
> @@ -38,17 +38,13 @@
>  
>  namespace WebCore {
>  
> -class GraphicsContext3D;
> -
> -// A Layer containing a WebGL or accelerated 2d canvas
> +// Base class for WebGL and accelerated 2d canvases.

Should description be:
"Base class for _layers_ containing WebGL and accelerated 2d canvases"? 

>  class CanvasLayerChromium : public LayerChromium {
>  public:
> -    static PassRefPtr<CanvasLayerChromium> create(GraphicsLayerChromium* owner = 0);
> -    virtual bool drawsContent() { return m_context; }
> -    virtual void updateContents();
> -    virtual void draw();
> +    explicit CanvasLayerChromium(GraphicsLayerChromium* owner);

Shouldn't the constructor be in the private or protected section?

> +    virtual ~CanvasLayerChromium();
>  
> -    void setContext(const GraphicsContext3D* context);
> +    virtual void draw();
>  
>      class SharedValues {
>      public:
> @@ -69,21 +65,16 @@ public:
>          bool m_initialized;
>      };
>  
> -    class PrepareTextureCallback : public Noncopyable {
> +
> +void WebGLLayerChromium::updateContents()
> +{
> +    ASSERT(m_context);
> +    if (m_textureChanged) {

See previous comments.

> +        glBindTexture(GL_TEXTURE_2D, m_textureId);
> +        // Set the min-mag filters to linear and wrap modes to GL_CLAMP_TO_EDGE
> +        // to get around NPOT texture limitations of GLES.
> +        glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
> +        glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
> +        glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
> +        glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
> +        m_textureChanged = false;
> +    }
> +    // Update the contents of the texture used by the compositor.
> +    if (m_contentsDirty) {
> +        m_context->prepareTexture();
> +        m_contentsDirty = false;
> +    }
> +}
> +
> +void WebGLLayerChromium::setContext(const GraphicsContext3D* context)
> +{
> +    m_context = const_cast<GraphicsContext3D*>(context);
> +
> +    unsigned int textureId = m_context->platformTexture();

You should probably check for textureId == 0 as that indicates a failure somewhere.

> +    if (textureId != m_textureId)
> +        m_textureChanged = true;
> +    m_textureId = textureId;
> +}
> +
> +}
> diff --git a/WebCore/platform/graphics/gpu/CanvasFramebuffer.h b/WebCore/platform/graphics/gpu/CanvasFramebuffer.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..d52aba1cb68acbced5d34eb6348c5c890333d903
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef CanvasFramebuffer_h
> +#define CanvasFramebuffer_h
> +
> +// Manages a framebuffer to use for a 2d canvas.

move the above comment down, right before the class definition?

> +
> +#include "IntSize.h"
> +
> +#include <wtf/Noncopyable.h>
> +#include <wtf/OwnPtr.h>
> +#include <wtf/PassOwnPtr.h>
> +
> +namespace WebCore {
> +
> +class SharedContext3D;
> +
> +class CanvasFramebuffer : public Noncopyable {
> +public:
> +    static PassOwnPtr<CanvasFramebuffer> create(SharedContext3D*, const IntSize&);
> +    ~CanvasFramebuffer();
> +
> +    void bind();
> diff --git a/WebCore/platform/graphics/gpu/SharedContext3D.cpp b/WebCore/platform/graphics/gpu/SharedContext3D.cpp
> new file mode 100644
> index 0000000000000000000000000000000000000000..3b738f01f7281392937fed5e0df257cc9cfb92f4
> --- /dev/null
> +
> +// static
> +PassRefPtr<SharedContext3D> SharedContext3D::create(PassOwnPtr<GraphicsContext3D> context)
> +{
> +    return adoptRef(new SharedContext3D(context));
> +}
> +
> +SharedContext3D::SharedContext3D(PassOwnPtr<GraphicsContext3D> context)
> +    : m_context(context)
> +    , m_quadVertices(0)
> +    , m_solidFillShader(SolidFillShader::create(m_context.get()))
> +    , m_texShader(TexShader::create(m_context.get()))
> +

Please remove necessary new-line.
> +{
> +}
> +
> +SharedContext3D::~SharedContext3D()
> +{

[Chris Marrin]

Comment on attachment 66074 [details]
compile fix for gtk/win/mac

> WebCore/platform/graphics/gpu/CanvasFramebuffer.cpp:67
> +    m_context->copyTextureToCompositor(m_offscreenColorTexture, m_offscreenParentColorTexture);

This model is wrong for EAGL (iOS). There, the model is that you are handing the FBO object to the PlatformLayer. The object you're passing is actually a RenderBuffer not a Texture, but it is still represented by an OpenGL Object handle. The model should change to hide the m_offscreenParentColorTexture in the PlatformLayer and have this call pass m_offscreenColorBuffer (renamed for clarity) and PlatformLayer.

[Chris Marrin]

(In reply to comment #15)
> (From update of attachment 66074 [details])
> > WebCore/platform/graphics/gpu/CanvasFramebuffer.cpp:67
> > +    m_context->copyTextureToCompositor(m_offscreenColorTexture, m_offscreenParentColorTexture);
> 
> This model is wrong for EAGL (iOS). There, the model is that you are handing the FBO object to the PlatformLayer. The object you're passing is actually a RenderBuffer not a Texture, but it is still represented by an OpenGL Object handle. The model should change to hide the m_offscreenParentColorTexture in the PlatformLayer and have this call pass m_offscreenColorBuffer (renamed for clarity) and PlatformLayer.

The function would also be better named copyColorBufferToPlatformLayer() or something. 

For more on the EAGL model, it is actually a copy model. Once you hand the RenderBuffer to the Layer, that same buffer can be cleared and used again immediately. Some magic happens under the covers to adopt the current RenderBuffer by the Layer and have it replaced by a new buffer.

[James Robinson]

Thanks for looking through this patch.  I'll do my best to address all the line-by-line comments.

The point you make about the FBO treatment is a valid one, Chris.  We'll also want to generalize this in order to support multisampling.  Here's what I propose to make this more general:

1.) No new functions on GraphicsContext3D
2.) CanvasFramebuffer adopts a more general interface to look something like:

create(SharedContext3D*, IntSize)
bind()
publishToPlatformLayer(PlatformLayer*)
publishToTexture(unsigned)

and not expose any specifics about what the FBO binds to.  Then implementations can bind a CanvasFramebuffer to either a color texture or a RenderBuffer and deal with the details of how to expose the rendering results to either a PlatformLayer (for compositing) or a texture (to draw the rendering results in another canvas).  The specifics of allocating a compositing target (texture, CALayer, etc) and doing the copy/blit/resolve will be handled by each platform.

Follow up incoming.

[Chris Marrin]

(In reply to comment #17)
> Thanks for looking through this patch.  I'll do my best to address all the line-by-line comments.
> 
> The point you make about the FBO treatment is a valid one, Chris.  We'll also want to generalize this in order to support multisampling.  Here's what I propose to make this more general:
> 
> 1.) No new functions on GraphicsContext3D
> 2.) CanvasFramebuffer adopts a more general interface to look something like:
> 
> create(SharedContext3D*, IntSize)
> bind()
> publishToPlatformLayer(PlatformLayer*)
> publishToTexture(unsigned)
> 
> and not expose any specifics about what the FBO binds to.  Then implementations can bind a CanvasFramebuffer to either a color texture or a RenderBuffer and deal with the details of how to expose the rendering results to either a PlatformLayer (for compositing) or a texture (to draw the rendering results in another canvas).  The specifics of allocating a compositing target (texture, CALayer, etc) and doing the copy/blit/resolve will be handled by each platform.

This looks pretty good. Since CanvasFrameBuffer is intimately tied to SharedContext3D, perhaps its ctor should not be public and there should be a createFramebuffer method on SharedContext? That would make the relationship more clear I think. I like the model of the context vending the buffer you'll be drawing into.

How are you creating the PlatformLayer? Doesn't the SharedContext need to vend that as well? Did I miss the call that does that?

Also, I'm not sure it's appropriate to call this a Framebuffer. It's really an FBO, plus Renderbuffers, plus context info. So maybe calling it a DrawingBuffer would be more appropriate. That's the term we use in the WebGL spec and it really refers to the combination of all these things. Also, using the term "Canvas" here is misleading. this is more general that just Canvas.

And while I'm being the pedantic naming police, SharedContext3D would be more accurately named SharedGraphicsContext3D.

[James Robinson]

(In reply to comment #12)
> > WebCore/html/canvas/CanvasRenderingContext2D.cpp:1283
> >      sourceCanvas->makeRenderingResultsAvailable();
> Are you sure it isn't necessary to do something here?  Obviously, reading back to the buffer isn't right in this case, but we need some way to make sure that the sourceCanvas is up-to-date (e.g., last software draw is uploaded to framebuffer) before we draw.
> 
> And actually, I think this will break the software path, since it needs to make this call.  If the GPU truly doesn't need this, it should probably be doing an !isAccelerated() check instead.
> 
> (Reading on a bit further, it seems you're doing the GPU equivalent of this call in drawImageBuffer().  Fair enough, although it would be kind of cool to unify them in the same makeRenderingResultsAvailable() function.  And you'll still need to fix the software path).

I'll make this change separately to keep things simpler.

> > WebCore/platform/graphics/chromium/GLES2Canvas.cpp:261
> > +    return m_context->createTexture(ptr, format, width, height);
> OT:  Now that uploaded textures are shared between all contexts, I think we could actually get rid of the hash table entirely, and put a RefPtr<Texture> in the NativeImageSkia 
itself, if we wanted to.  This should help tie its lifetime to that of the image cache.

No can do - you can have an arbitrary number of compositors in a process each with its own SharedContext3D.  It's not just one per process, it's one per compositor.

> > WebCore/platform/graphics/skia/PlatformContextSkia.cpp:791
> > +        m_uploadTexture = context->createTexture(0, Texture::BGRA8, bitmap.width(), bitmap.height());
> Hmm.. hashing on the NULL ptr (besides making it "magic" and fragile, in case anyone else tries the same trick) will share a single upload texture for all canvases.  This means we might hang onto an 8K texture long after a big canvas is gone, and we only need to upload little 150x150 canvases, e.g.

A NULL pointer means "do not cache" - look at the changes in createTexture().  It is slightly magic but we need a way to create and upload textures without any caching at all and this seemed as good as any.

> > WebCore/platform/graphics/skia/PlatformContextSkia.h:184
> > +    void setSharedContext3D(SharedContext3D*, CanvasFramebuffer*, const IntSize&);
> I'd rather the CanvasFramebuffer be owned by the GLES2Canvas, if possible.  But we can talk about that later.

Maybe later :)

Everything else fixed in my soon to be posted patch.

[James Robinson]

> > > WebCore/platform/graphics/skia/PlatformContextSkia.cpp:791
> > > +        m_uploadTexture = context->createTexture(0, Texture::BGRA8, bitmap.width(), bitmap.height());
> > Hmm.. hashing on the NULL ptr (besides making it "magic" and fragile, in case anyone else tries the same trick) will share a single upload texture for all canvases.  This means we might hang onto an 8K texture long after a big canvas is gone, and we only need to upload little 150x150 canvases, e.g.
> 
> A NULL pointer means "do not cache" - look at the changes in createTexture().  It is slightly magic but we need a way to create and upload textures without any caching at all and this seemed as good as any.

Actually, this idea sucked.  I'll add another API for creating a non-cached texture.

[James Robinson]

Created attachment 66296 [details]
Patch

[James Robinson]

Significant changes since previous patch:

rename CanvasFramebuffer to DrawingBuffer
change the interface to simply:
 bind();
 publishToPlatformLayer(PlatformLayer*);

where publishToPlatformLayer() is implemented in a platform-dependent way to do the right thing.

This class is also now RefCounted so that the CanvasRenderingContext2D and the compositing layer can keep references to one.  This is needed because the DrawingBuffer on a CanvasRenderingContext2D can be reset at any time and the compositing layer tree is lazily updated via the style recalc mechanism.  I removed the ability to publish to a texture for now, that will be in a follow-up.

Fix PassRefPtr<> for Texture and SharedContext3D in multiple places.  Turns out I didn't quite know what PRP was for, but I think I do now :)

Switch shader order around in SharedContext3D and GLES2Canvas to match the Shader interface.

Cleaned up the CanvasLayerChromium hierarchy a bit.  m_textureChanged and m_textureId now are protected members of the base class and are treated in the same way.  I didn't change the tex parameter setting for the WebGL side, although I agree that's a good cleanup to do someday.  In Canvas2DLayerChromium the texture is actually owned by the compositing layer.

[James Robinson]

(In reply to comment #18)
> (In reply to comment #17)
> > Thanks for looking through this patch.  I'll do my best to address all the line-by-line comments.
> > 
> > The point you make about the FBO treatment is a valid one, Chris.  We'll also want to generalize this in order to support multisampling.  Here's what I propose to make this more general:
> > 
> > 1.) No new functions on GraphicsContext3D
> > 2.) CanvasFramebuffer adopts a more general interface to look something like:
> > 
> > create(SharedContext3D*, IntSize)
> > bind()
> > publishToPlatformLayer(PlatformLayer*)
> > publishToTexture(unsigned)
> > 
> > and not expose any specifics about what the FBO binds to.  Then implementations can bind a CanvasFramebuffer to either a color texture or a RenderBuffer and deal with the details of how to expose the rendering results to either a PlatformLayer (for compositing) or a texture (to draw the rendering results in another canvas).  The specifics of allocating a compositing target (texture, CALayer, etc) and doing the copy/blit/resolve will be handled by each platform.
> 
> This looks pretty good. Since CanvasFrameBuffer is intimately tied to SharedContext3D, perhaps its ctor should not be public and there should be a createFramebuffer method on SharedContext? That would make the relationship more clear I think. I like the model of the context vending the buffer you'll be drawing into.
> 
> How are you creating the PlatformLayer? Doesn't the SharedContext need to vend that as well? Did I miss the call that does that?

The PlatformLayer is created by the platform-specific implementation of GraphicsLayer::setContentsToCanvas2D().  It's made aware of the DrawingBuffer at creation time and whenever the compositing layer tree is regenerated.  PlatformLayer creation and initialization is inherently platform-specific (as the name of the type would imply).

I left the DrawingBuffer::create() as a static function on DrawingBuffer so that it's easier to find the implementation.  There will eventually be a number of platform-dependent implementations of this create and I think it'd be better if the various DrawingBufferXXX.cpp files contained only DrawingBuffer:: functions instead of having a function declared on SharedContext3D.

> 
> Also, I'm not sure it's appropriate to call this a Framebuffer. It's really an FBO, plus Renderbuffers, plus context info. So maybe calling it a DrawingBuffer would be more appropriate. That's the term we use in the WebGL spec and it really refers to the combination of all these things. Also, using the term "Canvas" here is misleading. this is more general that just Canvas.

I like DrawingBuffer a lot, the latest patch uses this name.
> 
> And while I'm being the pedantic naming police, SharedContext3D would be more accurately named SharedGraphicsContext3D.

That would work, but I don't think it adds any extra information since there's only one possible sort of "...Context3D" in webkit.  I've thus left it as SharedContext3D.  I can change it if you feel strongly but it's a lotta letters to type :).

[Chris Marrin]

(In reply to comment #23)
>...
> > How are you creating the PlatformLayer? Doesn't the SharedContext need to vend that as well? Did I miss the call that does that?
> 
> The PlatformLayer is created by the platform-specific implementation of GraphicsLayer::setContentsToCanvas2D().  It's made aware of the DrawingBuffer at creation time and whenever the compositing layer tree is regenerated.  PlatformLayer creation and initialization is inherently platform-specific (as the name of the type would imply).

I see that. That seems less than ideal though because it is very specific to Canvas 2D. It also has different ownership that WebGL, where the GraphicsContext3D owns the PlatformLayer. Seems like it would be better if SharedContext3D just had a factory for PlatformLayers that you could then send along to GraphicsLayer::setContentsToCanvas(). I don't see any logic in setContentsToCanvas2D that make it necessary for GraphicsLayer to know it's a 2D Canvas, or have any knowledge at all about DrawingBuffer. Committing the rendered image to the PlatformLayer will still require a call to publishToPlatformLayer() and that all happens up in the Canvas logic.

I think the closer we can keep this to the WebGL model the better. We of course need to deal with the optimization of a single shared context, but that can all be done up in the SharedContext3D like you have in your latest patch (which looks good).

> 
> I left the DrawingBuffer::create() as a static function on DrawingBuffer so that it's easier to find the implementation.  There will eventually be a number of platform-dependent implementations of this create and I think it'd be better if the various DrawingBufferXXX.cpp files contained only DrawingBuffer:: functions instead of having a function declared on SharedContext3D.
> 

Fair enough...

> > 
> > Also, I'm not sure it's appropriate to call this a Framebuffer. It's really an FBO, plus Renderbuffers, plus context info. So maybe calling it a DrawingBuffer would be more appropriate. That's the term we use in the WebGL spec and it really refers to the combination of all these things. Also, using the term "Canvas" here is misleading. this is more general that just Canvas.
> 
> I like DrawingBuffer a lot, the latest patch uses this name.

Cool.

> > 
> > And while I'm being the pedantic naming police, SharedContext3D would be more accurately named SharedGraphicsContext3D.
> 
> That would work, but I don't think it adds any extra information since there's only one possible sort of "...Context3D" in webkit.  I've thus left it as SharedContext3D.  I can change it if you feel strongly but it's a lotta letters to type :).

Yeah, I thought about that. But when I typed it, it didn't look that bad. I'm concerned about other uses of the term "context", which is pretty generic. It's only 8 extra letters and one is an 'i' which is really narrow :-)

[Stephen White]

Comment on attachment 66296 [details]
Patch

Other than the mac chrome issue (GraphicsContext.h), looks OK to me.

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

> WebCore/html/canvas/CanvasRenderingContext2D.cpp:172
> +            m_drawingBuffer = DrawingBuffer::create(m_context3D.get(), IntSize(canvas()->width(), canvas()->height()));
This looks like it recreates the FBO etc on every reset.  Could be costly.  Perhaps we should be trying to reuse DrawingBuffer around if the context and size is the same.

> WebCore/platform/graphics/GraphicsContext.h:416
> +#if PLATFORM(SKIA)
I'm pretty sure that using #if PLATFORM(SKIA) here and #if ENABLE(ACCELERATED_2D_CANVAS) in CanvasRenderingContext2D is going to cause problems on Mac Chromium, since it has the latter but not the former.

This should either be ENABLE(ACCELERATED_2D_CANVAS) here, or (my preference) it should have a stub implementation for other ports as it did before.

> WebCore/platform/graphics/chromium/DrawingBufferChromium.cpp:95
> +    glCopyTextureToParentTexture(m_internal->offscreenColorTexture, parentTexture);
Hmm.. this is weird.  I guess I should've paid better attention to your GPU process CL.  Anyway, we should probably hide this function behind a google GLES2 extension at some point, if it isn't already.

[James Robinson]

Comment on attachment 66296 [details]
Patch

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

> WebCore/html/canvas/CanvasRenderingContext2D.cpp:172
> +            m_drawingBuffer = DrawingBuffer::create(m_context3D.get(), IntSize(canvas()->width(), canvas()->height()));
It does, same as the software drawing buffer.  We can improve this, but I wanted to keep things simple at first.

> WebCore/platform/graphics/GraphicsContext.h:416
> +#if PLATFORM(SKIA)
Good catch - I'll add the stubs back.

> WebCore/platform/graphics/chromium/DrawingBufferChromium.cpp:95
> +    glCopyTextureToParentTexture(m_internal->offscreenColorTexture, parentTexture);
It is.

[Chris Marrin]

Comment on attachment 66296 [details]
Patch

I've given this an r- until we resolve the issue I raised in https://bugs.webkit.org/show_bug.cgi?id=44926#c24

[James Robinson]

(In reply to comment #24)
> (In reply to comment #23)
> >...
> > > How are you creating the PlatformLayer? Doesn't the SharedContext need to vend that as well? Did I miss the call that does that?
> > 
> > The PlatformLayer is created by the platform-specific implementation of GraphicsLayer::setContentsToCanvas2D().  It's made aware of the DrawingBuffer at creation time and whenever the compositing layer tree is regenerated.  PlatformLayer creation and initialization is inherently platform-specific (as the name of the type would imply).
> 
> I see that. That seems less than ideal though because it is very specific to Canvas 2D. It also has different ownership that WebGL, where the GraphicsContext3D owns the PlatformLayer. Seems like it would be better if SharedContext3D just had a factory for PlatformLayers that you could then send along to GraphicsLayer::setContentsToCanvas(). I don't see any logic in setContentsToCanvas2D that make it necessary for GraphicsLayer to know it's a 2D Canvas, or have any knowledge at all about DrawingBuffer. Committing the rendered image to the PlatformLayer will still require a call to publishToPlatformLayer() and that all happens up in the Canvas logic.
> 
> I think the closer we can keep this to the WebGL model the better. We of course need to deal with the optimization of a single shared context, but that can all be done up in the SharedContext3D like you have in your latest patch (which looks good).
> 

I think this could work.  I don't think it would be an improvement, however.  The WebGL model is pretty inflexible since it ties up the compositing logic for the element with the behavior of the element.  I think it's cleaner for the canvas-specific logic (i.e. the SharedContext3D, ContextRenderingContext2D, etc) to be concerned with taking commands from javascript and producing rendering results in a DrawingBuffer and not worry about compositing at all.  When the layer is composited then the compositor only needs to know where the rendering results are, it doesn't need to be aware of the rest of the canvas stack.  The DrawingBuffer has to be aware of both, of course, so that it can take the rendered results and put them in a layer.

One concrete problem with the WebGL model is that since the GraphicsContext3D owns the PlatformLayer directly, the compositor can't decide when to create or destroy the PlatformLayer itself.  If a page is loaded in a background tab then it would be really handy for the compositor to ditch all the platformlayers and their associated backing stores and just let each canvas continue rendering to a DrawingBuffer.

[James Robinson]

Created attachment 66404 [details]
add GraphicsContext stubs instead of #if PLATFORM()ing the function declarations

[Chris Marrin]

(In reply to comment #28)
> ...
> > I think the closer we can keep this to the WebGL model the better. We of course need to deal with the optimization of a single shared context, but that can all be done up in the SharedContext3D like you have in your latest patch (which looks good).
> > 
> 
> I think this could work.  I don't think it would be an improvement, however.  The WebGL model is pretty inflexible since it ties up the compositing logic for the element with the behavior of the element.  I think it's cleaner for the canvas-specific logic (i.e. the SharedContext3D, ContextRenderingContext2D, etc) to be concerned with taking commands from javascript and producing rendering results in a DrawingBuffer and not worry about compositing at all.  When the layer is composited then the compositor only needs to know where the rendering results are, it doesn't need to be aware of the rest of the canvas stack.  The DrawingBuffer has to be aware of both, of course, so that it can take the rendered results and put them in a layer.
> 
> One concrete problem with the WebGL model is that since the GraphicsContext3D owns the PlatformLayer directly, the compositor can't decide when to create or destroy the PlatformLayer itself.  If a page is loaded in a background tab then it would be really handy for the compositor to ditch all the platformlayers and their associated backing stores and just let each canvas continue rendering to a DrawingBuffer.

But if you had such a capability then the GraphicsLayer would have to know about more classes, GraphicsContext3D in the case of WebGL, and DrawingBuffer in the case of accelerated Canvas 2D. I don't think it's valid to say that giving GraphicsLayer control of the platform layer is better because you conserve resources. The PlatformLayer might not be the expensive part. It might be somewhere in the GraphicsContext. If we wanted to conserve resources it would be best done with a "you're going into the background, please conserve your resources" call to the various GraphicsContext3D and SharedContexts.

Your implementation has already grown the GraphicsLayer call with the setContentsToCanvas2D method. And it would have to grow it again for each new accelerated content type, adding another context object that GraphicsLayer needs to know about, too.

The current model is the way it is because it's the renderer of the content (e.g., GraphicsContext3D, DrawingBuffer) that knows what the requirements for the PlatformLayer are, so it should be that renderer where the PlatformLayer is vended. This model comes from the implementation of accelerated video rendering. The movie player vends a PlatformLayer that it has created specifically for rendering movies into. I really think we should stick with that model.

[James Robinson]

For the record, I originally wrote this patch following WebGL's method of PlatformLayers.  It quickly got disgusting, which led to this approach.

In WebGL, the WebGLRenderingContext has a 1:1 relationship with a GraphicsContext3D.  Each GraphicsContext3D holds a reference to one PlatformLayer.  On the mac PlatformLayer is a typedef for CALayer.  The GraphicsContext3D holds a RetainPtr<WebGLLayer> where WebGLLayer is a subclass (indirectly) of CALayer.  The layer is eagerly constructed and held for the lifetime of the GC3D.  It's nearly the same for Chromium, except that the pointer is a RefPtr<LayerChromium> held on GraphicsContext3DInternal and it is lazily created on the first access rather than eagerly.  When WebGL content is composited RenderLayerBacking::updateGraphicsLayerConfiguration() grabs a pointer to the PlatformLayer from the GraphicsContext3D and passes it to GraphicsLayer::setLayerContentsTo..().  Then the GraphicsLayer implementation grabs another reference to the layer (on mac by storing it in a RetainPtr<CALayer>, in chromium by storing it in a RefPtr<LayerChromium).  The reason for the ref counting dance is that the lifetime of the WebGL canvas and the compositing layer are inherently decoupled.  The compositing layer owning the contents layer is generally created after the WebGL canvas and may be destroyed before or after the canvas.  Thus both have to keep a reference to the PlatformLayer.

For 2d canvas, we could (and I initially did) do a similar sort of dance where both the canvas logic and the compositor maintain a reference to the PlatformLayer.  The question is then who holds the reference outside of the compositor.  This can't be retained by the SharedGraphicsContext3D, because there is no 1:1 mapping between a SharedGraphicsContext3D and a canvas.  The PlatformLayer could be held by the CanvasRenderingContext2D, but that means adding a bunch of #if PLATFORM() gunk to CanvasRenderingContext2D in order to manage its lifetime.  Conceptually the CanvasRenderingContext2D should not need to know anything directly about compositing anyway.  The other possibility is to hold the PlatformLayer on the DrawingBuffer.  This ends up being strange as well since the DrawingBuffer can be reset by the page, so it doesn't live for as long as a CanvasRenderingContext2D or the compositing layer.

What I decided on here is having the DrawingBuffer be refcounted by the CanvasRenderingContext2D and the compositing layer and have the PlatformLayer management done completely on the compositor side.  This is analogous to how Image and contents layers are managed currently.  I think it's much cleaner to have a reference counted DrawingBuffer maintained on both sides since a DrawingBuffer is a completely cross-platform concept, unlike a PlatformLayer.  The other advantage is that we only have to create PlatformLayers when actually compositing a canvas.  If this followed the WebGL/mac pattern, we would have to create a PlatformLayer for canvases that aren't even in the page.  It's pretty common to create many offscreen 2d canvases to use as intermediate results (although I imagine this is less common in WebGL).

[James Robinson]

Created attachment 66436 [details]
rename SharedContext3D to SharedGraphicsContext3D

[James Robinson]

I believe the latest patch addresses all comments raised so far.  Mind taking another look?

[James Robinson]

Created attachment 66459 [details]
other PlatformLayer ownership model

[James Robinson]

I've attached a patch that implements the PlatformLayer ownership model you were suggesting, Chris.  Take your pick (and if you r+ one r- the other, please).

[James Robinson]

Created attachment 66461 [details]
other PlatformLayer ownership model

[Chris Marrin]

Comment on attachment 66461 [details]
other PlatformLayer ownership model

This looks great. Thanks for making the change.

[James Robinson]

Committed r66746: <http://trac.webkit.org/changeset/66746>

[WebKit Review Bot]

http://trac.webkit.org/changeset/66746 might have broken Chromium Win Release

[James Robinson]

Comment on attachment 66461 [details]
other PlatformLayer ownership model

Committed r66746: <http://trac.webkit.org/changeset/66746>