Vulkan: Fix transform feedback with in-render-pass clears

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -778,7 +778,9 @@ angle::Result ContextVk::setupDraw(const gl::Context *context,
     DirtyBits dirtyBits = mGraphicsDirtyBits & dirtyBitMask;
 
     if (dirtyBits.none())
+    {
         return angle::Result::Continue;
+    }
 
     // Flush any relevant dirty bits.
     for (size_t dirtyBit : dirtyBits)
@@ -3657,9 +3659,19 @@ void ContextVk::writeAtomicCounterBufferDriverUniformOffsets(uint32_t *offsetsOu
     }
 }
 
+void ContextVk::pauseTransformFeedbackIfStartedAndRebindBuffersOnResume()
+{
+    DirtyBits rebindTransformFeedbackOnResume;
+    rebindTransformFeedbackOnResume.set(DIRTY_BIT_TRANSFORM_FEEDBACK_STATE);
+    pauseTransformFeedbackIfStarted(rebindTransformFeedbackOnResume);
+}
+
 ANGLE_INLINE void ContextVk::pauseTransformFeedbackIfStarted(DirtyBits onResumeOps)
 {
-    if (mRenderPassCommands->isTransformFeedbackStarted())
+    // Note that UtilsVk may have already paused transform feedback, so don't pause it again if it's
+    // already paused.
+    if (mRenderPassCommands->isTransformFeedbackStarted() &&
+        !mGraphicsDirtyBits.test(DIRTY_BIT_TRANSFORM_FEEDBACK_RESUME))
     {
         ASSERT(getFeatures().supportsTransformFeedbackExtension.enabled);
         mGraphicsDirtyBits.set(DIRTY_BIT_TRANSFORM_FEEDBACK_RESUME);
@@ -4597,9 +4609,7 @@ angle::Result ContextVk::flushCommandsAndEndRenderPass()
 
     addOverlayUsedBuffersCount(mRenderPassCommands);
 
-    DirtyBits rebindTransformFeedbackOnResume;
-    rebindTransformFeedbackOnResume.set(DIRTY_BIT_TRANSFORM_FEEDBACK_STATE);
-    pauseTransformFeedbackIfStarted(rebindTransformFeedbackOnResume);
+    pauseTransformFeedbackIfStartedAndRebindBuffersOnResume();
 
     mRenderPassCommands->endRenderPass(this);
 

src/libANGLE/renderer/vulkan/ContextVk.h

@@ -320,6 +320,7 @@ class ContextVk : public ContextImpl, public vk::Context, public MultisampleText
         const gl::TransformFeedbackBuffersArray<vk::BufferHelper *> &buffers);
     void onEndTransformFeedback();
     angle::Result onPauseTransformFeedback();
+    void pauseTransformFeedbackIfStartedAndRebindBuffersOnResume();
 
     // When UtilsVk issues draw or dispatch calls, it binds descriptor sets that the context is not
     // aware of.  This function is called to make sure affected descriptor set bindings are dirtied

src/libANGLE/renderer/vulkan/UtilsVk.cpp

@@ -1597,6 +1597,9 @@ angle::Result UtilsVk::clearFramebuffer(ContextVk *contextVk,
                            imageClearProgram, &pipelineDesc, VK_NULL_HANDLE, &shaderParams,
                            sizeof(shaderParams), commandBuffer));
 
+    // Make sure transform feedback is paused
+    contextVk->pauseTransformFeedbackIfStartedAndRebindBuffersOnResume();
+
     // Make sure this draw call doesn't count towards occlusion query results.
     ANGLE_TRY(contextVk->pauseRenderPassQueriesIfActive());
     commandBuffer->setScissor(0, 1, &scissor);
@@ -1606,6 +1609,14 @@ angle::Result UtilsVk::clearFramebuffer(ContextVk *contextVk,
     // Make sure what's bound here is correctly reverted on the next draw.
     contextVk->invalidateGraphicsPipelineAndDescriptorSets();
 
+    // If transform feedback was active, we can't pause and resume it in the same render pass
+    // because we can't insert a memory barrier for the counter buffers.  In that case, break the
+    // render pass.
+    if (contextVk->getStartedRenderPassCommands().isTransformFeedbackStarted())
+    {
+        ANGLE_TRY(contextVk->flushCommandsAndEndRenderPass());
+    }
+
     return angle::Result::Continue;
 }
 

src/libANGLE/renderer/vulkan/doc/TransformFeedbackViaExtension.md

@@ -4,10 +4,7 @@
 
 ANGLE emulates transform feedback using the vertexPipelineStoresAndAtomics features in Vulkan.
 But some GPU vendors do not support these atomics. Also the emulation becomes more difficult in
-GLES 3.2. Therefore ANGLE must support using the VK_EXT_transform_feedback extension .
-
-But some GPU vendor does not support this feature, So we need another implementation using
-VK_EXT_transform_feedback.
+GLES 3.2. Therefore ANGLE must support using the VK_EXT_transform_feedback extension.
 
 We also expect a performance gain when we use this extension.
 
@@ -37,66 +34,12 @@ source access of `VK_ACCESS_TRANSFORM_FEEDBACK_COUNTER_WRITE_BIT_EXT` at pipelin
 There is no equivalent function for glTransformFeedbackVaryings in Vulkan. The Vulkan specification
 states that the last vertex processing stage shader must be declared with the XFB execution mode.
 
-So we need to modify gl shader code to have transform feedback qualifiers. The glsl code will be
-converted proper SPIR-V code.
-
-we add the below layout qualifier for built-in XFB varyings.
-
-```
-out gl_PerVertex
-
-{
-
-layout(xfb_buffer = buffer_num, xfb_offset = offset, xfb_stride = stride) varying_type varying_name;
-
-}
-```
-
- And for user xfb varyings.
-
-```
-layout(xfb_buffer = buffer_num, xfb_offset = offset, xfb_stride = stride, location = num )
-out varying_type varying_name;
-
-```
-
-There are some corner cases we should handle.
-
-If more than 2 built-in varyings are used in the shader, and only one varying is declared as a
-transformFeedback varying, we can generate a layout qualifier like this.
-
-```
-out gl_PerVertex
-
-{
-
-layout(xfb_buffer = buffer_num, xfb_offset = offset, xfb_stride = stride) varying_type varying_name1;
-
-varying_type varying_name2;
-
-...
-
-}
-```
+The SPIR-V transformer takes care of adding this execution mode, as well as decorating the variables
+that need to be captured.
 
 ANGLE modifies gl_position.z in vertex shader for the Vulkan coordinate system. So, if we capture
-the value of 'gl_position' in the XFB buffer, the captured values will be incorrect.
-
-To resolve this, we declare user declare an internal position varying and copy the value from
-'gl_position'. We capture the internal position varying during transform feedback operation.
-
-```
-layout(xfb_buffer = buffer_num, xfb_offset = offset, xfb_stride = stride, location = num )
-out vec4 xfbANGLEPosition;
-
-....
-
-void main(){
-
-...
-
-xfbANGLEPosition = gl_Position;
-(gl_Position.z = ((gl_Position.z + gl_Position.w) * 0.5));
-}
-```
-
+the value of 'gl_position' in the XFB buffer, the captured values will be incorrect. To resolve
+this, we declare an internal position varying and copy the value from 'gl_position'. We capture the
+internal position varying during transform feedback operation. For simplicity, we do this for every
+captured varying, even though we could decorate the `gl_PerVertex` struct members in SPIR-V
+directly.

src/tests/gl_tests/TransformFeedbackTest.cpp

@@ -100,8 +100,7 @@ TEST_P(TransformFeedbackTest, ZeroSizedViewport)
     glBeginTransformFeedback(GL_TRIANGLES);
 
     // Create a query to check how many primitives were written
-    GLuint primitivesWrittenQuery = 0;
-    glGenQueries(1, &primitivesWrittenQuery);
+    GLQuery primitivesWrittenQuery;
     glBeginQuery(GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN, primitivesWrittenQuery);
 
     // Set a viewport that would result in no pixels being written to the framebuffer and draw
@@ -148,8 +147,7 @@ TEST_P(TransformFeedbackTest, BufferRebinding)
                  GL_STATIC_DRAW);
 
     // Create a query to check how many primitives were written
-    GLuint primitivesWrittenQuery = 0;
-    glGenQueries(1, &primitivesWrittenQuery);
+    GLQuery primitivesWrittenQuery;
     glBeginQuery(GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN, primitivesWrittenQuery);
 
     const float finalZ = 0.95f;
@@ -241,8 +239,7 @@ TEST_P(TransformFeedbackTest, RecordAndDraw)
     glBeginTransformFeedback(GL_POINTS);
 
     // Create a query to check how many primitives were written
-    GLuint primitivesWrittenQuery = 0;
-    glGenQueries(1, &primitivesWrittenQuery);
+    GLQuery primitivesWrittenQuery;
     glBeginQuery(GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN, primitivesWrittenQuery);
 
     glDrawArrays(GL_POINTS, 0, 6);
@@ -320,8 +317,7 @@ TEST_P(TransformFeedbackTest, SpanMultipleRenderPasses)
     glBeginTransformFeedback(GL_POINTS);
 
     // Create a query to check how many primitives were written
-    GLuint primitivesWrittenQuery = 0;
-    glGenQueries(1, &primitivesWrittenQuery);
+    GLQuery primitivesWrittenQuery;
     glBeginQuery(GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN, primitivesWrittenQuery);
 
     // Draw the first set of three points
@@ -377,6 +373,101 @@ 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)
+{
+    // 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);
+
+    // Set the program's transform feedback varyings (just gl_Position)
+    std::vector<std::string> tfVaryings;
+    tfVaryings.push_back("gl_Position");
+    compileDefaultProgram(tfVaryings, GL_INTERLEAVED_ATTRIBS);
+
+    glUseProgram(mProgram);
+
+    GLint positionLocation = glGetAttribLocation(mProgram, essl1_shaders::PositionAttrib());
+
+    const GLfloat vertices[] = {
+        -0.5f, 0.5f, 0.5f, -0.5f, -0.5f, 0.5f, 0.5f, -0.5f, 0.5f,
+        -0.5f, 0.5f, 0.5f, 0.5f,  -0.5f, 0.5f, 0.5f, 0.5f,  0.5f,
+    };
+
+    glVertexAttribPointer(positionLocation, 3, GL_FLOAT, GL_FALSE, 0, vertices);
+    glEnableVertexAttribArray(positionLocation);
+
+    // Bind the buffer for transform feedback output and start transform feedback
+    glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, mTransformFeedbackBuffer);
+    glBeginTransformFeedback(GL_POINTS);
+
+    // Create a query to check how many primitives were written
+    GLQuery primitivesWrittenQuery;
+    glBeginQuery(GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN, primitivesWrittenQuery);
+
+    // 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);
+
+    // Draw the second set of three points
+    glVertexAttribPointer(positionLocation, 3, GL_FLOAT, GL_FALSE, 0, vertices + 9);
+    glDrawArrays(GL_POINTS, 0, 3);
+
+    glDisableVertexAttribArray(positionLocation);
+    glVertexAttribPointer(positionLocation, 4, GL_FLOAT, GL_FALSE, 0, nullptr);
+    // End the query and transform feedback
+    glEndQuery(GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN);
+    glEndTransformFeedback();
+
+    glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, 0);
+
+    // Verify the number of primitives written
+    GLuint primitivesWritten = 0;
+    glGetQueryObjectuiv(primitivesWrittenQuery, GL_QUERY_RESULT_EXT, &primitivesWritten);
+    EXPECT_GL_NO_ERROR();
+
+    EXPECT_EQ(6u, primitivesWritten);
+
+    // Verify the captured buffer.
+
+    glBindBuffer(GL_ARRAY_BUFFER, mTransformFeedbackBuffer);
+    glVertexAttribPointer(positionLocation, 4, GL_FLOAT, GL_FALSE, 0, 0);
+    glEnableVertexAttribArray(positionLocation);
+
+    glClearColor(0.0f, 0.0f, 0.0f, 1.0f);
+    glClear(GL_COLOR_BUFFER_BIT);
+    glDrawArrays(GL_TRIANGLES, 0, 6);
+
+    const int w = getWindowWidth();
+    const int h = getWindowHeight();
+
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::blue);
+    EXPECT_PIXEL_COLOR_EQ(w - 1, 0, GLColor::blue);
+    EXPECT_PIXEL_COLOR_EQ(0, h - 1, GLColor::blue);
+    EXPECT_PIXEL_COLOR_EQ(w - 1, h - 1, GLColor::blue);
+
+    EXPECT_PIXEL_COLOR_EQ(w / 4 + 1, h / 4 + 1, GLColor::magenta);
+    EXPECT_PIXEL_COLOR_EQ(3 * w / 4 - 1, h / 4 + 1, GLColor::magenta);
+    EXPECT_PIXEL_COLOR_EQ(w / 4 + 1, 3 * h / 4 - 1, GLColor::magenta);
+    EXPECT_PIXEL_COLOR_EQ(3 * w / 4 - 1, 3 * h / 4 - 1, GLColor::magenta);
+
+    EXPECT_PIXEL_COLOR_EQ(w / 2, h / 2, GLColor::magenta);
+
+    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.
@@ -2343,8 +2434,7 @@ TEST_P(TransformFeedbackTest, RecordAndDrawWithScissorTest)
     glBeginTransformFeedback(GL_POINTS);
 
     // Create a query to check how many primitives were written
-    GLuint primitivesWrittenQuery = 0;
-    glGenQueries(1, &primitivesWrittenQuery);
+    GLQuery primitivesWrittenQuery;
     glBeginQuery(GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN, primitivesWrittenQuery);
 
     glDrawArrays(GL_POINTS, 0, 3);
@@ -2421,8 +2511,7 @@ TEST_P(TransformFeedbackWithDepthBufferTest, RecordAndDrawWithDepthWriteEnabled)
     glBeginTransformFeedback(GL_POINTS);
 
     // Create a query to check how many primitives were written
-    GLuint primitivesWrittenQuery = 0;
-    glGenQueries(1, &primitivesWrittenQuery);
+    GLQuery primitivesWrittenQuery;
     glBeginQuery(GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN, primitivesWrittenQuery);
 
     glDrawArrays(GL_POINTS, 0, 3);

src/tests/test_utils/gl_raii.h

@@ -133,6 +133,7 @@ class GLQueryEXT : public GLWrapper
   public:
     GLQueryEXT() : GLWrapper(&glGenQueriesEXT, &glDeleteQueriesEXT) {}
 };
+using GLQuery = GLQueryEXT;
 
 class GLShader : angle::NonCopyable
 {