Store shader interface variables as per query spec
GLES 3.1 section 7.3.1.1 specifies how active variable entries should
be generated and how active variables are named. The specs for program
interface variable queries are built on top of this section.
ANGLE has already followed this spec for the most part for generating
variable lists in ProgramState, but now we also follow the naming spec
for arrays and include [0] at the end of the stored name.
This will make implementing arrays of arrays more straightforward.
Most logic for variable queries will just keep working as is when
arrays of arrays are added instead of needing more complex logic for
handling array indexing.
BUG=angleproject:2125
TEST=angle_end2end_tests
Change-Id: I3acd14253153e10bc312114b0303065da2efb506
Reviewed-on: https://chromium-review.googlesource.com/739826
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/common/string_utils.cpp b/src/common/string_utils.cpp
index f7564db..26f384b 100644
--- a/src/common/string_utils.cpp
+++ b/src/common/string_utils.cpp
@@ -158,6 +158,11 @@
return Optional<std::vector<wchar_t>>(wcstring);
}
+bool BeginsWith(const std::string &str, const std::string &prefix)
+{
+ return strncmp(str.c_str(), prefix.c_str(), prefix.length()) == 0;
+}
+
bool BeginsWith(const std::string &str, const char *prefix)
{
return strncmp(str.c_str(), prefix, strlen(prefix)) == 0;
@@ -168,6 +173,11 @@
return strncmp(str, prefix, strlen(prefix)) == 0;
}
+bool BeginsWith(const std::string &str, const std::string &prefix, const size_t prefixLength)
+{
+ return strncmp(str.c_str(), prefix.c_str(), prefixLength) == 0;
+}
+
bool EndsWith(const std::string &str, const char *suffix)
{
const auto len = strlen(suffix);
diff --git a/src/common/string_utils.h b/src/common/string_utils.h
index 4437fb3..07bf1ff 100644
--- a/src/common/string_utils.h
+++ b/src/common/string_utils.h
@@ -49,6 +49,10 @@
Optional<std::vector<wchar_t>> WidenString(size_t length, const char *cString);
// Check if the string str begins with the given prefix.
+// The comparison is case sensitive.
+bool BeginsWith(const std::string &str, const std::string &prefix);
+
+// Check if the string str begins with the given prefix.
// Prefix may not be NULL and needs to be NULL terminated.
// The comparison is case sensitive.
bool BeginsWith(const std::string &str, const char *prefix);
@@ -58,6 +62,11 @@
// The comparison is case sensitive.
bool BeginsWith(const char *str, const char *prefix);
+// Check if the string str begins with the first prefixLength characters of the given prefix.
+// The length of the prefix string should be greater than or equal to prefixLength.
+// The comparison is case sensitive.
+bool BeginsWith(const std::string &str, const std::string &prefix, const size_t prefixLength);
+
// Check if the string str ends with the given suffix.
// Suffix may not be NUL and needs to be NULL terminated.
// The comparison is case sensitive.
diff --git a/src/common/string_utils_unittest.cpp b/src/common/string_utils_unittest.cpp
index e7ce8a6..04e4b1f 100644
--- a/src/common/string_utils_unittest.cpp
+++ b/src/common/string_utils_unittest.cpp
@@ -138,18 +138,73 @@
// Note: ReadFileToString is harder to test
-
-TEST(StringUtilsTest, BeginsEndsWith)
+class BeginsWithTest : public testing::Test
{
- ASSERT_FALSE(BeginsWith("foo", "bar"));
- ASSERT_FALSE(BeginsWith("", "foo"));
- ASSERT_FALSE(BeginsWith("foo", "foobar"));
+ public:
+ BeginsWithTest() : mMode(TestMode::CHAR_ARRAY) {}
- ASSERT_TRUE(BeginsWith("foobar", "foo"));
- ASSERT_TRUE(BeginsWith("foobar", ""));
- ASSERT_TRUE(BeginsWith("foo", "foo"));
- ASSERT_TRUE(BeginsWith("", ""));
+ enum class TestMode
+ {
+ CHAR_ARRAY,
+ STRING_AND_CHAR_ARRAY,
+ STRING
+ };
+ void setMode(TestMode mode) { mMode = mode; }
+
+ bool runBeginsWith(const char *str, const char *prefix)
+ {
+ if (mMode == TestMode::CHAR_ARRAY)
+ {
+ return BeginsWith(str, prefix);
+ }
+ if (mMode == TestMode::STRING_AND_CHAR_ARRAY)
+ {
+ return BeginsWith(std::string(str), prefix);
+ }
+ return BeginsWith(std::string(str), std::string(prefix));
+ }
+
+ void runTest()
+ {
+ ASSERT_FALSE(runBeginsWith("foo", "bar"));
+ ASSERT_FALSE(runBeginsWith("", "foo"));
+ ASSERT_FALSE(runBeginsWith("foo", "foobar"));
+
+ ASSERT_TRUE(runBeginsWith("foobar", "foo"));
+ ASSERT_TRUE(runBeginsWith("foobar", ""));
+ ASSERT_TRUE(runBeginsWith("foo", "foo"));
+ ASSERT_TRUE(runBeginsWith("", ""));
+ }
+
+ private:
+ TestMode mMode;
+};
+
+// Test that BeginsWith works correctly for const char * arguments.
+TEST_F(BeginsWithTest, CharArrays)
+{
+ setMode(TestMode::CHAR_ARRAY);
+ runTest();
+}
+
+// Test that BeginsWith works correctly for std::string and const char * arguments.
+TEST_F(BeginsWithTest, StringAndCharArray)
+{
+ setMode(TestMode::STRING_AND_CHAR_ARRAY);
+ runTest();
+}
+
+// Test that BeginsWith works correctly for std::string arguments.
+TEST_F(BeginsWithTest, Strings)
+{
+ setMode(TestMode::STRING);
+ runTest();
+}
+
+// Test that EndsWith works correctly.
+TEST(EndsWithTest, EndsWith)
+{
ASSERT_FALSE(EndsWith("foo", "bar"));
ASSERT_FALSE(EndsWith("", "bar"));
ASSERT_FALSE(EndsWith("foo", "foobar"));
@@ -160,4 +215,4 @@
ASSERT_TRUE(EndsWith("", ""));
}
-}
\ No newline at end of file
+} // anonymous namespace
diff --git a/src/common/utilities.cpp b/src/common/utilities.cpp
index efb5323..677e800 100644
--- a/src/common/utilities.cpp
+++ b/src/common/utilities.cpp
@@ -770,19 +770,33 @@
return name.substr(0, baseNameLength);
}
-unsigned int ParseAndStripArrayIndex(std::string *name)
+unsigned int ParseArrayIndex(const std::string &name, size_t *nameLengthWithoutArrayIndexOut)
{
+ ASSERT(nameLengthWithoutArrayIndexOut != nullptr);
unsigned int subscript = GL_INVALID_INDEX;
// Strip any trailing array operator and retrieve the subscript
- size_t open = name->find_last_of('[');
- size_t close = name->find_last_of(']');
- if (open != std::string::npos && close == name->length() - 1)
+ size_t open = name.find_last_of('[');
+ if (open != std::string::npos && name.back() == ']')
{
- subscript = atoi(name->c_str() + open + 1);
- name->erase(open);
+ bool indexIsValidDecimalNumber = true;
+ for (size_t i = open + 1; i < name.length() - 1u; ++i)
+ {
+ if (!isdigit(name[i]))
+ {
+ indexIsValidDecimalNumber = false;
+ break;
+ }
+ }
+ if (indexIsValidDecimalNumber)
+ {
+ subscript = atoi(name.c_str() + open + 1);
+ *nameLengthWithoutArrayIndexOut = open;
+ return subscript;
+ }
}
+ *nameLengthWithoutArrayIndexOut = name.length();
return subscript;
}
diff --git a/src/common/utilities.h b/src/common/utilities.h
index e26f781..db0624f 100644
--- a/src/common/utilities.h
+++ b/src/common/utilities.h
@@ -70,7 +70,10 @@
bool IsTriangleMode(GLenum drawMode);
bool IsIntegerFormat(GLenum unsizedFormat);
-unsigned int ParseAndStripArrayIndex(std::string *name);
+// Return the array index at the end of name, and write the length of name before the final array
+// index into nameLengthWithoutArrayIndexOut. In case name doesn't include an array index, return
+// GL_INVALID_INDEX and write the length of the original string.
+unsigned int ParseArrayIndex(const std::string &name, size_t *nameLengthWithoutArrayIndexOut);
struct UniformTypeInfo final : angle::NonCopyable
{
diff --git a/src/common/utilities_unittest.cpp b/src/common/utilities_unittest.cpp
index 7447ae4..571c578 100644
--- a/src/common/utilities_unittest.cpp
+++ b/src/common/utilities_unittest.cpp
@@ -75,4 +75,97 @@
EXPECT_TRUE(indices.empty());
}
+// Parse a string without any index.
+TEST(ParseArrayIndex, NoArrayIndex)
+{
+ size_t nameLengthWithoutArrayIndex;
+ EXPECT_EQ(GL_INVALID_INDEX, gl::ParseArrayIndex("foo", &nameLengthWithoutArrayIndex));
+ EXPECT_EQ(3u, nameLengthWithoutArrayIndex);
}
+
+// Parse an empty string for an array index.
+TEST(ParseArrayIndex, EmptyString)
+{
+ size_t nameLengthWithoutArrayIndex;
+ EXPECT_EQ(GL_INVALID_INDEX, gl::ParseArrayIndex("", &nameLengthWithoutArrayIndex));
+ EXPECT_EQ(0u, nameLengthWithoutArrayIndex);
+}
+
+// A valid array index is parsed correctly from the end of the string.
+TEST(ParseArrayIndex, ArrayIndex)
+{
+ size_t nameLengthWithoutArrayIndex;
+ EXPECT_EQ(123u, gl::ParseArrayIndex("foo[123]", &nameLengthWithoutArrayIndex));
+ EXPECT_EQ(3u, nameLengthWithoutArrayIndex);
+}
+
+// An array index from the middle of the string is not parsed.
+TEST(ParseArrayIndex, ArrayIndexInMiddle)
+{
+ size_t nameLengthWithoutArrayIndex;
+ EXPECT_EQ(GL_INVALID_INDEX, gl::ParseArrayIndex("foo[123].bar", &nameLengthWithoutArrayIndex));
+ EXPECT_EQ(12u, nameLengthWithoutArrayIndex);
+}
+
+// Trailing whitespace in the parsed string is taken into account.
+TEST(ParseArrayIndex, TrailingWhitespace)
+{
+ size_t nameLengthWithoutArrayIndex;
+ EXPECT_EQ(GL_INVALID_INDEX, gl::ParseArrayIndex("foo[123] ", &nameLengthWithoutArrayIndex));
+ EXPECT_EQ(9u, nameLengthWithoutArrayIndex);
+}
+
+// Only the last index is parsed.
+TEST(ParseArrayIndex, MultipleArrayIndices)
+{
+ size_t nameLengthWithoutArrayIndex;
+ EXPECT_EQ(34u, gl::ParseArrayIndex("foo[12][34]", &nameLengthWithoutArrayIndex));
+ EXPECT_EQ(7u, nameLengthWithoutArrayIndex);
+}
+
+// GetProgramResourceLocation spec in GLES 3.1 November 2016 page 87 mentions "decimal" integer.
+// So an integer in hexadecimal format should not parse as an array index.
+TEST(ParseArrayIndex, HexArrayIndex)
+{
+ size_t nameLengthWithoutArrayIndex;
+ EXPECT_EQ(GL_INVALID_INDEX, gl::ParseArrayIndex("foo[0xff]", &nameLengthWithoutArrayIndex));
+ EXPECT_EQ(9u, nameLengthWithoutArrayIndex);
+}
+
+// GetProgramResourceLocation spec in GLES 3.1 November 2016 page 87 mentions that the array
+// index should not contain a leading plus sign.
+TEST(ParseArrayIndex, ArrayIndexLeadingPlus)
+{
+ size_t nameLengthWithoutArrayIndex;
+ EXPECT_EQ(GL_INVALID_INDEX, gl::ParseArrayIndex("foo[+1]", &nameLengthWithoutArrayIndex));
+ EXPECT_EQ(7u, nameLengthWithoutArrayIndex);
+}
+
+// GetProgramResourceLocation spec in GLES 3.1 November 2016 page 87 says that index should not
+// contain whitespace. Test leading whitespace.
+TEST(ParseArrayIndex, ArrayIndexLeadingWhiteSpace)
+{
+ size_t nameLengthWithoutArrayIndex;
+ EXPECT_EQ(GL_INVALID_INDEX, gl::ParseArrayIndex("foo[ 0]", &nameLengthWithoutArrayIndex));
+ EXPECT_EQ(7u, nameLengthWithoutArrayIndex);
+}
+
+// GetProgramResourceLocation spec in GLES 3.1 November 2016 page 87 says that index should not
+// contain whitespace. Test trailing whitespace.
+TEST(ParseArrayIndex, ArrayIndexTrailingWhiteSpace)
+{
+ size_t nameLengthWithoutArrayIndex;
+ EXPECT_EQ(GL_INVALID_INDEX, gl::ParseArrayIndex("foo[0 ]", &nameLengthWithoutArrayIndex));
+ EXPECT_EQ(7u, nameLengthWithoutArrayIndex);
+}
+
+// GetProgramResourceLocation spec in GLES 3.1 November 2016 page 87 says that index should only
+// contain an integer.
+TEST(ParseArrayIndex, ArrayIndexBogus)
+{
+ size_t nameLengthWithoutArrayIndex;
+ EXPECT_EQ(GL_INVALID_INDEX, gl::ParseArrayIndex("foo[0bogus]", &nameLengthWithoutArrayIndex));
+ EXPECT_EQ(11u, nameLengthWithoutArrayIndex);
+}
+
+} // anonymous namespace
diff --git a/src/libANGLE/MemoryProgramCache.cpp b/src/libANGLE/MemoryProgramCache.cpp
index c28e1be..1c0a453 100644
--- a/src/libANGLE/MemoryProgramCache.cpp
+++ b/src/libANGLE/MemoryProgramCache.cpp
@@ -316,15 +316,15 @@
}
unsigned int outputVarCount = stream.readInt<unsigned int>();
+ ASSERT(state->mOutputLocations.empty());
for (unsigned int outputIndex = 0; outputIndex < outputVarCount; ++outputIndex)
{
- int locationIndex = stream.readInt<int>();
VariableLocation locationData;
stream.readIntVector<unsigned int>(&locationData.arrayIndices);
stream.readInt(&locationData.index);
stream.readInt(&locationData.flattenedArrayOffset);
stream.readBool(&locationData.ignored);
- state->mOutputLocations[locationIndex] = locationData;
+ state->mOutputLocations.push_back(locationData);
}
unsigned int outputTypeCount = stream.readInt<unsigned int>();
@@ -481,13 +481,12 @@
}
stream.writeInt(state.getOutputLocations().size());
- for (const auto &outputPair : state.getOutputLocations())
+ for (const auto &outputVar : state.getOutputLocations())
{
- stream.writeInt(outputPair.first);
- stream.writeIntVector(outputPair.second.arrayIndices);
- stream.writeIntOrNegOne(outputPair.second.index);
- stream.writeInt(outputPair.second.flattenedArrayOffset);
- stream.writeInt(outputPair.second.ignored);
+ stream.writeIntVector(outputVar.arrayIndices);
+ stream.writeIntOrNegOne(outputVar.index);
+ stream.writeInt(outputVar.flattenedArrayOffset);
+ stream.writeInt(outputVar.ignored);
}
stream.writeInt(state.mOutputVariableTypes.size());
diff --git a/src/libANGLE/Program.cpp b/src/libANGLE/Program.cpp
index f36301e..286aedc 100644
--- a/src/libANGLE/Program.cpp
+++ b/src/libANGLE/Program.cpp
@@ -14,6 +14,7 @@
#include "common/bitset_utils.h"
#include "common/debug.h"
#include "common/platform.h"
+#include "common/string_utils.h"
#include "common/utilities.h"
#include "compiler/translator/blocklayout.h"
#include "libANGLE/Context.h"
@@ -140,35 +141,73 @@
template <typename VarT>
GLuint GetResourceIndexFromName(const std::vector<VarT> &list, const std::string &name)
{
- std::vector<unsigned int> subscripts;
- std::string baseName = ParseResourceName(name, &subscripts);
-
- // The app is not allowed to specify array indices other than 0 for arrays of basic types
- for (unsigned int subscript : subscripts)
- {
- if (subscript != 0u)
- {
- return GL_INVALID_INDEX;
- }
- }
-
+ std::string nameAsArrayName = name + "[0]";
for (size_t index = 0; index < list.size(); index++)
{
const VarT &resource = list[index];
- if (resource.name == baseName)
+ if (resource.name == name || (resource.isArray() && resource.name == nameAsArrayName))
{
- // TODO(oetuaho@nvidia.com): Check array nesting >= number of specified
- // subscripts once arrays of arrays are supported in ShaderVariable.
- if ((resource.isArray() || subscripts.empty()) && subscripts.size() <= 1u)
- {
- return static_cast<GLuint>(index);
- }
+ return static_cast<GLuint>(index);
}
}
return GL_INVALID_INDEX;
}
+template <typename VarT>
+GLint GetVariableLocation(const std::vector<VarT> &list,
+ const std::vector<VariableLocation> &locationList,
+ const std::string &name)
+{
+ size_t nameLengthWithoutArrayIndex;
+ unsigned int arrayIndex = ParseArrayIndex(name, &nameLengthWithoutArrayIndex);
+
+ for (size_t location = 0u; location < locationList.size(); ++location)
+ {
+ const VariableLocation &variableLocation = locationList[location];
+ if (!variableLocation.used())
+ {
+ continue;
+ }
+
+ const VarT &variable = list[variableLocation.index];
+
+ if (angle::BeginsWith(variable.name, name))
+ {
+ if (name.length() == variable.name.length())
+ {
+ ASSERT(name == variable.name);
+ // GLES 3.1 November 2016 page 87.
+ // The string exactly matches the name of the active variable.
+ return static_cast<GLint>(location);
+ }
+ if (name.length() + 3u == variable.name.length() && variable.isArray())
+ {
+ ASSERT(name + "[0]" == variable.name);
+ // The string identifies the base name of an active array, where the string would
+ // exactly match the name of the variable if the suffix "[0]" were appended to the
+ // string.
+ return static_cast<GLint>(location);
+ }
+ }
+ if (variable.isArray() && variableLocation.arrayIndices[0] == arrayIndex &&
+ nameLengthWithoutArrayIndex + 3u == variable.name.length() &&
+ angle::BeginsWith(variable.name, name, nameLengthWithoutArrayIndex))
+ {
+ ASSERT(name.substr(0u, nameLengthWithoutArrayIndex) + "[0]" == variable.name);
+ // The string identifies an active element of the array, where the string ends with the
+ // concatenation of the "[" character, an integer (with no "+" sign, extra leading
+ // zeroes, or whitespace) identifying an array element, and the "]" character, the
+ // integer is less than the number of active elements of the array variable, and where
+ // the string would exactly match the enumerated name of the array if the decimal
+ // integer were replaced with zero.
+ return static_cast<GLint>(location);
+ }
+ }
+
+ return -1;
+}
+
void CopyStringToBuffer(GLchar *buffer, const std::string &string, GLsizei bufSize, GLsizei *length)
{
ASSERT(bufSize > 0);
@@ -361,44 +400,6 @@
return mLabel;
}
-GLint ProgramState::getUniformLocation(const std::string &name) const
-{
- std::vector<unsigned int> subscripts;
- std::string baseName = ParseResourceName(name, &subscripts);
-
- for (size_t location = 0; location < mUniformLocations.size(); ++location)
- {
- const VariableLocation &uniformLocation = mUniformLocations[location];
- if (!uniformLocation.used())
- {
- continue;
- }
-
- const LinkedUniform &uniform = mUniforms[uniformLocation.index];
-
- if (uniform.name == baseName)
- {
- if (uniform.isArray())
- {
- if (uniformLocation.arrayIndices == subscripts ||
- (uniformLocation.areAllArrayIndicesZero() && subscripts.empty()))
- {
- return static_cast<GLint>(location);
- }
- }
- else
- {
- if (subscripts.empty())
- {
- return static_cast<GLint>(location);
- }
- }
- }
- }
-
- return -1;
-}
-
GLuint ProgramState::getUniformIndexFromName(const std::string &name) const
{
return GetResourceIndexFromName(mUniforms, name);
@@ -576,8 +577,7 @@
void Program::bindUniformLocation(GLuint index, const char *name)
{
- // Bind the base uniform name only since array indices other than 0 cannot be bound
- mUniformLocationBindings.bindLocation(index, ParseResourceName(name, nullptr));
+ mUniformLocationBindings.bindLocation(index, name);
}
void Program::bindFragmentInputLocation(GLint index, const char *name)
@@ -604,12 +604,13 @@
ret.valid = true;
- std::string originalName = binding.first;
- unsigned int arrayIndex = ParseAndStripArrayIndex(&originalName);
+ size_t nameLengthWithoutArrayIndex;
+ unsigned int arrayIndex = ParseArrayIndex(binding.first, &nameLengthWithoutArrayIndex);
for (const auto &in : inputs)
{
- if (in.name == originalName)
+ if (in.name.length() == nameLengthWithoutArrayIndex &&
+ angle::BeginsWith(in.name, binding.first, nameLengthWithoutArrayIndex))
{
if (in.isArray())
{
@@ -1133,15 +1134,7 @@
GLuint Program::getInputResourceIndex(const GLchar *name) const
{
- for (GLuint attributeIndex = 0; attributeIndex < mState.mAttributes.size(); ++attributeIndex)
- {
- const sh::Attribute &attribute = mState.mAttributes[attributeIndex];
- if (attribute.name == name)
- {
- return attributeIndex;
- }
- }
- return GL_INVALID_INDEX;
+ return GetResourceIndexFromName(mState.mAttributes, std::string(name));
}
GLuint Program::getOutputResourceIndex(const GLchar *name) const
@@ -1179,9 +1172,7 @@
if (bufSize > 0)
{
- std::string nameWithArray = (resource.isArray() ? resource.name + "[0]" : resource.name);
-
- CopyStringToBuffer(name, nameWithArray, bufSize, length);
+ CopyStringToBuffer(name, resource.name, bufSize, length);
}
}
@@ -1223,21 +1214,7 @@
GLint Program::getFragDataLocation(const std::string &name) const
{
- std::string baseName(name);
- unsigned int arrayIndex = ParseAndStripArrayIndex(&baseName);
- for (auto outputPair : mState.mOutputLocations)
- {
- const VariableLocation &locationInfo = outputPair.second;
- const sh::OutputVariable &outputVariable = mState.mOutputVariables[locationInfo.index];
- ASSERT(locationInfo.arrayIndices.size() <= 1);
- if (outputVariable.name == baseName &&
- (arrayIndex == GL_INVALID_INDEX || (!locationInfo.arrayIndices.empty() &&
- arrayIndex == locationInfo.arrayIndices.back())))
- {
- return static_cast<GLint>(outputPair.first);
- }
- }
- return -1;
+ return GetVariableLocation(mState.mOutputVariables, mState.mOutputLocations, name);
}
void Program::getActiveUniform(GLuint index,
@@ -1256,10 +1233,6 @@
if (bufsize > 0)
{
std::string string = uniform.name;
- if (uniform.isArray())
- {
- string += "[0]";
- }
CopyStringToBuffer(name, string, bufsize, length);
}
@@ -1350,7 +1323,7 @@
GLint Program::getUniformLocation(const std::string &name) const
{
- return mState.getUniformLocation(name);
+ return GetVariableLocation(mState.mUniforms, mState.mUniformLocations, name);
}
GLuint Program::getUniformIndex(const std::string &name) const
@@ -2075,6 +2048,11 @@
// Link attributes that have a binding location
for (sh::Attribute &attribute : mState.mAttributes)
{
+ // GLSL ES 3.10 January 2016 section 4.3.4: Vertex shader inputs can't be arrays or
+ // structures, so we don't need to worry about adjusting their names or generating entries
+ // for each member/element (unlike uniforms for example).
+ ASSERT(!attribute.isArray() && !attribute.isStruct());
+
int bindingLocation = mAttributeBindings.getBinding(attribute.name);
if (attribute.location == -1 && bindingLocation != -1)
{
@@ -2772,18 +2750,32 @@
{
const sh::OutputVariable &outputVariable = mState.mOutputVariables[outputVariableIndex];
+ if (outputVariable.isArray())
+ {
+ // We're following the GLES 3.1 November 2016 spec section 7.3.1.1 Naming Active
+ // Resources and including [0] at the end of array variable names.
+ mState.mOutputVariables[outputVariableIndex].name += "[0]";
+ mState.mOutputVariables[outputVariableIndex].mappedName += "[0]";
+ }
+
// Don't store outputs for gl_FragDepth, gl_FragColor, etc.
if (outputVariable.isBuiltIn())
continue;
// Since multiple output locations must be specified, use 0 for non-specified locations.
- int baseLocation = (outputVariable.location == -1 ? 0 : outputVariable.location);
+ unsigned int baseLocation =
+ (outputVariable.location == -1 ? 0u
+ : static_cast<unsigned int>(outputVariable.location));
for (unsigned int elementIndex = 0; elementIndex < outputVariable.elementCount();
elementIndex++)
{
- const int location = baseLocation + elementIndex;
- ASSERT(mState.mOutputLocations.count(location) == 0);
+ const unsigned int location = baseLocation + elementIndex;
+ if (location >= mState.mOutputLocations.size())
+ {
+ mState.mOutputLocations.resize(location + 1);
+ }
+ ASSERT(!mState.mOutputLocations.at(location).used());
if (outputVariable.isArray())
{
mState.mOutputLocations[location] =
@@ -2806,7 +2798,7 @@
const auto &samplerUniform = mState.mUniforms[samplerIndex];
if (samplerUniform.binding != -1)
{
- GLint location = mState.getUniformLocation(samplerUniform.name);
+ GLint location = getUniformLocation(samplerUniform.name);
ASSERT(location != -1);
std::vector<GLint> boundTextureUnits;
for (unsigned int elementIndex = 0; elementIndex < samplerUniform.elementCount();
@@ -2942,9 +2934,9 @@
{
for (const VarT &field : fields)
{
- const std::string &fullName = (prefix.empty() ? field.name : prefix + "." + field.name);
+ std::string fullName = (prefix.empty() ? field.name : prefix + "." + field.name);
- const std::string &fullMappedName =
+ std::string fullMappedName =
(mappedPrefix.empty() ? field.mappedName : mappedPrefix + "." + field.mappedName);
if (field.isStruct())
@@ -2968,6 +2960,12 @@
continue;
}
+ if (field.isArray())
+ {
+ fullName += "[0]";
+ fullMappedName += "[0]";
+ }
+
LinkedUniform newUniform(field.type, field.precision, fullName, field.arraySize, -1, -1,
-1, blockIndex, memberInfo);
newUniform.mappedName = fullMappedName;
diff --git a/src/libANGLE/Program.h b/src/libANGLE/Program.h
index cc81f29..50364ea 100644
--- a/src/libANGLE/Program.h
+++ b/src/libANGLE/Program.h
@@ -275,7 +275,7 @@
unsigned int getMaxActiveAttribLocation() const { return mMaxActiveAttribLocation; }
DrawBufferMask getActiveOutputVariables() const { return mActiveOutputVariables; }
const std::vector<sh::OutputVariable> &getOutputVariables() const { return mOutputVariables; }
- const std::map<int, VariableLocation> &getOutputLocations() const { return mOutputLocations; }
+ const std::vector<VariableLocation> &getOutputLocations() const { return mOutputLocations; }
const std::vector<LinkedUniform> &getUniforms() const { return mUniforms; }
const std::vector<VariableLocation> &getUniformLocations() const { return mUniformLocations; }
const std::vector<InterfaceBlock> &getUniformBlocks() const { return mUniformBlocks; }
@@ -299,7 +299,6 @@
return mAtomicCounterBuffers;
}
- GLint getUniformLocation(const std::string &name) const;
GLuint getUniformIndexFromName(const std::string &name) const;
GLuint getUniformIndexFromLocation(GLint location) const;
Optional<GLuint> getSamplerIndex(GLint location) const;
@@ -340,7 +339,12 @@
// 4. Atomic counter uniforms
// 5. Uniform block uniforms
// This makes opaque uniform validation easier, since we don't need a separate list.
+ // For generating the entries and naming them we follow the spec: GLES 3.1 November 2016 section
+ // 7.3.1.1 Naming Active Resources. There's a separate entry for each struct member and each
+ // inner array of an array of arrays. Names and mapped names of uniforms that are arrays include
+ // [0] in the end. This makes implementation of queries simpler.
std::vector<LinkedUniform> mUniforms;
+
std::vector<VariableLocation> mUniformLocations;
std::vector<InterfaceBlock> mUniformBlocks;
std::vector<InterfaceBlock> mShaderStorageBlocks;
@@ -355,8 +359,10 @@
// An array of the images that are used by the program
std::vector<gl::ImageBinding> mImageBindings;
+ // Names and mapped names of output variables that are arrays include [0] in the end, similarly
+ // to uniforms.
std::vector<sh::OutputVariable> mOutputVariables;
- std::map<int, VariableLocation> mOutputLocations;
+ std::vector<VariableLocation> mOutputLocations;
DrawBufferMask mActiveOutputVariables;
// Fragment output variable base types: FLOAT, INT, or UINT. Ordered by location.
diff --git a/src/libANGLE/UniformLinker.cpp b/src/libANGLE/UniformLinker.cpp
index 0cf2e7e..a14ac53 100644
--- a/src/libANGLE/UniformLinker.cpp
+++ b/src/libANGLE/UniformLinker.cpp
@@ -10,6 +10,7 @@
#include "libANGLE/UniformLinker.h"
+#include "common/string_utils.h"
#include "common/utilities.h"
#include "libANGLE/Caps.h"
#include "libANGLE/Context.h"
@@ -33,7 +34,21 @@
return nullptr;
}
-} // anonymouse namespace
+int GetUniformLocationBinding(const Program::Bindings &uniformLocationBindings,
+ const sh::Uniform &uniform)
+{
+ int binding = uniformLocationBindings.getBinding(uniform.name);
+ if (uniform.isArray() && binding == -1)
+ {
+ // Bindings for array uniforms can be set either with or without [0] in the end.
+ ASSERT(angle::EndsWith(uniform.name, "[0]"));
+ std::string nameWithoutIndex = uniform.name.substr(0u, uniform.name.length() - 3u);
+ return uniformLocationBindings.getBinding(nameWithoutIndex);
+ }
+ return binding;
+}
+
+} // anonymous namespace
UniformLinker::UniformLinker(const ProgramState &state) : mState(state)
{
@@ -191,7 +206,7 @@
continue;
}
- int preSetLocation = uniformLocationBindings.getBinding(uniform.name);
+ int preSetLocation = GetUniformLocationBinding(uniformLocationBindings, uniform);
int shaderLocation = uniform.location;
if (shaderLocation != -1)
@@ -264,7 +279,7 @@
continue;
}
- int apiBoundLocation = uniformLocationBindings.getBinding(uniform.name);
+ int apiBoundLocation = GetUniformLocationBinding(uniformLocationBindings, uniform);
int shaderLocation = uniform.location;
if (shaderLocation != -1)
@@ -518,7 +533,19 @@
{
uniformList = atomicCounterUniforms;
}
- LinkedUniform *existingUniform = FindUniform(*uniformList, fullName);
+
+ std::string fullNameWithArrayIndex(fullName);
+ std::string fullMappedNameWithArrayIndex(fullMappedName);
+
+ if (uniform.isArray())
+ {
+ // We're following the GLES 3.1 November 2016 spec section 7.3.1.1 Naming Active Resources
+ // and including [0] at the end of array variable names.
+ fullNameWithArrayIndex += "[0]";
+ fullMappedNameWithArrayIndex += "[0]";
+ }
+
+ LinkedUniform *existingUniform = FindUniform(*uniformList, fullNameWithArrayIndex);
if (existingUniform)
{
if (binding != -1)
@@ -541,10 +568,10 @@
}
else
{
- LinkedUniform linkedUniform(uniform.type, uniform.precision, fullName, uniform.arraySize,
- binding, offset, *location, -1,
+ LinkedUniform linkedUniform(uniform.type, uniform.precision, fullNameWithArrayIndex,
+ uniform.arraySize, binding, offset, *location, -1,
sh::BlockMemberInfo::getDefaultBlockInfo());
- linkedUniform.mappedName = fullMappedName;
+ linkedUniform.mappedName = fullMappedNameWithArrayIndex;
linkedUniform.staticUse = markStaticUse;
if (markStaticUse)
{
diff --git a/src/libANGLE/queryutils.cpp b/src/libANGLE/queryutils.cpp
index 7270276..80667d9 100644
--- a/src/libANGLE/queryutils.cpp
+++ b/src/libANGLE/queryutils.cpp
@@ -453,15 +453,8 @@
case GL_NAME_LENGTH:
{
- size_t length = var.name.size();
- if (var.isArray())
- {
- // Counts "[0]".
- length += 3;
- }
// ES31 spec p84: This counts the terminating null char.
- ++length;
- return clampCast<GLint>(length);
+ return clampCast<GLint>(var.name.size() + 1u);
}
case GL_LOCATION:
@@ -1471,7 +1464,7 @@
return program->getFragDataLocation(name);
case GL_UNIFORM:
- return program->getState().getUniformLocation(name);
+ return program->getUniformLocation(name);
default:
UNREACHABLE();
diff --git a/src/libANGLE/renderer/d3d/DynamicHLSL.cpp b/src/libANGLE/renderer/d3d/DynamicHLSL.cpp
index a589d89..e466915 100644
--- a/src/libANGLE/renderer/d3d/DynamicHLSL.cpp
+++ b/src/libANGLE/renderer/d3d/DynamicHLSL.cpp
@@ -1265,9 +1265,15 @@
const auto &shaderOutputVars =
metadata.getFragmentShader()->getData().getActiveOutputVariables();
- for (auto outputPair : programData.getOutputLocations())
+ for (size_t outputLocationIndex = 0u;
+ outputLocationIndex < programData.getOutputLocations().size(); ++outputLocationIndex)
{
- const VariableLocation &outputLocation = outputPair.second;
+ const VariableLocation &outputLocation =
+ programData.getOutputLocations().at(outputLocationIndex);
+ if (!outputLocation.used())
+ {
+ continue;
+ }
const sh::ShaderVariable &outputVariable = shaderOutputVars[outputLocation.index];
const std::string &variableName = "out_" + outputVariable.name;
@@ -1283,7 +1289,7 @@
outputKeyVariable.type = outputVariable.type;
outputKeyVariable.name = variableName + elementString;
outputKeyVariable.source = variableName + ArrayIndexString(outputLocation.arrayIndices);
- outputKeyVariable.outputIndex = outputPair.first;
+ outputKeyVariable.outputIndex = outputLocationIndex;
outPixelShaderKey->push_back(outputKeyVariable);
}
diff --git a/src/libANGLE/renderer/d3d/ProgramD3D.cpp b/src/libANGLE/renderer/d3d/ProgramD3D.cpp
index 9c633b0..41f9f9d 100644
--- a/src/libANGLE/renderer/d3d/ProgramD3D.cpp
+++ b/src/libANGLE/renderer/d3d/ProgramD3D.cpp
@@ -9,6 +9,7 @@
#include "libANGLE/renderer/d3d/ProgramD3D.h"
#include "common/bitset_utils.h"
+#include "common/string_utils.h"
#include "common/utilities.h"
#include "libANGLE/Context.h"
#include "libANGLE/Framebuffer.h"
@@ -1979,7 +1980,17 @@
if (!glUniform.isInDefaultBlock())
continue;
- auto mapEntry = uniformMap.find(glUniform.name);
+ std::string name = glUniform.name;
+ if (glUniform.isArray())
+ {
+ // In the program state, array uniform names include [0] as in the program resource
+ // spec. Here we don't include it.
+ // TODO(oetuaho@nvidia.com): consider using the same uniform naming here as in the GL
+ // layer.
+ ASSERT(angle::EndsWith(name, "[0]"));
+ name.resize(name.length() - 3);
+ }
+ auto mapEntry = uniformMap.find(name);
ASSERT(mapEntry != uniformMap.end());
mD3DUniforms.push_back(mapEntry->second);
}
@@ -2588,8 +2599,9 @@
const std::string & /* blockMappedName */,
size_t *sizeOut) const
{
- std::string baseName = blockName;
- gl::ParseAndStripArrayIndex(&baseName);
+ size_t nameLengthWithoutArrayIndex;
+ gl::ParseArrayIndex(blockName, &nameLengthWithoutArrayIndex);
+ std::string baseName = blockName.substr(0u, nameLengthWithoutArrayIndex);
auto sizeIter = mBlockDataSizes.find(baseName);
if (sizeIter == mBlockDataSizes.end())
diff --git a/src/libANGLE/renderer/d3d/ProgramD3D.h b/src/libANGLE/renderer/d3d/ProgramD3D.h
index e4ed275..d66a431 100644
--- a/src/libANGLE/renderer/d3d/ProgramD3D.h
+++ b/src/libANGLE/renderer/d3d/ProgramD3D.h
@@ -50,7 +50,7 @@
// Duplicated from the GL layer
const gl::UniformTypeInfo &typeInfo;
- std::string name;
+ std::string name; // Names of arrays don't include [0], unlike at the GL layer.
unsigned int arraySize;
// Pointer to a system copies of the data. Separate pointers for each uniform storage type.
diff --git a/src/libANGLE/renderer/gl/ProgramGL.cpp b/src/libANGLE/renderer/gl/ProgramGL.cpp
index 6f81fd0..4045eeb 100644
--- a/src/libANGLE/renderer/gl/ProgramGL.cpp
+++ b/src/libANGLE/renderer/gl/ProgramGL.cpp
@@ -663,14 +663,15 @@
// "Locations for sequential array indices are not required to be sequential."
const gl::LinkedUniform &uniform = uniforms[entry.index];
std::stringstream fullNameStr;
- fullNameStr << uniform.mappedName;
if (uniform.isArray())
{
- for (auto arrayElementIndexIt = entry.arrayIndices.rbegin();
- arrayElementIndexIt != entry.arrayIndices.rend(); ++arrayElementIndexIt)
- {
- fullNameStr << "[" << (*arrayElementIndexIt) << "]";
- }
+ ASSERT(angle::EndsWith(uniform.mappedName, "[0]"));
+ fullNameStr << uniform.mappedName.substr(0, uniform.mappedName.length() - 3);
+ fullNameStr << "[" << entry.arrayIndices[0] << "]";
+ }
+ else
+ {
+ fullNameStr << uniform.mappedName;
}
const std::string &fullName = fullNameStr.str();