GL: Mark unused uniform locations that were explicitly bound as ignored

src/libANGLE/Program.cpp

@@ -999,6 +999,18 @@ int ProgramAliasedBindings::getBindingByName(const std::string &name) const
     return (iter != mBindings.end()) ? iter->second.location : -1;
 }
 
+int ProgramAliasedBindings::getBindingByLocation(GLuint location) const
+{
+    for (const auto &iter : mBindings)
+    {
+        if (iter.second.location == location)
+        {
+            return iter.second.location;
+        }
+    }
+    return -1;
+}
+
 int ProgramAliasedBindings::getBinding(const sh::ShaderVariable &variable) const
 {
     const std::string &name = variable.name;
@@ -1349,7 +1361,7 @@ void Program::bindAttributeLocation(GLuint index, const char *name)
 void Program::bindUniformLocation(GLuint index, const char *name)
 {
     ASSERT(mLinkResolved);
-    mUniformLocationBindings.bindLocation(index, name);
+    mState.mUniformLocationBindings.bindLocation(index, name);
 }
 
 void Program::bindFragmentInputLocation(GLint index, const char *name)
@@ -1498,7 +1510,7 @@ angle::Result Program::link(const Context *context)
 
         GLuint combinedImageUniforms = 0u;
         if (!linkUniforms(context->getCaps(), context->getClientVersion(), mInfoLog,
-                          mUniformLocationBindings, &combinedImageUniforms,
+                          mState.mUniformLocationBindings, &combinedImageUniforms,
                           &resources->unusedUniforms))
         {
             return angle::Result::Continue;
@@ -1563,7 +1575,7 @@ angle::Result Program::link(const Context *context)
 
         GLuint combinedImageUniforms = 0u;
         if (!linkUniforms(context->getCaps(), context->getClientVersion(), mInfoLog,
-                          mUniformLocationBindings, &combinedImageUniforms,
+                          mState.mUniformLocationBindings, &combinedImageUniforms,
                           &resources->unusedUniforms))
         {
             return angle::Result::Continue;
@@ -2444,7 +2456,7 @@ const ProgramBindings &Program::getAttributeBindings() const
 const ProgramAliasedBindings &Program::getUniformLocationBindings() const
 {
     ASSERT(mLinkResolved);
-    return mUniformLocationBindings;
+    return mState.mUniformLocationBindings;
 }
 const ProgramBindings &Program::getFragmentInputBindings() const
 {

src/libANGLE/Program.h

@@ -293,6 +293,54 @@ struct ImageBinding
     bool unreferenced;
 };
 
+struct ProgramBinding
+{
+    ProgramBinding() : location(GL_INVALID_INDEX), aliased(false) {}
+    ProgramBinding(GLuint index) : location(index), aliased(false) {}
+
+    GLuint location;
+    // Whether another binding was set that may potentially alias this.
+    bool aliased;
+};
+
+class ProgramBindings final : angle::NonCopyable
+{
+  public:
+    ProgramBindings();
+    ~ProgramBindings();
+
+    void bindLocation(GLuint index, const std::string &name);
+    int getBindingByName(const std::string &name) const;
+    int getBinding(const sh::ShaderVariable &variable) const;
+
+    using const_iterator = std::unordered_map<std::string, GLuint>::const_iterator;
+    const_iterator begin() const;
+    const_iterator end() const;
+
+  private:
+    std::unordered_map<std::string, GLuint> mBindings;
+};
+
+// Uniforms and Fragment Outputs require special treatment due to array notation (e.g., "[0]")
+class ProgramAliasedBindings final : angle::NonCopyable
+{
+  public:
+    ProgramAliasedBindings();
+    ~ProgramAliasedBindings();
+
+    void bindLocation(GLuint index, const std::string &name);
+    int getBindingByName(const std::string &name) const;
+    int getBindingByLocation(GLuint location) const;
+    int getBinding(const sh::ShaderVariable &variable) const;
+
+    using const_iterator = std::unordered_map<std::string, ProgramBinding>::const_iterator;
+    const_iterator begin() const;
+    const_iterator end() const;
+
+  private:
+    std::unordered_map<std::string, ProgramBinding> mBindings;
+};
+
 class ProgramState final : angle::NonCopyable
 {
   public:
@@ -405,6 +453,11 @@ class ProgramState final : angle::NonCopyable
     ShaderType getFirstAttachedShaderStageType() const;
     ShaderType getLastAttachedShaderStageType() const;
 
+    const ProgramAliasedBindings &getUniformLocationBindings() const
+    {
+        return mUniformLocationBindings;
+    }
+
   private:
     friend class MemoryProgramCache;
     friend class Program;
@@ -517,53 +570,10 @@ class ProgramState final : angle::NonCopyable
 
     // Cached mask of active images.
     ActiveTextureMask mActiveImagesMask;
-};
 
-struct ProgramBinding
-{
-    ProgramBinding() : location(GL_INVALID_INDEX), aliased(false) {}
-    ProgramBinding(GLuint index) : location(index), aliased(false) {}
-
-    GLuint location;
-    // Whether another binding was set that may potentially alias this.
-    bool aliased;
-};
-
-class ProgramBindings final : angle::NonCopyable
-{
-  public:
-    ProgramBindings();
-    ~ProgramBindings();
-
-    void bindLocation(GLuint index, const std::string &name);
-    int getBindingByName(const std::string &name) const;
-    int getBinding(const sh::ShaderVariable &variable) const;
-
-    using const_iterator = std::unordered_map<std::string, GLuint>::const_iterator;
-    const_iterator begin() const;
-    const_iterator end() const;
-
-  private:
-    std::unordered_map<std::string, GLuint> mBindings;
-};
-
-// Uniforms and Fragment Outputs require special treatment due to array notation (e.g., "[0]")
-class ProgramAliasedBindings final : angle::NonCopyable
-{
-  public:
-    ProgramAliasedBindings();
-    ~ProgramAliasedBindings();
-
-    void bindLocation(GLuint index, const std::string &name);
-    int getBindingByName(const std::string &name) const;
-    int getBinding(const sh::ShaderVariable &variable) const;
-
-    using const_iterator = std::unordered_map<std::string, ProgramBinding>::const_iterator;
-    const_iterator begin() const;
-    const_iterator end() const;
-
-  private:
-    std::unordered_map<std::string, ProgramBinding> mBindings;
+    // Note that this has nothing to do with binding layout qualifiers that can be set for some
+    // uniforms in GLES3.1+. It is used to pre-set the location of uniforms.
+    ProgramAliasedBindings mUniformLocationBindings;
 };
 
 struct ProgramVaryingRef
@@ -1098,10 +1108,6 @@ class Program final : angle::NonCopyable, public LabeledObject
 
     ProgramBindings mAttributeBindings;
 
-    // Note that this has nothing to do with binding layout qualifiers that can be set for some
-    // uniforms in GLES3.1+. It is used to pre-set the location of uniforms.
-    ProgramAliasedBindings mUniformLocationBindings;
-
     // CHROMIUM_path_rendering
     ProgramBindings mFragmentInputBindings;
 

src/libANGLE/renderer/gl/ProgramGL.cpp

@@ -1166,7 +1166,16 @@ void ProgramGL::markUnusedUniformLocations(std::vector<gl::VariableLocation> *un
                 GLuint imageIndex = mState.getImageIndexFromUniformIndex(locationRef.index);
                 (*imageBindings)[imageIndex].unreferenced = true;
             }
-            locationRef.markUnused();
+            // If the location has been previously bound by a glBindUniformLocation call, it should
+            // be marked as ignored. Otherwise it's unused.
+            if (mState.getUniformLocationBindings().getBindingByLocation(location) != -1)
+            {
+                locationRef.markIgnored();
+            }
+            else
+            {
+                locationRef.markUnused();
+            }
         }
     }
 }

src/tests/gl_tests/BindUniformLocationTest.cpp

@@ -342,6 +342,48 @@ void main()
     EXPECT_GL_ERROR(GL_INVALID_OPERATION);
 }
 
+// GL backend optimizes away a uniform in the vertex shader if it's only used to
+// compute a varying that is never referenced in the fragment shader.
+TEST_P(BindUniformLocationTest, UnusedUniformUpdateComplex)
+{
+    ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_CHROMIUM_bind_uniform_location"));
+
+    ASSERT_NE(nullptr, glBindUniformLocationCHROMIUM);
+
+    constexpr char kVS[] = R"(precision highp float;
+attribute vec4 a_position;
+varying vec4 v_unused;
+uniform vec4 u_unused;
+void main()
+{
+    gl_Position = a_position;
+    v_unused = u_unused;
+}
+)";
+
+    constexpr char kFS[] = R"(precision mediump float;
+varying vec4 v_unused;
+void main()
+{
+    gl_FragColor = vec4(1.0, 0.0, 0.0, 1.0);
+})";
+
+    const GLint unusedLocation = 1;
+
+    mProgram = CompileProgram(kVS, kFS, [&](GLuint program) {
+        glBindUniformLocationCHROMIUM(program, unusedLocation, "u_unused");
+    });
+    ASSERT_NE(0u, mProgram);
+
+    glUseProgram(mProgram);
+
+    // No errors on bound locations of names that do not exist
+    // in the shader. Otherwise it would be inconsistent wrt the
+    // optimization case.
+    glUniform4f(unusedLocation, 0.25f, 0.25f, 0.25f, 0.25f);
+    EXPECT_GL_NO_ERROR();
+}
+
 // Test for a bug where using a sampler caused GL error if the mProgram had
 // uniforms that were optimized away by the driver. This was only a problem with
 // glBindUniformLocationCHROMIUM implementation. This could be reproed by