[Mesa-dev] [Mesa-stable] [PATCH] glsl: Allow overlapping locations for vertex input attributes
Anuj Phogat
anuj.phogat at gmail.com
Mon Mar 10 16:15:41 PDT 2014
On Mon, Mar 10, 2014 at 3:27 PM, Ian Romanick <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>
> > 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).
>
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."
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?
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-dev/attachments/20140310/d5aec8b0/attachment.html>
More information about the mesa-dev
mailing list