Vulkan: Remove inactive uniforms in the translator

By removing inactive uniforms in the translator, glslang wrapper doesn't
need to comment them out.  Additionally, inactive uniforms don't find
their way in the default uniform block, reducing its size if there's a
mix of active and inactive uniforms.

As collateral, it also fixes a bug where inactive uniforms of struct
type were not correctly removed by glslang wrapper.

Bug: angleproject:3394
Bug: angleproject:4211
Bug: angleproject:4248
Change-Id: I874747070e875fe24bf59d39d1322e319e280a16
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1999278
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>

Author: ShabbyX

src/compiler/translator/TranslatorVulkan.cpp

@@ -712,7 +712,7 @@ bool TranslatorVulkan::translateImpl(TIntermBlock *root,
     int atomicCounterCount            = 0;
     for (const auto &uniform : getUniforms())
     {
-        if (!uniform.isBuiltIn() && uniform.staticUse && !gl::IsOpaqueType(uniform.type))
+        if (!uniform.isBuiltIn() && uniform.active && !gl::IsOpaqueType(uniform.type))
         {
             ++defaultUniformCount;
         }

src/compiler/translator/blocklayout.cpp

@@ -48,10 +48,18 @@ void GetInterfaceBlockInfo(const std::vector<VarT> &fields,
                            const std::string &prefix,
                            BlockLayoutEncoder *encoder,
                            bool inRowMajorLayout,
+                           bool onlyActiveVariables,
                            BlockLayoutMap *blockInfoOut)
 {
     BlockLayoutMapVisitor visitor(blockInfoOut, prefix, encoder);
-    TraverseShaderVariables(fields, inRowMajorLayout, &visitor);
+    if (onlyActiveVariables)
+    {
+        TraverseActiveShaderVariables(fields, inRowMajorLayout, &visitor);
+    }
+    else
+    {
+        TraverseShaderVariables(fields, inRowMajorLayout, &visitor);
+    }
 }
 
 void TraverseStructVariable(const ShaderVariable &variable,
@@ -345,17 +353,19 @@ void GetInterfaceBlockInfo(const std::vector<ShaderVariable> &fields,
 {
     // Matrix packing is always recorded in individual fields, so they'll set the row major layout
     // flag to true if needed.
-    GetInterfaceBlockInfo(fields, prefix, encoder, false, blockInfoOut);
+    // Iterates over all variables.
+    GetInterfaceBlockInfo(fields, prefix, encoder, false, false, blockInfoOut);
 }
 
-void GetUniformBlockInfo(const std::vector<ShaderVariable> &uniforms,
-                         const std::string &prefix,
-                         BlockLayoutEncoder *encoder,
-                         BlockLayoutMap *blockInfoOut)
+void GetActiveUniformBlockInfo(const std::vector<ShaderVariable> &uniforms,
+                               const std::string &prefix,
+                               BlockLayoutEncoder *encoder,
+                               BlockLayoutMap *blockInfoOut)
 {
     // Matrix packing is always recorded in individual fields, so they'll set the row major layout
     // flag to true if needed.
-    GetInterfaceBlockInfo(uniforms, prefix, encoder, false, blockInfoOut);
+    // Iterates only over the active variables.
+    GetInterfaceBlockInfo(uniforms, prefix, encoder, false, true, blockInfoOut);
 }
 
 // VariableNameVisitor implementation.

src/compiler/translator/blocklayout.h

@@ -179,10 +179,10 @@ void GetInterfaceBlockInfo(const std::vector<ShaderVariable> &fields,
                            BlockLayoutMap *blockInfoOut);
 
 // Used for laying out the default uniform block on the Vulkan backend.
-void GetUniformBlockInfo(const std::vector<ShaderVariable> &uniforms,
-                         const std::string &prefix,
-                         BlockLayoutEncoder *encoder,
-                         BlockLayoutMap *blockInfoOut);
+void GetActiveUniformBlockInfo(const std::vector<ShaderVariable> &uniforms,
+                               const std::string &prefix,
+                               BlockLayoutEncoder *encoder,
+                               BlockLayoutMap *blockInfoOut);
 
 class ShaderVariableVisitor
 {
@@ -298,6 +298,20 @@ void TraverseShaderVariables(const std::vector<T> &vars,
         TraverseShaderVariable(var, isRowMajorLayout, visitor);
     }
 }
+
+template <typename T>
+void TraverseActiveShaderVariables(const std::vector<T> &vars,
+                                   bool isRowMajorLayout,
+                                   ShaderVariableVisitor *visitor)
+{
+    for (const T &var : vars)
+    {
+        if (var.active)
+        {
+            TraverseShaderVariable(var, isRowMajorLayout, visitor);
+        }
+    }
+}
 }  // namespace sh
 
 #endif  // COMMON_BLOCKLAYOUT_H_

src/compiler/translator/tree_ops/RemoveInactiveInterfaceVariables.cpp

@@ -41,7 +41,7 @@ RemoveInactiveInterfaceVariablesTraverser::RemoveInactiveInterfaceVariablesTrave
 {}
 
 template <typename Variable>
-bool isVariableActive(const std::vector<Variable> &mVars, const ImmutableString &name)
+bool IsVariableActive(const std::vector<Variable> &mVars, const ImmutableString &name)
 {
     for (const Variable &var : mVars)
     {
@@ -71,7 +71,7 @@ bool RemoveInactiveInterfaceVariablesTraverser::visitDeclaration(Visit visit,
 
     const TType &type = declarator->getType();
 
-    // Only remove opaque uniform and interface block declarations.
+    // Only remove uniform and interface block declarations.
     //
     // Note: Don't remove varyings.  Imagine a situation where the VS doesn't write to a varying
     // but the FS reads from it.  This is allowed, though the value of the varying is undefined.
@@ -81,11 +81,11 @@ bool RemoveInactiveInterfaceVariablesTraverser::visitDeclaration(Visit visit,
 
     if (type.isInterfaceBlock())
     {
-        removeDeclaration = !isVariableActive(mInterfaceBlocks, type.getInterfaceBlock()->name());
+        removeDeclaration = !IsVariableActive(mInterfaceBlocks, type.getInterfaceBlock()->name());
     }
-    else if (type.getQualifier() == EvqUniform && IsOpaqueType(type.getBasicType()))
+    else if (type.getQualifier() == EvqUniform)
     {
-        removeDeclaration = !isVariableActive(mUniforms, asSymbol->getName());
+        removeDeclaration = !IsVariableActive(mUniforms, asSymbol->getName());
     }
 
     if (removeDeclaration)

src/libANGLE/renderer/glslang_wrapper_utils.cpp

@@ -1045,20 +1045,18 @@ void CleanupUnusedEntities(bool useOldRewriteStructSamplers,
         }
     }
 
-    // Comment out unused default uniforms.  This relies on the fact that the shader compiler
-    // outputs uniforms to a single line.
+    // Comment out inactive samplers.  This relies on the fact that the shader compiler outputs
+    // uniforms to a single line.
     for (const gl::UnusedUniform &unusedUniform : resources.unusedUniforms)
     {
-        if (unusedUniform.isImage || unusedUniform.isAtomicCounter)
+        if (!unusedUniform.isSampler)
         {
             continue;
         }
 
-        std::string uniformName = unusedUniform.isSampler
-                                      ? useOldRewriteStructSamplers
-                                            ? GetMappedSamplerNameOld(unusedUniform.name)
-                                            : GlslangGetMappedSamplerName(unusedUniform.name)
-                                      : unusedUniform.name;
+        std::string uniformName = useOldRewriteStructSamplers
+                                      ? GetMappedSamplerNameOld(unusedUniform.name)
+                                      : GlslangGetMappedSamplerName(unusedUniform.name);
 
         for (IntermediateShaderSource &shaderSource : *shaderSources)
         {

src/libANGLE/renderer/metal/ProgramMtl.mm

@@ -124,7 +124,7 @@ void InitDefaultUniformBlock(const std::vector<sh::Uniform> &uniforms,
     }
 
     sh::Std140BlockEncoder blockEncoder;
-    sh::GetUniformBlockInfo(uniforms, "", &blockEncoder, blockLayoutMapOut);
+    sh::GetActiveUniformBlockInfo(uniforms, "", &blockEncoder, blockLayoutMapOut);
 
     size_t blockSize = blockEncoder.getCurrentOffset();
 

src/libANGLE/renderer/vulkan/ProgramVk.cpp

@@ -58,7 +58,7 @@ void InitDefaultUniformBlock(const std::vector<sh::ShaderVariable> &uniforms,
     }
 
     VulkanDefaultBlockEncoder blockEncoder;
-    sh::GetUniformBlockInfo(uniforms, "", &blockEncoder, blockLayoutMapOut);
+    sh::GetActiveUniformBlockInfo(uniforms, "", &blockEncoder, blockLayoutMapOut);
 
     size_t blockSize = blockEncoder.getCurrentOffset();