ShCheckVariablesWithinPackingLimits add sh::ShaderVariable overload

include/GLSLANG/ShaderLang.h

@@ -48,7 +48,7 @@ typedef unsigned int GLenum;
 
 // Version number for shader translation API.
 // It is incremented every time the API changes.
-#define ANGLE_SH_VERSION 147
+#define ANGLE_SH_VERSION 148
 
 typedef enum {
   SH_GLES2_SPEC = 0x8B40,
@@ -438,12 +438,16 @@ typedef struct
 // flag above.
 // Parameters:
 // maxVectors: the available rows of registers.
-// varInfoArray: an array of variable info (types and sizes).
+// varInfoArray / variables: an array of variables.
 // varInfoArraySize: the size of the variable array.
+// TODO(cwallez) remove the first overload once the second is rolled in Chromium
 COMPILER_EXPORT bool ShCheckVariablesWithinPackingLimits(
     int maxVectors,
     ShVariableInfo *varInfoArray,
     size_t varInfoArraySize);
+COMPILER_EXPORT bool ShCheckVariablesWithinPackingLimits(
+    int maxVectors,
+    const std::vector<sh::ShaderVariable> &variables);
 
 // Gives the compiler-assigned register for an interface block.
 // The method writes the value to the output variable "indexOut".

src/compiler/translator/ShaderLang.cpp

@@ -339,6 +339,11 @@ bool ShCheckVariablesWithinPackingLimits(
         sh::ShaderVariable var(varInfoArray[ii].type, varInfoArray[ii].size);
         variables.push_back(var);
     }
+    return ShCheckVariablesWithinPackingLimits(maxVectors, variables);
+}
+bool ShCheckVariablesWithinPackingLimits(int maxVectors,
+                                         const std::vector<sh::ShaderVariable> &variables)
+{
     VariablePacker packer;
     return packer.CheckVariablesWithinPackingLimits(maxVectors, variables);
 }
@@ -376,4 +381,4 @@ const std::map<std::string, unsigned int> *ShGetUniformRegisterMap(const ShHandl
     static std::map<std::string, unsigned int> map;
     return &map;
 #endif  // ANGLE_ENABLE_HLSL
-}
+}
\ No newline at end of file

src/compiler/translator/VariableInfo.cpp

@@ -31,58 +31,6 @@ void ExpandUserDefinedVariable(const ShaderVariable &variable,
                                const std::string &name,
                                const std::string &mappedName,
                                bool markStaticUse,
-                               std::vector<ShaderVariable> *expanded);
-
-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 ExpandUserDefinedVariable(const ShaderVariable &variable,
-                               const std::string &name,
-                               const std::string &mappedName,
-                               bool markStaticUse,
                                std::vector<ShaderVariable> *expanded)
 {
     ASSERT(variable.isStruct());
@@ -677,6 +625,52 @@ bool CollectVariables::visitBinary(Visit, TIntermBinary *binaryNode)
     return true;
 }
 
+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)
 {

src/compiler/translator/VariableInfo.h

@@ -69,6 +69,12 @@ class CollectVariables : public TIntermTraverser
     const TSymbolTable &mSymbolTable;
 };
 
+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);

src/compiler/translator/VariablePacker.cpp

@@ -148,15 +148,21 @@ bool VariablePacker::searchColumn(int column, int numRows, int* destRow, int* de
     return true;
 }
 
-template <typename VarT>
-bool VariablePacker::CheckVariablesWithinPackingLimits(unsigned int maxVectors,
-                                                       const std::vector<VarT> &in_variables)
+bool VariablePacker::CheckVariablesWithinPackingLimits(
+    unsigned int maxVectors,
+    const std::vector<sh::ShaderVariable> &in_variables)
 {
     ASSERT(maxVectors > 0);
     maxRows_ = maxVectors;
     topNonFullRow_ = 0;
     bottomNonFullRow_ = maxRows_ - 1;
-    std::vector<VarT> variables(in_variables);
+    std::vector<sh::ShaderVariable> variables;
+
+    for (const auto &variable : in_variables)
+    {
+        ExpandVariable(variable, variable.name, variable.mappedName, variable.staticUse,
+                       &variables);
+    }
 
     // Check whether each variable fits in the available vectors.
     for (size_t i = 0; i < variables.size(); i++) {
@@ -261,9 +267,3 @@ bool VariablePacker::CheckVariablesWithinPackingLimits(unsigned int maxVectors,
 
     return true;
 }
-
-// Instantiate all possible variable packings
-template bool VariablePacker::CheckVariablesWithinPackingLimits(unsigned int, const std::vector<sh::ShaderVariable> &);
-template bool VariablePacker::CheckVariablesWithinPackingLimits(unsigned int, const std::vector<sh::Attribute> &);
-template bool VariablePacker::CheckVariablesWithinPackingLimits(unsigned int, const std::vector<sh::Uniform> &);
-template bool VariablePacker::CheckVariablesWithinPackingLimits(unsigned int, const std::vector<sh::Varying> &);

src/compiler/translator/VariablePacker.h

@@ -12,30 +12,29 @@
 
 class VariablePacker {
  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.
-    template <typename VarT>
-    bool CheckVariablesWithinPackingLimits(unsigned int maxVectors,
-                                           const std::vector<VarT> &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_;
+   // 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_;
 };
 
 #endif // COMPILER_TRANSLATOR_VARIABLEPACKER_H_

src/tests/compiler_tests/VariablePacker_test.cpp

@@ -64,58 +64,60 @@ static sh::GLenum nonSqMatTypes[] = {
 };
 
 TEST(VariablePacking, Pack) {
-  VariablePacker packer;
-  std::vector<sh::ShaderVariable> vars;
-  const int kMaxRows = 16;
-  // test no vars.
-  EXPECT_TRUE(packer.CheckVariablesWithinPackingLimits(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);
-    // Check 1 of the type.
-    vars.clear();
-    vars.push_back(sh::ShaderVariable(type, 0));
+    VariablePacker packer;
+    std::vector<sh::ShaderVariable> vars;
+    const int kMaxRows = 16;
+    // test no vars.
     EXPECT_TRUE(packer.CheckVariablesWithinPackingLimits(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));
+    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);
+        // Check 1 of the type.
+        vars.clear();
+        vars.push_back(sh::ShaderVariable(type, 0));
+        EXPECT_TRUE(packer.CheckVariablesWithinPackingLimits(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));
+        // 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));
 
-    // Check exactly the right amount of 1 type as individual vars.
-    num_vars = kMaxRows / num_rows *
-        ((num_components_per_row > 2) ? 1 : (4 / num_components_per_row));
-    vars.clear();
-    for (int ii = 0; ii < num_vars; ++ii) {
-      vars.push_back(sh::ShaderVariable(type, 0));
-    }
-    EXPECT_TRUE(packer.CheckVariablesWithinPackingLimits(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));
 
-    // Check 1 too many.
-    vars.push_back(sh::ShaderVariable(type, 0));
-    EXPECT_FALSE(packer.CheckVariablesWithinPackingLimits(kMaxRows, vars));
-  }
+        // Check exactly the right amount of 1 type as individual vars.
+        num_vars =
+            kMaxRows / num_rows * ((num_components_per_row > 2) ? 1 : (4 / num_components_per_row));
+        vars.clear();
+        for (int ii = 0; ii < num_vars; ++ii)
+        {
+            vars.push_back(sh::ShaderVariable(type, 0));
+        }
+        EXPECT_TRUE(packer.CheckVariablesWithinPackingLimits(kMaxRows, vars));
 
-  // Test example from GLSL ES 3.0 spec chapter 11.
-  vars.clear();
-  vars.push_back(sh::ShaderVariable(GL_FLOAT_VEC4, 0));
-  vars.push_back(sh::ShaderVariable(GL_FLOAT_MAT3, 0));
-  vars.push_back(sh::ShaderVariable(GL_FLOAT_MAT3, 0));
-  vars.push_back(sh::ShaderVariable(GL_FLOAT_VEC2, 6));
-  vars.push_back(sh::ShaderVariable(GL_FLOAT_VEC2, 4));
-  vars.push_back(sh::ShaderVariable(GL_FLOAT_VEC2, 0));
-  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));
+        // Check 1 too many.
+        vars.push_back(sh::ShaderVariable(type, 0));
+        EXPECT_FALSE(packer.CheckVariablesWithinPackingLimits(kMaxRows, vars));
+    }
+
+    // Test example from GLSL ES 3.0 spec chapter 11.
+    vars.clear();
+    vars.push_back(sh::ShaderVariable(GL_FLOAT_VEC4, 0));
+    vars.push_back(sh::ShaderVariable(GL_FLOAT_MAT3, 0));
+    vars.push_back(sh::ShaderVariable(GL_FLOAT_MAT3, 0));
+    vars.push_back(sh::ShaderVariable(GL_FLOAT_VEC2, 6));
+    vars.push_back(sh::ShaderVariable(GL_FLOAT_VEC2, 4));
+    vars.push_back(sh::ShaderVariable(GL_FLOAT_VEC2, 0));
+    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));
 }
 
 TEST(VariablePacking, PackSizes) {
@@ -204,3 +206,32 @@ TEST(VariablePacking, ReuseRows)
         EXPECT_TRUE(packer.CheckVariablesWithinPackingLimits(kMaxRows, vars));
     }
 }
+
+// Check the packer supports and flattens structures.
+TEST(VariablePacking, Struct)
+{
+    VariablePacker packer;
+    std::vector<sh::ShaderVariable> fields;
+    const int kMaxRows = 16;
+
+    // Test example from GLSL ES 3.0 spec chapter 11, but with structs
+    std::vector<sh::ShaderVariable> vars;
+    vars.push_back(sh::ShaderVariable(GL_STRUCT_ANGLEX, 0));
+
+    sh::ShaderVariable &parentStruct = vars[0];
+    parentStruct.fields.push_back(sh::ShaderVariable(GL_FLOAT_VEC4, 0));
+    parentStruct.fields.push_back(sh::ShaderVariable(GL_FLOAT_MAT3, 0));
+
+    parentStruct.fields.push_back(sh::ShaderVariable(GL_STRUCT_ANGLEX, 0));
+    sh::ShaderVariable &innerStruct = parentStruct.fields.back();
+    innerStruct.fields.push_back(sh::ShaderVariable(GL_FLOAT_MAT3, 0));
+    innerStruct.fields.push_back(sh::ShaderVariable(GL_FLOAT_VEC2, 6));
+    innerStruct.fields.push_back(sh::ShaderVariable(GL_FLOAT_VEC2, 4));
+
+    parentStruct.fields.push_back(sh::ShaderVariable(GL_FLOAT_VEC2, 0));
+    parentStruct.fields.push_back(sh::ShaderVariable(GL_FLOAT, 3));
+    parentStruct.fields.push_back(sh::ShaderVariable(GL_FLOAT, 2));
+    parentStruct.fields.push_back(sh::ShaderVariable(GL_FLOAT, 0));
+
+    EXPECT_TRUE(packer.CheckVariablesWithinPackingLimits(kMaxRows, vars));
+}