On Tue, Mar 11, 2014 at 10:37 AM, Ian Romanick <i...@freedesktop.org> wrote:
> 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. > 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); > > > > > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
