On Wed, 2019-03-20 at 20:56 +1100, Timothy Arceri wrote: > > On 20/3/19 7:58 pm, Juan A. Suarez Romero wrote: > > For the 4 first patches in the series: > > > > Reviewed-by: Juan A. Suarez <jasua...@igalia.com> > > > > > > On Fri, 2019-02-01 at 19:55 +0200, Andres Gomez wrote: > > > Added tests which check component aliasing between types that have > > > different bit widths. > > > > > > From Section 4.4.1 (Input Layout Qualifiers) of the GLSL 4.60 spec: > > > > > > "Further, when location aliasing, the aliases sharing the location > > > must have the same underlying numerical type and bit > > > width (floating-point or integer, 32-bit versus 64-bit, etc.) > > > and the same auxiliary storage and interpolation > > > qualification. The one exception where component aliasing is > > > permitted is for two input variables (not block members) to a > > > vertex shader, which are allowed to have component aliasing. This > > > vertex-variable component aliasing is intended only to support > > > vertex shaders where each execution path accesses at most one > > > input per each aliased component. 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 aliased to any single component." > > > > > > Cc: Timothy Arceri <tarc...@itsqueeze.com> > > > Cc: Iago Toral Quiroga <ito...@igalia.com> > > > Cc: Ilia Mirkin <imir...@alum.mit.edu> > > > Signed-off-by: Andres Gomez <ago...@igalia.com> > > > --- > > > .../type-mismatch-signed-double.vert | 59 +++++++++++++++++++ > > > .../width-mismatch-float-double.vert | 59 +++++++++++++++++++ > > > ...s-width-mismatch-double-float.shader_test} | 25 ++++---- > > > 3 files changed, 131 insertions(+), 12 deletions(-) > > > create mode 100644 > > > tests/spec/arb_enhanced_layouts/compiler/component-layout/type-mismatch-signed-double.vert > > > create mode 100644 > > > tests/spec/arb_enhanced_layouts/compiler/component-layout/width-mismatch-float-double.vert > > > rename > > > tests/spec/arb_enhanced_layouts/linker/component-layout/{vs-to-fs-type-mismatch-double-float.shader_test > > > => vs-to-fs-width-mismatch-double-float.shader_test} (56%) > > > > > > diff --git > > > a/tests/spec/arb_enhanced_layouts/compiler/component-layout/type-mismatch-signed-double.vert > > > > > > b/tests/spec/arb_enhanced_layouts/compiler/component-layout/type-mismatch-signed-double.vert > > > new file mode 100644 > > > index 000000000..01bfb0df1 > > > --- /dev/null > > > +++ > > > b/tests/spec/arb_enhanced_layouts/compiler/component-layout/type-mismatch-signed-double.vert > > > @@ -0,0 +1,59 @@ > > > +// [config] > > > +// expect_result: pass > > > +// glsl_version: 1.50 > > > +// check_link: true > > > +// require_extensions: GL_ARB_enhanced_layouts > > > GL_ARB_explicit_attrib_location GL_ARB_gpu_shader_fp64 > > > GL_ARB_vertex_attrib_64bit > > > +// [end config] > > > +// > > > +// From Section 4.4.1 (Input Layout Qualifiers) of the GLSL 4.50 spec: > > > +// > > > +// "Further, when location aliasing, the aliases sharing the location > > > must > > > +// have the same underlying numerical type (floating-point or integer) > > > and > > > +// the same auxiliary storage and interpolation qualification" > > > +// > > > +// ... > > > +// > > > +// "The one exception where component aliasing is permitted is for two > > > input > > > +// variables (not block members) to a vertex shader, which are allowed > > > to > > > +// have component aliasing. This vertex-variable component aliasing is > > > +// intended only to support vertex shaders where each execution path > > > +// accesses at most one input per each aliased component. > > > 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 aliased to any single component." > > > +// > > > +// Issue 16 from the ARB_enhanced_layouts spec: > > > +// > > > +// "We do allow this for vertex shader inputs, because we've supported > > > +// "aliasing" behavior since OpenGL 2.0. This allows for an > > > "uber-shader" > > > +// with variables like: > > > +// > > > +// layout(location=3) in float var1; > > > +// layout(location=3) in int var2; > > > +// > > > +// where sometimes it uses <var1> and sometimes <var2>. Since we don't > > > +// treat the code above (with overlapping components) as an error, it > > > +// would be strange to treat non-overlapping component assignments as > > > an > > > +// error." > > > + > > > +#version 150 > > > +#extension GL_ARB_enhanced_layouts: require > > > +#extension GL_ARB_explicit_attrib_location: require > > > +#extension GL_ARB_gpu_shader_fp64: require > > > +#extension GL_ARB_vertex_attrib_64bit: require > > > + > > > +uniform int i; > > > + > > > +// consume X/Y components > > > +layout(location = 0) in ivec2 a; > > > + > > > +// consume Z/W components > > > +layout(location = 0, component = 2) in double b; > > > + > > > +void main() > > > +{ > > > + if (i == 1) > > > + gl_Position = vec4(a, 1.0, 1.0); > > > + else > > > + gl_Position = vec4(b); > > > +} > > > diff --git > > > a/tests/spec/arb_enhanced_layouts/compiler/component-layout/width-mismatch-float-double.vert > > > > > > b/tests/spec/arb_enhanced_layouts/compiler/component-layout/width-mismatch-float-double.vert > > > new file mode 100644 > > > index 000000000..74926c1ea > > > --- /dev/null > > > +++ > > > b/tests/spec/arb_enhanced_layouts/compiler/component-layout/width-mismatch-float-double.vert > > > @@ -0,0 +1,59 @@ > > > +// [config] > > > +// expect_result: pass > > > +// glsl_version: 1.50 > > > +// check_link: true > > > +// require_extensions: GL_ARB_enhanced_layouts > > > GL_ARB_explicit_attrib_location GL_ARB_gpu_shader_fp64 > > > GL_ARB_vertex_attrib_64bit > > > +// [end config] > > > +// > > > +// From Section 4.4.1 (Input Layout Qualifiers) of the GLSL 4.60 spec: > > > +// > > > +// "Further, when location aliasing, the aliases sharing the > > > +// location must have the same underlying numerical type and bit > > > +// width (floating-point or integer, 32-bit versus 64-bit, etc.) > > This directly contradicts the example just above this text: > > From section 4.4.1 (Input Layout Qualifiers) of the GLSL 4.6 spec: > > "layout(location=8) in dvec3 h; // components 0,1,2 and 3 of location 8 > // and components 0 and 1 of location 9 > layout(location=9, component=2) in float i; // okay, compts 2 and 3"
See my answer at: https://lists.freedesktop.org/archives/mesa-dev/2019-March/216979.html > > > +// and the same auxiliary storage and interpolation > > > +// qualification. The one exception where component aliasing is > > > +// permitted is for two input variables (not block members) to a > > > +// vertex shader, which are allowed to have component > > > +// aliasing. This vertex-variable component aliasing is intended > > > +// only to support vertex shaders where each execution path > > > +// accesses at most one input per each aliased > > > +// component. 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 aliased to > > > +// any single component." > > > +// > > > +// Issue 16 from the ARB_enhanced_layouts spec: > > > +// > > > +// "We do allow this for vertex shader inputs, because we've supported > > > +// "aliasing" behavior since OpenGL 2.0. This allows for an > > > "uber-shader" > > > +// with variables like: > > > +// > > > +// layout(location=3) in float var1; > > > +// layout(location=3) in int var2; > > > +// > > > +// where sometimes it uses <var1> and sometimes <var2>. Since we don't > > > +// treat the code above (with overlapping components) as an error, it > > > +// would be strange to treat non-overlapping component assignments as > > > an > > > +// error." > > > + > > > +#version 150 > > > +#extension GL_ARB_enhanced_layouts: require > > > +#extension GL_ARB_explicit_attrib_location: require > > > +#extension GL_ARB_gpu_shader_fp64: require > > > +#extension GL_ARB_vertex_attrib_64bit: require > > > + > > > +uniform int i; > > > + > > > +// consume X/Y components > > > +layout(location = 0) in vec2 a; > > > + > > > +// consume Z/W components > > > +layout(location = 0, component = 2) in double b; > > > + > > > +void main() > > > +{ > > > + if (i == 1) > > > + gl_Position = vec4(a, 1.0, 1.0); > > > + else > > > + gl_Position = vec4(b); > > > +} > > > diff --git > > > a/tests/spec/arb_enhanced_layouts/linker/component-layout/vs-to-fs-type-mismatch-double-float.shader_test > > > > > > b/tests/spec/arb_enhanced_layouts/linker/component-layout/vs-to-fs-width-mismatch-double-float.shader_test > > > similarity index 56% > > > rename from > > > tests/spec/arb_enhanced_layouts/linker/component-layout/vs-to-fs-type-mismatch-double-float.shader_test > > > rename to > > > tests/spec/arb_enhanced_layouts/linker/component-layout/vs-to-fs-width-mismatch-double-float.shader_test > > > index ed596b3dc..fefff6d78 100644 > > > --- > > > a/tests/spec/arb_enhanced_layouts/linker/component-layout/vs-to-fs-type-mismatch-double-float.shader_test > > > +++ > > > b/tests/spec/arb_enhanced_layouts/linker/component-layout/vs-to-fs-width-mismatch-double-float.shader_test > > > @@ -1,16 +1,17 @@ > > > -// From Section 4.4.1 (Input Layout Qualifiers) of the GLSL 4.50 spec: > > > +// From Section 4.4.1 (Input Layout Qualifiers) of the GLSL 4.60 spec: > > > // > > > -// "Location aliasing is causing two variables or block members to > > > have the > > > -// same location number. Component aliasing is assigning the same (or > > > -// overlapping) component numbers for two location aliases. (Recall if > > > -// component is not used, components are assigned starting with 0.) > > > With one > > > -// exception, location aliasing is allowed only if it does not cause > > > -// component aliasing; it is a compile-time or link-time error to cause > > > -// component aliasing." > > > -// > > > -// "Further, when location aliasing, the aliases sharing the location > > > must > > > -// have the same underlying numerical type (floating-point or integer) > > > and > > > -// the same auxiliary storage and interpolation qualification" > > > +// "Location aliasing is causing two variables or block members to > > > +// have the same location number. Component aliasing is assigning > > > +// the same (or overlapping) component numbers for two location > > > +// aliases. (Recall if component is not used, components are > > > +// assigned starting with 0.) With one exception, location aliasing > > > +// is allowed only if it does not cause component aliasing; it is a > > > +// compile-time or link-time error to cause component > > > +// aliasing. Further, when location aliasing, the aliases sharing > > > +// the location must have the same underlying numerical type and > > > +// bit width (floating-point or integer, 32-bit versus 64-bit, > > > +// etc.) and the same auxiliary storage and interpolation > > > +// qualification." > > > > > > [require] > > > GLSL >= 1.50 > > > > _______________________________________________ > > Piglit mailing list > > Piglit@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/piglit > > -- Br, Andres _______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit