Fix issues with clearing deleted attachments

src/libANGLE/Context.cpp

@@ -3545,9 +3545,14 @@ void Context::clear(GLbitfield mask)
 
 void Context::clearBufferfv(GLenum buffer, GLint drawbuffer, const GLfloat *values)
 {
+    // It's not an error to try to clear a non-existent buffer, but it's a no-op. We early out so
+    // that the backend doesn't need to take this case into account.
     if (buffer == GL_DEPTH && !getGLState().getDrawFramebuffer()->getDepthbuffer())
     {
-        // It's not an error to try to clear a non-existent depth buffer, but it's a no-op.
+        return;
+    }
+    if (buffer == GL_COLOR && !getGLState().getDrawFramebuffer()->getColorbuffer(drawbuffer))
+    {
         return;
     }
     ANGLE_CONTEXT_TRY(prepareForClearBuffer(buffer, drawbuffer));
@@ -3557,6 +3562,12 @@ void Context::clearBufferfv(GLenum buffer, GLint drawbuffer, const GLfloat *valu
 
 void Context::clearBufferuiv(GLenum buffer, GLint drawbuffer, const GLuint *values)
 {
+    // It's not an error to try to clear a non-existent buffer, but it's a no-op. We early out so
+    // that the backend doesn't need to take this case into account.
+    if (buffer == GL_COLOR && !getGLState().getDrawFramebuffer()->getColorbuffer(drawbuffer))
+    {
+        return;
+    }
     ANGLE_CONTEXT_TRY(prepareForClearBuffer(buffer, drawbuffer));
     ANGLE_CONTEXT_TRY(
         mGLState.getDrawFramebuffer()->clearBufferuiv(this, buffer, drawbuffer, values));
@@ -3564,9 +3575,14 @@ void Context::clearBufferuiv(GLenum buffer, GLint drawbuffer, const GLuint *valu
 
 void Context::clearBufferiv(GLenum buffer, GLint drawbuffer, const GLint *values)
 {
+    // It's not an error to try to clear a non-existent buffer, but it's a no-op. We early out so
+    // that the backend doesn't need to take this case into account.
     if (buffer == GL_STENCIL && !getGLState().getDrawFramebuffer()->getStencilbuffer())
     {
-        // It's not an error to try to clear a non-existent stencil buffer, but it's a no-op.
+        return;
+    }
+    if (buffer == GL_COLOR && !getGLState().getDrawFramebuffer()->getColorbuffer(drawbuffer))
+    {
         return;
     }
     ANGLE_CONTEXT_TRY(prepareForClearBuffer(buffer, drawbuffer));

src/libANGLE/Framebuffer.cpp

@@ -746,7 +746,7 @@ bool Framebuffer::detachResourceById(const Context *context, GLenum resourceType
     for (size_t colorIndex = 0; colorIndex < mState.mColorAttachments.size(); ++colorIndex)
     {
         if (detachMatchingAttachment(context, &mState.mColorAttachments[colorIndex], resourceType,
-                                     resourceId, DIRTY_BIT_COLOR_ATTACHMENT_0 + colorIndex))
+                                     resourceId))
         {
             found = true;
         }
@@ -759,23 +759,19 @@ bool Framebuffer::detachResourceById(const Context *context, GLenum resourceType
              &mState.mWebGLStencilAttachment}};
         for (FramebufferAttachment *attachment : attachments)
         {
-            if (attachment->isAttached() && attachment->type() == resourceType &&
-                attachment->id() == resourceId)
+            if (detachMatchingAttachment(context, attachment, resourceType, resourceId))
             {
-                resetAttachment(context, attachment->getBinding());
                 found = true;
             }
         }
     }
     else
     {
-        if (detachMatchingAttachment(context, &mState.mDepthAttachment, resourceType, resourceId,
-                                     DIRTY_BIT_DEPTH_ATTACHMENT))
+        if (detachMatchingAttachment(context, &mState.mDepthAttachment, resourceType, resourceId))
         {
             found = true;
         }
-        if (detachMatchingAttachment(context, &mState.mStencilAttachment, resourceType, resourceId,
-                                     DIRTY_BIT_STENCIL_ATTACHMENT))
+        if (detachMatchingAttachment(context, &mState.mStencilAttachment, resourceType, resourceId))
         {
             found = true;
         }
@@ -787,14 +783,13 @@ bool Framebuffer::detachResourceById(const Context *context, GLenum resourceType
 bool Framebuffer::detachMatchingAttachment(const Context *context,
                                            FramebufferAttachment *attachment,
                                            GLenum matchType,
-                                           GLuint matchId,
-                                           size_t dirtyBit)
+                                           GLuint matchId)
 {
     if (attachment->isAttached() && attachment->type() == matchType && attachment->id() == matchId)
     {
-        attachment->detach(context);
-        mDirtyBits.set(dirtyBit);
-        mState.mResourceNeedsInit.set(dirtyBit, false);
+        // We go through resetAttachment to make sure that all the required bookkeeping will be done
+        // such as updating enabled draw buffer state.
+        resetAttachment(context, attachment->getBinding());
         return true;
     }
 

src/libANGLE/Framebuffer.h

@@ -353,8 +353,7 @@ class Framebuffer final : public angle::ObserverInterface,
     bool detachMatchingAttachment(const Context *context,
                                   FramebufferAttachment *attachment,
                                   GLenum matchType,
-                                  GLuint matchId,
-                                  size_t dirtyBit);
+                                  GLuint matchId);
     GLenum checkStatusWithGLFrontEnd(const Context *context);
     GLenum checkStatusImpl(const Context *context);
     void setAttachment(const Context *context,

src/tests/gl_tests/FramebufferTest.cpp

@@ -606,6 +606,45 @@ TEST_P(FramebufferTest_ES3, ClearNonexistentDepthStencil)
     EXPECT_GL_NO_ERROR();
 }
 
+// Test that clearing a color attachment that has been deleted doesn't crash.
+TEST_P(FramebufferTest_ES3, ClearDeletedAttachment)
+{
+    // An INVALID_FRAMEBUFFER_OPERATION error was seen in this test on Mac, not sure where it might
+    // be originating from. http://anglebug.com/2834
+    ANGLE_SKIP_TEST_IF(IsOSX() && IsOpenGL());
+
+    GLFramebuffer fbo;
+    glBindFramebuffer(GL_FRAMEBUFFER, fbo);
+
+    // There used to be a bug where some draw buffer state used to remain set even after the
+    // attachment was detached via deletion. That's why we create, attach and delete this RBO here.
+    GLuint rbo = 0u;
+    glGenRenderbuffers(1, &rbo);
+    glBindRenderbuffer(GL_RENDERBUFFER, rbo);
+    glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_RENDERBUFFER, rbo);
+    glDeleteRenderbuffers(1, &rbo);
+
+    // There needs to be at least one color attachment to prevent early out from the clear calls.
+    GLRenderbuffer rbo2;
+    glBindRenderbuffer(GL_RENDERBUFFER, rbo2);
+    glRenderbufferStorage(GL_RENDERBUFFER, GL_RGBA8, 1, 1);
+    glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT1, GL_RENDERBUFFER, rbo2);
+
+    ASSERT_GL_NO_ERROR();
+
+    // There's no error specified for clearing nonexistent buffers, it's simply a no-op, so we
+    // expect no GL errors below.
+    std::array<GLfloat, 4> floatClearValue = {0.0f, 0.0f, 0.0f, 0.0f};
+    glClearBufferfv(GL_COLOR, 0, floatClearValue.data());
+    EXPECT_GL_NO_ERROR();
+    std::array<GLuint, 4> uintClearValue = {0u, 0u, 0u, 0u};
+    glClearBufferuiv(GL_COLOR, 0, uintClearValue.data());
+    EXPECT_GL_NO_ERROR();
+    std::array<GLint, 4> intClearValue = {0, 0, 0, 0};
+    glClearBufferiv(GL_COLOR, 0, intClearValue.data());
+    EXPECT_GL_NO_ERROR();
+}
+
 ANGLE_INSTANTIATE_TEST(FramebufferTest_ES3, ES3_D3D11(), ES3_OPENGL(), ES3_OPENGLES());
 
 class FramebufferTest_ES31 : public ANGLETest