Clean up checking variable packing limits
This encapsulates expanding struct variables inside the VariablePacker
class that packs variables according to the GLSL ES spec.
The variable expansion step is no longer run twice when checking
uniforms against the max uniforms limit.
BUG=angleproject:2068
TEST=angle_unittests
Change-Id: I012ddaa249f71c0a78d937c98007c61352e64888
Reviewed-on: https://chromium-review.googlesource.com/608367
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
diff --git a/src/compiler/translator/Compiler.cpp b/src/compiler/translator/Compiler.cpp
index 72a3259..8b7cb0e 100644
--- a/src/compiler/translator/Compiler.cpp
+++ b/src/compiler/translator/Compiler.cpp
@@ -484,13 +484,9 @@
}
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");
diff --git a/src/compiler/translator/ShaderLang.cpp b/src/compiler/translator/ShaderLang.cpp
index 07653c4..f9a5bb5 100644
--- a/src/compiler/translator/ShaderLang.cpp
+++ b/src/compiler/translator/ShaderLang.cpp
@@ -448,8 +448,7 @@
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,
diff --git a/src/compiler/translator/VariableInfo.cpp b/src/compiler/translator/VariableInfo.cpp
index 39baef9..f48abd3 100644
--- a/src/compiler/translator/VariableInfo.cpp
+++ b/src/compiler/translator/VariableInfo.cpp
@@ -49,24 +49,6 @@
}
}
-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 @@
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
diff --git a/src/compiler/translator/VariableInfo.h b/src/compiler/translator/VariableInfo.h
index 7dbaab7..7317414 100644
--- a/src/compiler/translator/VariableInfo.h
+++ b/src/compiler/translator/VariableInfo.h
@@ -29,15 +29,6 @@
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_
diff --git a/src/compiler/translator/VariablePacker.cpp b/src/compiler/translator/VariablePacker.cpp
index 9c4c937..1fa668b 100644
--- a/src/compiler/translator/VariablePacker.cpp
+++ b/src/compiler/translator/VariablePacker.cpp
@@ -3,6 +3,8 @@
// 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.
#include <algorithm>
@@ -11,62 +13,102 @@
#include "compiler/translator/VariablePacker.h"
#include "common/utilities.h"
-int VariablePacker::GetNumComponentsPerRow(sh::GLenum type)
+namespace sh
{
- switch (type)
+
+namespace
+{
+
+void ExpandVariable(const ShaderVariable &variable,
+ const std::string &name,
+ const std::string &mappedName,
+ bool markStaticUse,
+ std::vector<ShaderVariable> *expanded);
+
+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++)
{
- case GL_FLOAT_MAT4:
- case GL_FLOAT_MAT2:
- case GL_FLOAT_MAT2x4:
- case GL_FLOAT_MAT3x4:
- case GL_FLOAT_MAT4x2:
- case GL_FLOAT_MAT4x3:
- case GL_FLOAT_VEC4:
- case GL_INT_VEC4:
- case GL_BOOL_VEC4:
- case GL_UNSIGNED_INT_VEC4:
- return 4;
- case GL_FLOAT_MAT3:
- case GL_FLOAT_MAT2x3:
- case GL_FLOAT_MAT3x2:
- case GL_FLOAT_VEC3:
- case GL_INT_VEC3:
- case GL_BOOL_VEC3:
- case GL_UNSIGNED_INT_VEC3:
- return 3;
- case GL_FLOAT_VEC2:
- case GL_INT_VEC2:
- case GL_BOOL_VEC2:
- case GL_UNSIGNED_INT_VEC2:
- return 2;
- default:
- ASSERT(gl::VariableComponentCount(type) == 1);
- return 1;
+ const ShaderVariable &field = fields[fieldIndex];
+ ExpandVariable(field, name + "." + field.name, mappedName + "." + field.mappedName,
+ markStaticUse, expanded);
}
}
-int VariablePacker::GetNumRows(sh::GLenum type)
+void ExpandVariable(const ShaderVariable &variable,
+ const std::string &name,
+ const std::string &mappedName,
+ bool markStaticUse,
+ std::vector<ShaderVariable> *expanded)
{
- switch (type)
+ if (variable.isStruct())
{
- case GL_FLOAT_MAT4:
- case GL_FLOAT_MAT2x4:
- case GL_FLOAT_MAT3x4:
- case GL_FLOAT_MAT4x3:
- case GL_FLOAT_MAT4x2:
- return 4;
- case GL_FLOAT_MAT3:
- case GL_FLOAT_MAT2x3:
- case GL_FLOAT_MAT3x2:
- return 3;
- case GL_FLOAT_MAT2:
- return 2;
- default:
- ASSERT(gl::VariableRowCount(type) == 1);
- return 1;
+ 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);
}
}
+class VariablePacker
+{
+ public:
+ bool checkExpandedVariablesWithinPackingLimits(unsigned int maxVectors,
+ std::vector<sh::ShaderVariable> *variables);
+
+ 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_;
+};
+
struct TVariableInfoComparer
{
bool operator()(const sh::ShaderVariable &lhs, const sh::ShaderVariable &rhs) const
@@ -159,27 +201,21 @@
return true;
}
-bool VariablePacker::CheckVariablesWithinPackingLimits(
+bool VariablePacker::checkExpandedVariablesWithinPackingLimits(
unsigned int maxVectors,
- const std::vector<sh::ShaderVariable> &in_variables)
+ std::vector<sh::ShaderVariable> *variables)
{
ASSERT(maxVectors > 0);
maxRows_ = maxVectors;
topNonFullRow_ = 0;
bottomNonFullRow_ = maxRows_ - 1;
- 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++)
+ for (const sh::ShaderVariable &variable : *variables)
{
- const sh::ShaderVariable &variable = variables[i];
- if (variable.elementCount() > maxVectors / GetNumRows(variable.type))
+ // Structs should have been expanded before reaching here.
+ ASSERT(!variable.isStruct());
+ if (variable.elementCount() > maxVectors / GetVariablePackingRows(variable.type))
{
return false;
}
@@ -187,20 +223,20 @@
// As per GLSL 1.017 Appendix A, Section 7 variables are packed in specific
// order by type, then by size of array, largest first.
- std::sort(variables.begin(), variables.end(), TVariableInfoComparer());
+ std::sort(variables->begin(), variables->end(), TVariableInfoComparer());
rows_.clear();
rows_.resize(maxVectors, 0);
// Packs the 4 column variables.
size_t ii = 0;
- for (; ii < variables.size(); ++ii)
+ for (; ii < variables->size(); ++ii)
{
- const sh::ShaderVariable &variable = variables[ii];
- if (GetNumComponentsPerRow(variable.type) != 4)
+ const sh::ShaderVariable &variable = (*variables)[ii];
+ if (GetVariablePackingComponentsPerRow(variable.type) != 4)
{
break;
}
- topNonFullRow_ += GetNumRows(variable.type) * variable.elementCount();
+ topNonFullRow_ += GetVariablePackingRows(variable.type) * variable.elementCount();
}
if (topNonFullRow_ > maxRows_)
@@ -210,14 +246,14 @@
// Packs the 3 column variables.
int num3ColumnRows = 0;
- for (; ii < variables.size(); ++ii)
+ for (; ii < variables->size(); ++ii)
{
- const sh::ShaderVariable &variable = variables[ii];
- if (GetNumComponentsPerRow(variable.type) != 3)
+ const sh::ShaderVariable &variable = (*variables)[ii];
+ if (GetVariablePackingComponentsPerRow(variable.type) != 3)
{
break;
}
- num3ColumnRows += GetNumRows(variable.type) * variable.elementCount();
+ num3ColumnRows += GetVariablePackingRows(variable.type) * variable.elementCount();
}
if (topNonFullRow_ + num3ColumnRows > maxRows_)
@@ -232,14 +268,14 @@
int twoColumnRowsAvailable = maxRows_ - top2ColumnRow;
int rowsAvailableInColumns01 = twoColumnRowsAvailable;
int rowsAvailableInColumns23 = twoColumnRowsAvailable;
- for (; ii < variables.size(); ++ii)
+ for (; ii < variables->size(); ++ii)
{
- const sh::ShaderVariable &variable = variables[ii];
- if (GetNumComponentsPerRow(variable.type) != 2)
+ const sh::ShaderVariable &variable = (*variables)[ii];
+ if (GetVariablePackingComponentsPerRow(variable.type) != 2)
{
break;
}
- int numRows = GetNumRows(variable.type) * variable.elementCount();
+ int numRows = GetVariablePackingRows(variable.type) * variable.elementCount();
if (numRows <= rowsAvailableInColumns01)
{
rowsAvailableInColumns01 -= numRows;
@@ -260,11 +296,11 @@
fillColumns(maxRows_ - numRowsUsedInColumns23, numRowsUsedInColumns23, 2, 2);
// Packs the 1 column variables.
- for (; ii < variables.size(); ++ii)
+ for (; ii < variables->size(); ++ii)
{
- const sh::ShaderVariable &variable = variables[ii];
- ASSERT(1 == GetNumComponentsPerRow(variable.type));
- int numRows = GetNumRows(variable.type) * variable.elementCount();
+ const sh::ShaderVariable &variable = (*variables)[ii];
+ ASSERT(1 == GetVariablePackingComponentsPerRow(variable.type));
+ int numRows = GetVariablePackingRows(variable.type) * variable.elementCount();
int smallestColumn = -1;
int smallestSize = maxRows_ + 1;
int topRow = -1;
@@ -291,7 +327,86 @@
fillColumns(topRow, numRows, smallestColumn, 1);
}
- ASSERT(variables.size() == ii);
+ ASSERT(variables->size() == ii);
return true;
}
+
+} // anonymous namespace
+
+int GetVariablePackingComponentsPerRow(sh::GLenum type)
+{
+ switch (type)
+ {
+ case GL_FLOAT_MAT4:
+ case GL_FLOAT_MAT2:
+ case GL_FLOAT_MAT2x4:
+ case GL_FLOAT_MAT3x4:
+ case GL_FLOAT_MAT4x2:
+ case GL_FLOAT_MAT4x3:
+ case GL_FLOAT_VEC4:
+ case GL_INT_VEC4:
+ case GL_BOOL_VEC4:
+ case GL_UNSIGNED_INT_VEC4:
+ return 4;
+ case GL_FLOAT_MAT3:
+ case GL_FLOAT_MAT2x3:
+ case GL_FLOAT_MAT3x2:
+ case GL_FLOAT_VEC3:
+ case GL_INT_VEC3:
+ case GL_BOOL_VEC3:
+ case GL_UNSIGNED_INT_VEC3:
+ return 3;
+ case GL_FLOAT_VEC2:
+ case GL_INT_VEC2:
+ case GL_BOOL_VEC2:
+ case GL_UNSIGNED_INT_VEC2:
+ return 2;
+ default:
+ ASSERT(gl::VariableComponentCount(type) == 1);
+ return 1;
+ }
+}
+
+int GetVariablePackingRows(sh::GLenum type)
+{
+ switch (type)
+ {
+ case GL_FLOAT_MAT4:
+ case GL_FLOAT_MAT2x4:
+ case GL_FLOAT_MAT3x4:
+ case GL_FLOAT_MAT4x3:
+ case GL_FLOAT_MAT4x2:
+ return 4;
+ case GL_FLOAT_MAT3:
+ case GL_FLOAT_MAT2x3:
+ case GL_FLOAT_MAT3x2:
+ return 3;
+ case GL_FLOAT_MAT2:
+ return 2;
+ default:
+ ASSERT(gl::VariableRowCount(type) == 1);
+ return 1;
+ }
+}
+
+template <typename T>
+bool CheckVariablesInPackingLimits(unsigned int maxVectors, const std::vector<T> &variables)
+{
+ VariablePacker packer;
+ std::vector<sh::ShaderVariable> expandedVariables;
+ for (const ShaderVariable &variable : variables)
+ {
+ ExpandVariable(variable, variable.name, variable.mappedName, variable.staticUse,
+ &expandedVariables);
+ }
+ return packer.checkExpandedVariablesWithinPackingLimits(maxVectors, &expandedVariables);
+}
+
+template bool CheckVariablesInPackingLimits<ShaderVariable>(
+ unsigned int maxVectors,
+ const std::vector<ShaderVariable> &variables);
+template bool CheckVariablesInPackingLimits<Uniform>(unsigned int maxVectors,
+ const std::vector<Uniform> &variables);
+
+} // namespace sh
diff --git a/src/compiler/translator/VariablePacker.h b/src/compiler/translator/VariablePacker.h
index c6a3206..3bc73a0 100644
--- a/src/compiler/translator/VariablePacker.h
+++ b/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 components in a row a data type takes.
+int GetVariablePackingComponentsPerRow(sh::GLenum type);
- // Gets how many rows a data type takes.
- static int GetNumRows(sh::GLenum type);
+// Gets how many rows a data type takes.
+int GetVariablePackingRows(sh::GLenum type);
- private:
- static const int kNumColumns = 4;
- static const unsigned kColumnMask = (1 << kNumColumns) - 1;
+// 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);
- 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_;
-};
+} // namespace sh
#endif // COMPILER_TRANSLATOR_VARIABLEPACKER_H_