Fix attribute layout location linking.
When a vertex attribute has a GLSL layout location qualifier, it takes
precedence over the binding location provided through the
glBindAttribLocation API call.
OpenGL ES 3.0.5 spec:
"If an active attribute has a binding explicitly set within the shader
text and a different binding assigned by BindAttribLocation, the
assignment in the shader text is used."
Change-Id: If0bc0dc01a8ff6189703f2be26f1938fbff5f5ae
Reviewed-on: https://swiftshader-review.googlesource.com/20168
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
diff --git a/src/OpenGL/compiler/OutputASM.cpp b/src/OpenGL/compiler/OutputASM.cpp
index 9ed34b2..c18ecf7 100644
--- a/src/OpenGL/compiler/OutputASM.cpp
+++ b/src/OpenGL/compiler/OutputASM.cpp
@@ -400,12 +400,12 @@
registerIndex = 0;
}
- Attribute::Attribute(GLenum type, const std::string &name, int arraySize, int location, int registerIndex)
+ Attribute::Attribute(GLenum type, const std::string &name, int arraySize, int layoutLocation, int registerIndex)
{
this->type = type;
this->name = name;
this->arraySize = arraySize;
- this->location = location;
+ this->layoutLocation = layoutLocation;
this->registerIndex = registerIndex;
}
diff --git a/src/OpenGL/compiler/OutputASM.h b/src/OpenGL/compiler/OutputASM.h
index 2f90266..b037a67 100644
--- a/src/OpenGL/compiler/OutputASM.h
+++ b/src/OpenGL/compiler/OutputASM.h
@@ -144,12 +144,12 @@
struct Attribute
{
Attribute();
- Attribute(GLenum type, const std::string &name, int arraySize, int location, int registerIndex);
+ Attribute(GLenum type, const std::string &name, int arraySize, int layoutLocation, int registerIndex);
GLenum type;
std::string name;
int arraySize;
- int location;
+ int layoutLocation;
int registerIndex;
};
diff --git a/src/OpenGL/libGLESv2/Program.cpp b/src/OpenGL/libGLESv2/Program.cpp
index 77912cc..dc64813 100644
--- a/src/OpenGL/libGLESv2/Program.cpp
+++ b/src/OpenGL/libGLESv2/Program.cpp
@@ -277,19 +277,7 @@
GLint Program::getAttributeLocation(const char *name)
{
- if(name)
- {
- std::string strName(name);
- for(auto const &it : linkedAttribute)
- {
- if(it.name == strName)
- {
- return getAttributeBinding(it);
- }
- }
- }
-
- return -1;
+ return name ? getAttributeLocation(std::string(name)) : -1;
}
int Program::getAttributeStream(int attributeIndex)
@@ -1591,80 +1579,55 @@
// Determines the mapping between GL attributes and vertex stream usage indices
bool Program::linkAttributes()
{
+ static_assert(MAX_VERTEX_ATTRIBS <= 32, "attribute count exceeds bitfield count");
unsigned int usedLocations = 0;
- // Link attributes that have a binding location
+ // Link attributes that have a GLSL layout location qualifier
for(auto const &attribute : vertexShader->activeAttributes)
{
- int location = (attributeBinding.find(attribute.name) != attributeBinding.end()) ? attributeBinding[attribute.name] : -1;
-
- if(location != -1) // Set by glBindAttribLocation
+ if(attribute.layoutLocation != -1)
{
- int rows = VariableRegisterCount(attribute.type);
-
- if(rows + location > MAX_VERTEX_ATTRIBS)
+ if(!linkAttribute(attribute, attribute.layoutLocation, usedLocations))
{
- appendToInfoLog("Active attribute (%s) at location %d is too big to fit", attribute.name.c_str(), location);
return false;
}
-
- // In GLSL 3.00, attribute aliasing produces a link error
- // In GLSL 1.00, attribute aliasing is allowed
- if(vertexShader->getShaderVersion() >= 300)
- {
- for(auto const &it : linkedAttribute)
- {
- int itLocStart = getAttributeBinding(it);
- ASSERT(itLocStart >= 0);
- int itLocEnd = itLocStart + VariableRegisterCount(it.type);
- for(int i = 0; i < rows; i++)
- {
- int loc = location + i;
- if((loc >= itLocStart) && (loc < itLocEnd))
- {
- appendToInfoLog("Attribute '%s' aliases attribute '%s' at location %d", attribute.name.c_str(), it.name.c_str(), location);
- return false;
- }
- }
- }
- }
-
- linkedAttributeLocation[attribute.name] = location;
- linkedAttribute.push_back(attribute);
- for(int i = 0; i < rows; i++)
- {
- usedLocations |= 1 << (location + i);
- }
}
}
- // Link attributes that don't have a binding location
+ // Link attributes that have an API provided binding location but no GLSL layout location
for(auto const &attribute : vertexShader->activeAttributes)
{
- int location = (attributeBinding.find(attribute.name) != attributeBinding.end()) ? attributeBinding[attribute.name] : -1;
+ int bindingLocation = (attributeBinding.find(attribute.name) != attributeBinding.end()) ? attributeBinding[attribute.name] : -1;
- if(location == -1) // Not set by glBindAttribLocation
+ if(attribute.layoutLocation == -1 && bindingLocation != -1)
{
- int rows = VariableRegisterCount(attribute.type);
- int availableIndex = AllocateFirstFreeBits(&usedLocations, rows, MAX_VERTEX_ATTRIBS);
-
- if(availableIndex == -1 || availableIndex + rows > MAX_VERTEX_ATTRIBS)
+ if(!linkAttribute(attribute, bindingLocation, usedLocations))
{
- appendToInfoLog("Too many active attributes (%s)", attribute.name.c_str());
- return false; // Fail to link
+ return false;
}
-
- linkedAttributeLocation[attribute.name] = availableIndex;
- linkedAttribute.push_back(attribute);
}
}
- for(auto const &it : linkedAttribute)
+ // Link attributes that don't have a binding location nor a layout location
+ for(auto const &attribute : vertexShader->activeAttributes)
{
- int location = getAttributeBinding(it);
+ if(attribute.layoutLocation == -1 && attributeBinding.find(attribute.name) == attributeBinding.end())
+ {
+ if(!linkAttribute(attribute, -1, usedLocations))
+ {
+ return false;
+ }
+ }
+ }
+
+ ASSERT(linkedAttribute.size() == vertexShader->activeAttributes.size());
+
+ for(auto const &attribute : linkedAttribute)
+ {
+ int location = getAttributeLocation(attribute.name);
ASSERT(location >= 0);
- int index = vertexShader->getSemanticIndex(it.name);
- int rows = std::max(VariableRegisterCount(it.type), 1);
+ int index = vertexShader->getSemanticIndex(attribute.name);
+ int rows = VariableRegisterCount(attribute.type);
for(int r = 0; r < rows; r++)
{
@@ -1675,17 +1638,69 @@
return true;
}
- int Program::getAttributeBinding(const glsl::Attribute &attribute)
+ bool Program::linkAttribute(const glsl::Attribute &attribute, int location, unsigned int &usedLocations)
{
- if(attribute.location != -1)
+ int rows = VariableRegisterCount(attribute.type);
+
+ if(location == -1)
{
- return attribute.location;
+ location = AllocateFirstFreeBits(&usedLocations, rows, MAX_VERTEX_ATTRIBS);
+
+ if(location == -1 || location + rows > MAX_VERTEX_ATTRIBS)
+ {
+ appendToInfoLog("Too many active attributes (%s)", attribute.name.c_str());
+ return false; // Fail to link
+ }
+ }
+ else
+ {
+ if(rows + location > MAX_VERTEX_ATTRIBS)
+ {
+ appendToInfoLog("Active attribute (%s) at location %d is too big to fit", attribute.name.c_str(), location);
+ return false;
+ }
+
+ // In GLSL 3.00, attribute aliasing produces a link error
+ // In GLSL 1.00, attribute aliasing is allowed
+ if(vertexShader->getShaderVersion() >= 300)
+ {
+ for(auto const &previousAttrib : linkedAttribute)
+ {
+ int previousLocation = getAttributeLocation(previousAttrib.name);
+ int previousRows = VariableRegisterCount(previousAttrib.type);
+
+ if(location >= previousLocation && location < previousLocation + previousRows)
+ {
+ appendToInfoLog("Attribute '%s' aliases attribute '%s' at location %d", attribute.name.c_str(), previousAttrib.name.c_str(), location);
+ return false;
+ }
+
+ if(location <= previousLocation && location + rows > previousLocation)
+ {
+ appendToInfoLog("Attribute '%s' aliases attribute '%s' at location %d", attribute.name.c_str(), previousAttrib.name.c_str(), previousLocation);
+ return false;
+ }
+ }
+ }
+
+ for(int i = 0; i < rows; i++)
+ {
+ usedLocations |= 1 << (location + i);
+ }
}
- std::map<std::string, GLuint>::const_iterator it = linkedAttributeLocation.find(attribute.name);
- if(it != linkedAttributeLocation.end())
+ linkedAttributeLocation[attribute.name] = location;
+ linkedAttribute.push_back(attribute);
+
+ return true;
+ }
+
+ int Program::getAttributeLocation(const std::string &name)
+ {
+ std::map<std::string, GLuint>::const_iterator attribute = linkedAttributeLocation.find(name);
+ if(attribute != linkedAttributeLocation.end())
{
- return it->second;
+ return attribute->second;
}
return -1;
diff --git a/src/OpenGL/libGLESv2/Program.h b/src/OpenGL/libGLESv2/Program.h
index 501d081..a232926 100644
--- a/src/OpenGL/libGLESv2/Program.h
+++ b/src/OpenGL/libGLESv2/Program.h
@@ -228,7 +228,8 @@
bool linkTransformFeedback();
bool linkAttributes();
- int getAttributeBinding(const glsl::Attribute &attribute);
+ bool linkAttribute(const glsl::Attribute &attribute, int location, unsigned int &usedLocations);
+ int getAttributeLocation(const std::string &name);
Uniform *getUniform(const std::string &name) const;
bool linkUniforms(const Shader *shader);
diff --git a/src/OpenGL/libGLESv2/libGLESv2.cpp b/src/OpenGL/libGLESv2/libGLESv2.cpp
index 789b0c8..b3f1d20 100644
--- a/src/OpenGL/libGLESv2/libGLESv2.cpp
+++ b/src/OpenGL/libGLESv2/libGLESv2.cpp
@@ -2345,7 +2345,6 @@
if(context)
{
-
es2::Program *programObject = context->getProgram(program);
if(!programObject)
