OSDN Git Service

Fix attribute layout location linking.
authorNicolas Capens <capn@google.com>
Wed, 8 Aug 2018 03:57:21 +0000 (23:57 -0400)
committerNicolas Capens <nicolascapens@google.com>
Mon, 13 Aug 2018 17:19:30 +0000 (17:19 +0000)
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>
src/OpenGL/compiler/OutputASM.cpp
src/OpenGL/compiler/OutputASM.h
src/OpenGL/libGLESv2/Program.cpp
src/OpenGL/libGLESv2/Program.h
src/OpenGL/libGLESv2/libGLESv2.cpp
tests/unittests/unittests.cpp

index 9ed34b2..c18ecf7 100644 (file)
@@ -400,12 +400,12 @@ namespace glsl
                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;
        }
 
index 2f90266..b037a67 100644 (file)
@@ -144,12 +144,12 @@ namespace glsl
        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;
        };
index 77912cc..dc64813 100644 (file)
@@ -277,19 +277,7 @@ namespace es2
 
        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 @@ namespace es2
        // 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;
-                                                       }
-                                               }
-                                       }
-                               }
+               // Link attributes that have an API provided binding location but no GLSL layout location
+               for(auto const &attribute : vertexShader->activeAttributes)
+               {
+                       int bindingLocation = (attributeBinding.find(attribute.name) != attributeBinding.end()) ? attributeBinding[attribute.name] : -1;
 
-                               linkedAttributeLocation[attribute.name] = location;
-                               linkedAttribute.push_back(attribute);
-                               for(int i = 0; i < rows; i++)
+                       if(attribute.layoutLocation == -1 && bindingLocation != -1)
+                       {
+                               if(!linkAttribute(attribute, bindingLocation, usedLocations))
                                {
-                                       usedLocations |= 1 << (location + i);
+                                       return false;
                                }
                        }
                }
 
-               // Link attributes that don't have a binding location
+               // Link attributes that don't have a binding location nor a layout location
                for(auto const &attribute : vertexShader->activeAttributes)
                {
-                       int location = (attributeBinding.find(attribute.name) != attributeBinding.end()) ? attributeBinding[attribute.name] : -1;
-
-                       if(location == -1)   // Not set by glBindAttribLocation
+                       if(attribute.layoutLocation == -1 && attributeBinding.find(attribute.name) == attributeBinding.end())
                        {
-                               int rows = VariableRegisterCount(attribute.type);
-                               int availableIndex = AllocateFirstFreeBits(&usedLocations, rows, MAX_VERTEX_ATTRIBS);
-
-                               if(availableIndex == -1 || availableIndex + rows > MAX_VERTEX_ATTRIBS)
+                               if(!linkAttribute(attribute, -1, 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)
+               ASSERT(linkedAttribute.size() == vertexShader->activeAttributes.size());
+
+               for(auto const &attribute : linkedAttribute)
                {
-                       int location = getAttributeBinding(it);
+                       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 @@ namespace es2
                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;
index 501d081..a232926 100644 (file)
@@ -228,7 +228,8 @@ namespace es2
                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);
index 789b0c8..b3f1d20 100644 (file)
@@ -2345,7 +2345,6 @@ int GetAttribLocation(GLuint program, const GLchar* name)
 
        if(context)
        {
-
                es2::Program *programObject = context->getProgram(program);
 
                if(!programObject)
index 80bc887..91b34cd 100644 (file)
@@ -266,6 +266,8 @@ protected:
                glGetProgramiv(ph.program, GL_LINK_STATUS, &linkStatus);
                EXPECT_NE(linkStatus, 0);
 
+               EXPECT_GLENUM_EQ(GL_NONE, glGetError());
+
                return ph;
        }
 
@@ -274,6 +276,8 @@ protected:
                glDeleteShader(ph.fragmentShader);
                glDeleteShader(ph.vertexShader);
                glDeleteProgram(ph.program);
+
+               EXPECT_GLENUM_EQ(GL_NONE, glGetError());
        }
 
        void drawQuad(GLuint program, const char* textureName = nullptr)
@@ -411,8 +415,6 @@ TEST_F(SwiftShaderTest, DynamicLoop)
 {
        Initialize(3, false);
 
-       unsigned char green[4] = { 0, 255, 0, 255 };
-
        const std::string vs =
                "#version 300 es\n"
                "in vec4 position;\n"
@@ -464,6 +466,7 @@ TEST_F(SwiftShaderTest, DynamicLoop)
 
        deleteProgram(ph);
 
+       unsigned char green[4] = { 0, 255, 0, 255 };
        expectFramebufferColor(green);
 
        EXPECT_GLENUM_EQ(GL_NONE, glGetError());
@@ -476,8 +479,6 @@ TEST_F(SwiftShaderTest, DynamicIndexing)
 {
        Initialize(3, false);
 
-       unsigned char green[4] = { 0, 255, 0, 255 };
-
        const std::string vs =
                "#version 300 es\n"
                "in vec4 position;\n"
@@ -521,6 +522,160 @@ TEST_F(SwiftShaderTest, DynamicIndexing)
 
        deleteProgram(ph);
 
+       unsigned char green[4] = { 0, 255, 0, 255 };
+       expectFramebufferColor(green);
+
+       EXPECT_GLENUM_EQ(GL_NONE, glGetError());
+
+       Uninitialize();
+}
+
+// Test vertex attribute location linking
+TEST_F(SwiftShaderTest, AttributeLocation)
+{
+       Initialize(3, false);
+
+       const std::string vs =
+               "#version 300 es\n"
+               "layout(location = 0) in vec4 a0;\n"   // Explicitly bound in GLSL
+               "layout(location = 2) in vec4 a2;\n"   // Explicitly bound in GLSL
+               "in vec4 a5;\n"                        // Bound to location 5 by API
+               "in mat2 a3;\n"                        // Implicit location
+               "in vec4 a1;\n"                        // Implicit location
+               "in vec4 a6;\n"                        // Implicit location
+               "out vec4 color;\n"
+               "void main()\n"
+               "{\n"
+               "   vec4 a34 = vec4(a3[0], a3[1]);\n"
+               "       gl_Position = a0;\n"
+               "   color = (a2 == vec4(1.0, 2.0, 3.0, 4.0) &&\n"
+               "            a34 == vec4(5.0, 6.0, 7.0, 8.0) &&\n"
+               "            a5 == vec4(9.0, 10.0, 11.0, 12.0) &&\n"
+               "            a1 == vec4(13.0, 14.0, 15.0, 16.0) &&\n"
+               "            a6 == vec4(17.0, 18.0, 19.0, 20.0)) ?\n"
+               "           vec4(0.0, 1.0, 0.0, 1.0) :\n"
+               "           vec4(1.0, 0.0, 0.0, 1.0);"
+               "}\n";
+
+       const std::string fs =
+               "#version 300 es\n"
+               "precision mediump float;\n"
+               "in vec4 color;\n"
+               "out vec4 fragColor;\n"
+               "void main()\n"
+               "{\n"
+               "       fragColor = color;\n"
+               "}\n";
+
+       ProgramHandles ph;
+       ph.program = glCreateProgram();
+       EXPECT_GLENUM_EQ(GL_NONE, glGetError());
+
+       ph.vertexShader = glCreateShader(GL_VERTEX_SHADER);
+       const char* vsSource[1] = { vs.c_str() };
+       glShaderSource(ph.vertexShader, 1, vsSource, nullptr);
+       glCompileShader(ph.vertexShader);
+       EXPECT_GLENUM_EQ(GL_NONE, glGetError());
+       GLint vsCompileStatus = 0;
+       glGetShaderiv(ph.vertexShader, GL_COMPILE_STATUS, &vsCompileStatus);
+       EXPECT_EQ(vsCompileStatus, GL_TRUE);
+
+       ph.fragmentShader = glCreateShader(GL_FRAGMENT_SHADER);
+       const char* fsSource[1] = { fs.c_str() };
+       glShaderSource(ph.fragmentShader, 1, fsSource, nullptr);
+       glCompileShader(ph.fragmentShader);
+       EXPECT_GLENUM_EQ(GL_NONE, glGetError());
+       GLint fsCompileStatus = 0;
+       glGetShaderiv(ph.fragmentShader, GL_COMPILE_STATUS, &fsCompileStatus);
+       EXPECT_EQ(fsCompileStatus, GL_TRUE);
+
+       // Not assigned a layout location in GLSL. Bind it explicitly with the API.
+       glBindAttribLocation(ph.program, 5, "a5");
+       EXPECT_GLENUM_EQ(GL_NONE, glGetError());
+
+       // Should not override GLSL layout location qualifier
+       glBindAttribLocation(ph.program, 8, "a2");
+       EXPECT_GLENUM_EQ(GL_NONE, glGetError());
+
+       glAttachShader(ph.program, ph.vertexShader);
+       glAttachShader(ph.program, ph.fragmentShader);
+       glLinkProgram(ph.program);
+       EXPECT_GLENUM_EQ(GL_NONE, glGetError());
+
+       // Changes after linking should have no effect
+       glBindAttribLocation(ph.program, 0, "a1");
+       glBindAttribLocation(ph.program, 6, "a2");
+       glBindAttribLocation(ph.program, 2, "a6");
+
+       GLint linkStatus = 0;
+       glGetProgramiv(ph.program, GL_LINK_STATUS, &linkStatus);
+       EXPECT_NE(linkStatus, 0);
+       EXPECT_GLENUM_EQ(GL_NONE, glGetError());
+
+       float vertices[18] = { -1.0f,  1.0f, 0.5f,
+                              -1.0f, -1.0f, 0.5f,
+                               1.0f, -1.0f, 0.5f,
+                              -1.0f,  1.0f, 0.5f,
+                               1.0f, -1.0f, 0.5f,
+                               1.0f,  1.0f, 0.5f };
+
+       float attributes[5][4] = { 1.0f, 2.0f, 3.0f, 4.0f,
+                                  5.0f, 6.0f, 7.0f, 8.0f,
+                                  9.0f, 10.0f, 11.0f, 12.0f,
+                                  13.0f, 14.0f, 15.0f, 16.0f,
+                                  17.0f, 18.0f, 19.0f, 20.0f };
+
+       GLint a0 = glGetAttribLocation(ph.program, "a0");
+       EXPECT_EQ(a0, 0);
+       glVertexAttribPointer(a0, 3, GL_FLOAT, GL_FALSE, 0, vertices);
+       glEnableVertexAttribArray(a0);
+       EXPECT_GLENUM_EQ(GL_NONE, glGetError());
+
+       GLint a2 = glGetAttribLocation(ph.program, "a2");
+       EXPECT_EQ(a2, 2);
+       glVertexAttribPointer(a2, 4, GL_FLOAT, GL_FALSE, 0, attributes[0]);
+       glVertexAttribDivisor(a2, 1);
+       glEnableVertexAttribArray(a2);
+       EXPECT_GLENUM_EQ(GL_NONE, glGetError());
+
+       GLint a3 = glGetAttribLocation(ph.program, "a3");
+       EXPECT_EQ(a3, 3);   // Note: implementation specific
+       glVertexAttribPointer(a3 + 0, 2, GL_FLOAT, GL_FALSE, 0, &attributes[1][0]);
+       glVertexAttribPointer(a3 + 1, 2, GL_FLOAT, GL_FALSE, 0, &attributes[1][2]);
+       glVertexAttribDivisor(a3 + 0, 1);
+       glVertexAttribDivisor(a3 + 1, 1);
+       glEnableVertexAttribArray(a3 + 0);
+       glEnableVertexAttribArray(a3 + 1);
+       EXPECT_GLENUM_EQ(GL_NONE, glGetError());
+
+       GLint a5 = glGetAttribLocation(ph.program, "a5");
+       EXPECT_EQ(a5, 5);
+       glVertexAttribPointer(a5, 4, GL_FLOAT, GL_FALSE, 0, attributes[2]);
+       glVertexAttribDivisor(a5, 1);
+       glEnableVertexAttribArray(a5);
+       EXPECT_GLENUM_EQ(GL_NONE, glGetError());
+
+       GLint a1 = glGetAttribLocation(ph.program, "a1");
+       EXPECT_EQ(a1, 1);   // Note: implementation specific
+       glVertexAttribPointer(a1, 4, GL_FLOAT, GL_FALSE, 0, attributes[3]);
+       glVertexAttribDivisor(a1, 1);
+       glEnableVertexAttribArray(a1);
+       EXPECT_GLENUM_EQ(GL_NONE, glGetError());
+
+       GLint a6 = glGetAttribLocation(ph.program, "a6");
+       EXPECT_EQ(a6, 6);   // Note: implementation specific
+       glVertexAttribPointer(a6, 4, GL_FLOAT, GL_FALSE, 0, attributes[4]);
+       glVertexAttribDivisor(a6, 1);
+       glEnableVertexAttribArray(a6);
+       EXPECT_GLENUM_EQ(GL_NONE, glGetError());
+
+       glUseProgram(ph.program);
+       glDrawArrays(GL_TRIANGLES, 0, 6);
+       EXPECT_GLENUM_EQ(GL_NONE, glGetError());
+
+       deleteProgram(ph);
+
+       unsigned char green[4] = { 0, 255, 0, 255 };
        expectFramebufferColor(green);
 
        EXPECT_GLENUM_EQ(GL_NONE, glGetError());