On 03/10/2014 04:15 PM, Anuj Phogat wrote:
>
>
>
> On Mon, Mar 10, 2014 at 3:27 PM, Ian Romanick <i...@freedesktop.org
> <mailto:i...@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.pho...@gmail.com
> <mailto:anuj.pho...@gmail.com>>
> > Cc: "9.2 10.0 10.1" <mesa-sta...@lists.freedesktop.org
> <mailto:mesa-sta...@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);
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
