Clarify aliasing rules in CHROMIUM_bind_uniform_location

The CHROMIUM_bind_uniform_location spec previously had some conflicting information on when uniform location aliasing was allowed. Now the section on uniform location aliasing makes it clear that aliasing locations of two statically used uniforms is an error. This guarantees compatibility between different compiler versions that may treat a different subset of uniforms as active, depending on optimizations. It follows the spirit of GLSL ES 3.00.6 spec section 12.46, that has similar rules for attributes. The implementation is fixed to correctly follow the spec. When flattening uniforms, static use is tracked separately from activeness. BUG=angleproject:2262 TEST=angle_end2end_tests Change-Id: I570fd384064aec66ef0380a53fa01ac5e43eec5a Reviewed-on: https://chromium-review.googlesource.com/978144Reviewed-by: Corentin Wallez <cwallez@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>

Clarify aliasing rules in CHROMIUM_bind_uniform_location
44861c48 · Olli Etuaho · Commit Bot · a571f28d · 44861c48 · 44861c48
Commit 44861c48 authored Mar 23, 2018 by Olli Etuaho Committed by Commit Bot Mar 26, 2018
3 changed files
--- a/extensions/CHROMIUM_bind_uniform_location.txt
+++ b/extensions/CHROMIUM_bind_uniform_location.txt
@@ -96,7 +96,7 @@ New Procedures and Functions
    (MAX_VERTEX_UNIFORM_VECTORS + MAX_FRAGMENT_UNIFORM_VECTORS) * 4

    or less than 0. BindUniformLocation has no effect until the program is
-    linked. In particular, it doesn't modify the bindings of active uniforms
+    linked. In particular, it doesn't modify the bindings of uniform
    variables in a program that has already been linked.

    The error INVALID_OPERATION is generated if name starts with the reserved
@@ -117,10 +117,9 @@ New Procedures and Functions

    It is possible for an application to bind more than one uniform name to
    the same location.  This is referred to as aliasing.  This will only work
-    if only one of the aliased uniforms is active in the executable program,
-    or if no path through the shader consumes more than one uniform of a set
-    of uniforms aliased to the same location.  If two statically used uniforms
-    in a program are bound to the name location, link must fail.
+    if only one of the aliased uniforms is statically used in the executable
+    program.  If two statically used uniforms in a program are bound to the same
+    location, link must fail.

 Errors

@@ -139,3 +138,4 @@ Revision History
                 to behave like location -1.
    3/9/2017     Locations set in the shader override ones set by the binding
                 API.
+    3/26/2018    Clarify that aliasing rules apply to statically used uniforms.
\ No newline at end of file
--- a/src/libANGLE/ProgramLinkedResources.cpp
+++ b/src/libANGLE/ProgramLinkedResources.cpp
--- a/src/libANGLE/ProgramLinkedResources.h
+++ b/src/libANGLE/ProgramLinkedResources.h
@@ -115,6 +115,7 @@ class UniformLinker final : angle::NonCopyable
        std::vector<LinkedUniform> *atomicCounterUniforms,
        GLenum shaderType,
        bool markActive,
+        bool markStaticUse,
        int binding,
        int offset,
        int *location);
@@ -127,6 +128,7 @@ class UniformLinker final : angle::NonCopyable
                                            std::vector<LinkedUniform> *atomicCounterUniforms,
                                            GLenum shaderType,
                                            bool markActive,
+                                            bool markStaticUse,
                                            int binding,
                                            int offset,
                                            int *location);
@@ -139,12 +141,13 @@ class UniformLinker final : angle::NonCopyable
                                           std::vector<LinkedUniform> *atomicCounterUniforms,
                                           GLenum shaderType,
                                           bool markActive,
+                                           bool markStaticUse,
                                           int binding,
                                           int offset,
                                           int *location);

-    // markActive is given as a separate parameter because it is tracked here at struct
-    // granularity.
+    // markActive and markStaticUse are given as separate parameters because they are tracked here
+    // at struct granularity.
    ShaderUniformCount flattenUniformImpl(const sh::ShaderVariable &uniform,
                                          const std::string &fullName,
                                          const std::string &fullMappedName,
@@ -153,6 +156,7 @@ class UniformLinker final : angle::NonCopyable
                                          std::vector<LinkedUniform> *atomicCounterUniforms,
                                          GLenum shaderType,
                                          bool markActive,
+                                          bool markStaticUse,
                                          int binding,
                                          int offset,
                                          int *location);
@@ -160,7 +164,6 @@ class UniformLinker final : angle::NonCopyable
    bool indexUniforms(InfoLog &infoLog, const ProgramBindings &uniformLocationBindings);
    bool gatherUniformLocationsAndCheckConflicts(InfoLog &infoLog,
                                                 const ProgramBindings &uniformLocationBindings,
-                                                 std::set<GLuint> *reservedLocations,
                                                 std::set<GLuint> *ignoredLocations,
                                                 int *maxUniformLocation);
    void pruneUnusedUniforms();