Vulkan: Apply binding size in updateBuffersDescriptorSet.

src/libANGLE/renderer/vulkan/ProgramExecutableVk.cpp

@@ -20,6 +20,23 @@
 
 namespace rx
 {
+namespace
+{
+VkDeviceSize GetShaderBufferBindingSize(const gl::OffsetBindingPointer<gl::Buffer> &bufferBinding)
+{
+    if (bufferBinding.getSize() != 0)
+    {
+        return bufferBinding.getSize();
+    }
+    // If size is 0, we can't always use VK_WHOLE_SIZE (or bufferHelper.getSize()), as the
+    // backing buffer may be larger than max*BufferRange.  In that case, we use the backing
+    // buffer size (what's left after offset).
+    const gl::Buffer *bufferGL = bufferBinding.get();
+    ASSERT(bufferGL);
+    ASSERT(bufferGL->getSize() >= bufferBinding.getOffset());
+    return bufferGL->getSize() - bufferBinding.getOffset();
+}
+}  // namespace
 
 DefaultUniformBlock::DefaultUniformBlock() = default;
 
@@ -485,8 +502,9 @@ void ProgramExecutableVk::addTextureDescriptorSetDesc(const gl::ProgramState &pr
     }
 }
 
-void WriteBufferDescriptorSetBinding(const gl::OffsetBindingPointer<gl::Buffer> &bufferBinding,
-                                     VkDeviceSize maxSize,
+void WriteBufferDescriptorSetBinding(const vk::BufferHelper &buffer,
+                                     VkDeviceSize offset,
+                                     VkDeviceSize size,
                                      VkDescriptorSet descSet,
                                      VkDescriptorType descType,
                                      uint32_t bindingIndex,
@@ -495,29 +513,6 @@ void WriteBufferDescriptorSetBinding(const gl::OffsetBindingPointer<gl::Buffer>
                                      VkDescriptorBufferInfo *bufferInfoOut,
                                      VkWriteDescriptorSet *writeInfoOut)
 {
-    gl::Buffer *buffer = bufferBinding.get();
-    ASSERT(buffer != nullptr);
-
-    // Make sure there's no possible under/overflow with binding size.
-    static_assert(sizeof(VkDeviceSize) >= sizeof(bufferBinding.getSize()),
-                  "VkDeviceSize too small");
-    ASSERT(bufferBinding.getSize() >= 0);
-
-    BufferVk *bufferVk             = vk::GetImpl(buffer);
-    VkDeviceSize offset            = bufferBinding.getOffset();
-    VkDeviceSize size              = bufferBinding.getSize();
-    vk::BufferHelper &bufferHelper = bufferVk->getBuffer();
-
-    // If size is 0, we can't always use VK_WHOLE_SIZE (or bufferHelper.getSize()), as the
-    // backing buffer may be larger than max*BufferRange.  In that case, we use the minimum of
-    // the backing buffer size (what's left after offset) and the buffer size as defined by the
-    // shader.  That latter is only valid for UBOs, as SSBOs may have variable length arrays.
-    size = size > 0 ? size : (bufferHelper.getSize() - offset);
-    if (maxSize > 0)
-    {
-        size = std::min(size, maxSize);
-    }
-
     // If requiredOffsetAlignment is 0, the buffer offset is guaranteed to have the necessary
     // alignment through other means (the backend specifying the alignment through a GLES limit that
     // the frontend then enforces).  If it's not 0, we need to bind the buffer at an offset that's
@@ -531,7 +526,7 @@ void WriteBufferDescriptorSetBinding(const gl::OffsetBindingPointer<gl::Buffer>
         size += offsetDiff;
     }
 
-    bufferInfoOut->buffer = bufferHelper.getBuffer().getHandle();
+    bufferInfoOut->buffer = buffer.getBuffer().getHandle();
     bufferInfoOut->offset = offset;
     bufferInfoOut->range  = size;
 
@@ -916,17 +911,32 @@ void ProgramExecutableVk::updateBuffersDescriptorSet(ContextVk *contextVk,
         ShaderInterfaceVariableInfo info = mVariableInfoMap[shaderType][block.mappedName];
         uint32_t binding                 = info.binding;
         uint32_t arrayElement            = block.isArray ? block.arrayElement : 0;
-        VkDeviceSize maxBlockSize        = isStorageBuffer ? 0 : block.dataSize;
+
+        VkDeviceSize size;
+        if (!isStorageBuffer)
+        {
+            size = block.dataSize;
+        }
+        else
+        {
+            size = GetShaderBufferBindingSize(bufferBinding);
+        }
+
+        // Make sure there's no possible under/overflow with binding size.
+        static_assert(sizeof(VkDeviceSize) >= sizeof(bufferBinding.getSize()),
+                      "VkDeviceSize too small");
+        ASSERT(bufferBinding.getSize() >= 0);
 
         VkDescriptorBufferInfo &bufferInfo = descriptorBufferInfo[writeCount];
         VkWriteDescriptorSet &writeInfo    = writeDescriptorInfo[writeCount];
 
-        WriteBufferDescriptorSetBinding(bufferBinding, maxBlockSize, descriptorSet, descriptorType,
-                                        binding, arrayElement, 0, &bufferInfo, &writeInfo);
-
         BufferVk *bufferVk             = vk::GetImpl(bufferBinding.get());
         vk::BufferHelper &bufferHelper = bufferVk->getBuffer();
 
+        WriteBufferDescriptorSetBinding(bufferHelper, bufferBinding.getOffset(), size,
+                                        descriptorSet, descriptorType, binding, arrayElement, 0,
+                                        &bufferInfo, &writeInfo);
+
         if (isStorageBuffer)
         {
             // We set the SHADER_READ_BIT to be conservative.
@@ -997,13 +1007,15 @@ void ProgramExecutableVk::updateAtomicCounterBuffersDescriptorSet(
         VkDescriptorBufferInfo &bufferInfo = descriptorBufferInfo[binding];
         VkWriteDescriptorSet &writeInfo    = writeDescriptorInfo[binding];
 
-        WriteBufferDescriptorSetBinding(bufferBinding, 0, descriptorSet,
-                                        VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, info.binding, binding,
-                                        requiredOffsetAlignment, &bufferInfo, &writeInfo);
-
         BufferVk *bufferVk             = vk::GetImpl(bufferBinding.get());
         vk::BufferHelper &bufferHelper = bufferVk->getBuffer();
 
+        VkDeviceSize size = GetShaderBufferBindingSize(bufferBinding);
+        WriteBufferDescriptorSetBinding(bufferHelper, bufferBinding.getOffset(), size,
+                                        descriptorSet, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER,
+                                        info.binding, binding, requiredOffsetAlignment, &bufferInfo,
+                                        &writeInfo);
+
         // We set SHADER_READ_BIT to be conservative.
         commandBufferHelper->bufferWrite(
             resourceUseList, VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT, &bufferHelper);

src/tests/gl_tests/ShaderStorageBufferTest.cpp

@@ -1974,6 +1974,104 @@ void main()
     EXPECT_GL_NO_ERROR();
 }
 
+// Test back to back that the length of unsized array is correct after respecifying the buffer
+// size to be smaller than the first
+TEST_P(ShaderStorageBufferTest31, UnsizedArrayLengthRespecifySize)
+{
+    // http://anglebug.com/4566
+    ANGLE_SKIP_TEST_IF(IsD3D11() || (IsAndroid() && IsOpenGLES()));
+
+    constexpr char kComputeShaderSource[] =
+        R"(#version 310 es
+layout (local_size_x=1) in;
+layout(std430, binding = 0) buffer Storage0 {
+  uint buf1[2];
+  uint buf2[];
+} sb_load;
+layout(std430, binding = 1) buffer Storage1 {
+  int unsizedArrayLength;
+  uint buf1[2];
+  uint buf2[];
+} sb_store;
+
+void main()
+{
+  sb_store.unsizedArrayLength = sb_store.buf2.length();
+  for (int i = 0; i < sb_load.buf1.length(); i++) {
+    sb_store.buf1[i] = sb_load.buf1[i];
+  }
+  for (int i = 0; i < sb_load.buf2.length(); i++) {
+    sb_store.buf2[i] = sb_load.buf2[i];
+  }
+}
+)";
+
+    ANGLE_GL_COMPUTE_PROGRAM(program, kComputeShaderSource);
+    glUseProgram(program);
+
+    constexpr unsigned int kBytesPerComponent                       = sizeof(unsigned int);
+    constexpr unsigned int kLoadBlockElementCount                   = 5;
+    constexpr unsigned int kStoreBlockElementCount                  = 6;
+    constexpr unsigned int kInputValues[kLoadBlockElementCount]     = {1u, 2u, 3u, 4u, 5u};
+    constexpr unsigned int kExpectedValues[kStoreBlockElementCount] = {3u, 1u, 2u, 3u, 4u, 5u};
+    GLBuffer shaderStorageBuffer[2];
+    glBindBuffer(GL_SHADER_STORAGE_BUFFER, shaderStorageBuffer[0]);
+    glBufferData(GL_SHADER_STORAGE_BUFFER, kLoadBlockElementCount * kBytesPerComponent,
+                 &kInputValues, GL_STATIC_DRAW);
+    glBindBuffer(GL_SHADER_STORAGE_BUFFER, shaderStorageBuffer[1]);
+    glBufferData(GL_SHADER_STORAGE_BUFFER, kStoreBlockElementCount * kBytesPerComponent, nullptr,
+                 GL_STATIC_DRAW);
+
+    glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 0, shaderStorageBuffer[0]);
+    glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 1, shaderStorageBuffer[1]);
+
+    glDispatchCompute(1, 1, 1);
+    glFinish();
+
+    glBindBuffer(GL_SHADER_STORAGE_BUFFER, shaderStorageBuffer[1]);
+    const GLuint *ptr = reinterpret_cast<const GLuint *>(
+        glMapBufferRange(GL_SHADER_STORAGE_BUFFER, 0, kStoreBlockElementCount * kBytesPerComponent,
+                         GL_MAP_READ_BIT));
+    for (unsigned int i = 0; i < kStoreBlockElementCount; i++)
+    {
+        EXPECT_EQ(kExpectedValues[i], *(ptr + i));
+    }
+    glUnmapBuffer(GL_SHADER_STORAGE_BUFFER);
+
+    EXPECT_GL_NO_ERROR();
+
+    // Respecify these SSBOs to be smaller
+    constexpr unsigned int kSmallerLoadBlockElementCount                          = 3;
+    constexpr unsigned int kSmallerStoreBlockElementCount                         = 4;
+    constexpr unsigned int kSmallerInputValues[kSmallerLoadBlockElementCount]     = {1u, 2u, 3u};
+    constexpr unsigned int kSmallerExpectedValues[kSmallerStoreBlockElementCount] = {1u, 1u, 2u,
+                                                                                     3u};
+
+    glBindBuffer(GL_SHADER_STORAGE_BUFFER, shaderStorageBuffer[0]);
+    glBufferData(GL_SHADER_STORAGE_BUFFER, kSmallerLoadBlockElementCount * kBytesPerComponent,
+                 &kSmallerInputValues, GL_STATIC_DRAW);
+    glBindBuffer(GL_SHADER_STORAGE_BUFFER, shaderStorageBuffer[1]);
+    glBufferData(GL_SHADER_STORAGE_BUFFER, kSmallerStoreBlockElementCount * kBytesPerComponent,
+                 nullptr, GL_STATIC_DRAW);
+
+    glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 0, shaderStorageBuffer[0]);
+    glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 1, shaderStorageBuffer[1]);
+
+    glDispatchCompute(1, 1, 1);
+    glFinish();
+
+    glBindBuffer(GL_SHADER_STORAGE_BUFFER, shaderStorageBuffer[1]);
+    const GLuint *ptr2 = reinterpret_cast<const GLuint *>(
+        glMapBufferRange(GL_SHADER_STORAGE_BUFFER, 0,
+                         kSmallerStoreBlockElementCount * kBytesPerComponent, GL_MAP_READ_BIT));
+    for (unsigned int i = 0; i < kSmallerStoreBlockElementCount; i++)
+    {
+        EXPECT_EQ(kSmallerExpectedValues[i], *(ptr2 + i));
+    }
+
+    EXPECT_GL_NO_ERROR();
+}
+
 // Test that compond assignment operator for buffer variable is correctly handled.
 TEST_P(ShaderStorageBufferTest31, CompoundAssignmentOperator)
 {