Move special DEPTH_STENCIL attachment logic from command buffers to WebGL1

third_party/WebKit/Source/modules/webgl/WebGLFramebuffer.cpp

 /*
  * Copyright (C) 2009 Apple 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:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 185 matching lines @@
 WebGLFramebuffer* WebGLFramebuffer::create(WebGLRenderingContextBase* ctx)
 {
     return new WebGLFramebuffer(ctx);
 }
 
 WebGLFramebuffer::WebGLFramebuffer(WebGLRenderingContextBase* ctx)
     : WebGLContextObject(ctx)
     , m_object(0)
     , m_destructionInProgress(false)
     , m_hasEverBeenBound(false)
+    , m_webGL1DepthStencilConsistent(true)
     , m_readBuffer(GL_COLOR_ATTACHMENT0)
 {
     ctx->contextGL()->GenFramebuffers(1, &m_object);
 }
 
 WebGLFramebuffer::~WebGLFramebuffer()
 {
     // Attachments in |m_attachments| will be deleted from other
     // places, and we must not touch that map in deleteObjectImpl once
     // the destructor has been entered.
     m_destructionInProgress = true;
 
     // See the comment in WebGLObject::detachAndDeleteObject().
     detachAndDeleteObject();
 }
 
 void WebGLFramebuffer::setAttachmentForBoundFramebuffer(GLenum target, GLenum attachment, GLenum texTarget, WebGLTexture* texture, GLint level, GLint layer)
 {
-    ASSERT(isBound(target));
-    removeAttachmentFromBoundFramebuffer(target, attachment);
-    if (!m_object)
-        return;
-    if (texture && texture->object()) {
-        m_attachments.add(attachment, WebGLTextureAttachment::create(texture, texTarget, level, layer));
-        drawBuffersIfNecessary(false);
-        texture->onAttached();
+    DCHECK(m_object);
+    DCHECK(isBound(target));
+    if (context()->isWebGL2OrHigher()) {
+        if (attachment == GL_DEPTH_STENCIL_ATTACHMENT) {
+            setAttachmentInternal(target, GL_DEPTH_ATTACHMENT, texTarget, texture, level, layer);
+            setAttachmentInternal(target, GL_STENCIL_ATTACHMENT, texTarget, texture, level, layer);
+        } else {
+            setAttachmentInternal(target, attachment, texTarget, texture, level, layer);
+        }
+        GLuint textureId = objectOrZero(texture);
+        // texTarget can be 0 if detaching using framebufferTextureLayer.
+        DCHECK(texTarget || !textureId);
+        switch (texTarget) {
+        case 0:
+        case GL_TEXTURE_3D:
+        case GL_TEXTURE_2D_ARRAY:
+            context()->contextGL()->FramebufferTextureLayer(target, attachment, textureId, level, layer);
+            break;
+        default:
+            DCHECK_EQ(layer, 0);
+            context()->contextGL()->FramebufferTexture2D(target, attachment, texTarget, textureId, level);
+            break;
+        }
+    } else {
+        DCHECK_EQ(layer, 0);
+        setAttachmentInternal(target, attachment, texTarget, texture, level, layer);
+        switch (attachment) {
+        case GL_DEPTH_ATTACHMENT:
+        case GL_STENCIL_ATTACHMENT:
+        case GL_DEPTH_STENCIL_ATTACHMENT:
+            commitWebGL1DepthStencilIfConsistent(target);
+            break;
+        default:
+            context()->contextGL()->FramebufferTexture2D(target, attachment, texTarget, objectOrZero(texture), level);
+            break;
+        }
     }
 }
 
 void WebGLFramebuffer::setAttachmentForBoundFramebuffer(GLenum target, GLenum attachment, WebGLRenderbuffer* renderbuffer)
 {
-    ASSERT(isBound(target));
-    removeAttachmentFromBoundFramebuffer(target, attachment);
-    if (!m_object)
-        return;
-    if (renderbuffer && renderbuffer->object()) {
-        m_attachments.add(attachment, WebGLRenderbufferAttachment::create(renderbuffer));
-        drawBuffersIfNecessary(false);
-        renderbuffer->onAttached();
+    DCHECK(m_object);
+    DCHECK(isBound(target));
+    if (context()->isWebGL2OrHigher()) {
+        if (attachment == GL_DEPTH_STENCIL_ATTACHMENT) {
+            setAttachmentInternal(target, GL_DEPTH_ATTACHMENT, renderbuffer);
+            setAttachmentInternal(target, GL_STENCIL_ATTACHMENT, renderbuffer);
+        } else {
+            setAttachmentInternal(target, attachment, renderbuffer);
+        }
+        context()->contextGL()->FramebufferRenderbuffer(target, attachment, GL_RENDERBUFFER, objectOrZero(renderbuffer));
+    } else {
+        setAttachmentInternal(target, attachment, renderbuffer);
+        switch (attachment) {
+        case GL_DEPTH_ATTACHMENT:
+        case GL_STENCIL_ATTACHMENT:
+        case GL_DEPTH_STENCIL_ATTACHMENT:
+            commitWebGL1DepthStencilIfConsistent(target);
+            break;
+        default:
+            context()->contextGL()->FramebufferRenderbuffer(target, attachment, GL_RENDERBUFFER, objectOrZero(renderbuffer));
+            break;
+        }
     }
 }
 
-void WebGLFramebuffer::attach(GLenum target, GLenum attachment, GLenum attachmentPoint)
-{
-    ASSERT(isBound(target));
-    WebGLAttachment* attachmentObject = getAttachment(attachment);
-    if (attachmentObject)
-        attachmentObject->attach(context()->contextGL(), target, attachmentPoint);
-}
-
 WebGLSharedObject* WebGLFramebuffer::getAttachmentObject(GLenum attachment) const
 {
     if (!m_object)
         return nullptr;
     WebGLAttachment* attachmentObject = getAttachment(attachment);
     return attachmentObject ? attachmentObject->object() : nullptr;
 }
 
 WebGLFramebuffer::WebGLAttachment* WebGLFramebuffer::getAttachment(GLenum attachment) const
 {
     const AttachmentMap::const_iterator it = m_attachments.find(attachment);
     return (it != m_attachments.end()) ? it->value.get() : 0;
 }
 
-void WebGLFramebuffer::removeAttachmentFromBoundFramebuffer(GLenum target, GLenum attachment)
-{
-    ASSERT(isBound(target));
-    if (!m_object)
-        return;
-
-    WebGLAttachment* attachmentObject = getAttachment(attachment);
-    if (attachmentObject) {
-        attachmentObject->onDetached(context()->contextGL());
-        m_attachments.remove(attachment);
-        drawBuffersIfNecessary(false);
-        switch (attachment) {
-        case GL_DEPTH_STENCIL_ATTACHMENT:
-            attach(target, GL_DEPTH_ATTACHMENT, GL_DEPTH_ATTACHMENT);
-            attach(target, GL_STENCIL_ATTACHMENT, GL_STENCIL_ATTACHMENT);
-            break;
-        case GL_DEPTH_ATTACHMENT:
-        case GL_STENCIL_ATTACHMENT:
-            attach(target, GL_DEPTH_STENCIL_ATTACHMENT, GL_DEPTH_STENCIL_ATTACHMENT);
-            break;
-        }
-    }
-}
-
 void WebGLFramebuffer::removeAttachmentFromBoundFramebuffer(GLenum target, WebGLSharedObject* attachment)
 {
     ASSERT(isBound(target));
     if (!m_object)
         return;
     if (!attachment)
         return;
 
     bool checkMore = true;
+    bool isWebGL1 = !context()->isWebGL2OrHigher();
+    bool checkWebGL1DepthStencil = false;
     while (checkMore) {
         checkMore = false;
         for (const auto& it : m_attachments) {
             WebGLAttachment* attachmentObject = it.value.get();
             if (attachmentObject->isSharedObject(attachment)) {
                 GLenum attachmentType = it.key;
-                attachmentObject->unattach(context()->contextGL(), target, attachmentType);
-                removeAttachmentFromBoundFramebuffer(target, attachmentType);
+                switch (attachmentType) {
+                case GL_DEPTH_ATTACHMENT:
+                case GL_STENCIL_ATTACHMENT:
+                case GL_DEPTH_STENCIL_ATTACHMENT:
+                    if (isWebGL1) {
+                        checkWebGL1DepthStencil = true;
+                    } else {
+                        attachmentObject->unattach(context()->contextGL(), target, attachmentType);
+                    }
+                    break;
+                default:
+                    attachmentObject->unattach(context()->contextGL(), target, attachmentType);
+                    break;
+                }
+                removeAttachmentInternal(target, attachmentType);
                 checkMore = true;
                 break;
             }
         }
     }
+    if (checkWebGL1DepthStencil)
+        commitWebGL1DepthStencilIfConsistent(target);
 }
 
 GLenum WebGLFramebuffer::checkDepthStencilStatus(const char** reason) const
 {
-    if (context()->isWebGL2OrHigher())
+    if (context()->isWebGL2OrHigher() || m_webGL1DepthStencilConsistent)
         return GL_FRAMEBUFFER_COMPLETE;
-    WebGLAttachment* depthAttachment = nullptr;
-    WebGLAttachment* stencilAttachment = nullptr;
-    WebGLAttachment* depthStencilAttachment = nullptr;
-    for (const auto& it : m_attachments) {
-        WebGLAttachment* attachment = it.value.get();
-        ASSERT(attachment);
-        switch (it.key) {
-        case GL_DEPTH_ATTACHMENT:
-            depthAttachment = attachment;
-            break;
-        case GL_STENCIL_ATTACHMENT:
-            stencilAttachment = attachment;
-            break;
-        case GL_DEPTH_STENCIL_ATTACHMENT:
-            depthStencilAttachment = attachment;
-            break;
-        default:
-            break;
-        }
-    }
-    if ((depthStencilAttachment && (depthAttachment || stencilAttachment))
-        || (depthAttachment && stencilAttachment)) {
-        *reason = "conflicting DEPTH/STENCIL/DEPTH_STENCIL attachments";
-        return GL_FRAMEBUFFER_UNSUPPORTED;
-    }
-    return GL_FRAMEBUFFER_COMPLETE;
+    *reason = "conflicting DEPTH/STENCIL/DEPTH_STENCIL attachments";
+    return GL_FRAMEBUFFER_UNSUPPORTED;
 }
 
 bool WebGLFramebuffer::hasStencilBuffer() const
 {
     WebGLAttachment* attachment = getAttachment(GL_STENCIL_ATTACHMENT);
     if (!attachment)
         attachment = getAttachment(GL_DEPTH_STENCIL_ATTACHMENT);
     return attachment && attachment->valid();
 }
 
@@ skipping 46 matching lines @@
                 }
             }
         }
         if (reset) {
             context()->contextGL()->DrawBuffersEXT(
                 m_filteredDrawBuffers.size(), m_filteredDrawBuffers.data());
         }
     }
 }
 
+void WebGLFramebuffer::setAttachmentInternal(GLenum target, GLenum attachment, GLenum texTarget, WebGLTexture* texture, GLint level, GLint layer)
+{
+    DCHECK(isBound(target));
+    DCHECK(m_object);
+    removeAttachmentInternal(target, attachment);
+    if (texture && texture->object()) {
+        m_attachments.add(attachment, WebGLTextureAttachment::create(texture, texTarget, level, layer));
+        drawBuffersIfNecessary(false);
+        texture->onAttached();
+    }
+}
+
+void WebGLFramebuffer::setAttachmentInternal(GLenum target, GLenum attachment, WebGLRenderbuffer* renderbuffer)
+{
+    DCHECK(isBound(target));
+    DCHECK(m_object);
+    removeAttachmentInternal(target, attachment);
+    if (renderbuffer && renderbuffer->object()) {
+        m_attachments.add(attachment, WebGLRenderbufferAttachment::create(renderbuffer));
+        drawBuffersIfNecessary(false);
+        renderbuffer->onAttached();
+    }
+}
+
+void WebGLFramebuffer::removeAttachmentInternal(GLenum target, GLenum attachment)
+{
+    DCHECK(isBound(target));
+    DCHECK(m_object);
+
+    WebGLAttachment* attachmentObject = getAttachment(attachment);
+    if (attachmentObject) {
+        attachmentObject->onDetached(context()->contextGL());
+        m_attachments.remove(attachment);
+        drawBuffersIfNecessary(false);
+    }
+}
+
+void WebGLFramebuffer::commitWebGL1DepthStencilIfConsistent(GLenum target)
+{
+    DCHECK(!context()->isWebGL2OrHigher());
+    WebGLAttachment* depthAttachment = nullptr;
+    WebGLAttachment* stencilAttachment = nullptr;
+    WebGLAttachment* depthStencilAttachment = nullptr;
+    int count = 0;
+    for (const auto& it : m_attachments) {
+        WebGLAttachment* attachment = it.value.get();
+        DCHECK(attachment);
+        switch (it.key) {
+        case GL_DEPTH_ATTACHMENT:
+            depthAttachment = attachment;
+            ++count;
+            break;
+        case GL_STENCIL_ATTACHMENT:
+            stencilAttachment = attachment;
+            ++count;
+            break;
+        case GL_DEPTH_STENCIL_ATTACHMENT:
+            depthStencilAttachment = attachment;
+            ++count;
+            break;
+        default:
+            break;
+        }
+    }
+
+    m_webGL1DepthStencilConsistent = count <= 1;
+    if (!m_webGL1DepthStencilConsistent)
+        return;

[Ken Russell (switch to Gerrit), 2016/09/29 03:07:43]

Can this lazy flushing of the framebuffer's attachments cause Blink's idea of
the framebuffer's state to get out of sync with respect to the command buffer's
in a way that's observable? It looks like everything's OK to me:
 - calls which modify the draw framebuffer are all handled correctly via calls
to WebGLFramebuffer::checkDepthStencilStatus()
 - queries of the framebuffer's attachments return Blink's cached values
 - the command buffer's notion of framebuffer completeness includes the WebGL
1.0 rules per the change to Framebuffer::IsPossiblyComplete above

I can't think of anything that could be missing. Mo, can you?

+
+    gpu::gles2::GLES2Interface* gl = context()->contextGL();
+    if (depthAttachment) {
+        depthAttachment->attach(gl, target, GL_DEPTH_ATTACHMENT);
+        gl->FramebufferRenderbuffer(target, GL_STENCIL_ATTACHMENT, GL_RENDERBUFFER, 0);
+    } else if (stencilAttachment) {
+        gl->FramebufferRenderbuffer(target, GL_DEPTH_ATTACHMENT, GL_RENDERBUFFER, 0);
+        stencilAttachment->attach(gl, target, GL_STENCIL_ATTACHMENT);
+    } else if (depthStencilAttachment) {
+        depthStencilAttachment->attach(gl, target, GL_DEPTH_STENCIL_ATTACHMENT);
+    } else {
+        gl->FramebufferRenderbuffer(target, GL_DEPTH_STENCIL_ATTACHMENT, GL_RENDERBUFFER, 0);
+    }
+}
+
 GLenum WebGLFramebuffer::getDrawBuffer(GLenum drawBuffer)
 {
     int index = static_cast<int>(drawBuffer - GL_DRAW_BUFFER0_EXT);
     ASSERT(index >= 0);
     if (index < static_cast<int>(m_drawBuffers.size()))
         return m_drawBuffers[index];
     if (drawBuffer == GL_DRAW_BUFFER0_EXT)
         return GL_COLOR_ATTACHMENT0;
     return GL_NONE;
 }
 
 void WebGLFramebuffer::visitChildDOMWrappers(v8::Isolate* isolate, const v8::Persistent<v8::Object>& wrapper)
 {
     for (const auto& attachment : m_attachments) {
         DOMWrapperWorld::setWrapperReferencesInAllWorlds(wrapper, attachment.value->object(), isolate);
     }
 }
 
 DEFINE_TRACE(WebGLFramebuffer)
 {
     visitor->trace(m_attachments);
     WebGLContextObject::trace(visitor);
 }
 
 } // namespace blink