<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Tue, Mar 11, 2014 at 10:37 AM, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>On 03/10/2014 04:15 PM, Anuj Phogat wrote: > > > > On Mon, Mar 10, 2014 at 3:27 PM, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a> </div><div><div>> <mailto:<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>>> 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 <<a href="mailto:anuj.phogat@gmail.com" target="_blank">anuj.phogat@gmail.com</a> </div></div>> <mailto:<a href="mailto:anuj.phogat@gmail.com" target="_blank">anuj.phogat@gmail.com</a>>> <div>> > Cc: "9.2 10.0 10.1" <<a href="mailto:mesa-stable@lists.freedesktop.org" target="_blank">mesa-stable@lists.freedesktop.org</a> </div>> <mailto:<a href="mailto:mesa-stable@lists.freedesktop.org" target="_blank">mesa-stable@lists.freedesktop.org</a>>> <div><div>> > 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." </div></div>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 <div> 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? </div>You may be right. We'll have to prod other implementations. I'll also do some spec archaeology. </blockquote><div>Piglit tests I posted on mailing list passes on NVIDIA's OpenGL 4.3 linux</div><div>driver. NVIDIA allows the kind of overlapping you described above.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div><div> > 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); > > </div></div></blockquote></div> </div></div>