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

Ian Romanick idr at freedesktop.org
Tue Mar 11 10:37:23 PDT 2014


On 03/10/2014 04:15 PM, Anuj Phogat wrote:
> 
> 
> 
> On Mon, Mar 10, 2014 at 3:27 PM, Ian Romanick <idr at freedesktop.org
> <mailto:idr at freedesktop.org>> wrote:
> 
>     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
>     <mailto:anuj.phogat at gmail.com>>
>     > Cc: "9.2 10.0 10.1" <mesa-stable at lists.freedesktop.org
>     <mailto: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).
> 
> 
> This should not be a problem as along as there's only one active attribute
> in vertex shader. If a developer uses such locations, its his
> responsibility 
> to use just one of them in any path in shader program. Khronos CTS test
> also expects linking to happen in above case. 
> 
> OpenGL 4.0 spec makes similar looking statements at two different places.
> First  for vertex shader input locations at page 60:
> 
> "LinkProgram will fail if the attribute bindings assigned by BindAttri-
> bLocation do not leave not enough space to assign a location for an
> active matrix
> attribute or an active attribute array, both of which require multiple
> contiguous
> generic attributes."

Somewhere in the intervening specs, the language was changed.  In 4.4 it
says:

    "LinkProgram will fail if the attribute bindings specified either
    by BindAttribLocation or explicitly set within the shader text do
    not leave not enough space to assign a location for an active
    matrix attribute or an active attribute array, both of which
    require multiple contiguous generic attributes."

> This statement sounds little unclear to me. It doesn't say  what happens if
> locations set in shader text don't leave enough space. It also doesn't make
> clear who assigns the location in "to assign a location for an active
> matrix".
> explicit or automatic assignment?

You may be right.  We'll have to prod other implementations.  I'll also
do some spec archaeology.

> Second for fragment output locations at page 233:
> "if the explicit binding assigments do not leave enough space for the linker
> to automatically assign a location for a varying out array, which requires
> multiple contiguous locations."
> 
> This statement makes the condition more clear for linking error. Notice
> the words
>  'explicit' and 'automatically' in text from page 233.
> 
> So I interpreted the two statements as:
> Aliasing of explicit and automatic assignments is prohibited.
> But, aliasing is allowed in explicit assignments of attribute locations.
> 
> Do you interpret them differently ?
> 
> 
>     Does this patch affect this?
> 
> Yes. It allows linking in such cases. 
> 
> 
>     >            *
>     > -          * 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);
> 
> 



More information about the mesa-stable mailing list