Vulkan: Fix prerotation bug with glInvalidateSubFramebuffer

src/libANGLE/renderer/vulkan/FramebufferVk.cpp

@@ -128,7 +128,7 @@ void EarlyAdjustFlipYForPreRotation(SurfaceRotation blitAngleIn,
 
 void AdjustBlitAreaForPreRotation(SurfaceRotation framebufferAngle,
                                   const gl::Rectangle &blitAreaIn,
-                                  gl::Rectangle framebufferDimensions,
+                                  const gl::Rectangle &framebufferDimensions,
                                   gl::Rectangle *blitAreaOut)
 {
     switch (framebufferAngle)
@@ -359,9 +359,16 @@ angle::Result FramebufferVk::invalidateSub(const gl::Context *context,
 {
     ContextVk *contextVk = vk::GetImpl(context);
 
+    const gl::Rectangle nonRotatedCompleteRenderArea = getNonRotatedCompleteRenderArea();
+    gl::Rectangle rotatedInvalidateArea;
+    RotateRectangle(contextVk->getRotationDrawFramebuffer(),
+                    contextVk->isViewportFlipEnabledForDrawFBO(),
+                    nonRotatedCompleteRenderArea.width, nonRotatedCompleteRenderArea.height, area,
+                    &rotatedInvalidateArea);
+
     // If invalidateSub() covers the whole framebuffer area, make it behave as invalidate().
     const gl::Rectangle completeRenderArea = getRotatedCompleteRenderArea(contextVk);
-    if (area.encloses(completeRenderArea))
+    if (rotatedInvalidateArea.encloses(completeRenderArea))
     {
         return invalidateImpl(contextVk, count, attachments, false);
     }
@@ -372,7 +379,8 @@ angle::Result FramebufferVk::invalidateSub(const gl::Context *context,
     // glCopyTex[Sub]Image, shader storage image, etc).
     ANGLE_TRY(flushDeferredClears(contextVk, completeRenderArea));
 
-    if (area.encloses(contextVk->getStartedRenderPassCommands().getRenderArea()))
+    if (contextVk->hasStartedRenderPass() &&
+        rotatedInvalidateArea.encloses(contextVk->getStartedRenderPassCommands().getRenderArea()))
     {
         // Because the render pass's render area is within the invalidated area, it is fine for
         // invalidateImpl() to use a storeOp of DONT_CARE (i.e. fine to not store the contents of

src/tests/gl_tests/FramebufferTest.cpp

@@ -385,7 +385,21 @@ TEST_P(FramebufferFormatsTest, RGB565Renderbuffer)
 }
 
 class FramebufferTest_ES3 : public ANGLETest
-{};
+{
+  protected:
+    FramebufferTest_ES3()
+    {
+        setWindowWidth(kWidth);
+        setWindowHeight(kHeight);
+        setConfigRedBits(8);
+        setConfigGreenBits(8);
+        setConfigBlueBits(8);
+        setConfigAlphaBits(8);
+    }
+
+    static constexpr GLsizei kWidth  = 64;
+    static constexpr GLsizei kHeight = 256;
+};
 
 // Covers invalidating an incomplete framebuffer. This should be a no-op, but should not error.
 TEST_P(FramebufferTest_ES3, InvalidateIncomplete)
@@ -425,6 +439,42 @@ TEST_P(FramebufferTest_ES3, SubInvalidateIncomplete)
     EXPECT_GL_NO_ERROR();
 }
 
+// Test that subinvalidate with no prior command works.  Regression test for the Vulkan backend that
+// assumed a render pass is started when sub invalidate is called.
+TEST_P(FramebufferTest_ES3, SubInvalidateFirst)
+{
+    glBindFramebuffer(GL_FRAMEBUFFER, 0);
+
+    // Invalidate half of the framebuffer using swapped dimensions.
+    std::array<GLenum, 1> attachments = {GL_COLOR};
+    glInvalidateSubFramebuffer(GL_DRAW_FRAMEBUFFER, 1, attachments.data(), 0, 0, kHeight, kWidth);
+    EXPECT_GL_NO_ERROR();
+}
+
+// Test that subinvalidate doesn't discard data outside area.  Uses swapped width/height for
+// invalidate which results in a partial invalidate, but also prevents bugs with Vulkan
+// pre-rotation.
+TEST_P(FramebufferTest_ES3, SubInvalidatePartial)
+{
+    glBindFramebuffer(GL_FRAMEBUFFER, 0);
+
+    // Clear the attachment.
+    glClearColor(1.0f, 0.0f, 0.0f, 1.0f);
+    glClear(GL_COLOR_BUFFER_BIT);
+    EXPECT_GL_NO_ERROR();
+
+    // Invalidate half of the framebuffer using swapped dimensions.
+    std::array<GLenum, 1> attachments = {GL_COLOR};
+    glInvalidateSubFramebuffer(GL_DRAW_FRAMEBUFFER, 1, attachments.data(), 0, 0, kHeight, kWidth);
+    EXPECT_GL_NO_ERROR();
+
+    // Make sure the other half is correct.
+    EXPECT_PIXEL_COLOR_EQ(0, kWidth, GLColor::red);
+    EXPECT_PIXEL_COLOR_EQ(kWidth - 1, kWidth, GLColor::red);
+    EXPECT_PIXEL_COLOR_EQ(0, kHeight - 1, GLColor::red);
+    EXPECT_PIXEL_COLOR_EQ(kWidth - 1, kHeight - 1, GLColor::red);
+}
+
 // Test that the framebuffer state tracking robustly handles a depth-only attachment being set
 // as a depth-stencil attachment. It is equivalent to detaching the depth-stencil attachment.
 TEST_P(FramebufferTest_ES3, DepthOnlyAsDepthStencil)
@@ -2510,5 +2560,9 @@ TEST_P(FramebufferTest, BindAndDrawDifferentSizedFBOs)
 ANGLE_INSTANTIATE_TEST_ES2(AddMockTextureNoRenderTargetTest);
 ANGLE_INSTANTIATE_TEST_ES2(FramebufferTest);
 ANGLE_INSTANTIATE_TEST_ES2_AND_ES3(FramebufferFormatsTest);
-ANGLE_INSTANTIATE_TEST_ES3_AND(FramebufferTest_ES3, ES3_METAL());
+ANGLE_INSTANTIATE_TEST_ES3_AND(FramebufferTest_ES3,
+                               WithEmulatedPrerotation(ES3_VULKAN(), 90),
+                               WithEmulatedPrerotation(ES3_VULKAN(), 180),
+                               WithEmulatedPrerotation(ES3_VULKAN(), 270),
+                               ES3_METAL());
 ANGLE_INSTANTIATE_TEST_ES31(FramebufferTest_ES31);