<div dir="ltr"><div>Hello,</div><div> </div><div>Thanks a lot for review.</div><div> </div><div>Regards,</div><div>Andrii. </div></div> <div class="gmail_quote"><div dir="ltr">On Tue, Nov 27, 2018 at 11:23 AM Tapani Pälli <<a href="mailto:tapani.palli@intel.com">tapani.palli@intel.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi; On 10/31/18 5:05 PM, <a href="mailto:asimiklit.work@gmail.com" target="_blank">asimiklit.work@gmail.com</a> wrote: > From: Andrii Simiklit <<a href="mailto:andrii.simiklit@globallogic.com" target="_blank">andrii.simiklit@globallogic.com</a>> > > I guess we should not expect a valid location for: > "patch out vec4 tcs_patch;" > because this output variable is declareted > without "layout (location=X)" and according to spec: > "Not all active variables are assigned valid locations; the > following variables will have an effective location of -1: > > * uniforms declared as atomic counters; > > * members of a uniform block; > > * built-in inputs, outputs, and uniforms (starting with "gl_"); and > > * inputs or outputs not declared with a "location" layout qualifier, > except for vertex shader inputs and fragment shader outputs." I've verified that this is the way it is. It took me a while to digest this since the test uses PROGRAM_SEPARABLE here and the rules of the extension are quite complicated. However, I'm convinced now and this LGTM :) Reviewed-by: Tapani Pälli <<a href="mailto:tapani.palli@intel.com" target="_blank">tapani.palli@intel.com</a>> > Also I fixed some conflicting comments. > > Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=108612" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=108612</a> > Signed-off-by: Andrii Simiklit <<a href="mailto:andrii.simiklit@globallogic.com" target="_blank">andrii.simiklit@globallogic.com</a>> > --- > .../getprogramresourceiv.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/tests/spec/arb_program_interface_query/getprogramresourceiv.c b/tests/spec/arb_program_interface_query/getprogramresourceiv.c > index 2727a6c87..70212350e 100755 > --- a/tests/spec/arb_program_interface_query/getprogramresourceiv.c > +++ b/tests/spec/arb_program_interface_query/getprogramresourceiv.c > @@ -360,7 +360,7 @@ static const struct subtest_t subtests[] = { > { GL_REFERENCED_BY_GEOMETRY_SHADER, 1, { 0 } }, > { GL_REFERENCED_BY_FRAGMENT_SHADER, 1, { 0 } }, > { GL_REFERENCED_BY_COMPUTE_SHADER, 1, { 0 } }, > - { GL_LOCATION, 1, { -1 } }, /* valid index == anything but -1 */ > + { GL_LOCATION, 1, { -1 } }, /* invalid index */ > { 0, 0, { 0 } }} > }, > { &prog_loc, GL_PROGRAM_INPUT, "input0", NULL, { > @@ -396,12 +396,12 @@ static const struct subtest_t subtests[] = { > { GL_NAME_LENGTH, 1, { 6 } }, > { GL_TYPE, 1, { GL_FLOAT_VEC4 } }, > { GL_ARRAY_SIZE, 1, { 1 } }, > - { GL_OFFSET, 1, { -1 } }, /* valid index == anything but -1 */ > + { GL_OFFSET, 1, { -1 } }, /* invalid index */ > { GL_BLOCK_INDEX, 1, { -1 } }, /* invalid index */ > - { GL_ARRAY_STRIDE, 1, { -1 } }, /* valid index == anything but -1 */ > + { GL_ARRAY_STRIDE, 1, { -1 } }, /* invalid index */ > { GL_MATRIX_STRIDE, 1, { -1 } }, > { GL_IS_ROW_MAJOR, 1, { 0 } }, > - { GL_ATOMIC_COUNTER_BUFFER_INDEX, 1, { -1 } }, /* valid index == anything but -1 */ > + { GL_ATOMIC_COUNTER_BUFFER_INDEX, 1, { -1 } }, /* invalid index */ > { GL_REFERENCED_BY_VERTEX_SHADER, 1, { 0 } }, > { GL_REFERENCED_BY_TESS_CONTROL_SHADER, 1, { 0 } }, > { GL_REFERENCED_BY_TESS_EVALUATION_SHADER, 1, { 0 } }, > @@ -421,8 +421,8 @@ static const struct subtest_t subtests[] = { > { GL_REFERENCED_BY_GEOMETRY_SHADER, 1, { 0 } }, > { GL_REFERENCED_BY_FRAGMENT_SHADER, 1, { 0 } }, > { GL_REFERENCED_BY_COMPUTE_SHADER, 1, { 0 } }, > - { GL_LOCATION, 1, { 0 } }, /* valid index == anything but -1 */ > - { GL_LOCATION_INDEX, 1, { -1 } }, /* valid index == anything but -1 */ > + { GL_LOCATION, 1, { -1 } }, /* invalid index */ > + { GL_LOCATION_INDEX, 1, { -1 } }, /* invalid index */ > { GL_IS_PER_PATCH, 1, { 1 } }, > { 0, 0, { 0 } }} > }, > @@ -435,14 +435,14 @@ static const struct subtest_t subtests[] = { > { GL_ARRAY_STRIDE, 1, { 0 } }, /* valid index == anything but -1 */ > { GL_MATRIX_STRIDE, 1, { 0 } }, > { GL_IS_ROW_MAJOR, 1, { 0 } }, > - { GL_ATOMIC_COUNTER_BUFFER_INDEX, 1, { -1 } }, /* valid index == anything but -1 */ > + { GL_ATOMIC_COUNTER_BUFFER_INDEX, 1, { -1 } }, /* invalid index */ > { GL_REFERENCED_BY_VERTEX_SHADER, 1, { 0 } }, > { GL_REFERENCED_BY_TESS_CONTROL_SHADER, 1, { 0 } }, > { GL_REFERENCED_BY_TESS_EVALUATION_SHADER, 1, { 0 } }, > { GL_REFERENCED_BY_GEOMETRY_SHADER, 1, { 0 } }, > { GL_REFERENCED_BY_FRAGMENT_SHADER, 1, { 1 } }, > { GL_REFERENCED_BY_COMPUTE_SHADER, 1, { 0 } }, > - { GL_LOCATION, 1, { -1 } }, /* valid index == anything but -1 */ > + { GL_LOCATION, 1, { -1 } }, /* invalid index */ > { 0, 0, { 0 } }} > }, > { &prog_std, GL_UNIFORM_BLOCK, "fs_uniform_block", fs_std_fs_uniform_blk, { > </blockquote></div>