Vulkan: Simplify viewport / scissor updates

src/libANGLE/angletypes.h

@@ -45,7 +45,7 @@ PrimitiveType GetPrimitiveType(GLenum drawMode);
 struct Rectangle
 {
     Rectangle() : x(0), y(0), width(0), height(0) {}
-    Rectangle(int x_in, int y_in, int width_in, int height_in)
+    constexpr Rectangle(int x_in, int y_in, int width_in, int height_in)
         : x(x_in), y(y_in), width(width_in), height(height_in)
     {
     }

src/libANGLE/renderer/vulkan/CommandGraph.cpp

@@ -396,6 +396,11 @@ Error CommandGraphNode::visitAndExecute(VkDevice device,
     return NoError();
 }
 
+const gl::Rectangle &CommandGraphNode::getRenderPassRenderArea() const
+{
+    return mRenderPassRenderArea;
+}
+
 // CommandGraph implementation.
 CommandGraph::CommandGraph()
 {

src/libANGLE/renderer/vulkan/CommandGraph.h

@@ -132,6 +132,8 @@ class CommandGraphNode final : angle::NonCopyable
                           RenderPassCache *renderPassCache,
                           CommandBuffer *primaryCommandBuffer);
 
+    const gl::Rectangle &getRenderPassRenderArea() const;
+
   private:
     void setHasChildren();
 

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -37,6 +37,16 @@
 
 namespace rx
 {
+
+namespace
+{
+constexpr gl::Rectangle kMaxSizedScissor(0,
+                                         0,
+                                         std::numeric_limits<int>::max(),
+                                         std::numeric_limits<int>::max());
+
+}  // anonymous namespace
+
 ContextVk::ContextVk(const gl::ContextState &state, RendererVk *renderer)
     : ContextImpl(state),
       mRenderer(renderer),
@@ -344,15 +354,14 @@ void ContextVk::updateScissor(const gl::State &glState)
 {
     if (glState.isScissorTestEnabled())
     {
-        mPipelineDesc->updateScissor(glState.getScissor(),
-                                     glState.getDrawFramebuffer()->getDimensions());
+        mPipelineDesc->updateScissor(glState.getScissor());
     }
     else
     {
-        // If the scissor test isn't enabled, we have to also update the scissor to
-        // be equal to the framebuffer dimensions to make sure we keep rendering everything.
-        mPipelineDesc->updateScissor(glState.getViewport(),
-                                     glState.getDrawFramebuffer()->getDimensions());
+        // If the scissor test isn't enabled, we can simply use a really big scissor that's
+        // certainly larger than the current surface using the maximum size of a 2D texture
+        // for the width and height.
+        mPipelineDesc->updateScissor(kMaxSizedScissor);
     }
 }
 
@@ -373,16 +382,8 @@ void ContextVk::syncState(const gl::Context *context, const gl::State::DirtyBits
         switch (dirtyBit)
         {
             case gl::State::DIRTY_BIT_SCISSOR_TEST_ENABLED:
-                updateScissor(glState);
-                break;
             case gl::State::DIRTY_BIT_SCISSOR:
-                // Only modify the scissor region if the test is enabled, otherwise we want to keep
-                // the viewport size as the scissor region.
-                if (glState.isScissorTestEnabled())
-                {
-                    mPipelineDesc->updateScissor(glState.getScissor(),
-                                                 glState.getDrawFramebuffer()->getDimensions());
-                }
+                updateScissor(glState);
                 break;
             case gl::State::DIRTY_BIT_VIEWPORT:
                 mPipelineDesc->updateViewport(glState.getViewport(), glState.getNearPlane(),

src/libANGLE/renderer/vulkan/FramebufferVk.cpp

@@ -507,7 +507,19 @@ gl::Error FramebufferVk::clearAttachmentsWithScissorRegion(const gl::Context *co
     VkClearRect clearRect;
     clearRect.baseArrayLayer = 0;
     clearRect.layerCount     = 1;
-    clearRect.rect           = contextVk->getScissor();
+
+    // When clearing, the scissor region must be clipped to the renderArea per the validation rules
+    // in Vulkan.
+    gl::Rectangle intersection;
+    if (ClipRectangle(contextVk->getGLState().getScissor(), node->getRenderPassRenderArea(),
+                      &intersection))
+    {
+        clearRect.rect = gl_vk::GetRect(intersection);
+    }
+    else
+    {
+        clearRect.rect = gl_vk::GetRect(contextVk->getGLState().getScissor());
+    }
 
     commandBuffer->clearAttachments(static_cast<uint32_t>(clearAttachmentIndex),
                                     clearAttachments.data(), 1, &clearRect);

src/libANGLE/renderer/vulkan/vk_cache_utils.cpp

@@ -649,11 +649,6 @@ void PipelineDesc::updateViewport(const gl::Rectangle &viewport, float nearPlane
     mViewport.height   = static_cast<float>(viewport.height);
     mViewport.minDepth = nearPlane;
     mViewport.maxDepth = farPlane;
-
-    mScissor.offset.x      = viewport.x;
-    mScissor.offset.y      = viewport.y;
-    mScissor.extent.width  = viewport.width;
-    mScissor.extent.height = viewport.height;
 }
 
 void PipelineDesc::updateVertexInputInfo(const VertexInputBindings &bindings,
@@ -808,22 +803,9 @@ void PipelineDesc::updateRenderPassDesc(const RenderPassDesc &renderPassDesc)
     mRenderPassDesc = renderPassDesc;
 }
 
-void PipelineDesc::updateScissor(const gl::Rectangle &rect, gl::Box framebufferDimensions)
+void PipelineDesc::updateScissor(const gl::Rectangle &rect)
 {
-    gl::Rectangle intersection;
-    gl::Rectangle clipRect(static_cast<GLuint>(0), static_cast<GLuint>(0),
-                           static_cast<GLuint>(framebufferDimensions.width),
-                           static_cast<GLuint>(framebufferDimensions.height));
-    // Coordinates outside the framebuffer aren't valid in Vulkan but not error is returned, the
-    // scissor is just ignored.
-    if (ClipRectangle(rect, clipRect, &intersection))
-    {
-        mScissor = gl_vk::GetRect(intersection);
-    }
-    else
-    {
-        mScissor = gl_vk::GetRect(rect);
-    }
+    mScissor = gl_vk::GetRect(rect);
 }
 
 // AttachmentOpsArray implementation.

src/libANGLE/renderer/vulkan/vk_cache_utils.h

@@ -292,7 +292,7 @@ class PipelineDesc final
 
     // Scissor support
     const VkRect2D &getScissor() const { return mScissor; }
-    void updateScissor(const gl::Rectangle &rect, gl::Box surfaceDimensions);
+    void updateScissor(const gl::Rectangle &rect);
 
     // Blend states
     void updateBlendEnabled(bool isBlendEnabled);

src/tests/gl_tests/StateChangeTest.cpp

@@ -1490,6 +1490,89 @@ TEST_P(SimpleStateChangeTest, RedefineRenderbufferInUse)
     EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
 }
 
+// Validate that we can draw -> change frame buffer size -> draw and we'll be rendering
+// at the full size of the new framebuffer.
+TEST_P(SimpleStateChangeTest, ChangeFramebufferSizeBetweenTwoDraws)
+{
+    constexpr char vertexShader[] =
+        R"(attribute vec2 position;
+void main()
+{
+    gl_Position = vec4(position, 0, 1);
+})";
+
+    constexpr char fragShader[] = R"(precision mediump float;
+uniform vec4 color;
+void main() {
+    gl_FragColor = color;
+})";
+
+    constexpr size_t kSmallTextureSize = 2;
+    constexpr size_t kBigTextureSize   = 4;
+
+    // Create 2 textures, one of 2x2 and the other 4x4
+    GLTexture texture1;
+    glBindTexture(GL_TEXTURE_2D, texture1);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, kSmallTextureSize, kSmallTextureSize, 0, GL_RGBA,
+                 GL_UNSIGNED_BYTE, nullptr);
+    ASSERT_GL_NO_ERROR();
+
+    GLTexture texture2;
+    glBindTexture(GL_TEXTURE_2D, texture2);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, kBigTextureSize, kBigTextureSize, 0, GL_RGBA,
+                 GL_UNSIGNED_BYTE, nullptr);
+    ASSERT_GL_NO_ERROR();
+
+    // A framebuffer for each texture to draw on.
+    GLFramebuffer framebuffer1;
+    glBindFramebuffer(GL_FRAMEBUFFER, framebuffer1);
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, texture1, 0);
+    ASSERT_GL_NO_ERROR();
+    ASSERT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, glCheckFramebufferStatus(GL_FRAMEBUFFER));
+
+    GLFramebuffer framebuffer2;
+    glBindFramebuffer(GL_FRAMEBUFFER, framebuffer2);
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, texture2, 0);
+    ASSERT_GL_NO_ERROR();
+    ASSERT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, glCheckFramebufferStatus(GL_FRAMEBUFFER));
+
+    ANGLE_GL_PROGRAM(program, vertexShader, fragShader);
+    glUseProgram(program);
+    GLint uniformLocation = glGetUniformLocation(program, "color");
+    ASSERT_NE(uniformLocation, -1);
+
+    // Bind to the first framebuffer for drawing.
+    glBindFramebuffer(GL_FRAMEBUFFER, framebuffer1);
+
+    // Set a scissor, that will trigger setting the internal scissor state in Vulkan to
+    // (0,0,framebuffer.width, framebuffer.height) size since the scissor isn't enabled.
+    glScissor(0, 0, 16, 16);
+    ASSERT_GL_NO_ERROR();
+
+    // Set color to red.
+    glUniform4f(uniformLocation, 1.0f, 0.0f, 0.0f, 1.0f);
+    glViewport(0, 0, kSmallTextureSize, kSmallTextureSize);
+
+    // Draw a full sized red quad
+    drawQuad(program, "position", 1.0f, 1.0f, true);
+    ASSERT_GL_NO_ERROR();
+
+    // Bind to the second (bigger) framebuffer
+    glBindFramebuffer(GL_FRAMEBUFFER, framebuffer2);
+    glViewport(0, 0, kBigTextureSize, kBigTextureSize);
+
+    ASSERT_GL_NO_ERROR();
+
+    // Set color to green.
+    glUniform4f(uniformLocation, 0.0f, 1.0f, 0.0f, 1.0f);
+
+    // Draw again and we should fill everything with green and expect everything to be green.
+    drawQuad(program, "position", 1.0f, 1.0f, true);
+    ASSERT_GL_NO_ERROR();
+
+    EXPECT_PIXEL_RECT_EQ(0, 0, kBigTextureSize, kBigTextureSize, GLColor::green);
+}
+
 }  // anonymous namespace
 
 ANGLE_INSTANTIATE_TEST(StateChangeTest, ES2_D3D9(), ES2_D3D11(), ES2_OPENGL());