Vulkan: Fix query pause on framebuffer binding change

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -317,6 +317,13 @@ ANGLE_INLINE void ContextVk::flushDescriptorSetUpdates()
     mDescriptorImageInfos.clear();
 }
 
+ANGLE_INLINE void ContextVk::onRenderPassFinished()
+{
+    pauseRenderPassQueriesIfActive();
+
+    mRenderPassCommandBuffer = nullptr;
+}
+
 // ContextVk::ScopedDescriptorSetUpdates implementation.
 class ContextVk::ScopedDescriptorSetUpdates final : angle::NonCopyable
 {
@@ -4593,8 +4600,6 @@ angle::Result ContextVk::flushCommandsAndEndRenderPass()
         return angle::Result::Continue;
     }
 
-    pauseRenderPassQueriesIfActive();
-
     mCurrentTransformFeedbackBuffers.clear();
     mCurrentIndirectBuffer = nullptr;
 

src/libANGLE/renderer/vulkan/ContextVk.h

@@ -839,7 +839,7 @@ class ContextVk : public ContextImpl, public vk::Context, public MultisampleText
     angle::Result flushOutsideRenderPassCommands();
     void flushDescriptorSetUpdates();
 
-    ANGLE_INLINE void onRenderPassFinished() { mRenderPassCommandBuffer = nullptr; }
+    void onRenderPassFinished();
 
     void initIndexTypeMap();
 

src/tests/gl_tests/OcclusionQueriesTest.cpp

@@ -60,8 +60,7 @@ TEST_P(OcclusionQueriesTest, IsOccluded)
 
     EXPECT_GL_NO_ERROR();
 
-    GLuint query = 0;
-    glGenQueriesEXT(1, &query);
+    GLQueryEXT query;
     glBeginQueryEXT(GL_ANY_SAMPLES_PASSED_EXT, query);
     drawQuad(mProgram, essl1_shaders::PositionAttrib(),
              0.8f);  // this quad should be occluded by first quad
@@ -83,8 +82,6 @@ TEST_P(OcclusionQueriesTest, IsOccluded)
 
     EXPECT_GL_NO_ERROR();
 
-    glDeleteQueriesEXT(1, &query);
-
     EXPECT_GL_FALSE(result);
 }
 
@@ -105,8 +102,7 @@ TEST_P(OcclusionQueriesTest, IsNotOccluded)
 
     EXPECT_GL_NO_ERROR();
 
-    GLuint query = 0;
-    glGenQueriesEXT(1, &query);
+    GLQueryEXT query;
     glBeginQueryEXT(GL_ANY_SAMPLES_PASSED_EXT, query);
     drawQuad(mProgram, essl1_shaders::PositionAttrib(), 0.8f);  // this quad should not be occluded
     glEndQueryEXT(GL_ANY_SAMPLES_PASSED_EXT);
@@ -120,8 +116,6 @@ TEST_P(OcclusionQueriesTest, IsNotOccluded)
 
     EXPECT_GL_NO_ERROR();
 
-    glDeleteQueriesEXT(1, &query);
-
     EXPECT_GL_TRUE(result);
 }
 
@@ -146,8 +140,7 @@ TEST_P(OcclusionQueriesTest, ClearNotCounted)
 
     EXPECT_GL_NO_ERROR();
 
-    GLuint query[2] = {0};
-    glGenQueriesEXT(2, query);
+    GLQueryEXT query[2];
 
     // First query
     glBeginQueryEXT(GL_ANY_SAMPLES_PASSED_EXT, query[0]);
@@ -198,8 +191,6 @@ TEST_P(OcclusionQueriesTest, ClearNotCounted)
                            &result[1]);  // will block waiting for result
     EXPECT_GL_NO_ERROR();
 
-    glDeleteQueriesEXT(2, query);
-
     EXPECT_GL_FALSE(result[0]);
     EXPECT_GL_TRUE(result[1]);
 }
@@ -213,8 +204,7 @@ TEST_P(OcclusionQueriesTest, MaskedClearNotCounted)
     // http://anglebug.com/4925
     ANGLE_SKIP_TEST_IF(IsD3D());
 
-    GLuint query = 0;
-    glGenQueriesEXT(1, &query);
+    GLQueryEXT query;
 
     // Masked clear
     glBeginQueryEXT(GL_ANY_SAMPLES_PASSED_EXT, query);
@@ -230,8 +220,6 @@ TEST_P(OcclusionQueriesTest, MaskedClearNotCounted)
                            &result);  // will block waiting for result
     EXPECT_GL_NO_ERROR();
 
-    glDeleteQueriesEXT(1, &query);
-
     EXPECT_GL_FALSE(result);
 }
 
@@ -247,8 +235,7 @@ TEST_P(OcclusionQueriesTest, CopyNotCounted)
     // http://anglebug.com/5100
     ANGLE_SKIP_TEST_IF(IsMetal() && IsNVIDIA());
 
-    GLuint query = 0;
-    glGenQueriesEXT(1, &query);
+    GLQueryEXT query;
 
     // Unrelated draw before the query starts.
     drawQuad(mProgram, essl1_shaders::PositionAttrib(), 0.8f, 0.5f);
@@ -268,8 +255,6 @@ TEST_P(OcclusionQueriesTest, CopyNotCounted)
                            &result);  // will block waiting for result
     EXPECT_GL_NO_ERROR();
 
-    glDeleteQueriesEXT(1, &query);
-
     EXPECT_GL_FALSE(result);
 }
 
@@ -282,6 +267,9 @@ TEST_P(OcclusionQueriesTestES3, BlitNotCounted)
     // http://anglebug.com/5101
     ANGLE_SKIP_TEST_IF(IsWindows() && IsAMD() && IsVulkan());
 
+    // http://anglebug.com/5443
+    ANGLE_SKIP_TEST_IF(IsOSX() && IsMetal() && IsNVIDIA());
+
     constexpr GLuint kSize = 64;
 
     GLFramebuffer srcFbo;
@@ -300,8 +288,7 @@ TEST_P(OcclusionQueriesTestES3, BlitNotCounted)
     glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, kSize, kSize, 0, GL_RGB, GL_UNSIGNED_BYTE, nullptr);
     glFramebufferTexture2D(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, dstTex, 0);
 
-    GLuint query = 0;
-    glGenQueriesEXT(1, &query);
+    GLQueryEXT query;
 
     // Unrelated draw before the query starts.
     drawQuad(mProgram, essl1_shaders::PositionAttrib(), 0.8f, 0.5f);
@@ -319,8 +306,6 @@ TEST_P(OcclusionQueriesTestES3, BlitNotCounted)
                            &result);  // will block waiting for result
     EXPECT_GL_NO_ERROR();
 
-    glDeleteQueriesEXT(1, &query);
-
     EXPECT_GL_FALSE(result);
 }
 
@@ -360,8 +345,7 @@ TEST_P(OcclusionQueriesTestES3, UnresolveNotCounted)
     glBindTexture(GL_TEXTURE_2D, texture);
     glCopyTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 0, 0, kSize, kSize, 0);
 
-    GLuint query = 0;
-    glGenQueriesEXT(1, &query);
+    GLQueryEXT query;
 
     // Make a draw call that will fail the depth test, and therefore shouldn't contribute to
     // occlusion query.
@@ -379,11 +363,61 @@ TEST_P(OcclusionQueriesTestES3, UnresolveNotCounted)
                            &result);  // will block waiting for result
     EXPECT_GL_NO_ERROR();
 
-    glDeleteQueriesEXT(1, &query);
-
     EXPECT_GL_FALSE(result);
 }
 
+// Test that changing framebuffers work
+TEST_P(OcclusionQueriesTest, FramebufferBindingChange)
+{
+    ANGLE_SKIP_TEST_IF(getClientMajorVersion() < 3 &&
+                       !IsGLExtensionEnabled("GL_EXT_occlusion_query_boolean"));
+
+    constexpr GLsizei kSize = 4;
+
+    // Create two framebuffers, and make sure they are synced.
+    GLFramebuffer fbo[2];
+    GLTexture color[2];
+
+    for (size_t index = 0; index < 2; ++index)
+    {
+        glBindTexture(GL_TEXTURE_2D, color[index]);
+        glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, kSize, kSize, 0, GL_RGBA, GL_UNSIGNED_BYTE,
+                     nullptr);
+
+        glBindFramebuffer(GL_FRAMEBUFFER, fbo[index]);
+        glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, color[index],
+                               0);
+
+        glClearColor(0, index, 1 - index, 1);
+        glClear(GL_COLOR_BUFFER_BIT);
+
+        EXPECT_PIXEL_COLOR_EQ(0, 0, index ? GLColor::green : GLColor::blue);
+    }
+    EXPECT_GL_NO_ERROR();
+
+    glViewport(0, 0, kSize, kSize);
+
+    // Start an occlusion query and issue a draw call to each framebuffer.
+    GLQueryEXT query;
+
+    glBeginQueryEXT(GL_ANY_SAMPLES_PASSED_EXT, query);
+
+    for (size_t index = 0; index < 2; ++index)
+    {
+        glBindFramebuffer(GL_FRAMEBUFFER, fbo[index]);
+        drawQuad(mProgram, essl1_shaders::PositionAttrib(), 0.5f);
+    }
+
+    glEndQueryEXT(GL_ANY_SAMPLES_PASSED_EXT);
+    EXPECT_GL_NO_ERROR();
+
+    GLuint result = GL_FALSE;
+    glGetQueryObjectuivEXT(query, GL_QUERY_RESULT_EXT, &result);
+    EXPECT_GL_NO_ERROR();
+
+    EXPECT_GL_TRUE(result);
+}
+
 // Test multiple occlusion queries.
 TEST_P(OcclusionQueriesTest, MultiQueries)
 {
@@ -405,8 +439,7 @@ TEST_P(OcclusionQueriesTest, MultiQueries)
     ANGLE_SKIP_TEST_IF(IsOSX() && IsARM64() && IsDesktopOpenGL());
     ANGLE_SKIP_TEST_IF(IsOSX() && IsARM64() && IsMetal());
 
-    GLuint query[5] = {};
-    glGenQueriesEXT(5, query);
+    GLQueryEXT query[5];
 
     // First query
     glBeginQueryEXT(GL_ANY_SAMPLES_PASSED_EXT, query[0]);
@@ -504,8 +537,6 @@ TEST_P(OcclusionQueriesTest, MultiQueries)
                            &result);  // will block waiting for result
     EXPECT_GL_NO_ERROR();
     EXPECT_GL_TRUE(result);
-
-    glDeleteQueriesEXT(5, query);
 }
 
 TEST_P(OcclusionQueriesTest, Errors)

src/tests/gl_tests/TransformFeedbackTest.cpp

@@ -82,6 +82,8 @@ class TransformFeedbackTest : public TransformFeedbackTestBase
     }
 
     void setupOverrunTest(const std::vector<GLfloat> &vertices);
+
+    void midRecordOpDoesNotContributeTest(std::function<void()> op);
 };
 
 TEST_P(TransformFeedbackTest, ZeroSizedViewport)
@@ -374,18 +376,8 @@ TEST_P(TransformFeedbackTest, SpanMultipleRenderPasses)
     EXPECT_GL_NO_ERROR();
 }
 
-// Test that draw-based clear between draws does not contribute to transform feedback.
-TEST_P(TransformFeedbackTest, ClearWhileRecordingDoesNotContribute)
+void TransformFeedbackTest::midRecordOpDoesNotContributeTest(std::function<void()> op)
 {
-    // TODO(anglebug.com/4533) This fails after the upgrade to the 26.20.100.7870 driver.
-    ANGLE_SKIP_TEST_IF(IsWindows() && IsIntel() && IsVulkan());
-
-    // Fails on Mac GL drivers. http://anglebug.com/4992
-    ANGLE_SKIP_TEST_IF(IsOpenGL() && IsOSX());
-
-    // anglebug.com/5434
-    ANGLE_SKIP_TEST_IF(IsAndroid() && IsOpenGLES());
-
     glClearColor(0.0f, 0.0f, 0.0f, 1.0f);
     glClear(GL_COLOR_BUFFER_BIT);
 
@@ -417,10 +409,8 @@ TEST_P(TransformFeedbackTest, ClearWhileRecordingDoesNotContribute)
     // Draw the first set of three points
     glDrawArrays(GL_POINTS, 0, 3);
 
-    glColorMask(GL_FALSE, GL_FALSE, GL_TRUE, GL_FALSE);
-    glClearColor(1.0f, 1.0f, 1.0f, 1.0f);
-    glClear(GL_COLOR_BUFFER_BIT);
-    glColorMask(GL_TRUE, GL_TRUE, GL_FALSE, GL_TRUE);
+    // Perform the operation in the middle of recording
+    op();
 
     // Draw the second set of three points
     glVertexAttribPointer(positionLocation, 3, GL_FLOAT, GL_FALSE, 0, vertices + 9);
@@ -442,7 +432,6 @@ TEST_P(TransformFeedbackTest, ClearWhileRecordingDoesNotContribute)
     EXPECT_EQ(6u, primitivesWritten);
 
     // Verify the captured buffer.
-
     glBindBuffer(GL_ARRAY_BUFFER, mTransformFeedbackBuffer);
     glVertexAttribPointer(positionLocation, 4, GL_FLOAT, GL_FALSE, 0, 0);
     glEnableVertexAttribArray(positionLocation);
@@ -450,6 +439,28 @@ TEST_P(TransformFeedbackTest, ClearWhileRecordingDoesNotContribute)
     glClearColor(0.0f, 0.0f, 0.0f, 1.0f);
     glClear(GL_COLOR_BUFFER_BIT);
     glDrawArrays(GL_TRIANGLES, 0, 6);
+}
+
+// Test that draw-based clear between draws does not contribute to transform feedback.
+TEST_P(TransformFeedbackTest, ClearWhileRecordingDoesNotContribute)
+{
+    // TODO(anglebug.com/4533) This fails after the upgrade to the 26.20.100.7870 driver.
+    ANGLE_SKIP_TEST_IF(IsWindows() && IsIntel() && IsVulkan());
+
+    // Fails on Mac GL drivers. http://anglebug.com/4992
+    ANGLE_SKIP_TEST_IF(IsOpenGL() && IsOSX());
+
+    // anglebug.com/5434
+    ANGLE_SKIP_TEST_IF(IsAndroid() && IsOpenGLES());
+
+    auto clear = []() {
+        glColorMask(GL_FALSE, GL_FALSE, GL_TRUE, GL_FALSE);
+        glClearColor(1.0f, 1.0f, 1.0f, 1.0f);
+        glClear(GL_COLOR_BUFFER_BIT);
+        glColorMask(GL_TRUE, GL_TRUE, GL_FALSE, GL_TRUE);
+    };
+
+    midRecordOpDoesNotContributeTest(clear);
 
     const int w = getWindowWidth();
     const int h = getWindowHeight();
@@ -469,6 +480,89 @@ TEST_P(TransformFeedbackTest, ClearWhileRecordingDoesNotContribute)
     EXPECT_GL_NO_ERROR();
 }
 
+// Test that copy in the middle of rendering doesn't contribute to transform feedback.
+TEST_P(TransformFeedbackTest, CopyWhileRecordingDoesNotContribute)
+{
+    // TODO(anglebug.com/4533) This fails after the upgrade to the 26.20.100.7870 driver.
+    ANGLE_SKIP_TEST_IF(IsWindows() && IsIntel() && IsVulkan());
+
+    // Fails on Mac GL drivers. http://anglebug.com/4992
+    ANGLE_SKIP_TEST_IF(IsOpenGL() && IsOSX());
+
+    // anglebug.com/5434
+    ANGLE_SKIP_TEST_IF(IsAndroid() && IsOpenGLES());
+
+    auto copy = []() {
+        GLTexture texture;
+        glCopyTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 0, 0, 1, 1, 0);
+    };
+
+    midRecordOpDoesNotContributeTest(copy);
+
+    const int w = getWindowWidth();
+    const int h = getWindowHeight();
+
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::black);
+    EXPECT_PIXEL_COLOR_EQ(w - 1, 0, GLColor::black);
+    EXPECT_PIXEL_COLOR_EQ(0, h - 1, GLColor::black);
+    EXPECT_PIXEL_COLOR_EQ(w - 1, h - 1, GLColor::black);
+
+    EXPECT_PIXEL_COLOR_EQ(w / 4 + 1, h / 4 + 1, GLColor::red);
+    EXPECT_PIXEL_COLOR_EQ(3 * w / 4 - 1, h / 4 + 1, GLColor::red);
+    EXPECT_PIXEL_COLOR_EQ(w / 4 + 1, 3 * h / 4 - 1, GLColor::red);
+    EXPECT_PIXEL_COLOR_EQ(3 * w / 4 - 1, 3 * h / 4 - 1, GLColor::red);
+
+    EXPECT_PIXEL_COLOR_EQ(w / 2, h / 2, GLColor::red);
+
+    EXPECT_GL_NO_ERROR();
+}
+
+// Test that blit in the middle of rendering doesn't contribute to transform feedback.
+TEST_P(TransformFeedbackTest, BlitWhileRecordingDoesNotContribute)
+{
+    // TODO(anglebug.com/4533) This fails after the upgrade to the 26.20.100.7870 driver.
+    ANGLE_SKIP_TEST_IF(IsWindows() && IsIntel() && IsVulkan());
+
+    // Fails on Mac GL drivers. http://anglebug.com/4992
+    ANGLE_SKIP_TEST_IF(IsOpenGL() && IsOSX());
+
+    // anglebug.com/5434
+    ANGLE_SKIP_TEST_IF(IsAndroid() && IsOpenGLES());
+
+    auto blit = []() {
+        GLFramebuffer dstFbo;
+        glBindFramebuffer(GL_DRAW_FRAMEBUFFER, dstFbo);
+
+        GLTexture dstTex;
+        glBindTexture(GL_TEXTURE_2D, dstTex);
+        glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, 1, 1, 0, GL_RGB, GL_UNSIGNED_BYTE, nullptr);
+        glFramebufferTexture2D(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, dstTex, 0);
+
+        glBlitFramebuffer(0, 0, 1, 1, 1, 1, 0, 0, GL_COLOR_BUFFER_BIT, GL_LINEAR);
+
+        glBindFramebuffer(GL_DRAW_FRAMEBUFFER, 0);
+    };
+
+    midRecordOpDoesNotContributeTest(blit);
+
+    const int w = getWindowWidth();
+    const int h = getWindowHeight();
+
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::black);
+    EXPECT_PIXEL_COLOR_EQ(w - 1, 0, GLColor::black);
+    EXPECT_PIXEL_COLOR_EQ(0, h - 1, GLColor::black);
+    EXPECT_PIXEL_COLOR_EQ(w - 1, h - 1, GLColor::black);
+
+    EXPECT_PIXEL_COLOR_EQ(w / 4 + 1, h / 4 + 1, GLColor::red);
+    EXPECT_PIXEL_COLOR_EQ(3 * w / 4 - 1, h / 4 + 1, GLColor::red);
+    EXPECT_PIXEL_COLOR_EQ(w / 4 + 1, 3 * h / 4 - 1, GLColor::red);
+    EXPECT_PIXEL_COLOR_EQ(3 * w / 4 - 1, 3 * h / 4 - 1, GLColor::red);
+
+    EXPECT_PIXEL_COLOR_EQ(w / 2, h / 2, GLColor::red);
+
+    EXPECT_GL_NO_ERROR();
+}
+
 // Test that XFB does not allow writing more vertices than fit in the bound buffers.
 // TODO(jmadill): Enable this test after fixing the last case where the buffer size changes after
 // calling glBeginTransformFeedback.