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

Anuj Phogat anuj.phogat at gmail.com
Thu Mar 13 15:39:58 PDT 2014


On Tue, Mar 11, 2014 at 10:37 AM, Ian Romanick <idr at freedesktop.org> wrote:

> 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.
>
Piglit tests I posted on mailing list passes on NVIDIA's OpenGL 4.3 linux
driver. NVIDIA allows the kind of overlapping you described above.

>
> > 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);
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-stable/attachments/20140313/aea0e640/attachment-0001.html>


More information about the mesa-stable mailing list