Vulkan: Fix FBO cache when updating disabled attachments.

src/libANGLE/renderer/vulkan/FramebufferVk.cpp

@@ -1079,14 +1079,8 @@ angle::Result FramebufferVk::syncState(const gl::Context *context,
                 break;
             case gl::Framebuffer::DIRTY_BIT_DEPTH_BUFFER_CONTENTS:
             case gl::Framebuffer::DIRTY_BIT_STENCIL_BUFFER_CONTENTS:
-            {
-                RenderTargetVk *depthStencilRT = getDepthStencilRenderTarget();
-                ASSERT(depthStencilRT != nullptr);
-                ANGLE_TRY(depthStencilRT->flushStagedUpdates(contextVk));
-                mCurrentFramebufferDesc.update(vk::kFramebufferDescDepthStencilIndex,
-                                               depthStencilRT->getAssignSerial(contextVk));
+                updateDepthStencilAttachmentSerial(contextVk);
                 break;
-            }
             case gl::Framebuffer::DIRTY_BIT_READ_BUFFER:
                 ANGLE_TRY(mRenderTargetCache.update(context, mState, dirtyBits));
                 break;
@@ -1112,38 +1106,35 @@ angle::Result FramebufferVk::syncState(const gl::Context *context,
             default:
             {
                 static_assert(gl::Framebuffer::DIRTY_BIT_COLOR_ATTACHMENT_0 == 0, "FB dirty bits");
+                uint32_t colorIndexGL;
                 if (dirtyBit < gl::Framebuffer::DIRTY_BIT_COLOR_ATTACHMENT_MAX)
                 {
-                    size_t colorIndexGL = static_cast<size_t>(
+                    colorIndexGL = static_cast<uint32_t>(
                         dirtyBit - gl::Framebuffer::DIRTY_BIT_COLOR_ATTACHMENT_0);
                     ANGLE_TRY(updateColorAttachment(context, colorIndexGL));
-                    if (mRenderTargetCache.getColors()[colorIndexGL] != nullptr &&
-                        mState.getEnabledDrawBuffers()[colorIndexGL])
-                    {
-                        mCurrentFramebufferDesc.update(
-                            static_cast<uint32_t>(colorIndexGL),
-                            mRenderTargetCache.getColors()[colorIndexGL]->getAssignSerial(
-                                contextVk));
-                    }
-                    else
-                    {
-                        mCurrentFramebufferDesc.update(static_cast<uint32_t>(colorIndexGL),
-                                                       vk::kZeroAttachmentSerial);
-                    }
                 }
                 else
                 {
                     ASSERT(dirtyBit >= gl::Framebuffer::DIRTY_BIT_COLOR_BUFFER_CONTENTS_0 &&
                            dirtyBit < gl::Framebuffer::DIRTY_BIT_COLOR_BUFFER_CONTENTS_MAX);
-                    size_t colorIndexGL = static_cast<size_t>(
+                    colorIndexGL = static_cast<uint32_t>(
                         dirtyBit - gl::Framebuffer::DIRTY_BIT_COLOR_BUFFER_CONTENTS_0);
                     ANGLE_TRY(mRenderTargetCache.getColors()[colorIndexGL]->flushStagedUpdates(
                         contextVk));
-                    ASSERT(mRenderTargetCache.getColors()[colorIndexGL] != nullptr);
-                    mCurrentFramebufferDesc.update(
-                        static_cast<uint32_t>(colorIndexGL),
-                        mRenderTargetCache.getColors()[colorIndexGL]->getAssignSerial(contextVk));
                 }
+
+                RenderTargetVk *renderTarget = mRenderTargetCache.getColors()[colorIndexGL];
+
+                if (renderTarget && mState.getEnabledDrawBuffers()[colorIndexGL])
+                {
+                    mCurrentFramebufferDesc.update(colorIndexGL,
+                                                   renderTarget->getAssignSerial(contextVk));
+                }
+                else
+                {
+                    mCurrentFramebufferDesc.update(colorIndexGL, vk::kZeroAttachmentSerial);
+                }
+                break;
             }
         }
     }
@@ -1293,6 +1284,9 @@ angle::Result FramebufferVk::getFramebuffer(ContextVk *contextVk, vk::Framebuffe
     vk::FramebufferHelper newFramebuffer;
     ANGLE_TRY(newFramebuffer.init(contextVk, framebufferInfo));
 
+    // Sanity check that our description matches our attachments. Can catch implementation bugs.
+    ASSERT(static_cast<uint32_t>(attachments.size()) == mCurrentFramebufferDesc.attachmentCount());
+
     mFramebufferCache[mCurrentFramebufferDesc] = std::move(newFramebuffer);
     mFramebuffer                               = &mFramebufferCache[mCurrentFramebufferDesc];
     *framebufferOut                            = &mFramebuffer->getFramebuffer();

src/libANGLE/renderer/vulkan/vk_cache_utils.cpp

@@ -1645,6 +1645,8 @@ bool TextureDescriptorDesc::operator==(const TextureDescriptorDesc &other) const
     return memcmp(mSerials.data(), other.mSerials.data(), sizeof(TexUnitSerials) * mMaxIndex) == 0;
 }
 
+// FramebufferDesc implementation.
+
 FramebufferDesc::FramebufferDesc()
 {
     reset();
@@ -1676,6 +1678,18 @@ bool FramebufferDesc::operator==(const FramebufferDesc &other) const
     return memcmp(&mSerials, &other.mSerials, sizeof(Serial) * kMaxFramebufferAttachments) == 0;
 }
 
+uint32_t FramebufferDesc::attachmentCount() const
+{
+    uint32_t count = 0;
+    for (const AttachmentSerial &serial : mSerials)
+    {
+        if (serial.imageSerial != 0)
+        {
+            count++;
+        }
+    }
+    return count;
+}
 }  // namespace vk
 
 // RenderPassCache implementation.

src/libANGLE/renderer/vulkan/vk_cache_utils.h

@@ -750,6 +750,8 @@ class FramebufferDesc
 
     bool operator==(const FramebufferDesc &other) const;
 
+    uint32_t attachmentCount() const;
+
   private:
     gl::AttachmentArray<AttachmentSerial> mSerials;
 };

src/tests/gl_tests/FramebufferTest.cpp

@@ -1320,6 +1320,55 @@ void main()
     EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
 }
 
+// Covers a bug in ANGLE's Vulkan back-end. Our VkFramebuffer cache would in some cases forget to
+// check the draw states when computing a cache key.
+TEST_P(FramebufferTest_ES3, DisabledAttachmentRedefinition)
+{
+    constexpr GLuint kSize = 2;
+
+    // Make a Framebuffer with two attachments with one enabled and one disabled.
+    GLTexture texA, texB;
+    glBindTexture(GL_TEXTURE_2D, texA);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, kSize, kSize, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
+    glBindTexture(GL_TEXTURE_2D, texB);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, kSize, kSize, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
+
+    GLFramebuffer fbo;
+    glBindFramebuffer(GL_FRAMEBUFFER, fbo);
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, texA, 0);
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT1, GL_TEXTURE_2D, texB, 0);
+
+    // Mask out the second texture.
+    constexpr GLenum kOneDrawBuf = GL_COLOR_ATTACHMENT0;
+    glDrawBuffers(1, &kOneDrawBuf);
+
+    ASSERT_GL_NO_ERROR();
+    ASSERT_GL_FRAMEBUFFER_COMPLETE(GL_FRAMEBUFFER);
+
+    // Set up a very simple shader.
+    ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), essl1_shaders::fs::Green());
+    glViewport(0, 0, kSize, kSize);
+
+    // Draw
+    drawQuad(program, essl1_shaders::PositionAttrib(), 0.5f, 1.0f, true);
+    ASSERT_GL_NO_ERROR();
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
+
+    // Update the masked out attachment and draw again.
+    std::vector<GLColor> redPixels(kSize * kSize, GLColor::red);
+    glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, kSize, kSize, GL_RGBA, GL_UNSIGNED_BYTE,
+                    redPixels.data());
+
+    // Draw
+    drawQuad(program, essl1_shaders::PositionAttrib(), 0.5f, 1.0f, true);
+    ASSERT_GL_NO_ERROR();
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
+
+    glReadBuffer(GL_COLOR_ATTACHMENT1);
+    ASSERT_GL_NO_ERROR();
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
+}
+
 ANGLE_INSTANTIATE_TEST_ES2(AddDummyTextureNoRenderTargetTest);
 ANGLE_INSTANTIATE_TEST_ES2_AND_ES3(FramebufferFormatsTest);
 ANGLE_INSTANTIATE_TEST_ES3(FramebufferTest_ES3);