Remove alpha/depth/stencil/antialias from WGC3D::Attributes.

third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp

Index: third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp b/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
index ab504062f627b280d82d5475ec062c5aaa88e8ab..15b3f5d454005aa1daa1e868d199e00187439fd8 100644
--- a/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
+++ b/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
@@ -90,7 +90,7 @@ static bool shouldFailDrawingBufferCreationForTesting = false;
 
 } // namespace
 
-PassRefPtr<DrawingBuffer> DrawingBuffer::create(PassOwnPtr<WebGraphicsContext3DProvider> contextProvider, const IntSize& size, bool premultipliedAlpha, PreserveDrawingBuffer preserve, WebGraphicsContext3D::Attributes requestedAttributes)
+PassRefPtr<DrawingBuffer> DrawingBuffer::create(PassOwnPtr<WebGraphicsContext3DProvider> contextProvider, const IntSize& size, bool premultipliedAlpha, bool wantAlphaChannel, bool wantDepthBuffer, bool wantStencilBuffer, bool wantAntialiasing, PreserveDrawingBuffer preserve)
 {
     ASSERT(contextProvider);
 
@@ -106,8 +106,9 @@ PassRefPtr<DrawingBuffer> DrawingBuffer::create(PassOwnPtr<WebGraphicsContext3DP
     }
     ASSERT(extensionsUtil->supportsExtension("GL_OES_packed_depth_stencil"));
     extensionsUtil->ensureExtensionEnabled("GL_OES_packed_depth_stencil");
-    bool multisampleSupported = (extensionsUtil->supportsExtension("GL_CHROMIUM_framebuffer_multisample")
-        || extensionsUtil->supportsExtension("GL_EXT_multisampled_render_to_texture"))
+    bool multisampleSupported = wantAntialiasing
+        && (extensionsUtil->supportsExtension("GL_CHROMIUM_framebuffer_multisample")
+            || extensionsUtil->supportsExtension("GL_EXT_multisampled_render_to_texture"))
         && extensionsUtil->supportsExtension("GL_OES_rgb8_rgba8");
     if (multisampleSupported) {
         extensionsUtil->ensureExtensionEnabled("GL_OES_rgb8_rgba8");
@@ -120,8 +121,8 @@ PassRefPtr<DrawingBuffer> DrawingBuffer::create(PassOwnPtr<WebGraphicsContext3DP
     if (discardFramebufferSupported)
         extensionsUtil->ensureExtensionEnabled("GL_EXT_discard_framebuffer");
 
-    RefPtr<DrawingBuffer> drawingBuffer = adoptRef(new DrawingBuffer(std::move(contextProvider), extensionsUtil.release(), multisampleSupported, discardFramebufferSupported, premultipliedAlpha, preserve, requestedAttributes));
-    if (!drawingBuffer->initialize(size)) {
+    RefPtr<DrawingBuffer> drawingBuffer = adoptRef(new DrawingBuffer(std::move(contextProvider), extensionsUtil.release(), discardFramebufferSupported, wantAlphaChannel, premultipliedAlpha, preserve));
+    if (!drawingBuffer->initialize(size, wantDepthBuffer, wantStencilBuffer, multisampleSupported)) {
         drawingBuffer->beginDestruction();
         return PassRefPtr<DrawingBuffer>();
     }
@@ -133,7 +134,13 @@ void DrawingBuffer::forceNextDrawingBufferCreationToFail()
     shouldFailDrawingBufferCreationForTesting = true;
 }
 
-DrawingBuffer::DrawingBuffer(PassOwnPtr<WebGraphicsContext3DProvider> contextProvider, PassOwnPtr<Extensions3DUtil> extensionsUtil, bool multisampleExtensionSupported, bool discardFramebufferSupported, bool premultipliedAlpha, PreserveDrawingBuffer preserve, WebGraphicsContext3D::Attributes requestedAttributes)
+DrawingBuffer::DrawingBuffer(
+    PassOwnPtr<WebGraphicsContext3DProvider> contextProvider,
+    PassOwnPtr<Extensions3DUtil> extensionsUtil,
+    bool discardFramebufferSupported,
+    bool wantAlphaChannel,
+    bool premultipliedAlpha,
+    PreserveDrawingBuffer preserve)
     : m_preserveDrawingBuffer(preserve)
     , m_scissorEnabled(false)
     , m_texture2DBinding(0)
@@ -145,10 +152,12 @@ DrawingBuffer::DrawingBuffer(PassOwnPtr<WebGraphicsContext3DProvider> contextPro
     , m_gl(m_contextProvider->contextGL())
     , m_extensionsUtil(std::move(extensionsUtil))
     , m_size(-1, -1)
-    , m_requestedAttributes(requestedAttributes)
-    , m_multisampleExtensionSupported(multisampleExtensionSupported)
     , m_discardFramebufferSupported(discardFramebufferSupported)
+    , m_wantAlphaChannel(wantAlphaChannel)
     , m_premultipliedAlpha(premultipliedAlpha)
+    , m_hasDepthBuffer(false)
+    , m_hasStencilBuffer(false)
+    , m_hasImplicitStencilBuffer(false)
     , m_fbo(0)
     , m_depthStencilBuffer(0)
     , m_multisampleFBO(0)
@@ -268,7 +277,7 @@ bool DrawingBuffer::prepareMailbox(WebExternalTextureMailbox* outMailbox, WebExt
         bitmap->setSize(size());
 
         unsigned char* pixels = bitmap->pixels();
-        bool needPremultiply = m_actualAttributes.alpha && !m_premultipliedAlpha;
+        bool needPremultiply = m_wantAlphaChannel && !m_premultipliedAlpha;
         WebGLImageConversion::AlphaOp op = needPremultiply ? WebGLImageConversion::AlphaDoPremultiply : WebGLImageConversion::AlphaDoNothing;
         if (pixels)
             readBackFramebuffer(pixels, size().width(), size().height(), ReadbackSkia, op);
@@ -282,9 +291,8 @@ bool DrawingBuffer::prepareMailbox(WebExternalTextureMailbox* outMailbox, WebExt
     RefPtr<MailboxInfo> frontColorBufferMailbox = recycledMailbox();
 
     // No buffer available to recycle, create a new one.
-    if (!frontColorBufferMailbox) {
+    if (!frontColorBufferMailbox)
         frontColorBufferMailbox = createNewMailbox(createTextureAndAllocateMemory(m_size));
-    }
 
     if (m_preserveDrawingBuffer == Discard) {
         std::swap(frontColorBufferMailbox->textureInfo, m_colorBuffer);
@@ -367,7 +375,7 @@ DrawingBuffer::TextureParameters DrawingBuffer::defaultTextureParameters()
 {
     TextureParameters parameters;
     parameters.target = GL_TEXTURE_2D;
-    if (m_requestedAttributes.alpha) {
+    if (m_wantAlphaChannel) {
         parameters.internalColorFormat = GL_RGBA;
         parameters.colorFormat = GL_RGBA;
         parameters.internalRenderbufferFormat = GL_RGBA8_OES;
@@ -449,7 +457,7 @@ void DrawingBuffer::deleteMailbox(const WebExternalTextureMailbox& mailbox)
     ASSERT_NOT_REACHED();
 }
 
-bool DrawingBuffer::initialize(const IntSize& size)
+bool DrawingBuffer::initialize(const IntSize& size, bool wantDepthBuffer, bool wantStencilBuffer, bool multisampleExtensionSupported)
 {
     if (m_gl->GetGraphicsResetStatusKHR() != GL_NO_ERROR) {
         // Need to try to restore the context again later.
@@ -460,7 +468,7 @@ bool DrawingBuffer::initialize(const IntSize& size)
 
     int maxSampleCount = 0;
     m_antiAliasingMode = None;
-    if (m_requestedAttributes.antialias && m_multisampleExtensionSupported) {
+    if (multisampleExtensionSupported) {
         m_gl->GetIntegerv(GL_MAX_SAMPLES_ANGLE, &maxSampleCount);
         m_antiAliasingMode = MSAAExplicitResolve;
         if (m_extensionsUtil->supportsExtension("GL_EXT_multisampled_render_to_texture")) {
@@ -475,27 +483,15 @@ bool DrawingBuffer::initialize(const IntSize& size)
 
     m_gl->BindFramebuffer(GL_FRAMEBUFFER, m_fbo);
     createSecondaryBuffers();
-    // We first try to initialize everything with the requested attributes.
-    if (!reset(size))
+    if (!reset(size, wantDepthBuffer || wantStencilBuffer))
         return false;
-    // If that succeeds, we then see what we actually got and update our actual attributes to reflect that.
-    m_actualAttributes = m_requestedAttributes;
-    if (m_requestedAttributes.alpha) {
-        GLint alphaBits = 0;
-        m_gl->GetIntegerv(GL_ALPHA_BITS, &alphaBits);
-        m_actualAttributes.alpha = alphaBits > 0;
-    }
-    if (m_requestedAttributes.depth) {
-        GLint depthBits = 0;
-        m_gl->GetIntegerv(GL_DEPTH_BITS, &depthBits);
-        m_actualAttributes.depth = depthBits > 0;
-    }
-    if (m_requestedAttributes.stencil) {
-        GLint stencilBits = 0;
-        m_gl->GetIntegerv(GL_STENCIL_BITS, &stencilBits);
-        m_actualAttributes.stencil = stencilBits > 0;
+
+    if (m_depthStencilBuffer) {

[danakj, 2016/03/31 22:13:02]

Explanation of preserving old behaviour:

If requested was false, we didn't branch before at all, so actual would be false
for any one of these properties. That includes depth even tho stencil was true
and we actually have depth bits, and vice versa.

So here, we continue to set hasDepth and hasStencil to false if the request was
false.

hasImplicitStencilBuffer was implemented as m_depthStencilBuffer &&
m_requestedAttributes.depth && !m_requestedAttributes.stencil

Now we check m_depthStencilBuffer still. If we have a buffer than wantDepth or
wantStencil was true (and we DCHECK that). If wantStencil is false, then that
implies that wantDepth was true, so we don't both to actually check that in the
code.

+        DCHECK(wantDepthBuffer || wantStencilBuffer);
+        m_hasDepthBuffer = wantDepthBuffer;

[piman, 2016/04/01 00:24:15]

I'm not sure hasDepthBuffer/hasStencilBuffer are that useful if they're lies.
The only non-test user (WebGLRenderingContextBase) already &&'s the result with
the requested values, so I think we could just tell the truth?

[danakj, 2016/04/01 00:40:50]

That's an excellent point. I had to think this through to confirm:

Depth lies if
- depth buffer exists
- requested depth buffer was false

In that case WebGLRCBase will not query hasDepthBuffer() at all:
    if (m_requestedAttributes.depth() && !drawingBuffer()->hasDepthBuffer())

So the fact that it lies doesn't matter.

If the request is true, it will query. But if the request is true, we never lie.
We report true if it exists and false otherwise.

So ya I agree.

There is, however, one other user of these lies, which is line 774ish in this
file. Except there they read different lies, as I've commented there.

They will be false always when called from initialize() so we won't clear.

But when reset() is called externally, we won't clear the thing we didn't
request. If we don't maintain these lies internally, we will clear the depth
buffer when one wasn't requested, or stencil.

WDYT?

[danakj, 2016/04/01 00:41:46]

Oh, and if we use !!m_depthStencilBuffer there, then we'd clear on initialize()
too.

+        m_hasStencilBuffer = wantStencilBuffer;
+        m_hasImplicitStencilBuffer = !wantStencilBuffer;
     }
-    m_actualAttributes.antialias = multisample();
 
     if (m_gl->GetGraphicsResetStatusKHR() != GL_NO_ERROR) {
         // It's possible that the drawing buffer allocation provokes a context loss, so check again just in case. http://crbug.com/512302
@@ -545,9 +541,9 @@ bool DrawingBuffer::copyToPlatformTexture(WebGraphicsContext3D* context, gpu::gl
 
     GLboolean unpackPremultiplyAlphaNeeded = GL_FALSE;
     GLboolean unpackUnpremultiplyAlphaNeeded = GL_FALSE;
-    if (m_actualAttributes.alpha && m_premultipliedAlpha && !premultiplyAlpha)
+    if (m_wantAlphaChannel && m_premultipliedAlpha && !premultiplyAlpha)
         unpackUnpremultiplyAlphaNeeded = GL_TRUE;
-    else if (m_actualAttributes.alpha && !m_premultipliedAlpha && premultiplyAlpha)
+    else if (m_wantAlphaChannel && !m_premultipliedAlpha && premultiplyAlpha)
         unpackPremultiplyAlphaNeeded = GL_TRUE;
 
     gl->CopyTextureCHROMIUM(sourceTexture, texture, internalFormat, destType, flipY, unpackPremultiplyAlphaNeeded, unpackUnpremultiplyAlphaNeeded);
@@ -574,8 +570,8 @@ WebLayer* DrawingBuffer::platformLayer()
     if (!m_layer) {
         m_layer = adoptPtr(Platform::current()->compositorSupport()->createExternalTextureLayer(this));
 
-        m_layer->setOpaque(!m_actualAttributes.alpha);
-        m_layer->setBlendBackgroundColor(m_actualAttributes.alpha);
+        m_layer->setOpaque(!m_wantAlphaChannel);
+        m_layer->setBlendBackgroundColor(m_wantAlphaChannel);
         m_layer->setPremultipliedAlpha(m_premultipliedAlpha);
         m_layer->setNearestNeighbor(m_filterQuality == kNone_SkFilterQuality);
         GraphicsLayer::registerContentsLayer(m_layer->layer());
@@ -654,10 +650,10 @@ void DrawingBuffer::createSecondaryBuffers()
     }
 }
 
-bool DrawingBuffer::resizeFramebuffer(const IntSize& size)
+bool DrawingBuffer::resizeFramebuffer(const IntSize& size, bool wantDepthOrStencilBuffer)
 {
     m_gl->BindFramebuffer(GL_FRAMEBUFFER, m_fbo);
-    if (m_antiAliasingMode != MSAAExplicitResolve)
+    if (m_antiAliasingMode != MSAAExplicitResolve && wantDepthOrStencilBuffer)
         resizeDepthStencil(size);
     if (m_gl->CheckFramebufferStatus(GL_FRAMEBUFFER) != GL_FRAMEBUFFER_COMPLETE)
         return false;
@@ -665,7 +661,7 @@ bool DrawingBuffer::resizeFramebuffer(const IntSize& size)
     return true;
 }
 
-bool DrawingBuffer::resizeMultisampleFramebuffer(const IntSize& size)
+bool DrawingBuffer::resizeMultisampleFramebuffer(const IntSize& size, bool wantDepthOrStencilBuffer)
 {
     if (m_antiAliasingMode == MSAAExplicitResolve) {
         m_gl->BindFramebuffer(GL_FRAMEBUFFER, m_multisampleFBO);
@@ -677,7 +673,8 @@ bool DrawingBuffer::resizeMultisampleFramebuffer(const IntSize& size)
             return false;
 
         m_gl->FramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_RENDERBUFFER, m_multisampleColorBuffer);
-        resizeDepthStencil(size);
+        if (wantDepthOrStencilBuffer)
+            resizeDepthStencil(size);
         if (m_gl->CheckFramebufferStatus(GL_FRAMEBUFFER) != GL_FRAMEBUFFER_COMPLETE)
             return false;
     }
@@ -687,9 +684,6 @@ bool DrawingBuffer::resizeMultisampleFramebuffer(const IntSize& size)
 
 void DrawingBuffer::resizeDepthStencil(const IntSize& size)
 {
-    if (!m_requestedAttributes.depth && !m_requestedAttributes.stencil)
-        return;
-
     if (!m_depthStencilBuffer)
         m_gl->GenRenderbuffers(1, &m_depthStencilBuffer);
     m_gl->BindRenderbuffer(GL_RENDERBUFFER, m_depthStencilBuffer);
@@ -717,11 +711,6 @@ void DrawingBuffer::clearFramebuffers(GLbitfield clearMask)
     m_gl->Clear(clearMask);
 }
 
-bool DrawingBuffer::hasImplicitStencilBuffer() const
-{
-    return m_depthStencilBuffer && m_requestedAttributes.depth && !m_requestedAttributes.stencil;
-}
-
 void DrawingBuffer::setSize(const IntSize& size)
 {
     if (m_size == size)
@@ -744,7 +733,7 @@ IntSize DrawingBuffer::adjustSize(const IntSize& desiredSize, const IntSize& cur
     return adjustedSize;
 }
 
-bool DrawingBuffer::reset(const IntSize& newSize)
+bool DrawingBuffer::reset(const IntSize& newSize, bool wantDepthOrStencilBuffer)
 {
     ASSERT(!newSize.isEmpty());
     IntSize adjustedSize = adjustSize(newSize, m_size, m_maxTextureSize);
@@ -763,7 +752,8 @@ bool DrawingBuffer::reset(const IntSize& newSize)
             attachColorBufferToCurrentFBO();
 
             // resize multisample FBO
-            if (!resizeMultisampleFramebuffer(adjustedSize) || !resizeFramebuffer(adjustedSize)) {
+            if (!resizeMultisampleFramebuffer(adjustedSize, wantDepthOrStencilBuffer)
+                || !resizeFramebuffer(adjustedSize, wantDepthOrStencilBuffer)) {
                 adjustedSize.scale(s_resourceAdjustedRatio);
                 continue;
             }
@@ -781,12 +771,12 @@ bool DrawingBuffer::reset(const IntSize& newSize)
     m_gl->ColorMask(true, true, true, true);
 
     GLbitfield clearMask = GL_COLOR_BUFFER_BIT;
-    if (m_actualAttributes.depth) {
+    if (m_hasDepthBuffer) {

[danakj, 2016/03/31 22:13:02]

I will point out while I'm here that it's a bit weird we're branching on *has*
depth/stencil here (or on the actual attributes before), since those are not set
when this is called from initialize() until after this method returns.

         m_gl->ClearDepthf(1.0f);
         clearMask |= GL_DEPTH_BUFFER_BIT;
         m_gl->DepthMask(true);
     }
-    if (m_actualAttributes.stencil) {
+    if (m_hasStencilBuffer) {
         m_gl->ClearStencil(0);
         clearMask |= GL_STENCIL_BUFFER_BIT;
         m_gl->StencilMaskSeparate(GL_FRONT, 0xFFFFFFFF);
@@ -967,7 +957,7 @@ DrawingBuffer::TextureInfo DrawingBuffer::createTextureAndAllocateMemory(const I
     // TODO(erikchen): Add support for a CHROMIUM_image back buffer whose
     // behavior mimics a texture with internal format GL_RGB.
     // https://crbug.com/581777.
-    if (!m_requestedAttributes.alpha)
+    if (!m_wantAlphaChannel)
         return createDefaultTextureAndAllocateMemory(size);
 
     if (!RuntimeEnabledFeatures::webGLImageChromiumEnabled())