Bug 89700 – [chromium] Use WebGraphicsContext3D in compositor implementation

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[James Robinson]

[chromium] Use WebGraphicsContext3D in compositor implementation

[James Robinson]

Created attachment 148907 [details]
wip, not ready for review

[James Robinson]

Created attachment 148940 [details]
checkpoint

[James Robinson]

Created attachment 149203 [details]
Patch

[James Robinson]

From the WebCore ChangeLog:


        This converts the compositor implementation from using WebCore::GraphicsContext3D to using the Platform-provided
        WebGraphicsContext3D. This removes several unnecessary layers of indirection/wrapping and cuts down the
        compositor's implementation dependencies. GraphicsContext3D.h is still widely used to provide GL enum values.
        Most of the changes are purely mechanical - changing type names and the like.

        Ownership is changed a bit. Instead of multiple components holding references to the compositor's context, the
        context is now owned by the CCGraphicsContext, which is now owned directly by CCLayerTreeHostImpl.
        CCLayerTreeHostImpl also has ownership of its CCRenderer (LayerRendererChromium in 3D mode) and passes a
        non-owning pointer down to the CCRenderer.

        Extension checking is a bit different. The compositor does not (and never has) used extensions provided by
        WebGL's request/ensure mechanism. It simply checks for the existence of extensions it needs in the GL_EXTENSIONS
        string.

        FrameBufferSkPictureCanvasLayerTextureUpdater had to be patched as well, since it was grabbing a GrContext off
        of the compositor's GraphicsContext3D. This caused many problems. It was inefficient, since it required a full
        state flush when switching between ganesh and compositor calls. The gpu memory management was completely broken
        since the compositor clobbered ganesh's onMemoryAllocationChanged callback. This moves FBSkPCLTU over to using
        the appropriate SharedGraphicsContext3D, like filters.

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Adrienne Walker]

Comment on attachment 149203 [details]
Patch

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

I feel like I should have more substantial comments on a 186k patch, but R=me.

> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:1365
> -    RefPtr<CCGraphicsContext> ccContext = CCGraphicsContext::create3D(m_context);
> -    texture->framebufferTexture2D(ccContext.get(), m_implTextureAllocator.get());
> +    if (!texture->textureId())
> +        texture->allocate(m_implTextureAllocator.get());
> +    GLC(m_context, m_context->framebufferTexture2D(GraphicsContext3D::FRAMEBUFFER, GraphicsContext3D::COLOR_ATTACHMENT0, GraphicsContext3D::TEXTURE_2D, texture->textureId(), 0));

Can the ManagedTexture::framebufferTexture2d and bindTexture functions get removed?

[James Robinson]

(In reply to comment #6)
> (From update of attachment 149203 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=149203&action=review
> 
> I feel like I should have more substantial comments on a 186k patch, but R=me.
> 
> > Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:1365
> > -    RefPtr<CCGraphicsContext> ccContext = CCGraphicsContext::create3D(m_context);
> > -    texture->framebufferTexture2D(ccContext.get(), m_implTextureAllocator.get());
> > +    if (!texture->textureId())
> > +        texture->allocate(m_implTextureAllocator.get());
> > +    GLC(m_context, m_context->framebufferTexture2D(GraphicsContext3D::FRAMEBUFFER, GraphicsContext3D::COLOR_ATTACHMENT0, GraphicsContext3D::TEXTURE_2D, texture->textureId(), 0));
> 
> Can the ManagedTexture::framebufferTexture2d and bindTexture functions get removed?

I'll check if that can be done as a follow-up (this patch makes some of the GraphicsContext3DPrivate extension callback stuff become dead code and I want to delete that too).

[James Robinson]

Committed r121204: <http://trac.webkit.org/changeset/121204>

[James Robinson]

(In reply to comment #6)
> Can the ManagedTexture::framebufferTexture2d and bindTexture functions get removed?

ManagedTexture::bindTexture() is still used in ImageLayerChromium and CCHeadsUpDisplay. The first one, at least, appears to be a useful use for this abstraction. ManagedTexture::framebufferTexture2D() is now dead code, removal patch here: https://bugs.webkit.org/show_bug.cgi?id=89930