Vulkan: Enable inactive SSBO with unsized array tests

src/libANGLE/ProgramLinkedResources.cpp

@@ -483,7 +483,9 @@ class FlattenUniformVisitor : public sh::VariableNameVisitor
             }
             else
             {
-                mUnusedUniforms->emplace_back(linkedUniform.name, linkedUniform.isSampler());
+                mUnusedUniforms->emplace_back(linkedUniform.name, linkedUniform.isSampler(),
+                                              linkedUniform.isImage(),
+                                              linkedUniform.isAtomicCounter());
             }
 
             uniformList->push_back(linkedUniform);
@@ -912,7 +914,8 @@ void UniformLinker::pruneUnusedUniforms()
         }
         else
         {
-            mUnusedUniforms.emplace_back(uniformIter->name, uniformIter->isSampler());
+            mUnusedUniforms.emplace_back(uniformIter->name, uniformIter->isSampler(),
+                                         uniformIter->isImage(), uniformIter->isAtomicCounter());
             uniformIter = mUniforms.erase(uniformIter);
         }
     }
@@ -940,7 +943,9 @@ bool UniformLinker::flattenUniformsAndCheckCapsForShader(
         }
         else
         {
-            unusedUniforms.emplace_back(uniform.name, IsSamplerType(uniform.type));
+            unusedUniforms.emplace_back(uniform.name, IsSamplerType(uniform.type),
+                                        IsImageType(uniform.type),
+                                        IsAtomicCounterType(uniform.type));
         }
     }
 

src/libANGLE/ProgramLinkedResources.h

@@ -190,14 +190,18 @@ class AtomicCounterBufferLinker final : angle::NonCopyable
 // TODO(jmadill): Integrate uniform linking/filtering as well as interface blocks.
 struct UnusedUniform
 {
-    UnusedUniform(std::string name, bool isSampler)
+    UnusedUniform(std::string name, bool isSampler, bool isImage, bool isAtomicCounter)
     {
-        this->name      = name;
-        this->isSampler = isSampler;
+        this->name            = name;
+        this->isSampler       = isSampler;
+        this->isImage         = isImage;
+        this->isAtomicCounter = isAtomicCounter;
     }
 
     std::string name;
     bool isSampler;
+    bool isImage;
+    bool isAtomicCounter;
 };
 
 struct ProgramLinkedResources

src/libANGLE/renderer/glslang_wrapper_utils.cpp

@@ -55,7 +55,6 @@ constexpr char kParamsBegin                        = '(';
 constexpr char kParamsEnd                          = ')';
 constexpr char kUniformQualifier[]                 = "uniform";
 constexpr char kSSBOQualifier[]                    = "buffer";
-constexpr char kUnusedBlockSubstitution[]          = "struct";
 constexpr char kUnusedUniformSubstitution[]        = "// ";
 constexpr char kVersionDefine[]                    = "#version 450 core\n";
 constexpr char kLineRasterDefine[]                 = R"(#version 450 core
@@ -865,7 +864,7 @@ void AssignResourceBinding(gl::ShaderBitSet activeShaders,
                 shaderSource.insertLayoutSpecifier(name, bindingString);
                 shaderSource.insertQualifierSpecifier(name, qualifier);
             }
-            else
+            else if (unusedSubstitution)
             {
                 shaderSource.eraseLayoutAndQualifierSpecifiers(name, unusedSubstitution);
             }
@@ -891,7 +890,7 @@ uint32_t AssignInterfaceBlockBindings(const GlslangSourceOptions &options,
                 resourcesDescriptorSet + ", binding = " + Str(bindingIndex++);
 
             AssignResourceBinding(block.activeShaders(), block.name, bindingString, qualifier,
-                                  kUnusedBlockSubstitution, shaderSources);
+                                  nullptr, shaderSources);
         }
     }
 
@@ -950,7 +949,7 @@ uint32_t AssignImageBindings(const GlslangSourceOptions &options,
         }
 
         AssignResourceBinding(imageUniform.activeShaders(), name, bindingString, kUniformQualifier,
-                              kUnusedUniformSubstitution, shaderSources);
+                              nullptr, shaderSources);
     }
 
     return bindingIndex;
@@ -1049,20 +1048,15 @@ void CleanupUnusedEntities(bool useOldRewriteStructSamplers,
         }
     }
 
-    // Remove all the markers for unused interface blocks, and replace them with |struct|.
-    for (const std::string &unusedInterfaceBlock : resources.unusedInterfaceBlocks)
+    // Comment out unused default uniforms.  This relies on the fact that the shader compiler
+    // outputs uniforms to a single line.
+    for (const gl::UnusedUniform &unusedUniform : resources.unusedUniforms)
     {
-        for (IntermediateShaderSource &shaderSource : *shaderSources)
+        if (unusedUniform.isImage || unusedUniform.isAtomicCounter)
         {
-            shaderSource.eraseLayoutAndQualifierSpecifiers(unusedInterfaceBlock,
-                                                           kUnusedBlockSubstitution);
+            continue;
         }
-    }
 
-    // Comment out unused uniforms.  This relies on the fact that the shader compiler outputs
-    // uniforms to a single line.
-    for (const gl::UnusedUniform &unusedUniform : resources.unusedUniforms)
-    {
         std::string uniformName = unusedUniform.isSampler
                                       ? useOldRewriteStructSamplers
                                             ? GetMappedSamplerNameOld(unusedUniform.name)

src/tests/deqp_support/deqp_gles31_test_expectations.txt

@@ -681,9 +681,6 @@
 // Cannot create 2D (array) view of 3D texture.
 3886 VULKAN : dEQP-GLES31.functional.image_load_store.3d.*layer = FAIL
 
-// Inactive SSBOs with flexible array member (about 20% of these tests are affected):
-3714 VULKAN : dEQP-GLES31.functional.ssbo.layout.random.* = FAIL
-
 // OpenGL ES 3.1 conformance failures
 4098 SWIFTSHADER : dEQP-GLES31.functional.compute.basic.write_multiple_unsized_arr_multiple_groups = FAIL
 4098 SWIFTSHADER : dEQP-GLES31.functional.compute.basic.write_multiple_unsized_arr_single_invocation = FAIL

src/tests/gl_tests/ShaderStorageBufferTest.cpp

@@ -2224,6 +2224,38 @@ void main()
     EXPECT_GL_NO_ERROR();
 }
 
+// Test that inactive but statically used SSBOs with unsized array are handled correctly.
+//
+// Glslang wrapper used to replace the layout/qualifier of an inactive SSBO with |struct|,
+// effectively turning the interface block declaration into a struct definition.  This generally
+// worked except for SSBOs with an unsized array.  This test makes sure this special case is
+// now properly handled.
+TEST_P(ShaderStorageBufferTest31, InactiveButStaticallyUsedWithUnsizedArray)
+{
+    constexpr char kComputeShaderSource[] =
+        R"(#version 310 es
+layout (local_size_x=1) in;
+layout(binding=0, std140) buffer Storage
+{
+    uint data[];
+} sb;
+void main()
+{
+    if (false)
+    {
+        sb.data[0] = 1u;
+    }
+}
+)";
+
+    ANGLE_GL_COMPUTE_PROGRAM(program, kComputeShaderSource);
+    EXPECT_GL_NO_ERROR();
+
+    glUseProgram(program);
+    glDispatchCompute(1, 1, 1);
+    EXPECT_GL_NO_ERROR();
+}
+
 ANGLE_INSTANTIATE_TEST_ES31(ShaderStorageBufferTest31);
 
 }  // namespace