Clean up checking variable packing limits

src/compiler/translator/Compiler.cpp

@@ -484,13 +484,9 @@ TIntermBlock *TCompiler::compileTreeImpl(const char *const shaderStrings[],
             }
             if (compileOptions & SH_ENFORCE_PACKING_RESTRICTIONS)
             {
-                std::vector<sh::ShaderVariable> expandedUniforms;
-                sh::ExpandUniforms(uniforms, &expandedUniforms);
-                VariablePacker packer;
                 // Returns true if, after applying the packing rules in the GLSL ES 1.00.17 spec
                 // Appendix A, section 7, the shader does not use too many uniforms.
-                success =
-                    packer.CheckVariablesWithinPackingLimits(maxUniformVectors, expandedUniforms);
+                success = CheckVariablesInPackingLimits(maxUniformVectors, uniforms);
                 if (!success)
                 {
                     mDiagnostics.globalError("too many uniforms");

src/compiler/translator/ShaderLang.cpp

@@ -448,8 +448,7 @@ int GetVertexShaderNumViews(const ShHandle handle)
 
 bool CheckVariablesWithinPackingLimits(int maxVectors, const std::vector<ShaderVariable> &variables)
 {
-    VariablePacker packer;
-    return packer.CheckVariablesWithinPackingLimits(maxVectors, variables);
+    return CheckVariablesInPackingLimits(maxVectors, variables);
 }
 
 bool GetUniformBlockRegister(const ShHandle handle,

src/compiler/translator/VariableInfo.cpp

@@ -49,24 +49,6 @@ BlockType GetBlockType(TQualifier qualifier)
     }
 }
 
-void ExpandUserDefinedVariable(const ShaderVariable &variable,
-                               const std::string &name,
-                               const std::string &mappedName,
-                               bool markStaticUse,
-                               std::vector<ShaderVariable> *expanded)
-{
-    ASSERT(variable.isStruct());
-
-    const std::vector<ShaderVariable> &fields = variable.fields;
-
-    for (size_t fieldIndex = 0; fieldIndex < fields.size(); fieldIndex++)
-    {
-        const ShaderVariable &field = fields[fieldIndex];
-        ExpandVariable(field, name + "." + field.name, mappedName + "." + field.mappedName,
-                       markStaticUse, expanded);
-    }
-}
-
 template <class VarT>
 VarT *FindVariable(const TString &name, std::vector<VarT> *infoList)
 {
@@ -716,58 +698,4 @@ void CollectVariables(TIntermBlock *root,
     root->traverse(&collect);
 }
 
-void ExpandVariable(const ShaderVariable &variable,
-                    const std::string &name,
-                    const std::string &mappedName,
-                    bool markStaticUse,
-                    std::vector<ShaderVariable> *expanded)
-{
-    if (variable.isStruct())
-    {
-        if (variable.isArray())
-        {
-            for (unsigned int elementIndex = 0; elementIndex < variable.elementCount();
-                 elementIndex++)
-            {
-                std::string lname       = name + ::ArrayString(elementIndex);
-                std::string lmappedName = mappedName + ::ArrayString(elementIndex);
-                ExpandUserDefinedVariable(variable, lname, lmappedName, markStaticUse, expanded);
-            }
-        }
-        else
-        {
-            ExpandUserDefinedVariable(variable, name, mappedName, markStaticUse, expanded);
-        }
-    }
-    else
-    {
-        ShaderVariable expandedVar = variable;
-
-        expandedVar.name       = name;
-        expandedVar.mappedName = mappedName;
-
-        // Mark all expanded fields as used if the parent is used
-        if (markStaticUse)
-        {
-            expandedVar.staticUse = true;
-        }
-
-        if (expandedVar.isArray())
-        {
-            expandedVar.name += "[0]";
-            expandedVar.mappedName += "[0]";
-        }
-
-        expanded->push_back(expandedVar);
-    }
-}
-
-void ExpandUniforms(const std::vector<Uniform> &compact, std::vector<ShaderVariable> *expanded)
-{
-    for (const Uniform &variable : compact)
-    {
-        ExpandVariable(variable, variable.name, variable.mappedName, variable.staticUse, expanded);
-    }
-}
-
 }  // namespace sh

src/compiler/translator/VariableInfo.h

@@ -29,15 +29,6 @@ void CollectVariables(TIntermBlock *root,
                       TSymbolTable *symbolTable,
                       int shaderVersion,
                       const TExtensionBehavior &extensionBehavior);
-
-void ExpandVariable(const ShaderVariable &variable,
-                    const std::string &name,
-                    const std::string &mappedName,
-                    bool markStaticUse,
-                    std::vector<ShaderVariable> *expanded);
-
-// Expand struct uniforms to flattened lists of split variables
-void ExpandUniforms(const std::vector<Uniform> &compact, std::vector<ShaderVariable> *expanded);
 }
 
 #endif  // COMPILER_TRANSLATOR_VARIABLEINFO_H_

src/compiler/translator/VariablePacker.h

@@ -3,39 +3,30 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
+// Check whether variables fit within packing limits according to the packing rules from the GLSL ES
+// 1.00.17 spec, Appendix A, section 7.
 
 #ifndef COMPILER_TRANSLATOR_VARIABLEPACKER_H_
 #define COMPILER_TRANSLATOR_VARIABLEPACKER_H_
 
 #include <vector>
-#include "compiler/translator/VariableInfo.h"
 
-class VariablePacker
+#include <GLSLANG/ShaderLang.h>
+
+namespace sh
 {
-  public:
-    // Returns true if the passed in variables pack in maxVectors following
-    // the packing rules from the GLSL 1.017 spec, Appendix A, section 7.
-    bool CheckVariablesWithinPackingLimits(unsigned int maxVectors,
-                                           const std::vector<sh::ShaderVariable> &in_variables);
-
-    // Gets how many components in a row a data type takes.
-    static int GetNumComponentsPerRow(sh::GLenum type);
-
-    // Gets how many rows a data type takes.
-    static int GetNumRows(sh::GLenum type);
-
-  private:
-    static const int kNumColumns      = 4;
-    static const unsigned kColumnMask = (1 << kNumColumns) - 1;
-
-    unsigned makeColumnFlags(int column, int numComponentsPerRow);
-    void fillColumns(int topRow, int numRows, int column, int numComponentsPerRow);
-    bool searchColumn(int column, int numRows, int *destRow, int *destSize);
-
-    int topNonFullRow_;
-    int bottomNonFullRow_;
-    int maxRows_;
-    std::vector<unsigned> rows_;
-};
+
+// Gets how many components in a row a data type takes.
+int GetVariablePackingComponentsPerRow(sh::GLenum type);
+
+// Gets how many rows a data type takes.
+int GetVariablePackingRows(sh::GLenum type);
+
+// Returns true if the passed in variables pack in maxVectors.
+// T should be ShaderVariable or one of the subclasses of ShaderVariable.
+template <typename T>
+bool CheckVariablesInPackingLimits(unsigned int maxVectors, const std::vector<T> &variables);
+
+}  // namespace sh
 
 #endif  // COMPILER_TRANSLATOR_VARIABLEPACKER_H_

src/tests/compiler_tests/VariablePacker_test.cpp

@@ -9,6 +9,9 @@
 #include "common/angleutils.h"
 #include "compiler/translator/VariablePacker.h"
 
+namespace
+{
+
 static sh::GLenum types[] = {
     GL_FLOAT_MAT4,                     // 0
     GL_FLOAT_MAT2,                     // 1
@@ -54,42 +57,38 @@ static sh::GLenum types[] = {
     GL_UNSIGNED_INT_SAMPLER_2D_ARRAY,  // 41
 };
 
-static sh::GLenum nonSqMatTypes[] = {
-  GL_FLOAT_MAT2x3,
-  GL_FLOAT_MAT2x4,
-  GL_FLOAT_MAT3x2,
-  GL_FLOAT_MAT3x4,
-  GL_FLOAT_MAT4x2,
-  GL_FLOAT_MAT4x3
-};
+static sh::GLenum nonSqMatTypes[] = {GL_FLOAT_MAT2x3, GL_FLOAT_MAT2x4, GL_FLOAT_MAT3x2,
+                                     GL_FLOAT_MAT3x4, GL_FLOAT_MAT4x2, GL_FLOAT_MAT4x3};
+
+}  // anonymous namespace
 
-TEST(VariablePacking, Pack) {
-    VariablePacker packer;
+TEST(VariablePacking, Pack)
+{
     std::vector<sh::ShaderVariable> vars;
     const int kMaxRows = 16;
     // test no vars.
-    EXPECT_TRUE(packer.CheckVariablesWithinPackingLimits(kMaxRows, vars));
+    EXPECT_TRUE(CheckVariablesInPackingLimits(kMaxRows, vars));
 
     for (size_t tt = 0; tt < ArraySize(types); ++tt)
     {
         sh::GLenum type            = types[tt];
-        int num_rows               = VariablePacker::GetNumRows(type);
-        int num_components_per_row = VariablePacker::GetNumComponentsPerRow(type);
+        int num_rows               = sh::GetVariablePackingRows(type);
+        int num_components_per_row = sh::GetVariablePackingComponentsPerRow(type);
         // Check 1 of the type.
         vars.clear();
         vars.push_back(sh::ShaderVariable(type, 0));
-        EXPECT_TRUE(packer.CheckVariablesWithinPackingLimits(kMaxRows, vars));
+        EXPECT_TRUE(CheckVariablesInPackingLimits(kMaxRows, vars));
 
         // Check exactly the right amount of 1 type as an array.
         int num_vars = kMaxRows / num_rows;
         vars.clear();
         vars.push_back(sh::ShaderVariable(type, num_vars == 1 ? 0 : num_vars));
-        EXPECT_TRUE(packer.CheckVariablesWithinPackingLimits(kMaxRows, vars));
+        EXPECT_TRUE(CheckVariablesInPackingLimits(kMaxRows, vars));
 
         // test too many
         vars.clear();
         vars.push_back(sh::ShaderVariable(type, num_vars == 0 ? 0 : (num_vars + 1)));
-        EXPECT_FALSE(packer.CheckVariablesWithinPackingLimits(kMaxRows, vars));
+        EXPECT_FALSE(CheckVariablesInPackingLimits(kMaxRows, vars));
 
         // Check exactly the right amount of 1 type as individual vars.
         num_vars =
@@ -99,11 +98,11 @@ TEST(VariablePacking, Pack) {
         {
             vars.push_back(sh::ShaderVariable(type, 0));
         }
-        EXPECT_TRUE(packer.CheckVariablesWithinPackingLimits(kMaxRows, vars));
+        EXPECT_TRUE(CheckVariablesInPackingLimits(kMaxRows, vars));
 
         // Check 1 too many.
         vars.push_back(sh::ShaderVariable(type, 0));
-        EXPECT_FALSE(packer.CheckVariablesWithinPackingLimits(kMaxRows, vars));
+        EXPECT_FALSE(CheckVariablesInPackingLimits(kMaxRows, vars));
     }
 
     // Test example from GLSL ES 3.0 spec chapter 11.
@@ -117,66 +116,70 @@ TEST(VariablePacking, Pack) {
     vars.push_back(sh::ShaderVariable(GL_FLOAT, 3));
     vars.push_back(sh::ShaderVariable(GL_FLOAT, 2));
     vars.push_back(sh::ShaderVariable(GL_FLOAT, 0));
-    EXPECT_TRUE(packer.CheckVariablesWithinPackingLimits(kMaxRows, vars));
+    EXPECT_TRUE(CheckVariablesInPackingLimits(kMaxRows, vars));
 }
 
-TEST(VariablePacking, PackSizes) {
-  for (size_t tt = 0; tt < ArraySize(types); ++tt) {
-    GLenum type = types[tt];
+TEST(VariablePacking, PackSizes)
+{
+    for (size_t tt = 0; tt < ArraySize(types); ++tt)
+    {
+        sh::GLenum type = types[tt];
 
-    int expectedComponents = gl::VariableComponentCount(type);
-    int expectedRows = gl::VariableRowCount(type);
+        int expectedComponents = gl::VariableComponentCount(type);
+        int expectedRows       = gl::VariableRowCount(type);
 
-    if (type == GL_FLOAT_MAT2) {
-      expectedComponents = 4;
-    } else if (gl::IsMatrixType(type)) {
-      int squareSize = std::max(gl::VariableRowCount(type),
-          gl::VariableColumnCount(type));
-      expectedComponents = squareSize;
-      expectedRows = squareSize;
-    }
+        if (type == GL_FLOAT_MAT2)
+        {
+            expectedComponents = 4;
+        }
+        else if (gl::IsMatrixType(type))
+        {
+            int squareSize = std::max(gl::VariableRowCount(type), gl::VariableColumnCount(type));
+            expectedComponents = squareSize;
+            expectedRows       = squareSize;
+        }
 
-    EXPECT_EQ(expectedComponents,
-      VariablePacker::GetNumComponentsPerRow(type));
-    EXPECT_EQ(expectedRows, VariablePacker::GetNumRows(type));
-  }
+        EXPECT_EQ(expectedComponents, sh::GetVariablePackingComponentsPerRow(type));
+        EXPECT_EQ(expectedRows, sh::GetVariablePackingRows(type));
+    }
 }
 
 // Check special assumptions about packing non-square mats
-TEST(VariablePacking, NonSquareMats) {
+TEST(VariablePacking, NonSquareMats)
+{
 
-  for (size_t mt = 0; mt < ArraySize(nonSqMatTypes); ++mt) {
-    
-    GLenum type = nonSqMatTypes[mt];
+    for (size_t mt = 0; mt < ArraySize(nonSqMatTypes); ++mt)
+    {
 
-    int rows = gl::VariableRowCount(type);
-    int cols = gl::VariableColumnCount(type);
-    int squareSize = std::max(rows, cols);
+        sh::GLenum type = nonSqMatTypes[mt];
 
-    std::vector<sh::ShaderVariable> vars;
-    vars.push_back(sh::ShaderVariable(type, 0));
+        int rows       = gl::VariableRowCount(type);
+        int cols       = gl::VariableColumnCount(type);
+        int squareSize = std::max(rows, cols);
 
-    // Fill columns
-    for (int row = 0; row < squareSize; row++) {
-      for (int col = squareSize; col < 4; ++col) {
-        vars.push_back(sh::ShaderVariable(GL_FLOAT, 0));
-      }
-    }
+        std::vector<sh::ShaderVariable> vars;
+        vars.push_back(sh::ShaderVariable(type, 0));
 
-    VariablePacker packer;
+        // Fill columns
+        for (int row = 0; row < squareSize; row++)
+        {
+            for (int col = squareSize; col < 4; ++col)
+            {
+                vars.push_back(sh::ShaderVariable(GL_FLOAT, 0));
+            }
+        }
 
-    EXPECT_TRUE(packer.CheckVariablesWithinPackingLimits(squareSize, vars));
+        EXPECT_TRUE(CheckVariablesInPackingLimits(squareSize, vars));
 
-    // and one scalar and packing should fail
-    vars.push_back(sh::ShaderVariable(GL_FLOAT, 0));
-    EXPECT_FALSE(packer.CheckVariablesWithinPackingLimits(squareSize, vars));
-  }
+        // and one scalar and packing should fail
+        vars.push_back(sh::ShaderVariable(GL_FLOAT, 0));
+        EXPECT_FALSE(CheckVariablesInPackingLimits(squareSize, vars));
+    }
 }
 
 // Scalar type variables can be packed sharing rows with other variables.
 TEST(VariablePacking, ReuseRows)
 {
-    VariablePacker packer;
     std::vector<sh::ShaderVariable> vars;
     const int kMaxRows = 512;
 
@@ -185,13 +188,13 @@ TEST(VariablePacking, ReuseRows)
     // uniform bool u2[129];
     // uniform bool u3[129];
     {
-        int num_arrays = 4;
+        int num_arrays             = 4;
         int num_elements_per_array = kMaxRows / num_arrays + 1;
         for (int ii = 0; ii < num_arrays; ++ii)
         {
             vars.push_back(sh::ShaderVariable(GL_BOOL, num_elements_per_array));
         }
-        EXPECT_TRUE(packer.CheckVariablesWithinPackingLimits(kMaxRows, vars));
+        EXPECT_TRUE(CheckVariablesInPackingLimits(kMaxRows, vars));
     }
 
     vars.clear();
@@ -203,14 +206,13 @@ TEST(VariablePacking, ReuseRows)
         vars.push_back(sh::ShaderVariable(GL_FLOAT_VEC2, num_elements_per_array));
         vars.push_back(sh::ShaderVariable(GL_FLOAT, num_elements_per_array));
         vars.push_back(sh::ShaderVariable(GL_INT, num_elements_per_array));
-        EXPECT_TRUE(packer.CheckVariablesWithinPackingLimits(kMaxRows, vars));
+        EXPECT_TRUE(CheckVariablesInPackingLimits(kMaxRows, vars));
     }
 }
 
 // Check the packer supports and flattens structures.
 TEST(VariablePacking, Struct)
 {
-    VariablePacker packer;
     std::vector<sh::ShaderVariable> fields;
     const int kMaxRows = 16;
 
@@ -233,5 +235,5 @@ TEST(VariablePacking, Struct)
     parentStruct.fields.push_back(sh::ShaderVariable(GL_FLOAT, 2));
     parentStruct.fields.push_back(sh::ShaderVariable(GL_FLOAT, 0));
 
-    EXPECT_TRUE(packer.CheckVariablesWithinPackingLimits(kMaxRows, vars));
+    EXPECT_TRUE(CheckVariablesInPackingLimits(kMaxRows, vars));
 }