[Mesa-stable] [PATCH] glsl: Allow overlapping locations for vertex input attributes

[Ian Romanick]

On 03/10/2014 11:19 AM, Anuj Phogat wrote:
> Currently overlapping locations of input variables are not allowed for all
> the shader types in OpenGL and OpenGL ES.
> 
>>From OpenGL ES 3.0 spec, page 56:
>    "Binding more than one attribute name to the same location is referred
>     to as aliasing, and is not permitted in OpenGL ES Shading Language
>     3.00 vertex shaders. LinkProgram will fail when this condition exists.
>     However, aliasing is possible in OpenGL ES Shading Language 1.00 vertex
>     shaders."
> 
> Taking in to account what different versions of OpenGL and OpenGL ES specs
> say about aliasing:
>    - It is allowed only on vertex shader input attributes in OpenGL (2.0 and
>      above) and OpenGL ES 2.0.
>    - It is explictly disallowed in OpenGL ES 3.0.
> 
> Fixes Khronos CTS failing test:
> explicit_attrib_location_vertex_input_aliased.test
> See more details about this at below mentioned khronos bug.
> 
> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> Cc: "9.2 10.0 10.1" <mesa-stable at lists.freedesktop.org>
> Bugzilla: Khronos #9609
> ---
>  src/glsl/linker.cpp | 74 +++++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 61 insertions(+), 13 deletions(-)
> 
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index a619bc8..9bd698b 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -1798,10 +1798,12 @@ assign_attribute_or_color_locations(gl_shader_program *prog,
>  	     *     active attribute array, both of which require multiple
>  	     *     contiguous generic attributes."
>  	     *
> -	     * Previous versions of the spec contain similar language but omit
> -	     * the bit about attribute arrays.
> +	     * I think above text prohibits the aliasing of explicit and
> +	     * automatic assignments. But, aliasing is allowed in manual
> +	     * assignments of attribute locations. See below comments for
> +	     * the details.

The text above (outside the diff) forbids things like:

layout(location=0) in mat4 m;
layout(location=1) in vec4 v;

where v and m now partially overlap because m consumes 4 slots (0
through 3, inclusive).

Does this patch affect this?

>  	     *
> -	     * Page 61 of the OpenGL 4.0 spec also says:
> +	     * From OpenGL 4.0 spec, page 61:
>  	     *
>  	     *     "It is possible for an application to bind more than one
>  	     *     attribute name to the same location. This is referred to as
> @@ -1814,12 +1816,48 @@ assign_attribute_or_color_locations(gl_shader_program *prog,
>  	     *     but implementations are not required to generate an error
>  	     *     in this case."
>  	     *
> -	     * These two paragraphs are either somewhat contradictory, or I
> -	     * don't fully understand one or both of them.
> -	     */
> -	    /* FINISHME: The code as currently written does not support
> -	     * FINISHME: attribute location aliasing (see comment above).
> +	     * From GLSL 4.30 spec, page 54:
> +	     *
> +	     *    "A program will fail to link if any two non-vertex shader
> +	     *     input variables are assigned to the same location. For
> +	     *     vertex shaders, multiple input variables may be assigned
> +	     *     to the same location using either layout qualifiers or via
> +	     *     the OpenGL API. However, such aliasing is intended only to
> +	     *     support vertex shaders where each execution path accesses
> +	     *     at most one input per each location. Implementations are
> +	     *     permitted, but not required, to generate link-time errors
> +	     *     if they detect that every path through the vertex shader
> +	     *     executable accesses multiple inputs assigned to any single
> +	     *     location. For all shader types, a program will fail to link
> +	     *     if explicit location assignments leave the linker unable
> +	     *     to find space for other variables without explicit
> +	     *     assignments."
> +	     *
> +	     * From OpenGL ES 3.0 spec, page 56:
> +	     *
> +	     *    "Binding more than one attribute name to the same location
> +	     *     is referred to as aliasing, and is not permitted in OpenGL
> +	     *     ES Shading Language 3.00 vertex shaders. LinkProgram will
> +	     *     fail when this condition exists. However, aliasing is
> +	     *     possible in OpenGL ES Shading Language 1.00 vertex shaders.
> +	     *     This will only work if only one of the aliased attributes
> +	     *     is active in the executable program, or if no path through
> +	     *     the shader consumes more than one attribute of a set of
> +	     *     attributes aliased to the same location. A link error can
> +	     *     occur if the linker determines that every path through the
> +	     *     shader consumes multiple aliased attributes, but implemen-
> +	     *     tations are not required to generate an error in this case."
> +	     *
> +	     *     After looking at above references from OpenGL, OpenGL ES
> +	     *     and GLSL specifications, we allow aliasing of vertex input
> +	     *     variables in: OpenGL 2.0 (and above) and OpenGL ES 2.0.
> +	     *
> +	     *     NOTE: This is not required by the spec but its worth
> +	     *     mentioning here that we're not doing anything to make sure
> +	     *     that no path through the vertex shader executable accesses
> +	     *     multiple inputs assigned to any single location.

These last two paragraphis should be un-indented.

>  	     */
> +
>  	    /* Mask representing the contiguous slots that will be used by
>  	     * this attribute.
>  	     */
> @@ -1832,11 +1870,21 @@ assign_attribute_or_color_locations(gl_shader_program *prog,
>  	    if ((~(use_mask << attr) & used_locations) != used_locations) {
>  	       const char *const string = (target_index == MESA_SHADER_VERTEX)
>  		  ? "vertex shader input" : "fragment shader output";
> -	       linker_error(prog,
> -			    "insufficient contiguous locations "
> -			    "available for %s `%s' %d %d %d", string,
> -			    var->name, used_locations, use_mask, attr);
> -	       return false;
> +
> +               if (target_index == MESA_SHADER_FRAGMENT ||
> +                   (prog->IsES && prog->Version >= 300)) {
> +
> +                  linker_error(prog,
> +                               "overlapping location is assigned "
> +                               "to %s `%s' %d %d %d\n", string,
> +                               var->name, used_locations, use_mask, attr);
> +                  return false;
> +               } else {
> +                  linker_warning(prog,
> +                                 "overlapping location is assigned "
> +                                 "to %s `%s' %d %d %d\n", string,
> +                                 var->name, used_locations, use_mask, attr);
> +               }
>  	    }
>  
>  	    used_locations |= (use_mask << attr);