Vulkan: Implement invalidate for color buffers

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -619,6 +619,7 @@ ContextVk::ContextVk(const gl::State &state, gl::ErrorSet *errorSet, RendererVk
       mUseOldRewriteStructSamplers(false),
       mOutsideRenderPassCommands(nullptr),
       mRenderPassCommands(nullptr),
+      mRenderPassFramebuffer(VK_NULL_HANDLE),
       mHasPrimaryCommands(false),
       mGpuEventsEnabled(false),
       mGpuClockSync{std::numeric_limits<double>::max(), std::numeric_limits<double>::max()},
@@ -4213,6 +4214,7 @@ angle::Result ContextVk::flushAndBeginRenderPass(
 
     mRenderPassCommands->beginRenderPass(framebuffer, renderArea, renderPassDesc,
                                          renderPassAttachmentOps, clearValues, commandBufferOut);
+    mRenderPassFramebuffer = framebuffer.getHandle();
     return angle::Result::Continue;
 }
 

src/libANGLE/renderer/vulkan/ContextVk.h

@@ -412,6 +412,11 @@ class ContextVk : public ContextImpl, public vk::Context
     }
 
     RenderPassCache &getRenderPassCache() { return mRenderPassCache; }
+    bool isCurrentRenderPassOfFramebuffer(vk::Framebuffer *framebuffer)
+    {
+        return mRenderPassFramebuffer != VK_NULL_HANDLE && framebuffer != nullptr &&
+               mRenderPassFramebuffer == framebuffer->getHandle();
+    }
 
     vk::DescriptorSetLayoutDesc getDriverUniformsDescriptorSetDesc(
         VkShaderStageFlags shaderStages) const;
@@ -795,7 +800,11 @@ class ContextVk : public ContextImpl, public vk::Context
     void dumpCommandStreamDiagnostics();
     angle::Result flushOutsideRenderPassCommands();
 
-    ANGLE_INLINE void onRenderPassFinished() { mRenderPassCommandBuffer = nullptr; }
+    ANGLE_INLINE void onRenderPassFinished()
+    {
+        mRenderPassCommandBuffer = nullptr;
+        mRenderPassFramebuffer   = VK_NULL_HANDLE;
+    }
 
     angle::Result onBufferRead(VkAccessFlags readAccessType,
                                vk::PipelineStage readStage,
@@ -949,6 +958,7 @@ class ContextVk : public ContextImpl, public vk::Context
 
     vk::CommandBufferHelper *mOutsideRenderPassCommands;
     vk::CommandBufferHelper *mRenderPassCommands;
+    VkFramebuffer mRenderPassFramebuffer;
     vk::PrimaryCommandBuffer mPrimaryCommands;
     // Function recycleCommandBuffer() is public above
     bool mHasPrimaryCommands;

src/libANGLE/renderer/vulkan/FramebufferVk.cpp

@@ -225,7 +225,9 @@ angle::Result FramebufferVk::invalidate(const gl::Context *context,
                                         size_t count,
                                         const GLenum *attachments)
 {
-    // TODO(jmadill): Re-enable. See http://anglebug.com/4444
+    ContextVk *contextVk = vk::GetImpl(context);
+
+    ANGLE_TRY(invalidateImpl(contextVk, count, attachments));
     return angle::Result::Continue;
 }
 
@@ -234,7 +236,13 @@ angle::Result FramebufferVk::invalidateSub(const gl::Context *context,
                                            const GLenum *attachments,
                                            const gl::Rectangle &area)
 {
-    // TODO(jmadill): Re-enable. See http://anglebug.com/4444
+    ContextVk *contextVk = vk::GetImpl(context);
+
+    if (area.encloses(contextVk->getStartedRenderPassCommands().getRenderArea()))
+    {
+        ANGLE_TRY(invalidateImpl(contextVk, count, attachments));
+    }
+
     return angle::Result::Continue;
 }
 
@@ -1144,8 +1152,6 @@ angle::Result FramebufferVk::invalidateImpl(ContextVk *contextVk,
                                             size_t count,
                                             const GLenum *attachments)
 {
-    ASSERT(contextVk->hasStartedRenderPass());
-
     gl::DrawBufferMask invalidateColorBuffers;
     bool invalidateDepthBuffer   = false;
     bool invalidateStencilBuffer = false;
@@ -1178,48 +1184,80 @@ angle::Result FramebufferVk::invalidateImpl(ContextVk *contextVk,
         }
     }
 
-    // Set the appropriate storeOp for attachments.
-    size_t attachmentIndexVk = 0;
-    for (size_t colorIndexGL : mState.getEnabledDrawBuffers())
+    const auto &colorRenderTargets           = mRenderTargetCache.getColors();
+    RenderTargetVk *depthStencilRenderTarget = mRenderTargetCache.getDepthStencil(true);
+
+    // To ensure we invalidate the right renderpass we require that the current framebuffer be the
+    // same as the current renderpass' framebuffer. E.g. prevent sequence like:
+    //- Bind FBO 1, draw
+    //- Bind FBO 2, draw
+    //- Bind FBO 1, invalidate D/S
+    // to invalidate the D/S of FBO 2 since it would be the currently active renderpass.
+    vk::Framebuffer *currentFramebuffer = nullptr;
+    ANGLE_TRY(getFramebuffer(contextVk, &currentFramebuffer));
+
+    if (contextVk->hasStartedRenderPass() &&
+        contextVk->isCurrentRenderPassOfFramebuffer(currentFramebuffer))
     {
-        if (invalidateColorBuffers.test(colorIndexGL))
+        // Set the appropriate storeOp for attachments.
+        size_t attachmentIndexVk = 0;
+        for (size_t colorIndexGL : mState.getEnabledDrawBuffers())
         {
-            contextVk->getStartedRenderPassCommands().invalidateRenderPassColorAttachment(
-                attachmentIndexVk);
+            if (invalidateColorBuffers.test(colorIndexGL))
+            {
+                contextVk->getStartedRenderPassCommands().invalidateRenderPassColorAttachment(
+                    attachmentIndexVk);
+            }
+            ++attachmentIndexVk;
         }
-        ++attachmentIndexVk;
-    }
 
-    RenderTargetVk *depthStencilRenderTarget = mRenderTargetCache.getDepthStencil(true);
-    if (depthStencilRenderTarget)
-    {
-        if (invalidateDepthBuffer)
+        if (depthStencilRenderTarget)
         {
-            contextVk->getStartedRenderPassCommands().invalidateRenderPassDepthAttachment(
-                attachmentIndexVk);
+            if (invalidateDepthBuffer)
+            {
+                contextVk->getStartedRenderPassCommands().invalidateRenderPassDepthAttachment(
+                    attachmentIndexVk);
+            }
+
+            if (invalidateStencilBuffer)
+            {
+                contextVk->getStartedRenderPassCommands().invalidateRenderPassStencilAttachment(
+                    attachmentIndexVk);
+            }
         }
 
-        if (invalidateStencilBuffer)
+        // NOTE: Possible future optimization is to delay setting the storeOp and only do so if the
+        // render pass is closed by itself before another draw call.  Otherwise, in a situation like
+        // this:
+        //
+        //     draw()
+        //     invalidate()
+        //     draw()
+        //
+        // We would be discarding the attachments only to load them for the next draw (which is less
+        // efficient than keeping the render pass open and not do the discard at all).  While dEQP
+        // tests this pattern, this optimization may not be necessary if no application does this.
+        // It is expected that an application would invalidate() when it's done with the
+        // framebuffer, so the render pass would have closed either way.
+        ANGLE_TRY(contextVk->endRenderPass());
+    }
+
+    for (size_t colorIndexGL : mState.getEnabledDrawBuffers())
+    {
+        if (invalidateColorBuffers.test(colorIndexGL))
         {
-            contextVk->getStartedRenderPassCommands().invalidateRenderPassStencilAttachment(
-                attachmentIndexVk);
+            RenderTargetVk *colorRenderTarget = colorRenderTargets[colorIndexGL];
+            ASSERT(colorRenderTarget);
+            colorRenderTarget->invalidateContent();
         }
     }
 
-    // NOTE: Possible future optimization is to delay setting the storeOp and only do so if the
-    // render pass is closed by itself before another draw call.  Otherwise, in a situation like
-    // this:
-    //
-    //     draw()
-    //     invalidate()
-    //     draw()
-    //
-    // We would be discarding the attachments only to load them for the next draw (which is less
-    // efficient than keeping the render pass open and not do the discard at all).  While dEQP tests
-    // this pattern, this optimization may not be necessary if no application does this.  It is
-    // expected that an application would invalidate() when it's done with the framebuffer, so the
-    // render pass would have closed either way.
-    ANGLE_TRY(contextVk->endRenderPass());
+    // If we have a depth / stencil render target AND we invalidate both we'll mark it as
+    // invalid. Maybe in the future add separate depth & stencil invalid flags.
+    if (depthStencilRenderTarget && invalidateDepthBuffer && invalidateStencilBuffer)
+    {
+        depthStencilRenderTarget->invalidateContent();
+    }
 
     return angle::Result::Continue;
 }
@@ -1757,7 +1795,10 @@ angle::Result FramebufferVk::startNewRenderPass(ContextVk *contextVk,
         }
         else
         {
-            renderPassAttachmentOps.setOps(currentAttachmentCount, VK_ATTACHMENT_LOAD_OP_LOAD,
+            renderPassAttachmentOps.setOps(currentAttachmentCount,
+                                           colorRenderTarget->hasDefinedContent()
+                                               ? VK_ATTACHMENT_LOAD_OP_LOAD
+                                               : VK_ATTACHMENT_LOAD_OP_DONT_CARE,
                                            VK_ATTACHMENT_STORE_OP_STORE);
             packedClearValues.store(currentAttachmentCount, VK_IMAGE_ASPECT_COLOR_BIT,
                                     kUninitializedClearValue);

src/tests/gl_tests/StateChangeTest.cpp

@@ -847,6 +847,47 @@ TEST_P(StateChangeRenderTest, DepthRangeUpdates)
                          GLColor::green);
 }
 
+class StateChangeRenderTestES3 : public StateChangeRenderTest
+{};
+
+TEST_P(StateChangeRenderTestES3, InvalidateNonCurrentFramebuffer)
+{
+    glBindTexture(GL_TEXTURE_2D, mTextures[0]);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 16, 16, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
+    glBindFramebuffer(GL_FRAMEBUFFER, mFramebuffer);
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, mTextures[0], 0);
+    glBindTexture(GL_TEXTURE_2D, 0);
+    EXPECT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, glCheckFramebufferStatus(GL_FRAMEBUFFER));
+    ASSERT_GL_NO_ERROR();
+
+    // Draw with red to the FBO.
+    GLColor red(255, 0, 0, 255);
+    setUniformColor(red);
+    drawQuad(mProgram, "position", 0.5f);
+    EXPECT_PIXEL_COLOR_EQ(0, 0, red);
+
+    // Go back to default framebuffer, draw green
+    glBindFramebuffer(GL_FRAMEBUFFER, 0);
+    GLColor green(0, 255, 0, 255);
+    setUniformColor(green);
+    drawQuad(mProgram, "position", 0.5f);
+    EXPECT_PIXEL_COLOR_EQ(0, 0, green);
+
+    glBindFramebuffer(GL_FRAMEBUFFER, mFramebuffer);
+
+    // Invalidate color buffer of FBO
+    GLenum attachments1[] = {GL_COLOR_ATTACHMENT0};
+    glInvalidateFramebuffer(GL_FRAMEBUFFER, 1, attachments1);
+    ASSERT_GL_NO_ERROR();
+
+    // Verify drawing blue gives blue.
+    GLColor blue(0, 0, 255, 255);
+    setUniformColor(blue);
+    drawQuad(mProgram, "position", 0.5f);
+    EXPECT_PIXEL_COLOR_EQ(0, 0, blue);
+}
+
 // Tests that D3D11 dirty bit updates don't forget about BufferSubData attrib updates.
 TEST_P(StateChangeTest, VertexBufferUpdatedAfterDraw)
 {
@@ -5134,6 +5175,7 @@ ANGLE_INSTANTIATE_TEST_ES2(StateChangeTest);
 ANGLE_INSTANTIATE_TEST_ES2(LineLoopStateChangeTest);
 ANGLE_INSTANTIATE_TEST_ES2(StateChangeRenderTest);
 ANGLE_INSTANTIATE_TEST_ES3(StateChangeTestES3);
+ANGLE_INSTANTIATE_TEST_ES3(StateChangeRenderTestES3);
 ANGLE_INSTANTIATE_TEST_ES2(SimpleStateChangeTest);
 ANGLE_INSTANTIATE_TEST_ES3(SimpleStateChangeTestES3);
 ANGLE_INSTANTIATE_TEST_ES3(ImageRespecificationTest);