Vulkan: Reorder descriptor sets

src/libANGLE/renderer/vulkan/ProgramVk.cpp

@@ -1724,9 +1724,10 @@ angle::Result ProgramVk::updateDescriptorSets(ContextVk *contextVk,
     // Find the maximum non-null descriptor set.  This is used in conjunction with a driver
     // workaround to bind empty descriptor sets only for gaps in between 0 and max and avoid
     // binding unnecessary empty descriptor sets for the sets beyond max.
-    size_t descriptorSetRange = 0;
-    for (size_t descriptorSetIndex = 0; descriptorSetIndex < mDescriptorSets.size();
-         ++descriptorSetIndex)
+    const size_t descriptorSetStart = kUniformsAndXfbDescriptorSetIndex;
+    size_t descriptorSetRange       = 0;
+    for (size_t descriptorSetIndex = descriptorSetStart;
+         descriptorSetIndex < mDescriptorSets.size(); ++descriptorSetIndex)
     {
         if (mDescriptorSets[descriptorSetIndex] != VK_NULL_HANDLE)
         {
@@ -1737,7 +1738,7 @@ angle::Result ProgramVk::updateDescriptorSets(ContextVk *contextVk,
     const VkPipelineBindPoint pipelineBindPoint =
         mState.isCompute() ? VK_PIPELINE_BIND_POINT_COMPUTE : VK_PIPELINE_BIND_POINT_GRAPHICS;
 
-    for (uint32_t descriptorSetIndex = 0; descriptorSetIndex < descriptorSetRange;
+    for (uint32_t descriptorSetIndex = descriptorSetStart; descriptorSetIndex < descriptorSetRange;
          ++descriptorSetIndex)
     {
         VkDescriptorSet descSet = mDescriptorSets[descriptorSetIndex];

src/libANGLE/renderer/vulkan/vk_cache_utils.h

@@ -900,23 +900,27 @@ class PipelineLayoutCache final : angle::NonCopyable
 //
 // The set/binding assignment is done as following:
 //
-// - Set 0 contains uniform blocks created to encompass default uniforms.  1 binding is used per
+// - Set 0 contains the ANGLE driver uniforms at binding 0.  Note that driver uniforms are updated
+//   only under rare circumstances, such as viewport or depth range change.  However, there is only
+//   one binding in this set.  This set is placed before Set 1 containing transform feedback
+//   buffers, so that switching between xfb and non-xfb programs doesn't require rebinding this set.
+//   Otherwise, as the layout of Set 1 changes (due to addition and removal of xfb buffers), and all
+//   subsequent sets need to be rebound (due to Vulkan pipeline layout validation rules), we would
+//   have needed to invalidateGraphicsDriverUniforms().
+// - Set 1 contains uniform blocks created to encompass default uniforms.  1 binding is used per
 //   pipeline stage.  Additionally, transform feedback buffers are bound from binding 2 and up.
-// - Set 1 contains all textures.
-// - Set 2 contains all other shader resources, such as uniform and storage blocks, atomic counter
+// - Set 2 contains all textures.
+// - Set 3 contains all other shader resources, such as uniform and storage blocks, atomic counter
 //   buffers and images.
-// - Set 3 contains the ANGLE driver uniforms at binding 0.  Note that driver uniforms are updated
-//   only under rare circumstances, such as viewport or depth range change.  However, there is only
-//   one binding in this set.
 
+// ANGLE driver uniforms set index (binding is always 0):
+constexpr uint32_t kDriverUniformsDescriptorSetIndex = 0;
 // Uniforms set index:
-constexpr uint32_t kUniformsAndXfbDescriptorSetIndex = 0;
+constexpr uint32_t kUniformsAndXfbDescriptorSetIndex = 1;
 // Textures set index:
-constexpr uint32_t kTextureDescriptorSetIndex = 1;
+constexpr uint32_t kTextureDescriptorSetIndex = 2;
 // Other shader resources set index:
-constexpr uint32_t kShaderResourceDescriptorSetIndex = 2;
-// ANGLE driver uniforms set index (binding is always 3):
-constexpr uint32_t kDriverUniformsDescriptorSetIndex = 3;
+constexpr uint32_t kShaderResourceDescriptorSetIndex = 3;
 
 // Only 1 driver uniform binding is used.
 constexpr uint32_t kReservedDriverUniformBindingCount = 1;

src/tests/gl_tests/StateChangeTest.cpp

@@ -382,6 +382,39 @@ void main (void)
     EXPECT_PIXEL_COLOR_NEAR(w - 1, h - 1, GLColor::black, kPixelTolerance);
 }
 
+// Tests that drawing with transform feedback paused, then lines without transform feedback works
+// without Vulkan validation errors.
+TEST_P(StateChangeTestES3, DrawPausedXfbThenNonXfbLines)
+{
+    // glTransformFeedbackVaryings for program2 returns GL_INVALID_OPERATION on both Linux and
+    // windows.  http://anglebug.com/4265
+    ANGLE_SKIP_TEST_IF(IsIntel() && IsOpenGL());
+
+    std::vector<std::string> tfVaryings = {"gl_Position"};
+    ANGLE_GL_PROGRAM_TRANSFORM_FEEDBACK(program1, essl1_shaders::vs::Simple(),
+                                        essl1_shaders::fs::Blue(), tfVaryings, GL_SEPARATE_ATTRIBS);
+
+    GLBuffer xfbBuffer;
+    glBindBuffer(GL_TRANSFORM_FEEDBACK_BUFFER, xfbBuffer);
+    glBufferData(GL_TRANSFORM_FEEDBACK_BUFFER, 6 * sizeof(float[4]), nullptr, GL_STATIC_DRAW);
+
+    GLTransformFeedback xfb;
+    glBindTransformFeedback(GL_TRANSFORM_FEEDBACK, xfb);
+    glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, xfbBuffer);
+
+    glUseProgram(program1);
+    glBeginTransformFeedback(GL_TRIANGLES);
+    glPauseTransformFeedback();
+    glDrawArrays(GL_TRIANGLES, 0, 6);
+
+    ANGLE_GL_PROGRAM(program2, essl1_shaders::vs::Simple(), essl1_shaders::fs::Blue());
+    glUseProgram(program2);
+    glDrawArrays(GL_LINES, 0, 6);
+    glEndTransformFeedback();
+
+    ASSERT_GL_NO_ERROR();
+}
+
 // Tests that vertex attribute value is preserved across context switches.
 TEST_P(StateChangeTest, MultiContextVertexAttribute)
 {