Vulkan: Don't break the render pass on scissor change

src/libANGLE/angletypes.cpp

@@ -29,6 +29,11 @@ bool IsStencilNoOp(GLenum stencilFunc,
     return isNeverAndKeep || isAlwaysAndKeepOrAllKeep;
 }
 
+// Calculate whether the range [outsideLow, outsideHigh] encloses the range [insideLow, insideHigh]
+bool EnclosesRange(int outsideLow, int outsideHigh, int insideLow, int insideHigh)
+{
+    return outsideLow <= insideLow && outsideHigh >= insideHigh;
+}
 }  // anonymous namespace
 
 RasterizerState::RasterizerState()
@@ -644,6 +649,117 @@ bool ClipRectangle(const Rectangle &source, const Rectangle &clip, Rectangle *in
     return width != 0 && height != 0;
 }
 
+void GetEnclosingRectangle(const Rectangle &rect1, const Rectangle &rect2, Rectangle *rectUnion)
+{
+    // All callers use non-flipped framebuffer-size-clipped rectangles, so both flip and overflow
+    // are impossible.
+    ASSERT(!rect1.isReversedX() && !rect1.isReversedY());
+    ASSERT(!rect2.isReversedX() && !rect2.isReversedY());
+    ASSERT((angle::CheckedNumeric<int>(rect1.x) + rect1.width).IsValid());
+    ASSERT((angle::CheckedNumeric<int>(rect1.y) + rect1.height).IsValid());
+    ASSERT((angle::CheckedNumeric<int>(rect2.x) + rect2.width).IsValid());
+    ASSERT((angle::CheckedNumeric<int>(rect2.y) + rect2.height).IsValid());
+
+    // This function calculates a rectangle that covers both input rectangles:
+    //
+    //                     +---------+
+    //          rect1 -->  |         |
+    //                     |     +---+-----+
+    //                     |     |   |     | <-- rect2
+    //                     +-----+---+     |
+    //                           |         |
+    //                           +---------+
+    //
+    //   xy0 = min(rect1.xy0, rect2.xy0)
+    //                    \
+    //                     +---------+-----+
+    //          union -->  |         .     |
+    //                     |     + . + . . +
+    //                     |     .   .     |
+    //                     + . . + . +     |
+    //                     |     .         |
+    //                     +-----+---------+
+    //                                    /
+    //                         xy1 = max(rect1.xy1, rect2.xy1)
+
+    int x0 = std::min(rect1.x0(), rect2.x0());
+    int y0 = std::min(rect1.y0(), rect2.y0());
+
+    int x1 = std::max(rect1.x1(), rect2.x1());
+    int y1 = std::max(rect1.y1(), rect2.y1());
+
+    rectUnion->x      = x0;
+    rectUnion->y      = y0;
+    rectUnion->width  = x1 - x0;
+    rectUnion->height = y1 - y0;
+}
+
+void ExtendRectangle(const Rectangle &source, const Rectangle &extend, Rectangle *extended)
+{
+    // All callers use non-flipped framebuffer-size-clipped rectangles, so both flip and overflow
+    // are impossible.
+    ASSERT(!source.isReversedX() && !source.isReversedY());
+    ASSERT(!extend.isReversedX() && !extend.isReversedY());
+    ASSERT((angle::CheckedNumeric<int>(source.x) + source.width).IsValid());
+    ASSERT((angle::CheckedNumeric<int>(source.y) + source.height).IsValid());
+    ASSERT((angle::CheckedNumeric<int>(extend.x) + extend.width).IsValid());
+    ASSERT((angle::CheckedNumeric<int>(extend.y) + extend.height).IsValid());
+
+    int x0 = source.x0();
+    int x1 = source.x1();
+    int y0 = source.y0();
+    int y1 = source.y1();
+
+    const int extendX0 = extend.x0();
+    const int extendX1 = extend.x1();
+    const int extendY0 = extend.y0();
+    const int extendY1 = extend.y1();
+
+    // For each side of the rectangle, calculate whether it can be extended by the second rectangle.
+    // If so, extend it and continue for the next side with the new dimensions.
+
+    // Left: Reduce x0 if the second rectangle's vertical edge covers the source's:
+    //
+    //     +--- - - -                +--- - - -
+    //     |                         |
+    //     |  +--------------+       +-----------------+
+    //     |  |    source    |  -->  |       source    |
+    //     |  +--------------+       +-----------------+
+    //     |                         |
+    //     +--- - - -                +--- - - -
+    //
+    const bool enclosesHeight = EnclosesRange(extendY0, extendY1, y0, y1);
+    if (extendX0 < x0 && extendX1 >= x0 && enclosesHeight)
+    {
+        x0 = extendX0;
+    }
+
+    // Right: Increase x1 simiarly.
+    if (extendX0 <= x1 && extendX1 > x1 && enclosesHeight)
+    {
+        x1 = extendX1;
+    }
+
+    // Top: Reduce y0 if the second rectangle's horizontal edge covers the source's potentially
+    // extended edge.
+    const bool enclosesWidth = EnclosesRange(extendX0, extendX1, x0, x1);
+    if (extendY0 < y0 && extendY1 >= y0 && enclosesWidth)
+    {
+        y0 = extendY0;
+    }
+
+    // Right: Increase y1 simiarly.
+    if (extendY0 <= y1 && extendY1 > y1 && enclosesWidth)
+    {
+        y1 = extendY1;
+    }
+
+    extended->x      = x0;
+    extended->y      = y0;
+    extended->width  = x1 - x0;
+    extended->height = y1 - y0;
+}
+
 bool Box::operator==(const Box &other) const
 {
     return (x == other.x && y == other.y && z == other.z && width == other.width &&

src/libANGLE/angletypes.h

@@ -72,6 +72,8 @@ struct Rectangle
 
     bool encloses(const gl::Rectangle &inside) const;
 
+    bool empty() const { return width == 0 && height == 0; }
+
     int x;
     int y;
     int width;
@@ -81,7 +83,27 @@ struct Rectangle
 bool operator==(const Rectangle &a, const Rectangle &b);
 bool operator!=(const Rectangle &a, const Rectangle &b);
 
+// Calculate the intersection of two rectangles.  Returns false if the intersection is empty.
 bool ClipRectangle(const Rectangle &source, const Rectangle &clip, Rectangle *intersection);
+// Calculate the smallest rectangle that covers both rectangles.  This rectangle may cover areas
+// not covered by the two rectangles, for example in this situation:
+//
+//   +--+        +----+
+//   | ++-+  ->  |    |
+//   +-++ |      |    |
+//     +--+      +----+
+//
+void GetEnclosingRectangle(const Rectangle &rect1, const Rectangle &rect2, Rectangle *rectUnion);
+// Extend the source rectangle to cover parts (or all of) the second rectangle, in such a way that
+// no area is covered that isn't covered by both rectangles.  For example:
+//
+//             +--+        +--+
+//  source --> |  |        |  |
+//            ++--+-+  ->  |  |
+//            |+--+ |      |  |
+//            +-----+      +--+
+//
+void ExtendRectangle(const Rectangle &source, const Rectangle &extend, Rectangle *extended);
 
 struct Offset
 {

src/libANGLE/angletypes_unittest.cpp

@@ -287,4 +287,215 @@ TEST(Rectangle, Clip)
     ASSERT_FALSE(gl::ClipRectangle(clip10, source, nullptr));
 }
 
+// Test combine rectangles
+TEST(Rectangle, Combine)
+{
+    const gl::Rectangle rect1(0, 0, 100, 200);
+    const gl::Rectangle rect2(0, 0, 50, 100);
+    gl::Rectangle result;
+
+    gl::GetEnclosingRectangle(rect1, rect2, &result);
+    ASSERT_EQ(result.x0(), 0);
+    ASSERT_EQ(result.y0(), 0);
+    ASSERT_EQ(result.x1(), 100);
+    ASSERT_EQ(result.y1(), 200);
+
+    const gl::Rectangle rect3(50, 100, 100, 200);
+
+    gl::GetEnclosingRectangle(rect1, rect3, &result);
+    ASSERT_EQ(result.x0(), 0);
+    ASSERT_EQ(result.y0(), 0);
+    ASSERT_EQ(result.x1(), 150);
+    ASSERT_EQ(result.y1(), 300);
+
+    const gl::Rectangle rect4(-20, -30, 100, 200);
+
+    gl::GetEnclosingRectangle(rect1, rect4, &result);
+    ASSERT_EQ(result.x0(), -20);
+    ASSERT_EQ(result.y0(), -30);
+    ASSERT_EQ(result.x1(), 100);
+    ASSERT_EQ(result.y1(), 200);
+
+    const gl::Rectangle rect5(10, -30, 100, 200);
+
+    gl::GetEnclosingRectangle(rect1, rect5, &result);
+    ASSERT_EQ(result.x0(), 0);
+    ASSERT_EQ(result.y0(), -30);
+    ASSERT_EQ(result.x1(), 110);
+    ASSERT_EQ(result.y1(), 200);
+}
+
+// Test extend rectangles
+TEST(Rectangle, Extend)
+{
+    const gl::Rectangle source(0, 0, 100, 200);
+    const gl::Rectangle extend1(0, 0, 50, 100);
+    gl::Rectangle result;
+
+    //  +------+       +------+
+    //  |   |  |       |      |
+    //  +---+  |  -->  |      |
+    //  |      |       |      |
+    //  +------+       +------+
+    //
+    gl::ExtendRectangle(source, extend1, &result);
+    ASSERT_EQ(result.x0(), 0);
+    ASSERT_EQ(result.y0(), 0);
+    ASSERT_EQ(result.x1(), 100);
+    ASSERT_EQ(result.y1(), 200);
+
+    //  +------+           +------+
+    //  |S     |           |      |
+    //  |   +--+---+  -->  |      |
+    //  |   |  |   |       |      |
+    //  +---+--+   +       +------+
+    //      |      |
+    //      +------+
+    //
+    const gl::Rectangle extend2(50, 100, 100, 200);
+
+    gl::ExtendRectangle(source, extend2, &result);
+    ASSERT_EQ(result.x0(), 0);
+    ASSERT_EQ(result.y0(), 0);
+    ASSERT_EQ(result.x1(), 100);
+    ASSERT_EQ(result.y1(), 200);
+
+    //    +------+           +------+
+    //    |S     |           |      |
+    //  +-+------+---+  -->  |      |
+    //  | |      |   |       |      |
+    //  | +------+   +       |      |
+    //  |            |       |      |
+    //  +------------+       +------+
+    //
+    const gl::Rectangle extend3(-10, 100, 200, 200);
+
+    gl::ExtendRectangle(source, extend3, &result);
+    ASSERT_EQ(result.x0(), 0);
+    ASSERT_EQ(result.y0(), 0);
+    ASSERT_EQ(result.x1(), 100);
+    ASSERT_EQ(result.y1(), 300);
+
+    //    +------+           +------+
+    //    |S     |           |      |
+    //    |      |      -->  |      |
+    //    |      |           |      |
+    //  +-+------+---+       |      |
+    //  |            |       |      |
+    //  +------------+       +------+
+    //
+    for (int offsetLeft = 10; offsetLeft >= 0; offsetLeft -= 10)
+    {
+        for (int offsetRight = 10; offsetRight >= 0; offsetRight -= 10)
+        {
+            const gl::Rectangle extend4(-offsetLeft, 200, 100 + offsetLeft + offsetRight, 100);
+
+            gl::ExtendRectangle(source, extend4, &result);
+            ASSERT_EQ(result.x0(), 0) << offsetLeft << " " << offsetRight;
+            ASSERT_EQ(result.y0(), 0) << offsetLeft << " " << offsetRight;
+            ASSERT_EQ(result.x1(), 100) << offsetLeft << " " << offsetRight;
+            ASSERT_EQ(result.y1(), 300) << offsetLeft << " " << offsetRight;
+        }
+    }
+
+    // Similar to extend4, but with the second rectangle on the top, left and right.
+    for (int offsetLeft = 10; offsetLeft >= 0; offsetLeft -= 10)
+    {
+        for (int offsetRight = 10; offsetRight >= 0; offsetRight -= 10)
+        {
+            const gl::Rectangle extend4(-offsetLeft, -100, 100 + offsetLeft + offsetRight, 100);
+
+            gl::ExtendRectangle(source, extend4, &result);
+            ASSERT_EQ(result.x0(), 0) << offsetLeft << " " << offsetRight;
+            ASSERT_EQ(result.y0(), -100) << offsetLeft << " " << offsetRight;
+            ASSERT_EQ(result.x1(), 100) << offsetLeft << " " << offsetRight;
+            ASSERT_EQ(result.y1(), 200) << offsetLeft << " " << offsetRight;
+        }
+    }
+    for (int offsetTop = 10; offsetTop >= 0; offsetTop -= 10)
+    {
+        for (int offsetBottom = 10; offsetBottom >= 0; offsetBottom -= 10)
+        {
+            const gl::Rectangle extend4(-50, -offsetTop, 50, 200 + offsetTop + offsetBottom);
+
+            gl::ExtendRectangle(source, extend4, &result);
+            ASSERT_EQ(result.x0(), -50) << offsetTop << " " << offsetBottom;
+            ASSERT_EQ(result.y0(), 0) << offsetTop << " " << offsetBottom;
+            ASSERT_EQ(result.x1(), 100) << offsetTop << " " << offsetBottom;
+            ASSERT_EQ(result.y1(), 200) << offsetTop << " " << offsetBottom;
+        }
+    }
+    for (int offsetTop = 10; offsetTop >= 0; offsetTop -= 10)
+    {
+        for (int offsetBottom = 10; offsetBottom >= 0; offsetBottom -= 10)
+        {
+            const gl::Rectangle extend4(100, -offsetTop, 50, 200 + offsetTop + offsetBottom);
+
+            gl::ExtendRectangle(source, extend4, &result);
+            ASSERT_EQ(result.x0(), 0) << offsetTop << " " << offsetBottom;
+            ASSERT_EQ(result.y0(), 0) << offsetTop << " " << offsetBottom;
+            ASSERT_EQ(result.x1(), 150) << offsetTop << " " << offsetBottom;
+            ASSERT_EQ(result.y1(), 200) << offsetTop << " " << offsetBottom;
+        }
+    }
+
+    //    +------+           +------+
+    //    |S     |           |      |
+    //    |      |      -->  |      |
+    //    |      |           |      |
+    //    +------+           +------+
+    //  +------------+
+    //  |            |
+    //  +------------+
+    //
+    const gl::Rectangle extend5(-10, 201, 120, 100);
+
+    gl::ExtendRectangle(source, extend5, &result);
+    ASSERT_EQ(result.x0(), 0);
+    ASSERT_EQ(result.y0(), 0);
+    ASSERT_EQ(result.x1(), 100);
+    ASSERT_EQ(result.y1(), 200);
+
+    // Similar to extend5, but with the second rectangle on the top, left and right.
+    const gl::Rectangle extend6(-10, -101, 120, 100);
+
+    gl::ExtendRectangle(source, extend6, &result);
+    ASSERT_EQ(result.x0(), 0);
+    ASSERT_EQ(result.y0(), 0);
+    ASSERT_EQ(result.x1(), 100);
+    ASSERT_EQ(result.y1(), 200);
+
+    const gl::Rectangle extend7(-101, -10, 100, 220);
+
+    gl::ExtendRectangle(source, extend7, &result);
+    ASSERT_EQ(result.x0(), 0);
+    ASSERT_EQ(result.y0(), 0);
+    ASSERT_EQ(result.x1(), 100);
+    ASSERT_EQ(result.y1(), 200);
+
+    const gl::Rectangle extend8(101, -10, 100, 220);
+
+    gl::ExtendRectangle(source, extend8, &result);
+    ASSERT_EQ(result.x0(), 0);
+    ASSERT_EQ(result.y0(), 0);
+    ASSERT_EQ(result.x1(), 100);
+    ASSERT_EQ(result.y1(), 200);
+
+    //  +-------------+       +-------------+
+    //  |   +------+  |       |             |
+    //  |   |S     |  |       |             |
+    //  |   |      |  |  -->  |             |
+    //  |   |      |  |       |             |
+    //  |   +------+  |       |             |
+    //  +-------------+       +-------------+
+    //
+    const gl::Rectangle extend9(-100, -100, 300, 400);
+
+    gl::ExtendRectangle(source, extend9, &result);
+    ASSERT_EQ(result.x0(), -100);
+    ASSERT_EQ(result.y0(), -100);
+    ASSERT_EQ(result.x1(), 200);
+    ASSERT_EQ(result.y1(), 300);
+}
+
 }  // namespace angle

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -2691,8 +2691,10 @@ void ContextVk::optimizeRenderPassForPresent(VkFramebuffer framebufferHandle)
     {
         // Change depthstencil attachment storeOp to DONT_CARE
         const gl::DepthStencilState &dsState = mState.getDepthStencilState();
-        mRenderPassCommands->invalidateRenderPassStencilAttachment(dsState);
-        mRenderPassCommands->invalidateRenderPassDepthAttachment(dsState);
+        mRenderPassCommands->invalidateRenderPassStencilAttachment(
+            dsState, mRenderPassCommands->getRenderArea());
+        mRenderPassCommands->invalidateRenderPassDepthAttachment(
+            dsState, mRenderPassCommands->getRenderArea());
         // Mark content as invalid so that we will not load them in next renderpass
         depthStencilRenderTarget->invalidateEntireContent(this);
         depthStencilRenderTarget->invalidateEntireStencilContent(this);
@@ -2931,7 +2933,7 @@ void ContextVk::updateDepthRange(float nearPlane, float farPlane)
     mGraphicsPipelineDesc->updateDepthRange(&mGraphicsPipelineTransition, nearPlane, farPlane);
 }
 
-angle::Result ContextVk::updateScissorImpl(const gl::State &glState, bool shouldEndRenderPass)
+void ContextVk::updateScissor(const gl::State &glState)
 {
     FramebufferVk *framebufferVk = vk::GetImpl(glState.getDrawFramebuffer());
     gl::Rectangle renderArea     = framebufferVk->getNonRotatedCompleteRenderArea();
@@ -2949,26 +2951,15 @@ angle::Result ContextVk::updateScissorImpl(const gl::State &glState, bool should
     mGraphicsPipelineDesc->updateScissor(&mGraphicsPipelineTransition,
                                          gl_vk::GetRect(rotatedScissoredArea));
 
-    // If the scissor has grown beyond the previous scissoredRenderArea, make sure the render pass
-    // is restarted.  Otherwise, we can continue using the same renderpass area.
-    //
-    // Without a scissor, the render pass area covers the whole of the framebuffer.  With a
-    // scissored clear, the render pass area could be smaller than the framebuffer size.  When the
-    // scissor changes, if the scissor area is completely encompassed by the render pass area, it's
-    // possible to continue using the same render pass.  However, if the current render pass area
-    // is too small, we need to start a new one.  The latter can happen if a scissored clear starts
-    // a render pass, the scissor is disabled and a draw call is issued to affect the whole
-    // framebuffer.
-    gl::Rectangle scissoredRenderArea = framebufferVk->getRotatedScissoredRenderArea(this);
-    if (shouldEndRenderPass && mRenderPassCommands->started())
-    {
-        if (!mRenderPassCommands->getRenderArea().encloses(scissoredRenderArea))
-        {
-            ANGLE_TRY(flushCommandsAndEndRenderPass());
-        }
+    // If the scissor has grown beyond the previous scissoredRenderArea, grow the render pass render
+    // area.  The only undesirable effect this may have is that if the render area does not cover a
+    // previously invalidated area, that invalidate will have to be discarded.
+    if (mRenderPassCommandBuffer &&
+        !mRenderPassCommands->getRenderArea().encloses(rotatedScissoredArea))
+    {
+        ASSERT(mRenderPassCommands->started());
+        mRenderPassCommands->growRenderArea(this, rotatedScissoredArea);
     }
-
-    return angle::Result::Continue;
 }
 
 void ContextVk::invalidateProgramBindingHelper(const gl::State &glState)
@@ -3051,7 +3042,7 @@ angle::Result ContextVk::syncState(const gl::Context *context,
         {
             case gl::State::DIRTY_BIT_SCISSOR_TEST_ENABLED:
             case gl::State::DIRTY_BIT_SCISSOR:
-                ANGLE_TRY(updateScissorAndEndRenderPass(glState));
+                updateScissor(glState);
                 break;
             case gl::State::DIRTY_BIT_VIEWPORT:
             {
@@ -3059,7 +3050,7 @@ angle::Result ContextVk::syncState(const gl::Context *context,
                 updateViewport(framebufferVk, glState.getViewport(), glState.getNearPlane(),
                                glState.getFarPlane(), isViewportFlipEnabledForDrawFBO());
                 // Update the scissor, which will be constrained to the viewport
-                ANGLE_TRY(updateScissorAndEndRenderPass(glState));
+                updateScissor(glState);
                 break;
             }
             case gl::State::DIRTY_BIT_DEPTH_RANGE:
@@ -3226,8 +3217,10 @@ angle::Result ContextVk::syncState(const gl::Context *context,
                 // FramebufferVk::syncState signals that we should start a new command buffer.
                 // But changing the binding can skip FramebufferVk::syncState if the Framebuffer
                 // has no dirty bits. Thus we need to explicitly clear the current command
-                // buffer to ensure we start a new one. Note that we always start a new command
-                // buffer because we currently can only support one open RenderPass at a time.
+                // buffer to ensure we start a new one. We don't actually close the render pass here
+                // as some optimizations in non-draw commands require the render pass to remain
+                // open, such as invalidate or blit. Note that we always start a new command buffer
+                // because we currently can only support one open RenderPass at a time.
                 onRenderPassFinished();
 
                 gl::Framebuffer *drawFramebuffer = glState.getDrawFramebuffer();
@@ -3244,7 +3237,7 @@ angle::Result ContextVk::syncState(const gl::Context *context,
                 mGraphicsPipelineDesc->updateFrontFace(&mGraphicsPipelineTransition,
                                                        glState.getRasterizerState(),
                                                        isViewportFlipEnabledForDrawFBO());
-                ANGLE_TRY(updateScissor(glState));
+                updateScissor(glState);
                 const gl::DepthStencilState depthStencilState = glState.getDepthStencilState();
                 mGraphicsPipelineDesc->updateDepthTestEnabled(&mGraphicsPipelineTransition,
                                                               depthStencilState, drawFramebuffer);
@@ -3630,17 +3623,13 @@ void ContextVk::invalidateDriverUniforms()
     mComputeDirtyBits.set(DIRTY_BIT_DRIVER_UNIFORMS_BINDING);
 }
 
-angle::Result ContextVk::onFramebufferChange(FramebufferVk *framebufferVk)
+void 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)
+    // synced is the read framebuffer (which is not equal the draw framebuffer).
+    if (framebufferVk != vk::GetImpl(mState.getDrawFramebuffer()))
     {
-        return angle::Result::Continue;
+        return;
     }
 
     // Ensure that the pipeline description is updated.
@@ -3652,11 +3641,9 @@ angle::Result ContextVk::onFramebufferChange(FramebufferVk *framebufferVk)
     }
 
     // Update scissor.
-    ANGLE_TRY(updateScissor(mState));
+    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);
-    angle::Result onFramebufferChange(FramebufferVk *framebufferVk);
+    void onFramebufferChange(FramebufferVk *framebufferVk);
     void onDrawFramebufferRenderPassDescChange(FramebufferVk *framebufferVk);
     void onHostVisibleBufferWrite() { mIsAnyHostVisibleBufferWritten = true; }
 
@@ -469,14 +469,7 @@ class ContextVk : public ContextImpl, public vk::Context
     // avoid calling vkAllocateDesctiporSets each texture update.
     const vk::TextureDescriptorDesc &getActiveTexturesDesc() const { return mActiveTexturesDesc; }
 
-    angle::Result updateScissor(const gl::State &glState)
-    {
-        return updateScissorImpl(glState, false);
-    }
-    angle::Result updateScissorAndEndRenderPass(const gl::State &glState)
-    {
-        return updateScissorImpl(glState, true);
-    }
+    void updateScissor(const gl::State &glState);
 
     bool emulateSeamfulCubeMapSampling() const { return mEmulateSeamfulCubeMapSampling; }
 
@@ -992,7 +985,6 @@ class ContextVk : public ContextImpl, public vk::Context
     template <typename T, const T *VkWriteDescriptorSet::*pInfo>
     void growDesciptorCapacity(std::vector<T> *descriptorVector, size_t newSize);
 
-    angle::Result updateScissorImpl(const gl::State &glState, bool shouldEndRenderPass);
     angle::Result updateRenderPassDepthStencilAccess();
     bool shouldSwitchToReadOnlyDepthFeedbackLoopMode(const gl::Context *context,
                                                      gl::Texture *texture) const;

src/libANGLE/renderer/vulkan/FramebufferVk.cpp

@@ -348,7 +348,8 @@ angle::Result FramebufferVk::invalidate(const gl::Context *context,
 {
     ContextVk *contextVk = vk::GetImpl(context);
 
-    ANGLE_TRY(invalidateImpl(contextVk, count, attachments, false));
+    ANGLE_TRY(invalidateImpl(contextVk, count, attachments, false,
+                             getRotatedCompleteRenderArea(contextVk)));
     return angle::Result::Continue;
 }
 
@@ -367,10 +368,12 @@ angle::Result FramebufferVk::invalidateSub(const gl::Context *context,
                     &rotatedInvalidateArea);
 
     // If invalidateSub() covers the whole framebuffer area, make it behave as invalidate().
+    // The invalidate area is clipped to the render area for use inside invalidateImpl.
     const gl::Rectangle completeRenderArea = getRotatedCompleteRenderArea(contextVk);
-    if (rotatedInvalidateArea.encloses(completeRenderArea))
+    if (ClipRectangle(rotatedInvalidateArea, completeRenderArea, &rotatedInvalidateArea) &&
+        rotatedInvalidateArea == completeRenderArea)
     {
-        return invalidateImpl(contextVk, count, attachments, false);
+        return invalidate(context, count, attachments);
     }
 
     // If there are deferred clears, flush them.  syncState may have accumulated deferred clears,
@@ -385,7 +388,7 @@ angle::Result FramebufferVk::invalidateSub(const gl::Context *context,
         // 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
         // the invalidated area).
-        ANGLE_TRY(invalidateImpl(contextVk, count, attachments, true));
+        ANGLE_TRY(invalidateImpl(contextVk, count, attachments, true, rotatedInvalidateArea));
     }
     else
     {
@@ -1403,7 +1406,8 @@ bool FramebufferVk::checkStatus(const gl::Context *context) const
 angle::Result FramebufferVk::invalidateImpl(ContextVk *contextVk,
                                             size_t count,
                                             const GLenum *attachments,
-                                            bool isSubInvalidate)
+                                            bool isSubInvalidate,
+                                            const gl::Rectangle &invalidateArea)
 {
     gl::DrawBufferMask invalidateColorBuffers;
     bool invalidateDepthBuffer   = false;
@@ -1493,13 +1497,13 @@ angle::Result FramebufferVk::invalidateImpl(ContextVk *contextVk,
             if (invalidateDepthBuffer)
             {
                 contextVk->getStartedRenderPassCommands().invalidateRenderPassDepthAttachment(
-                    dsState);
+                    dsState, invalidateArea);
             }
 
             if (invalidateStencilBuffer)
             {
                 contextVk->getStartedRenderPassCommands().invalidateRenderPassStencilAttachment(
-                    dsState);
+                    dsState, invalidateArea);
             }
         }
         if (invalidateColorBuffers.any())
@@ -1780,7 +1784,7 @@ angle::Result FramebufferVk::syncState(const gl::Context *context,
     mFramebuffer = nullptr;
 
     // Notify the ContextVk to update the pipeline desc.
-    ANGLE_TRY(contextVk->onFramebufferChange(this));
+    contextVk->onFramebufferChange(this);
 
     return angle::Result::Continue;
 }
@@ -2150,6 +2154,11 @@ angle::Result FramebufferVk::clearWithCommand(ContextVk *contextVk,
                                               vk::CommandBufferHelper *renderpassCommands,
                                               const gl::Rectangle &scissoredRenderArea)
 {
+    // Clear is not affected by viewport, so ContextVk::updateScissor may have decided on a smaller
+    // render area.  Grow the render area to the full framebuffer size as this clear path is taken
+    // when not scissored.
+    contextVk->getStartedRenderPassCommands().growRenderArea(contextVk, scissoredRenderArea);
+
     gl::AttachmentVector<VkClearAttachment> attachments;
 
     // Go through deferred clears and add them to the list of attachments to clear.

src/libANGLE/renderer/vulkan/FramebufferVk.h

@@ -198,7 +198,8 @@ class FramebufferVk : public FramebufferImpl
     angle::Result invalidateImpl(ContextVk *contextVk,
                                  size_t count,
                                  const GLenum *attachments,
-                                 bool isSubInvalidate);
+                                 bool isSubInvalidate,
+                                 const gl::Rectangle &invalidateArea);
     // Release all FramebufferVk objects in the cache and clear cache
     void clearCache(ContextVk *contextVk);
     angle::Result updateDepthStencilAttachment(const gl::Context *context, bool deferClears);

src/libANGLE/renderer/vulkan/vk_helpers.cpp

@@ -593,6 +593,18 @@ bool IsAnySubresourceContentDefined(const gl::TexLevelArray<angle::BitSet8<8>> &
     }
     return false;
 }
+
+void ExtendRenderPassInvalidateArea(const gl::Rectangle &invalidateArea, gl::Rectangle *out)
+{
+    if (out->empty())
+    {
+        *out = invalidateArea;
+    }
+    else
+    {
+        gl::ExtendRectangle(*out, invalidateArea, out);
+    }
+}
 }  // anonymous namespace
 
 // This is an arbitrary max. We can change this later if necessary.
@@ -902,6 +914,7 @@ void CommandBufferHelper::restoreDepthContent()
         ASSERT(mDepthStencilImage->valid());
         mDepthStencilImage->restoreSubresourceContent(mDepthStencilLevelIndex,
                                                       mDepthStencilLayerIndex);
+        mDepthInvalidateArea = gl::Rectangle();
     }
 }
 
@@ -913,6 +926,7 @@ void CommandBufferHelper::restoreStencilContent()
         ASSERT(mDepthStencilImage->valid());
         mDepthStencilImage->restoreSubresourceStencilContent(mDepthStencilLevelIndex,
                                                              mDepthStencilLayerIndex);
+        mStencilInvalidateArea = gl::Rectangle();
     }
 }
 
@@ -1153,6 +1167,47 @@ void CommandBufferHelper::endTransformFeedback()
     mValidTransformFeedbackBufferCount = 0;
 }
 
+void CommandBufferHelper::invalidateRenderPassColorAttachment(PackedAttachmentIndex attachmentIndex)
+{
+    ASSERT(mIsRenderPassCommandBuffer);
+    SetBitField(mAttachmentOps[attachmentIndex].storeOp, vk::RenderPassStoreOp::DontCare);
+    mAttachmentOps[attachmentIndex].isInvalidated = true;
+}
+
+void CommandBufferHelper::invalidateRenderPassDepthAttachment(const gl::DepthStencilState &dsState,
+                                                              const gl::Rectangle &invalidateArea)
+{
+    ASSERT(mIsRenderPassCommandBuffer);
+    // Keep track of the size of commands in the command buffer.  If the size grows in the
+    // future, that implies that drawing occured since invalidated.
+    mDepthCmdSizeInvalidated = mCommandBuffer.getCommandSize();
+
+    // Also track the size if the attachment is currently disabled.
+    const bool isDepthWriteEnabled = dsState.depthTest && dsState.depthMask;
+    mDepthCmdSizeDisabled = isDepthWriteEnabled ? kInfiniteCmdSize : mDepthCmdSizeInvalidated;
+
+    // Set/extend the invalidate area.
+    ExtendRenderPassInvalidateArea(invalidateArea, &mDepthInvalidateArea);
+}
+
+void CommandBufferHelper::invalidateRenderPassStencilAttachment(
+    const gl::DepthStencilState &dsState,
+    const gl::Rectangle &invalidateArea)
+{
+    ASSERT(mIsRenderPassCommandBuffer);
+    // Keep track of the size of commands in the command buffer.  If the size grows in the
+    // future, that implies that drawing occured since invalidated.
+    mStencilCmdSizeInvalidated = mCommandBuffer.getCommandSize();
+
+    // Also track the size if the attachment is currently disabled.
+    const bool isStencilWriteEnabled =
+        dsState.stencilTest && (!dsState.isStencilNoOp() || !dsState.isStencilBackNoOp());
+    mStencilCmdSizeDisabled = isStencilWriteEnabled ? kInfiniteCmdSize : mStencilCmdSizeInvalidated;
+
+    // Set/extend the invalidate area.
+    ExtendRenderPassInvalidateArea(invalidateArea, &mStencilInvalidateArea);
+}
+
 angle::Result CommandBufferHelper::getRenderPassWithOps(ContextVk *contextVk,
                                                         RenderPass **renderPass)
 {
@@ -1342,6 +1397,8 @@ void CommandBufferHelper::reset()
         mStencilCmdSizeInvalidated         = kInfiniteCmdSize;
         mStencilCmdSizeDisabled            = kInfiniteCmdSize;
         mDepthStencilAttachmentIndex       = kAttachmentIndexInvalid;
+        mDepthInvalidateArea               = gl::Rectangle();
+        mStencilInvalidateArea             = gl::Rectangle();
         mRenderPassUsedImages.clear();
         mDepthStencilImage        = nullptr;
         mDepthStencilResolveImage = nullptr;
@@ -1410,6 +1467,32 @@ void CommandBufferHelper::updateRenderPassDepthStencilClear(VkImageAspectFlags a
     mClearValues.storeNoDepthStencil(mDepthStencilAttachmentIndex, combinedClearValue);
 }
 
+void CommandBufferHelper::growRenderArea(ContextVk *contextVk, const gl::Rectangle &newRenderArea)
+{
+    ASSERT(mIsRenderPassCommandBuffer);
+
+    // The render area is grown such that it covers both the previous and the new render areas.
+    gl::GetEnclosingRectangle(mRenderArea, newRenderArea, &mRenderArea);
+
+    // Remove invalidates that are no longer applicable.
+    if (!mDepthInvalidateArea.empty() && !mDepthInvalidateArea.encloses(mRenderArea))
+    {
+        ANGLE_PERF_WARNING(
+            contextVk->getDebug(), GL_DEBUG_SEVERITY_LOW,
+            "InvalidateSubFramebuffer for depth discarded due to increased scissor region");
+        mDepthInvalidateArea     = gl::Rectangle();
+        mDepthCmdSizeInvalidated = kInfiniteCmdSize;
+    }
+    if (!mStencilInvalidateArea.empty() && !mStencilInvalidateArea.encloses(mRenderArea))
+    {
+        ANGLE_PERF_WARNING(
+            contextVk->getDebug(), GL_DEBUG_SEVERITY_LOW,
+            "InvalidateSubFramebuffer for stencil discarded due to increased scissor region");
+        mStencilInvalidateArea     = gl::Rectangle();
+        mStencilCmdSizeInvalidated = kInfiniteCmdSize;
+    }
+}
+
 // DynamicBuffer implementation.
 DynamicBuffer::DynamicBuffer()
     : mUsage(0),

src/libANGLE/renderer/vulkan/vk_helpers.h

@@ -1017,38 +1017,11 @@ class CommandBufferHelper : angle::NonCopyable
 
     void endTransformFeedback();
 
-    void invalidateRenderPassColorAttachment(PackedAttachmentIndex attachmentIndex)
-    {
-        ASSERT(mIsRenderPassCommandBuffer);
-        SetBitField(mAttachmentOps[attachmentIndex].storeOp, vk::RenderPassStoreOp::DontCare);
-        mAttachmentOps[attachmentIndex].isInvalidated = true;
-    }
-
-    void invalidateRenderPassDepthAttachment(const gl::DepthStencilState &dsState)
-    {
-        ASSERT(mIsRenderPassCommandBuffer);
-        // Keep track of the size of commands in the command buffer.  If the size grows in the
-        // future, that implies that drawing occured since invalidated.
-        mDepthCmdSizeInvalidated = mCommandBuffer.getCommandSize();
-
-        // Also track the size if the attachment is currently disabled.
-        const bool isDepthWriteEnabled = dsState.depthTest && dsState.depthMask;
-        mDepthCmdSizeDisabled = isDepthWriteEnabled ? kInfiniteCmdSize : mDepthCmdSizeInvalidated;
-    }
-
-    void invalidateRenderPassStencilAttachment(const gl::DepthStencilState &dsState)
-    {
-        ASSERT(mIsRenderPassCommandBuffer);
-        // Keep track of the size of commands in the command buffer.  If the size grows in the
-        // future, that implies that drawing occured since invalidated.
-        mStencilCmdSizeInvalidated = mCommandBuffer.getCommandSize();
-
-        // Also track the size if the attachment is currently disabled.
-        const bool isStencilWriteEnabled =
-            dsState.stencilTest && (!dsState.isStencilNoOp() || !dsState.isStencilBackNoOp());
-        mStencilCmdSizeDisabled =
-            isStencilWriteEnabled ? kInfiniteCmdSize : mStencilCmdSizeInvalidated;
-    }
+    void invalidateRenderPassColorAttachment(PackedAttachmentIndex attachmentIndex);
+    void invalidateRenderPassDepthAttachment(const gl::DepthStencilState &dsState,
+                                             const gl::Rectangle &invalidateArea);
+    void invalidateRenderPassStencilAttachment(const gl::DepthStencilState &dsState,
+                                               const gl::Rectangle &invalidateArea);
 
     bool hasWriteAfterInvalidate(uint32_t cmdCountInvalidated, uint32_t cmdCountDisabled)
     {
@@ -1082,6 +1055,10 @@ class CommandBufferHelper : angle::NonCopyable
         return mRenderArea;
     }
 
+    // If render pass is started with a small render area due to a small scissor, and if a new
+    // larger scissor is specified, grow the render area to accomodate it.
+    void growRenderArea(ContextVk *contextVk, const gl::Rectangle &newRenderArea);
+
     void resumeTransformFeedback();
     void pauseTransformFeedback();
     bool isTransformFeedbackStarted() const { return mValidTransformFeedbackBufferCount > 0; }
@@ -1176,6 +1153,8 @@ class CommandBufferHelper : angle::NonCopyable
     uint32_t mDepthCmdSizeDisabled;
     uint32_t mStencilCmdSizeInvalidated;
     uint32_t mStencilCmdSizeDisabled;
+    gl::Rectangle mDepthInvalidateArea;
+    gl::Rectangle mStencilInvalidateArea;
 
     // Keep track of the depth/stencil attachment index
     PackedAttachmentIndex mDepthStencilAttachmentIndex;

src/tests/gl_tests/ClearTest.cpp

@@ -2095,10 +2095,6 @@ TEST_P(ClearTestES3, ClearThenMixedMaskedClear)
 {
     constexpr GLsizei kSize = 16;
 
-    GLint maxDrawBuffers = 0;
-    glGetIntegerv(GL_MAX_DRAW_BUFFERS, &maxDrawBuffers);
-    ASSERT_GE(maxDrawBuffers, 4);
-
     // Setup framebuffer.
     GLRenderbuffer color;
     glBindRenderbuffer(GL_RENDERBUFFER, color);
@@ -2170,10 +2166,6 @@ TEST_P(ClearTestES3, DrawClearThenDrawWithoutStateChange)
     swapBuffers();
     constexpr GLsizei kSize = 16;
 
-    GLint maxDrawBuffers = 0;
-    glGetIntegerv(GL_MAX_DRAW_BUFFERS, &maxDrawBuffers);
-    ASSERT_GE(maxDrawBuffers, 4);
-
     // Setup framebuffer.
     GLRenderbuffer color;
     glBindRenderbuffer(GL_RENDERBUFFER, color);
@@ -2353,6 +2345,54 @@ TEST_P(ClearTestES3, ClearBufferfiNoStencilAttachment)
     verifyDepth(0.5f, kSize);
 }
 
+// Test that scissored clear followed by non-scissored draw works.  Ensures that when scissor size
+// is expanded, the clear operation remains limited to the scissor region.  Written to catch
+// potential future bugs if loadOp=CLEAR is used in the Vulkan backend for a small render pass and
+// then the render area is mistakenly enlarged.
+TEST_P(ClearTest, ScissoredClearThenNonScissoredDraw)
+{
+    constexpr GLsizei kSize = 16;
+    const std::vector<GLColor> kInitialData(kSize * kSize, GLColor::red);
+
+    // Setup framebuffer.  Initialize color with red.
+    GLTexture color;
+    glBindTexture(GL_TEXTURE_2D, color);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, kSize, kSize, 0, GL_RGBA, GL_UNSIGNED_BYTE,
+                 kInitialData.data());
+
+    GLFramebuffer fb;
+    glBindFramebuffer(GL_FRAMEBUFFER, fb);
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, color, 0);
+    EXPECT_GL_NO_ERROR();
+    ASSERT_GL_FRAMEBUFFER_COMPLETE(GL_FRAMEBUFFER);
+
+    // Issue a scissored clear to green.
+    glClearColor(0.0f, 1.0f, 0.0f, 1.0f);
+    glScissor(kSize / 2, 0, kSize / 2, kSize);
+    glEnable(GL_SCISSOR_TEST);
+    glClear(GL_COLOR_BUFFER_BIT);
+
+    // Expand the scissor and blend blue into the framebuffer.
+    glScissor(0, 0, kSize, kSize);
+    glEnable(GL_BLEND);
+    glBlendFunc(GL_ONE, GL_ONE);
+
+    ANGLE_GL_PROGRAM(drawBlue, essl1_shaders::vs::Simple(), essl1_shaders::fs::Blue());
+    drawQuad(drawBlue, essl1_shaders::PositionAttrib(), 0.5f);
+    ASSERT_GL_NO_ERROR();
+
+    // Verify that the left half is magenta, and the right half is cyan.
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::magenta);
+    EXPECT_PIXEL_COLOR_EQ(kSize / 2 - 1, 0, GLColor::magenta);
+    EXPECT_PIXEL_COLOR_EQ(0, kSize - 1, GLColor::magenta);
+    EXPECT_PIXEL_COLOR_EQ(kSize / 2 - 1, kSize - 1, GLColor::magenta);
+
+    EXPECT_PIXEL_COLOR_EQ(kSize / 2, 0, GLColor::cyan);
+    EXPECT_PIXEL_COLOR_EQ(kSize / 2, kSize - 1, GLColor::cyan);
+    EXPECT_PIXEL_COLOR_EQ(kSize - 1, 0, GLColor::cyan);
+    EXPECT_PIXEL_COLOR_EQ(kSize - 1, kSize - 1, GLColor::cyan);
+}
+
 #ifdef Bool
 // X11 craziness.
 #    undef Bool

src/tests/gl_tests/FramebufferTest.cpp

@@ -395,6 +395,8 @@ class FramebufferTest_ES3 : public ANGLETest
         setConfigGreenBits(8);
         setConfigBlueBits(8);
         setConfigAlphaBits(8);
+        setConfigDepthBits(24);
+        setConfigStencilBits(8);
     }
 
     static constexpr GLsizei kWidth  = 64;
@@ -475,6 +477,71 @@ TEST_P(FramebufferTest_ES3, SubInvalidatePartial)
     EXPECT_PIXEL_COLOR_EQ(kWidth - 1, kHeight - 1, GLColor::red);
 }
 
+// Test that a scissored draw followed by subinvalidate followed by a non-scissored draw retains the
+// part that is not invalidated.  Uses swapped width/height for invalidate which results in a
+// partial invalidate, but also prevents bugs with Vulkan pre-rotation.
+TEST_P(FramebufferTest_ES3, ScissoredDrawSubInvalidateThenNonScissoredDraw)
+{
+    swapBuffers();
+    glBindFramebuffer(GL_FRAMEBUFFER, 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);
+
+    // Clear color to red and the depth/stencil buffer to 1.0 and 0x55
+    glClearColor(1.0f, 0.0f, 0.0f, 1.0f);
+    glClearDepthf(1);
+    glClearStencil(0x55);
+    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT);
+    EXPECT_GL_NO_ERROR();
+
+    // Break rendering so the following draw call starts rendering with a scissored area.
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
+
+    // Issue a scissored draw call that changes depth to 0.5 and stencil 0x3C
+    glScissor(0, 0, kHeight, kWidth);
+    glEnable(GL_SCISSOR_TEST);
+
+    glEnable(GL_DEPTH_TEST);
+    glDepthFunc(GL_ALWAYS);
+
+    glEnable(GL_STENCIL_TEST);
+    glStencilFunc(GL_ALWAYS, 0x3C, 0xFF);
+    glStencilOp(GL_REPLACE, GL_REPLACE, GL_REPLACE);
+    glStencilMask(0xFF);
+
+    glUniform4f(colorUniformLocation, 0.0f, 1.0f, 0.0f, 1.0f);
+    drawQuad(drawColor, essl1_shaders::PositionAttrib(), 0);
+
+    // Invalidate the draw region (half of the framebuffer using swapped dimensions).
+    std::array<GLenum, 3> attachments = {GL_COLOR, GL_DEPTH, GL_STENCIL};
+    glInvalidateSubFramebuffer(GL_DRAW_FRAMEBUFFER, 3, attachments.data(), 0, 0, kHeight, kWidth);
+    EXPECT_GL_NO_ERROR();
+
+    // Match the scissor to the framebuffer size and issue a draw call that blends blue, and expects
+    // depth to be 1 and stencil to be 0x55.  This is only valid for the half that was not
+    // invalidated.
+    glScissor(0, 0, kWidth, kHeight);
+    glDepthFunc(GL_LESS);
+    glStencilFunc(GL_EQUAL, 0x55, 0xFF);
+    glStencilOp(GL_KEEP, GL_KEEP, GL_KEEP);
+
+    glEnable(GL_BLEND);
+    glBlendFunc(GL_ONE, GL_ONE);
+    glUniform4f(colorUniformLocation, 0.0f, 0.0f, 1.0f, 1.0f);
+    drawQuad(drawColor, essl1_shaders::PositionAttrib(), 0.95f);
+    ASSERT_GL_NO_ERROR();
+
+    // Make sure the half that was not invalidated is correct.
+    EXPECT_PIXEL_COLOR_EQ(0, kWidth, GLColor::magenta);
+    EXPECT_PIXEL_COLOR_EQ(kWidth - 1, kWidth, GLColor::magenta);
+    EXPECT_PIXEL_COLOR_EQ(0, kHeight - 1, GLColor::magenta);
+    EXPECT_PIXEL_COLOR_EQ(kWidth - 1, kHeight - 1, GLColor::magenta);
+}
+
 // 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)