Vulkan: Redesign buffer descriptor set cache key.

Instead of writing the bitset masks, iterate up until the last active
buffer. Write zeros instead of skipping spaces. This is a bit simpler
to implement and also fixes a bug where empty buffers could cause us
to write invalid handles.

Bug: angleproject:5736
Change-Id: I785ef18ef5ae45109ec7d6e0b079b79a9984a1f8
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2837848
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Charlie Lao <cclao@google.com>
Reviewed-by: Tim Van Patten <timvp@google.com>

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -292,27 +292,48 @@ egl::ContextPriority GetContextPriority(const gl::State &state)
 template <typename MaskT>
 void AppendBufferVectorToDesc(vk::ShaderBuffersDescriptorDesc *desc,
                               const gl::BufferVector &buffers,
-                              const MaskT &mask,
+                              const MaskT &buffersMask,
                               bool appendOffset)
 {
-    for (size_t bufferIndex : mask)
+    if (buffersMask.any())
     {
-        const gl::OffsetBindingPointer<gl::Buffer> &binding = buffers[bufferIndex];
-        const gl::Buffer *bufferGL                          = binding.get();
-        BufferVk *bufferVk                                  = vk::GetImpl(bufferGL);
-        vk::BufferSerial bufferSerial = bufferVk->getBuffer().getBufferSerial();
-
-        desc->appendBufferSerial(bufferSerial);
-        ASSERT(static_cast<uint64_t>(binding.getSize()) <=
-               static_cast<uint64_t>(std::numeric_limits<uint32_t>::max()));
-        desc->append32BitValue(static_cast<uint32_t>(binding.getSize()));
-        if (appendOffset)
+        typename MaskT::param_type lastBufferIndex = buffersMask.last();
+        for (typename MaskT::param_type bufferIndex = 0; bufferIndex <= lastBufferIndex;
+             ++bufferIndex)
         {
-            ASSERT(static_cast<uint64_t>(binding.getOffset()) <
+            const gl::OffsetBindingPointer<gl::Buffer> &binding = buffers[bufferIndex];
+            const gl::Buffer *bufferGL                          = binding.get();
+
+            if (!bufferGL)
+            {
+                desc->append32BitValue(0);
+                continue;
+            }
+
+            BufferVk *bufferVk = vk::GetImpl(bufferGL);
+
+            if (!bufferVk->isBufferValid())
+            {
+                desc->append32BitValue(0);
+                continue;
+            }
+
+            vk::BufferSerial bufferSerial = bufferVk->getBuffer().getBufferSerial();
+
+            desc->appendBufferSerial(bufferSerial);
+            ASSERT(static_cast<uint64_t>(binding.getSize()) <=
                    static_cast<uint64_t>(std::numeric_limits<uint32_t>::max()));
-            desc->append32BitValue(static_cast<uint32_t>(binding.getOffset()));
+            desc->append32BitValue(static_cast<uint32_t>(binding.getSize()));
+            if (appendOffset)
+            {
+                ASSERT(static_cast<uint64_t>(binding.getOffset()) <
+                       static_cast<uint64_t>(std::numeric_limits<uint32_t>::max()));
+                desc->append32BitValue(static_cast<uint32_t>(binding.getOffset()));
+            }
         }
     }
+
+    desc->append32BitValue(std::numeric_limits<uint32_t>::max());
 }
 }  // anonymous namespace
 
@@ -3939,20 +3960,6 @@ angle::Result ContextVk::invalidateCurrentShaderResources()
     {
         mShaderBuffersDescriptorDesc.reset();
 
-#if defined(ANGLE_IS_64_BIT_CPU)
-        mShaderBuffersDescriptorDesc.append64BitValue(mState.getUniformBuffersMask().bits(0));
-        mShaderBuffersDescriptorDesc.append64BitValue(mState.getUniformBuffersMask().bits(1));
-        mShaderBuffersDescriptorDesc.append64BitValue(mState.getShaderStorageBuffersMask().bits());
-        mShaderBuffersDescriptorDesc.append64BitValue(mState.getAtomicCounterBuffersMask().bits());
-#else
-        mShaderBuffersDescriptorDesc.append32BitValue(mState.getUniformBuffersMask().bits(0));
-        mShaderBuffersDescriptorDesc.append32BitValue(mState.getUniformBuffersMask().bits(1));
-        mShaderBuffersDescriptorDesc.append32BitValue(mState.getUniformBuffersMask().bits(2));
-        mShaderBuffersDescriptorDesc.append32BitValue(mState.getShaderStorageBuffersMask().bits(0));
-        mShaderBuffersDescriptorDesc.append32BitValue(mState.getShaderStorageBuffersMask().bits(1));
-        mShaderBuffersDescriptorDesc.append32BitValue(mState.getAtomicCounterBuffersMask().bits());
-#endif  // defined(ANGLE_IS_64_BIT_CPU)
-
         ProgramExecutableVk *executableVk = nullptr;
         if (mState.getProgram())
         {

src/tests/gl_tests/UniformBufferTest.cpp

@@ -3349,6 +3349,46 @@ void main(void){
     EXPECT_GL_NO_ERROR();
 }
 
+// Tests rendering with a bound, unreferenced UBO that has no data. Covers a paticular back-end bug.
+TEST_P(UniformBufferTest, EmptyUnusedUniformBuffer)
+{
+    constexpr GLuint kBasicUBOIndex = 0;
+    constexpr GLuint kEmptyUBOIndex = 1;
+
+    // Create two UBOs. One is empty and the other is used.
+    constexpr GLfloat basicUBOData[4] = {1.0, 2.0, 3.0, 4.0};
+    GLBuffer basicUBO;
+    glBindBuffer(GL_UNIFORM_BUFFER, basicUBO);
+    glBufferData(GL_UNIFORM_BUFFER, sizeof(basicUBOData), basicUBOData, GL_STATIC_READ);
+    glBindBufferBase(GL_UNIFORM_BUFFER, kBasicUBOIndex, basicUBO);
+
+    GLBuffer emptyUBO;
+    glBindBufferBase(GL_UNIFORM_BUFFER, kEmptyUBOIndex, emptyUBO);
+
+    // Create a simple UBO program.
+    constexpr char kFS[] = R"(#version 300 es
+precision mediump float;
+uniform basicBlock {
+    vec4 basicVec4;
+};
+
+out vec4 outColor;
+
+void main() {
+   if (basicVec4 == vec4(1, 2, 3, 4)) {
+       outColor = vec4(0, 1, 0, 1);
+   } else {
+       outColor = vec4(1, 0, 0, 1);
+   }
+})";
+
+    // Draw and check result. Should not crash.
+    ANGLE_GL_PROGRAM(uboProgram, essl3_shaders::vs::Simple(), kFS);
+    drawQuad(uboProgram, essl1_shaders::PositionAttrib(), 0.5f, 1.0f);
+    EXPECT_GL_NO_ERROR();
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
+}
+
 GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(UniformBufferTest);
 ANGLE_INSTANTIATE_TEST_ES3(UniformBufferTest);