Vulkan: Fix UtilsVk clear in non-zero subpass

Mid-render-pass clears (through UtilsVk) run on the current subpass,
which in the presence of multisampled-render-to-texture unresolve
would be subpass 1.  The graphics pipeline for that draw call should
set the correct subpass index.

Bug: angleproject:4836
Change-Id: Iba4a03ea96a63b0f5d09c27e5283ff8a8b534e05
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2441509
Commit-Queue: Shahbaz Youssefi <[email protected]>
Reviewed-by: Shahbaz Youssefi <[email protected]>
Reviewed-by: Jamie Madill <[email protected]>

Author: ShabbyX

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -4687,6 +4687,11 @@ void ContextVk::restoreFinishedRenderPass(vk::Framebuffer *framebuffer)
     }
 }
 
+uint32_t ContextVk::getCurrentSubpassIndex() const
+{
+    return mGraphicsPipelineDesc->getSubpass();
+}
+
 angle::Result ContextVk::flushCommandsAndEndRenderPass()
 {
     // Ensure we flush the RenderPass *after* the prior commands.

src/libANGLE/renderer/vulkan/ContextVk.h

@@ -593,6 +593,8 @@ class ContextVk : public ContextImpl, public vk::Context
     // TODO(https://anglebug.com/4968): Support multiple open render passes.
     void restoreFinishedRenderPass(vk::Framebuffer *framebuffer);
 
+    uint32_t getCurrentSubpassIndex() const;
+
     egl::ContextPriority getContextPriority() const override { return mContextPriority; }
     angle::Result startRenderPass(gl::Rectangle renderArea, vk::CommandBuffer **commandBufferOut);
     void startNextSubpass();

src/libANGLE/renderer/vulkan/UtilsVk.cpp

@@ -1429,6 +1429,9 @@ angle::Result UtilsVk::clearFramebuffer(ContextVk *contextVk,
     // Note: depth test is disabled by default so this should be unnecessary, but works around an
     // Intel bug on windows.  http://anglebug.com/3348
     pipelineDesc.setDepthWriteEnabled(false);
+    // Clears can be done on a currently open render pass, so make sure the correct subpass index is
+    // used.
+    pipelineDesc.setSubpass(contextVk->getCurrentSubpassIndex());
 
     // Clear stencil by enabling stencil write with the right mask.
     if (params.clearStencil)

src/libANGLE/renderer/vulkan/vk_cache_utils.cpp

@@ -2228,6 +2228,16 @@ void GraphicsPipelineDesc::nextSubpass(GraphicsPipelineTransitionBits *transitio
     updateSubpass(transition, mRasterizationAndMultisampleStateInfo.bits.subpass + 1);
 }
 
+void GraphicsPipelineDesc::setSubpass(uint32_t subpass)
+{
+    SetBitField(mRasterizationAndMultisampleStateInfo.bits.subpass, subpass);
+}
+
+uint32_t GraphicsPipelineDesc::getSubpass() const
+{
+    return mRasterizationAndMultisampleStateInfo.bits.subpass;
+}
+
 void GraphicsPipelineDesc::updateRenderPassDesc(GraphicsPipelineTransitionBits *transition,
                                                 const RenderPassDesc &renderPassDesc)
 {

src/libANGLE/renderer/vulkan/vk_cache_utils.h

@@ -670,6 +670,8 @@ class GraphicsPipelineDesc final
     // Subpass
     void resetSubpass(GraphicsPipelineTransitionBits *transition);
     void nextSubpass(GraphicsPipelineTransitionBits *transition);
+    void setSubpass(uint32_t subpass);
+    uint32_t getSubpass() const;
 
   private:
     void updateSubpass(GraphicsPipelineTransitionBits *transition, uint32_t subpass);

src/tests/gl_tests/MultisampledRenderToTextureTest.cpp

@@ -101,6 +101,7 @@ class MultisampledRenderToTextureTest : public ANGLETest
         glDisable(GL_DEPTH_TEST);
         glDisable(GL_STENCIL_TEST);
         glDisable(GL_BLEND);
+        glColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
         drawQuad(mCopyTextureProgram, essl1_shaders::PositionAttrib(), 0.5f);
 
         // Expect that the rendered quad has the same color as the source texture
@@ -144,6 +145,7 @@ class MultisampledRenderToTextureTest : public ANGLETest
     void copyTexSubImageTestCommon(bool useRenderbuffer);
     void drawCopyThenBlendCommon(bool useRenderbuffer);
     void clearDrawCopyThenBlendSameProgramCommon(bool useRenderbuffer);
+    void drawCopyDrawThenMaskedClearCommon(bool useRenderbuffer);
     void clearThenBlendCommon(bool useRenderbuffer);
 
     GLProgram mCopyTextureProgram;
@@ -1066,6 +1068,14 @@ void MultisampledRenderToTextureTest::clearDrawCopyThenBlendSameProgramCommon(bo
     drawQuad(drawColor, essl1_shaders::PositionAttrib(), 0.5f);
     ASSERT_GL_NO_ERROR();
 
+    // Blend green into it.  This makes sure that the blend after the resolve doesn't have different
+    // state from the one used here.
+    glEnable(GL_BLEND);
+    glBlendFunc(GL_ONE, GL_ONE);
+    glUniform4f(colorUniformLocation, 0.0f, 1.0f, 0.0f, 1.0f);
+    drawQuad(drawColor, essl1_shaders::PositionAttrib(), 0.5f);
+    ASSERT_GL_NO_ERROR();
+
     // Create a texture and copy into it.
     GLTexture texture;
     glBindTexture(GL_TEXTURE_2D, texture);
@@ -1077,35 +1087,31 @@ void MultisampledRenderToTextureTest::clearDrawCopyThenBlendSameProgramCommon(bo
     // residing in the single-sampled texture, is available to the multisampled intermediate image
     // for blending.
 
-    // Blend half-transparent green into the multisampled color buffer.
-    glUniform4f(colorUniformLocation, 0.0f, 1.0f, 0.0f, 0.5f);
-    glEnable(GL_BLEND);
-    glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);
+    // Blend blue into the multisampled color buffer.
+    glUniform4f(colorUniformLocation, 0.0f, 0.0f, 1.0f, 1.0f);
     drawQuad(drawColor, essl1_shaders::PositionAttrib(), 0.5f);
     ASSERT_GL_NO_ERROR();
 
-    // Verify that the texture is now yellow
-    const GLColor kExpected(127, 127, 0, 191);
-    EXPECT_PIXEL_COLOR_NEAR(0, 0, kExpected, 1);
-    EXPECT_PIXEL_COLOR_NEAR(kSize - 1, 0, kExpected, 1);
-    EXPECT_PIXEL_COLOR_NEAR(0, kSize - 1, kExpected, 1);
-    EXPECT_PIXEL_COLOR_NEAR(kSize - 1, kSize - 1, kExpected, 1);
+    // Verify that the texture is now white
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::white);
+    EXPECT_PIXEL_COLOR_EQ(kSize - 1, 0, GLColor::white);
+    EXPECT_PIXEL_COLOR_EQ(0, kSize - 1, GLColor::white);
+    EXPECT_PIXEL_COLOR_EQ(kSize - 1, kSize - 1, GLColor::white);
 
     // Once again, clear and draw so the program is used again in the way it was first used.
     glClear(GL_COLOR_BUFFER_BIT);
     glDisable(GL_BLEND);
     glUniform4f(colorUniformLocation, 0.0f, 0.0f, 1.0f, 1.0f);
     drawQuad(drawColor, essl1_shaders::PositionAttrib(), 0.5f);
 
-    EXPECT_PIXEL_COLOR_NEAR(0, 0, GLColor::blue, 1);
-    EXPECT_PIXEL_COLOR_NEAR(kSize - 1, 0, GLColor::blue, 1);
-    EXPECT_PIXEL_COLOR_NEAR(0, kSize - 1, GLColor::blue, 1);
-    EXPECT_PIXEL_COLOR_NEAR(kSize - 1, kSize - 1, GLColor::blue, 1);
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::blue);
+    EXPECT_PIXEL_COLOR_EQ(kSize - 1, 0, GLColor::blue);
+    EXPECT_PIXEL_COLOR_EQ(0, kSize - 1, GLColor::blue);
+    EXPECT_PIXEL_COLOR_EQ(kSize - 1, kSize - 1, GLColor::blue);
 
-    // For completeness, verify that the texture used as copy target is red.
+    // For completeness, verify that the texture used as copy target is yellow.
     ASSERT_GL_NO_ERROR();
-    const GLColor kExpectedCopyResult(255, 0, 0, 255);
-    verifyResults(texture, kExpectedCopyResult, kSize, 0, 0, kSize, kSize);
+    verifyResults(texture, GLColor::yellow, kSize, 0, 0, kSize, kSize);
 
     ASSERT_GL_NO_ERROR();
 }
@@ -1126,6 +1132,200 @@ TEST_P(MultisampledRenderToTextureTest, RenderbufferClearDrawCopyThenBlendSamePr
     clearDrawCopyThenBlendSameProgramCommon(true);
 }
 
+// Similar to RenderbufferClearDrawCopyThenBlendSameProgram, but with the depth/stencil attachment
+// being unresolved only.
+TEST_P(MultisampledRenderToTextureES3Test,
+       RenderbufferClearDrawCopyThenBlendWithDepthStencilSameProgram)
+{
+    ANGLE_SKIP_TEST_IF(!EnsureGLExtensionEnabled("GL_EXT_multisampled_render_to_texture"));
+    constexpr GLsizei kSize = 64;
+
+    setupCopyTexProgram();
+
+    GLFramebuffer fboMS;
+    glBindFramebuffer(GL_FRAMEBUFFER, fboMS);
+
+    // Create multisampled framebuffer to draw into
+    GLTexture color;
+    glBindTexture(GL_TEXTURE_2D, color);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, kSize, kSize, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
+    glFramebufferTexture2DMultisampleEXT(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, color,
+                                         0, 4);
+
+    GLRenderbuffer depthStencil;
+    glBindRenderbuffer(GL_RENDERBUFFER, depthStencil);
+    glRenderbufferStorageMultisampleEXT(GL_RENDERBUFFER, 4, GL_DEPTH24_STENCIL8, kSize, kSize);
+    glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_STENCIL_ATTACHMENT, GL_RENDERBUFFER,
+                              depthStencil);
+    ASSERT_GL_NO_ERROR();
+    EXPECT_GL_FRAMEBUFFER_COMPLETE(GL_FRAMEBUFFER);
+
+    // Draw red into the multisampled color buffer.
+    ANGLE_GL_PROGRAM(drawColor, essl1_shaders::vs::Simple(), essl1_shaders::fs::UniformColor());
+    glUseProgram(drawColor);
+    GLint colorUniformLocation =
+        glGetUniformLocation(drawColor, angle::essl1_shaders::ColorUniform());
+    ASSERT_NE(colorUniformLocation, -1);
+
+    // Enable write to depth/stencil so the attachment has valid contents, but always pass the test.
+    glEnable(GL_DEPTH_TEST);
+    glDepthFunc(GL_ALWAYS);
+
+    glEnable(GL_STENCIL_TEST);
+    glStencilFunc(GL_ALWAYS, 0x55, 0xFF);
+    glStencilOp(GL_REPLACE, GL_REPLACE, GL_REPLACE);
+    glStencilMask(0xFF);
+
+    // Clear the framebuffer.
+    glClearColor(0.1f, 0.9f, 0.2f, 0.8f);
+    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT);
+
+    // Then draw into it.
+    glUniform4f(colorUniformLocation, 1.0f, 0.0f, 0.0f, 1.0f);
+    drawQuad(drawColor, essl1_shaders::PositionAttrib(), 1.0f);
+    ASSERT_GL_NO_ERROR();
+
+    // Blend green into it.  This makes sure that the blend after the resolve doesn't have different
+    // state from the one used here.  Additionally, test that the previous draw output the correct
+    // depth/stencil data.  Again, this makes sure that the draw call after the resolve doesn't have
+    // different has depth/stencil test state.
+
+    // If depth is not set to 1, rendering would fail.
+    glDepthFunc(GL_LESS);
+
+    // If stencil is not set to 0x55, rendering would fail.
+    glStencilFunc(GL_EQUAL, 0x55, 0xFF);
+    glStencilOp(GL_KEEP, GL_KEEP, GL_KEEP);
+
+    glEnable(GL_BLEND);
+    glBlendFunc(GL_ONE, GL_ONE);
+    glUniform4f(colorUniformLocation, 0.0f, 1.0f, 0.0f, 1.0f);
+    drawQuad(drawColor, essl1_shaders::PositionAttrib(), 0.95f);
+    ASSERT_GL_NO_ERROR();
+
+    // Create a texture and copy into it.
+    GLTexture texture;
+    glBindTexture(GL_TEXTURE_2D, texture);
+    glCopyTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 0, 0, kSize, kSize, 0);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+
+    // Clear color (but not depth/stencil), and draw again into the framebuffer, this time blending.
+    // Additionally, make sure the depth/stencil data are retained.
+
+    // Clear color (to blue), but not depth/stencil.
+    glClearColor(0.0f, 0.0f, 1.0f, 1.0f);
+    glClear(GL_COLOR_BUFFER_BIT);
+
+    // Blend green into the multisampled color buffer.
+    glUniform4f(colorUniformLocation, 0.0f, 1.0f, 0.0f, 1.0f);
+    drawQuad(drawColor, essl1_shaders::PositionAttrib(), 0.9f);
+    ASSERT_GL_NO_ERROR();
+
+    // Verify that the texture is now cyan
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::cyan);
+    EXPECT_PIXEL_COLOR_EQ(kSize - 1, 0, GLColor::cyan);
+    EXPECT_PIXEL_COLOR_EQ(0, kSize - 1, GLColor::cyan);
+    EXPECT_PIXEL_COLOR_EQ(kSize - 1, kSize - 1, GLColor::cyan);
+
+    // Once again, clear and draw so the program is used again in the way it was first used.
+    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT);
+    glDisable(GL_BLEND);
+    glDepthFunc(GL_ALWAYS);
+    glStencilFunc(GL_ALWAYS, 0x55, 0xFF);
+    glStencilOp(GL_REPLACE, GL_REPLACE, GL_REPLACE);
+    glUniform4f(colorUniformLocation, 0.0f, 0.0f, 1.0f, 1.0f);
+    drawQuad(drawColor, essl1_shaders::PositionAttrib(), 0.5f);
+
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::blue);
+    EXPECT_PIXEL_COLOR_EQ(kSize - 1, 0, GLColor::blue);
+    EXPECT_PIXEL_COLOR_EQ(0, kSize - 1, GLColor::blue);
+    EXPECT_PIXEL_COLOR_EQ(kSize - 1, kSize - 1, GLColor::blue);
+
+    // For completeness, verify that the texture used as copy target is yellow.
+    ASSERT_GL_NO_ERROR();
+    verifyResults(texture, GLColor::yellow, kSize, 0, 0, kSize, kSize);
+
+    ASSERT_GL_NO_ERROR();
+}
+
+void MultisampledRenderToTextureTest::drawCopyDrawThenMaskedClearCommon(bool useRenderbuffer)
+{
+    ANGLE_SKIP_TEST_IF(!EnsureGLExtensionEnabled("GL_EXT_multisampled_render_to_texture"));
+    constexpr GLsizei kSize = 64;
+
+    setupCopyTexProgram();
+
+    GLFramebuffer fboMS;
+    glBindFramebuffer(GL_FRAMEBUFFER, fboMS);
+
+    // Create multisampled framebuffer to draw into
+    GLTexture textureMS;
+    GLRenderbuffer renderbufferMS;
+    createAndAttachColorAttachment(useRenderbuffer, kSize, GL_COLOR_ATTACHMENT0, nullptr,
+                                   &textureMS, &renderbufferMS);
+    EXPECT_GL_FRAMEBUFFER_COMPLETE(GL_FRAMEBUFFER);
+
+    // Draw red into the multisampled color buffer.
+    ANGLE_GL_PROGRAM(drawColor, essl1_shaders::vs::Simple(), essl1_shaders::fs::UniformColor());
+    glUseProgram(drawColor);
+    GLint colorUniformLocation =
+        glGetUniformLocation(drawColor, angle::essl1_shaders::ColorUniform());
+    ASSERT_NE(colorUniformLocation, -1);
+
+    // Draw into framebuffer.
+    glUniform4f(colorUniformLocation, 1.0f, 0.0f, 0.0f, 1.0f);
+    drawQuad(drawColor, essl1_shaders::PositionAttrib(), 0.5f);
+    ASSERT_GL_NO_ERROR();
+
+    // Create a texture and copy into it.
+    GLTexture texture;
+    glBindTexture(GL_TEXTURE_2D, texture);
+    glCopyTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 0, 0, kSize, kSize, 0);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+
+    // Draw again into the framebuffer, this time blending.  Afterwards, issue a masked clear.  This
+    // ensures that previous resolved data is unresolved, and mid-render-pass clears work correctly.
+
+    // Draw green into the multisampled color buffer.
+    glUniform4f(colorUniformLocation, 0.0f, 1.0f, 0.0f, 1.0f);
+    drawQuad(drawColor, essl1_shaders::PositionAttrib(), 0.5f);
+    ASSERT_GL_NO_ERROR();
+
+    // Issue a masked clear.
+    glClearColor(0.1f, 0.9f, 1.0f, 0.8f);
+    glColorMask(GL_FALSE, GL_FALSE, GL_TRUE, GL_FALSE);
+    glClear(GL_COLOR_BUFFER_BIT);
+
+    // Verify that the texture is now cyan
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::cyan);
+    EXPECT_PIXEL_COLOR_EQ(kSize - 1, 0, GLColor::cyan);
+    EXPECT_PIXEL_COLOR_EQ(0, kSize - 1, GLColor::cyan);
+    EXPECT_PIXEL_COLOR_EQ(kSize - 1, kSize - 1, GLColor::cyan);
+
+    // For completeness, verify that the texture used as copy target is red.
+    ASSERT_GL_NO_ERROR();
+    verifyResults(texture, GLColor::red, kSize, 0, 0, kSize, kSize);
+
+    ASSERT_GL_NO_ERROR();
+}
+
+// Draw, copy, draw then issue a masked clear.  The copy will make sure an implicit resolve
+// happens.  The second draw should retain the data written by the first draw command ("unresolve"
+// operation).  The final clear uses a draw call to perform the clear in the Vulkan backend, and it
+// should use the correct subpass index.
+TEST_P(MultisampledRenderToTextureTest, DrawCopyDrawThenMaskedClear)
+{
+    drawCopyDrawThenMaskedClearCommon(false);
+}
+
+// Same as DrawCopyDrawThenMaskedClearCommon but with renderbuffers
+TEST_P(MultisampledRenderToTextureTest, RenderbufferDrawCopyDrawThenMaskedClear)
+{
+    drawCopyDrawThenMaskedClearCommon(true);
+}
+
 void MultisampledRenderToTextureTest::clearThenBlendCommon(bool useRenderbuffer)
 {
     ANGLE_SKIP_TEST_IF(!EnsureGLExtensionEnabled("GL_EXT_multisampled_render_to_texture"));