<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Mar 10, 2014 at 3:27 PM, Ian Romanick <span dir="ltr"><<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class=""><div class="h5">On 03/10/2014 11:19 AM, Anuj Phogat wrote:<br>
> Currently overlapping locations of input variables are not allowed for all<br>
> the shader types in OpenGL and OpenGL ES.<br>
><br>
>>From OpenGL ES 3.0 spec, page 56:<br>
> "Binding more than one attribute name to the same location is referred<br>
> to as aliasing, and is not permitted in OpenGL ES Shading Language<br>
> 3.00 vertex shaders. LinkProgram will fail when this condition exists.<br>
> However, aliasing is possible in OpenGL ES Shading Language 1.00 vertex<br>
> shaders."<br>
><br>
> Taking in to account what different versions of OpenGL and OpenGL ES specs<br>
> say about aliasing:<br>
> - It is allowed only on vertex shader input attributes in OpenGL (2.0 and<br>
> above) and OpenGL ES 2.0.<br>
> - It is explictly disallowed in OpenGL ES 3.0.<br>
><br>
> Fixes Khronos CTS failing test:<br>
> explicit_attrib_location_vertex_input_aliased.test<br>
> See more details about this at below mentioned khronos bug.<br>
><br>
> Signed-off-by: Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com">anuj.phogat@gmail.com</a>><br>
> Cc: "9.2 10.0 10.1" <<a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.freedesktop.org</a>><br>
> Bugzilla: Khronos #9609<br>
> ---<br>
> src/glsl/linker.cpp | 74 +++++++++++++++++++++++++++++++++++++++++++----------<br>
> 1 file changed, 61 insertions(+), 13 deletions(-)<br>
><br>
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp<br>
> index a619bc8..9bd698b 100644<br>
> --- a/src/glsl/linker.cpp<br>
> +++ b/src/glsl/linker.cpp<br>
> @@ -1798,10 +1798,12 @@ assign_attribute_or_color_locations(gl_shader_program *prog,<br>
> * active attribute array, both of which require multiple<br>
> * contiguous generic attributes."<br>
> *<br>
> - * Previous versions of the spec contain similar language but omit<br>
> - * the bit about attribute arrays.<br>
> + * I think above text prohibits the aliasing of explicit and<br>
> + * automatic assignments. But, aliasing is allowed in manual<br>
> + * assignments of attribute locations. See below comments for<br>
> + * the details.<br>
<br>
</div></div>The text above (outside the diff) forbids things like:<br>
<br>
layout(location=0) in mat4 m;<br>
layout(location=1) in vec4 v;<br>
<br>
where v and m now partially overlap because m consumes 4 slots (0<br>
through 3, inclusive).<br></blockquote><div><div><br></div><div>This should not be a problem as along as there's only one active attribute</div><div>in vertex shader. If a developer uses such locations, its his responsibility </div>
<div>to use just one of them in any path in shader program. Khronos CTS test</div><div>also expects linking to happen in above case. </div><div><br></div><div>OpenGL 4.0 spec makes similar looking statements at two different places.</div>
<div>First for vertex shader input locations at page 60:<br></div><div><br></div><div><div>"LinkProgram will fail if the attribute bindings assigned by BindAttri-</div><div>bLocation do not leave not enough space to assign a location for an active matrix</div>
<div>attribute or an active attribute array, both of which require multiple contiguous</div><div>generic attributes."</div></div><div><br></div><div>This statement sounds little unclear to me. It doesn't say what happens if</div>
<div>locations set in shader text don't leave enough space. It also doesn't make</div><div>clear who assigns the location in "to assign a location for an active matrix".</div><div>explicit or automatic assignment?</div>
<div><br></div><div>Second for fragment output locations at page 233:</div><div>"if the explicit binding assigments do not leave enough space for the linker</div><div>to automatically assign a location for a varying out array, which requires</div>
<div>multiple contiguous locations."</div></div><div><br></div><div>This statement makes the condition more clear for linking error. Notice the words</div><div> 'explicit' and 'automatically' in text from page 233.</div>
<div><br></div><div>So I interpreted the two statements as:</div><div><div>Aliasing of explicit and automatic assignments is prohibited.</div><div>But, aliasing is allowed in explicit assignments of attribute locations.</div>
</div><div><br></div><div>Do you interpret them differently ?</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
Does this patch affect this?<br></blockquote><div>Yes. It allows linking in such cases. </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><div class="h5"><br>
> *<br>
> - * Page 61 of the OpenGL 4.0 spec also says:<br>
> + * From OpenGL 4.0 spec, page 61:<br>
> *<br>
> * "It is possible for an application to bind more than one<br>
> * attribute name to the same location. This is referred to as<br>
> @@ -1814,12 +1816,48 @@ assign_attribute_or_color_locations(gl_shader_program *prog,<br>
> * but implementations are not required to generate an error<br>
> * in this case."<br>
> *<br>
> - * These two paragraphs are either somewhat contradictory, or I<br>
> - * don't fully understand one or both of them.<br>
> - */<br>
> - /* FINISHME: The code as currently written does not support<br>
> - * FINISHME: attribute location aliasing (see comment above).<br>
> + * From GLSL 4.30 spec, page 54:<br>
> + *<br>
> + * "A program will fail to link if any two non-vertex shader<br>
> + * input variables are assigned to the same location. For<br>
> + * vertex shaders, multiple input variables may be assigned<br>
> + * to the same location using either layout qualifiers or via<br>
> + * the OpenGL API. However, such aliasing is intended only to<br>
> + * support vertex shaders where each execution path accesses<br>
> + * at most one input per each location. Implementations are<br>
> + * permitted, but not required, to generate link-time errors<br>
> + * if they detect that every path through the vertex shader<br>
> + * executable accesses multiple inputs assigned to any single<br>
> + * location. For all shader types, a program will fail to link<br>
> + * if explicit location assignments leave the linker unable<br>
> + * to find space for other variables without explicit<br>
> + * assignments."<br>
> + *<br>
> + * From OpenGL ES 3.0 spec, page 56:<br>
> + *<br>
> + * "Binding more than one attribute name to the same location<br>
> + * is referred to as aliasing, and is not permitted in OpenGL<br>
> + * ES Shading Language 3.00 vertex shaders. LinkProgram will<br>
> + * fail when this condition exists. However, aliasing is<br>
> + * possible in OpenGL ES Shading Language 1.00 vertex shaders.<br>
> + * This will only work if only one of the aliased attributes<br>
> + * is active in the executable program, or if no path through<br>
> + * the shader consumes more than one attribute of a set of<br>
> + * attributes aliased to the same location. A link error can<br>
> + * occur if the linker determines that every path through the<br>
> + * shader consumes multiple aliased attributes, but implemen-<br>
> + * tations are not required to generate an error in this case."<br>
> + *<br>
> + * After looking at above references from OpenGL, OpenGL ES<br>
> + * and GLSL specifications, we allow aliasing of vertex input<br>
> + * variables in: OpenGL 2.0 (and above) and OpenGL ES 2.0.<br>
> + *<br>
> + * NOTE: This is not required by the spec but its worth<br>
> + * mentioning here that we're not doing anything to make sure<br>
> + * that no path through the vertex shader executable accesses<br>
> + * multiple inputs assigned to any single location.<br>
<br>
</div></div>These last two paragraphis should be un-indented.<br>
<div class=""><div class="h5"><br>
> */<br>
> +<br>
> /* Mask representing the contiguous slots that will be used by<br>
> * this attribute.<br>
> */<br>
> @@ -1832,11 +1870,21 @@ assign_attribute_or_color_locations(gl_shader_program *prog,<br>
> if ((~(use_mask << attr) & used_locations) != used_locations) {<br>
> const char *const string = (target_index == MESA_SHADER_VERTEX)<br>
> ? "vertex shader input" : "fragment shader output";<br>
> - linker_error(prog,<br>
> - "insufficient contiguous locations "<br>
> - "available for %s `%s' %d %d %d", string,<br>
> - var->name, used_locations, use_mask, attr);<br>
> - return false;<br>
> +<br>
> + if (target_index == MESA_SHADER_FRAGMENT ||<br>
> + (prog->IsES && prog->Version >= 300)) {<br>
> +<br>
> + linker_error(prog,<br>
> + "overlapping location is assigned "<br>
> + "to %s `%s' %d %d %d\n", string,<br>
> + var->name, used_locations, use_mask, attr);<br>
> + return false;<br>
> + } else {<br>
> + linker_warning(prog,<br>
> + "overlapping location is assigned "<br>
> + "to %s `%s' %d %d %d\n", string,<br>
> + var->name, used_locations, use_mask, attr);<br>
> + }<br>
> }<br>
><br>
> used_locations |= (use_mask << attr);<br>
<br>
</div></div></blockquote></div><br></div></div>