Vulkan: Fix scissor update in FramebufferVk::syncState

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -3630,8 +3630,19 @@ void ContextVk::invalidateDriverUniforms()
     mComputeDirtyBits.set(DIRTY_BIT_DRIVER_UNIFORMS_BINDING);
 }
 
-void ContextVk::onDrawFramebufferChange(FramebufferVk *framebufferVk)
+angle::Result ContextVk::onFramebufferChange(FramebufferVk *framebufferVk)
 {
+    // This is called from FramebufferVk::syncState.  Skip these updates if the framebuffer being
+    // synced is:
+    //
+    // - The read framebuffer (which is not equal the draw framebuffer)
+    // - A newly bound draw framebuffer.  ContextVk::syncState which follows will update all these
+    //   states, so this is redundant.
+    if (framebufferVk != mDrawFramebuffer)
+    {
+        return angle::Result::Continue;
+    }
+
     // Ensure that the pipeline description is updated.
     if (mGraphicsPipelineDesc->getRasterizationSamples() !=
         static_cast<uint32_t>(framebufferVk->getSamples()))
@@ -3640,7 +3651,12 @@ void ContextVk::onDrawFramebufferChange(FramebufferVk *framebufferVk)
                                                           framebufferVk->getSamples());
     }
 
+    // Update scissor.
+    ANGLE_TRY(updateScissor(mState));
+
     onDrawFramebufferRenderPassDescChange(framebufferVk);
+
+    return angle::Result::Continue;
 }
 
 void ContextVk::onDrawFramebufferRenderPassDescChange(FramebufferVk *framebufferVk)

src/libANGLE/renderer/vulkan/ContextVk.h

@@ -360,7 +360,7 @@ class ContextVk : public ContextImpl, public vk::Context
 
     void invalidateDefaultAttribute(size_t attribIndex);
     void invalidateDefaultAttributes(const gl::AttributesMask &dirtyMask);
-    void onDrawFramebufferChange(FramebufferVk *framebufferVk);
+    angle::Result onFramebufferChange(FramebufferVk *framebufferVk);
     void onDrawFramebufferRenderPassDescChange(FramebufferVk *framebufferVk);
     void onHostVisibleBufferWrite() { mIsAnyHostVisibleBufferWritten = true; }
 

src/libANGLE/renderer/vulkan/FramebufferVk.cpp

@@ -1763,13 +1763,6 @@ angle::Result FramebufferVk::syncState(const gl::Context *context,
         return angle::Result::Continue;
     }
 
-    // The FBO's new attachment may have changed the renderable area
-    if (binding == GL_DRAW_FRAMEBUFFER)
-    {
-        const gl::State &glState = context->getState();
-        ANGLE_TRY(contextVk->updateScissor(glState));
-    }
-
     if (command != gl::Command::Blit)
     {
         // Don't end the render pass when handling a blit to resolve, since we may be able to
@@ -1783,14 +1776,12 @@ angle::Result FramebufferVk::syncState(const gl::Context *context,
 
     updateRenderPassDesc();
 
-    // Notify the ContextVk to update the pipeline desc.
-    if (binding == GL_DRAW_FRAMEBUFFER)
-    {
-        contextVk->onDrawFramebufferChange(this);
-    }
     // Deactivate Framebuffer
     mFramebuffer = nullptr;
 
+    // Notify the ContextVk to update the pipeline desc.
+    ANGLE_TRY(contextVk->onFramebufferChange(this));
+
     return angle::Result::Continue;
 }
 

src/tests/gl_tests/FramebufferTest.cpp

@@ -2354,6 +2354,75 @@ TEST_P(FramebufferTest_ES3, DisabledAttachmentRedefinition)
     EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
 }
 
+// Test that changing the attachment of a framebuffer then sync'ing both READ and DRAW framebuffer
+// (currently possible with glInvalidateFramebuffer) updates the scissor correctly.
+TEST_P(FramebufferTest_ES3, ChangeAttachmentThenInvalidateAndDraw)
+{
+    constexpr GLsizei kSizeLarge = 32;
+    constexpr GLsizei kSizeSmall = 16;
+
+    GLTexture color1;
+    glBindTexture(GL_TEXTURE_2D, color1);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, kSizeSmall, kSizeSmall, 0, GL_RGBA, GL_UNSIGNED_BYTE,
+                 nullptr);
+
+    GLTexture color2;
+    glBindTexture(GL_TEXTURE_2D, color2);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, kSizeLarge, kSizeLarge, 0, GL_RGBA, GL_UNSIGNED_BYTE,
+                 nullptr);
+
+    GLFramebuffer fbo;
+    glBindFramebuffer(GL_FRAMEBUFFER, fbo);
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, color1, 0);
+
+    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);
+
+    glViewport(0, 0, kSizeLarge, kSizeLarge);
+
+    // Draw red into the framebuffer.
+    glUniform4f(colorUniformLocation, 1.0f, 0.0f, 0.0f, 1.0f);
+    drawQuad(drawColor, essl1_shaders::PositionAttrib(), 0.5f);
+    ASSERT_GL_NO_ERROR();
+
+    // Change the attachment, invalidate it and draw green.
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, color2, 0);
+    ASSERT_GL_FRAMEBUFFER_COMPLETE(GL_FRAMEBUFFER);
+
+    std::array<GLenum, 1> attachments = {GL_COLOR_ATTACHMENT0};
+    glInvalidateFramebuffer(GL_FRAMEBUFFER, 1, attachments.data());
+    ASSERT_GL_NO_ERROR();
+
+    glUniform4f(colorUniformLocation, 0.0f, 1.0f, 0.0f, 1.0f);
+    drawQuad(drawColor, essl1_shaders::PositionAttrib(), 0.5f);
+    ASSERT_GL_NO_ERROR();
+
+    // Validate the result.
+    EXPECT_PIXEL_RECT_EQ(0, 0, kSizeLarge, kSizeLarge, GLColor::green);
+
+    // Do the same, but changing from the large to small attachment.
+
+    // Draw red into the framebuffer.
+    glUniform4f(colorUniformLocation, 1.0f, 0.0f, 0.0f, 1.0f);
+    drawQuad(drawColor, essl1_shaders::PositionAttrib(), 0.5f);
+    ASSERT_GL_NO_ERROR();
+
+    // Change the attachment, invalidate it and draw blue.
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, color1, 0);
+    ASSERT_GL_FRAMEBUFFER_COMPLETE(GL_FRAMEBUFFER);
+    glInvalidateFramebuffer(GL_FRAMEBUFFER, 1, attachments.data());
+
+    glUniform4f(colorUniformLocation, 0.0f, 0.0f, 1.0f, 1.0f);
+    drawQuad(drawColor, essl1_shaders::PositionAttrib(), 0.5f);
+    ASSERT_GL_NO_ERROR();
+
+    // Validate the result.
+    EXPECT_PIXEL_RECT_EQ(0, 0, kSizeSmall, kSizeSmall, GLColor::blue);
+}
+
 class FramebufferTest : public ANGLETest
 {};