Re: [Mesa-dev] [PATCH 1/3] glsl: XFB TSC per-vertex output varyings match as not declared as arrays

[Chema Casanova]

V2 of this series that address the fix for
KHR-GL*.tessellation_shader.single.xfb_captures_data_from_correct_stage
regressions is available at Gitlab MR!300

https://gitlab.freedesktop.org/mesa/mesa/merge_requests/300

On 21/11/18 19:45, Jose Maria Casanova Crespo wrote:
> Recent change on OpenGL CTS ("Use non-arrayed varying name for TCS blocks")
> on KHR-GL*.tessellation_shader.single.xfb_captures_data_from_correct_stage
> tests changed how to name per-vertex Tessellation Control Shader output
> varyings in transform feedback using interface block as "BLOCK_INOUT.value"
> rather than "BLOCK_INOUT[0].value"
> 
> So Tessellation control shader per-vertex output variables and blocks that
> are required to be declared as arrays, with each element representing output
> values for a single vertex of a multi-vertex primitive are expected to be
> named as they were not declared as arrays.
> 
> This patch adds a new is_xfb_per_vertex_output flag at ir_variable level so
> we mark when an ir_variable is an per-vertex TCS output varying. So we
> treat it in terms on XFB its naming as a non array variable.
> 
> As we don't support NV_gpu_shader5, so PATCHES mode is not accepted as
> primitiveMode parameter of BeginTransformFeedback the test expects a
> failure as we can use the XFB results.
> 
> This patch uncovers that we were passing the GLES version of the tests
> because candidates naming didn't match, not because on GLES the Tessellation
> Control stage varyings shouldn't be XFB candidates in any case. This
> is addressed in the following patch.
> 
> Fixes: 
> KHR-GL4*.tessellation_shader.single.xfb_captures_data_from_correct_stage
> 
> Cc: mesa-sta...@lists.freedesktop.org
> ---
>  src/compiler/glsl/ir.cpp| 1 +
>  src/compiler/glsl/ir.h  | 6 ++
>  src/compiler/glsl/link_uniforms.cpp | 6 --
>  src/compiler/glsl/link_varyings.cpp | 8 +++-
>  4 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
> index 1d1a56ae9a5..582111d71f5 100644
> --- a/src/compiler/glsl/ir.cpp
> +++ b/src/compiler/glsl/ir.cpp
> @@ -1750,6 +1750,7 @@ ir_variable::ir_variable(const struct glsl_type *type, 
> const char *name,
> this->data.fb_fetch_output = false;
> this->data.bindless = false;
> this->data.bound = false;
> +   this->data.is_xfb_per_vertex_output = false;
>  
> if (type != NULL) {
>if (type->is_interface())
> diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
> index f478b29a6b5..e09f053b77c 100644
> --- a/src/compiler/glsl/ir.h
> +++ b/src/compiler/glsl/ir.h
> @@ -766,6 +766,12 @@ public:
> */
>unsigned is_xfb_only:1;
>  
> +  /**
> +   * Is this varying a TSC per-vertex output candidate for transform
> +   * feedback?
> +   */
> +  unsigned is_xfb_per_vertex_output:1;
> +
>/**
> * Was a transfor feedback buffer set in the shader?
> */
> diff --git a/src/compiler/glsl/link_uniforms.cpp 
> b/src/compiler/glsl/link_uniforms.cpp
> index 63e688b19a7..547da68e216 100644
> --- a/src/compiler/glsl/link_uniforms.cpp
> +++ b/src/compiler/glsl/link_uniforms.cpp
> @@ -72,8 +72,10 @@ program_resource_visitor::process(ir_variable *var, bool 
> use_std430_as_default)
>   get_internal_ifc_packing(use_std430_as_default) :
>var->type->get_internal_ifc_packing(use_std430_as_default);
>  
> -   const glsl_type *t =
> -  var->data.from_named_ifc_block ? var->get_interface_type() : var->type;
> +   const glsl_type *t = var->data.from_named_ifc_block ?
> +  (var->data.is_xfb_per_vertex_output ?
> +   var->get_interface_type()->without_array() :
> +   var->get_interface_type()) : var->type;
> const glsl_type *t_without_array = t->without_array();
>  
> /* false is always passed for the row_major parameter to the other
> diff --git a/src/compiler/glsl/link_varyings.cpp 
> b/src/compiler/glsl/link_varyings.cpp
> index 52e493cb599..1964dcc0a22 100644
> --- a/src/compiler/glsl/link_varyings.cpp
> +++ b/src/compiler/glsl/link_varyings.cpp
> @@ -2150,7 +2150,10 @@ private:
>tfeedback_candidate *candidate
>   = rzalloc(this->mem_ctx, tfeedback_candidate);
>candidate->toplevel_var = this->toplevel_var;
> -  candidate->type = type;
> +  if (this->toplevel_var->data.is_xfb_per_vertex_output)
> + candidate->type = type->without_array();
> +  else
> + candidate->type = type;
>candidate->offset = this->varying_floats;
>_mesa_hash_table_insert(this->tfeedback_candidates,
>ralloc_strdup(this->mem_ctx, name),
> @@ -2499,6 +2502,9 @@ assign_varying_locations(struct gl_context *ctx,
>  
>   if (num_tfeedback_decls > 0) {
>  tfeedback_candidate_generator g(mem_ctx, tfeedback_candidates);
> +if (producer->Stage == MESA_SHADER_TESS_CTRL &&
> +!output_var->data.patch)
> +   

Re: [Mesa-dev] [PATCH 1/3] glsl: XFB TSC per-vertex output varyings match as not declared as arrays

[Ilia Mirkin]

IMHO this is done at the wrong level. You're effectively keeping track
of a per-variable "this variable belongs to a tcs output that's not a
patch" bit. Doesn't seem worthy of a whole separate bit.

Also, you use ->without_array() a lot, but what you really need to do
is remove *one* layer of array-ness, since with AoA there could be
multiple.

There's already a function in link_varyings.cpp called
"get_varying_type()" I believe it does what you want.

Cheers,

  -ilia

On Thu, Dec 13, 2018 at 12:20 PM Chema Casanova  wrote:
>
> Ping.
>
> El 22/11/18 a las 0:28, Chema Casanova escribió:
> >
> >
> > On 21/11/18 20:04, Ilia Mirkin wrote:
> >> On Wed, Nov 21, 2018 at 1:45 PM Jose Maria Casanova Crespo
> >>  wrote:
> >>>
> >>> Recent change on OpenGL CTS ("Use non-arrayed varying name for TCS 
> >>> blocks")
> >>> on KHR-GL*.tessellation_shader.single.xfb_captures_data_from_correct_stage
> >>> tests changed how to name per-vertex Tessellation Control Shader output
> >>> varyings in transform feedback using interface block as 
> >>> "BLOCK_INOUT.value"
> >>> rather than "BLOCK_INOUT[0].value"
> >>>
> >>> So Tessellation control shader per-vertex output variables and blocks that
> >>> are required to be declared as arrays, with each element representing 
> >>> output
> >>> values for a single vertex of a multi-vertex primitive are expected to be
> >>> named as they were not declared as arrays.
> >>>
> >>> This patch adds a new is_xfb_per_vertex_output flag at ir_variable level 
> >>> so
> >>> we mark when an ir_variable is an per-vertex TCS output varying. So we
> >>> treat it in terms on XFB its naming as a non array variable.
> >>>
> >>> As we don't support NV_gpu_shader5, so PATCHES mode is not accepted as
> >>> primitiveMode parameter of BeginTransformFeedback the test expects a
> >>> failure as we can use the XFB results.
> >>>
> >>> This patch uncovers that we were passing the GLES version of the tests
> >>> because candidates naming didn't match, not because on GLES the 
> >>> Tessellation
> >>> Control stage varyings shouldn't be XFB candidates in any case. This
> >>> is addressed in the following patch.
> >>>
> >>> Fixes: 
> >>> KHR-GL4*.tessellation_shader.single.xfb_captures_data_from_correct_stage
> >>>
> >>> Cc: mesa-sta...@lists.freedesktop.org
> >>> ---
> >>>  src/compiler/glsl/ir.cpp| 1 +
> >>>  src/compiler/glsl/ir.h  | 6 ++
> >>>  src/compiler/glsl/link_uniforms.cpp | 6 --
> >>>  src/compiler/glsl/link_varyings.cpp | 8 +++-
> >>>  4 files changed, 18 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
> >>> index 1d1a56ae9a5..582111d71f5 100644
> >>> --- a/src/compiler/glsl/ir.cpp
> >>> +++ b/src/compiler/glsl/ir.cpp
> >>> @@ -1750,6 +1750,7 @@ ir_variable::ir_variable(const struct glsl_type 
> >>> *type, const char *name,
> >>> this->data.fb_fetch_output = false;
> >>> this->data.bindless = false;
> >>> this->data.bound = false;
> >>> +   this->data.is_xfb_per_vertex_output = false;
> >>>
> >>> if (type != NULL) {
> >>>if (type->is_interface())
> >>> diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
> >>> index f478b29a6b5..e09f053b77c 100644
> >>> --- a/src/compiler/glsl/ir.h
> >>> +++ b/src/compiler/glsl/ir.h
> >>> @@ -766,6 +766,12 @@ public:
> >>> */
> >>>unsigned is_xfb_only:1;
> >>>
> >>> +  /**
> >>> +   * Is this varying a TSC per-vertex output candidate for transform
> >>
> >> TCS?
> >
> >
> > Yes. I've fixed it locally at the commit summary too.
> >
> >
> >>> +   * feedback?
> >>> +   */
> >>> +  unsigned is_xfb_per_vertex_output:1;
> >>> +
> >>>/**
> >>> * Was a transfor feedback buffer set in the shader?
> >>
> >> ugh, not your problem, but "transform" :(
> >>
> >>> */
> >>> diff --git a/src/compiler/glsl/link_uniforms.cpp 
> >>> b/src/compiler/glsl/link_uniforms.cpp
> >>> index 63e688b19a7..547da68e216 100644
> >>> --- a/src/compiler/glsl/link_uniforms.cpp
> >>> +++ b/src/compiler/glsl/link_uniforms.cpp
> >>> @@ -72,8 +72,10 @@ program_resource_visitor::process(ir_variable *var, 
> >>> bool use_std430_as_default)
> >>>   get_internal_ifc_packing(use_std430_as_default) :
> >>>var->type->get_internal_ifc_packing(use_std430_as_default);
> >>>
> >>> -   const glsl_type *t =
> >>> -  var->data.from_named_ifc_block ? var->get_interface_type() : 
> >>> var->type;
> >>> +   const glsl_type *t = var->data.from_named_ifc_block ?
> >>> +  (var->data.is_xfb_per_vertex_output ?
> >>> +   var->get_interface_type()->without_array() :
> >>> +   var->get_interface_type()) : var->type;
> >>> const glsl_type *t_without_array = t->without_array();
> >>>
> >>> /* false is always passed for the row_major parameter to the other
> >>> diff --git a/src/compiler/glsl/link_varyings.cpp 
> >>> b/src/compiler/glsl/link_varyings.cpp
> >>> index 52e493cb599..1964dcc0a22 100644
> >>> --- 

Re: [Mesa-dev] [PATCH 1/3] glsl: XFB TSC per-vertex output varyings match as not declared as arrays

[Chema Casanova]

Ping.

El 22/11/18 a las 0:28, Chema Casanova escribió:
> 
> 
> On 21/11/18 20:04, Ilia Mirkin wrote:
>> On Wed, Nov 21, 2018 at 1:45 PM Jose Maria Casanova Crespo
>>  wrote:
>>>
>>> Recent change on OpenGL CTS ("Use non-arrayed varying name for TCS blocks")
>>> on KHR-GL*.tessellation_shader.single.xfb_captures_data_from_correct_stage
>>> tests changed how to name per-vertex Tessellation Control Shader output
>>> varyings in transform feedback using interface block as "BLOCK_INOUT.value"
>>> rather than "BLOCK_INOUT[0].value"
>>>
>>> So Tessellation control shader per-vertex output variables and blocks that
>>> are required to be declared as arrays, with each element representing output
>>> values for a single vertex of a multi-vertex primitive are expected to be
>>> named as they were not declared as arrays.
>>>
>>> This patch adds a new is_xfb_per_vertex_output flag at ir_variable level so
>>> we mark when an ir_variable is an per-vertex TCS output varying. So we
>>> treat it in terms on XFB its naming as a non array variable.
>>>
>>> As we don't support NV_gpu_shader5, so PATCHES mode is not accepted as
>>> primitiveMode parameter of BeginTransformFeedback the test expects a
>>> failure as we can use the XFB results.
>>>
>>> This patch uncovers that we were passing the GLES version of the tests
>>> because candidates naming didn't match, not because on GLES the Tessellation
>>> Control stage varyings shouldn't be XFB candidates in any case. This
>>> is addressed in the following patch.
>>>
>>> Fixes: 
>>> KHR-GL4*.tessellation_shader.single.xfb_captures_data_from_correct_stage
>>>
>>> Cc: mesa-sta...@lists.freedesktop.org
>>> ---
>>>  src/compiler/glsl/ir.cpp| 1 +
>>>  src/compiler/glsl/ir.h  | 6 ++
>>>  src/compiler/glsl/link_uniforms.cpp | 6 --
>>>  src/compiler/glsl/link_varyings.cpp | 8 +++-
>>>  4 files changed, 18 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
>>> index 1d1a56ae9a5..582111d71f5 100644
>>> --- a/src/compiler/glsl/ir.cpp
>>> +++ b/src/compiler/glsl/ir.cpp
>>> @@ -1750,6 +1750,7 @@ ir_variable::ir_variable(const struct glsl_type 
>>> *type, const char *name,
>>> this->data.fb_fetch_output = false;
>>> this->data.bindless = false;
>>> this->data.bound = false;
>>> +   this->data.is_xfb_per_vertex_output = false;
>>>
>>> if (type != NULL) {
>>>if (type->is_interface())
>>> diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
>>> index f478b29a6b5..e09f053b77c 100644
>>> --- a/src/compiler/glsl/ir.h
>>> +++ b/src/compiler/glsl/ir.h
>>> @@ -766,6 +766,12 @@ public:
>>> */
>>>unsigned is_xfb_only:1;
>>>
>>> +  /**
>>> +   * Is this varying a TSC per-vertex output candidate for transform
>>
>> TCS?
> 
> 
> Yes. I've fixed it locally at the commit summary too.
> 
> 
>>> +   * feedback?
>>> +   */
>>> +  unsigned is_xfb_per_vertex_output:1;
>>> +
>>>/**
>>> * Was a transfor feedback buffer set in the shader?
>>
>> ugh, not your problem, but "transform" :(
>>
>>> */
>>> diff --git a/src/compiler/glsl/link_uniforms.cpp 
>>> b/src/compiler/glsl/link_uniforms.cpp
>>> index 63e688b19a7..547da68e216 100644
>>> --- a/src/compiler/glsl/link_uniforms.cpp
>>> +++ b/src/compiler/glsl/link_uniforms.cpp
>>> @@ -72,8 +72,10 @@ program_resource_visitor::process(ir_variable *var, bool 
>>> use_std430_as_default)
>>>   get_internal_ifc_packing(use_std430_as_default) :
>>>var->type->get_internal_ifc_packing(use_std430_as_default);
>>>
>>> -   const glsl_type *t =
>>> -  var->data.from_named_ifc_block ? var->get_interface_type() : 
>>> var->type;
>>> +   const glsl_type *t = var->data.from_named_ifc_block ?
>>> +  (var->data.is_xfb_per_vertex_output ?
>>> +   var->get_interface_type()->without_array() :
>>> +   var->get_interface_type()) : var->type;
>>> const glsl_type *t_without_array = t->without_array();
>>>
>>> /* false is always passed for the row_major parameter to the other
>>> diff --git a/src/compiler/glsl/link_varyings.cpp 
>>> b/src/compiler/glsl/link_varyings.cpp
>>> index 52e493cb599..1964dcc0a22 100644
>>> --- a/src/compiler/glsl/link_varyings.cpp
>>> +++ b/src/compiler/glsl/link_varyings.cpp
>>> @@ -2150,7 +2150,10 @@ private:
>>>tfeedback_candidate *candidate
>>>   = rzalloc(this->mem_ctx, tfeedback_candidate);
>>>candidate->toplevel_var = this->toplevel_var;
>>> -  candidate->type = type;
>>> +  if (this->toplevel_var->data.is_xfb_per_vertex_output)
>>> + candidate->type = type->without_array();
>>> +  else
>>> + candidate->type = type;
>>>candidate->offset = this->varying_floats;
>>>_mesa_hash_table_insert(this->tfeedback_candidates,
>>>ralloc_strdup(this->mem_ctx, name),
>>> @@ -2499,6 +2502,9 @@ assign_varying_locations(struct gl_context *ctx,
>>>
>>>   if 

Re: [Mesa-dev] [PATCH 1/3] glsl: XFB TSC per-vertex output varyings match as not declared as arrays

[Chema Casanova]

On 21/11/18 20:04, Ilia Mirkin wrote:
> On Wed, Nov 21, 2018 at 1:45 PM Jose Maria Casanova Crespo
>  wrote:
>>
>> Recent change on OpenGL CTS ("Use non-arrayed varying name for TCS blocks")
>> on KHR-GL*.tessellation_shader.single.xfb_captures_data_from_correct_stage
>> tests changed how to name per-vertex Tessellation Control Shader output
>> varyings in transform feedback using interface block as "BLOCK_INOUT.value"
>> rather than "BLOCK_INOUT[0].value"
>>
>> So Tessellation control shader per-vertex output variables and blocks that
>> are required to be declared as arrays, with each element representing output
>> values for a single vertex of a multi-vertex primitive are expected to be
>> named as they were not declared as arrays.
>>
>> This patch adds a new is_xfb_per_vertex_output flag at ir_variable level so
>> we mark when an ir_variable is an per-vertex TCS output varying. So we
>> treat it in terms on XFB its naming as a non array variable.
>>
>> As we don't support NV_gpu_shader5, so PATCHES mode is not accepted as
>> primitiveMode parameter of BeginTransformFeedback the test expects a
>> failure as we can use the XFB results.
>>
>> This patch uncovers that we were passing the GLES version of the tests
>> because candidates naming didn't match, not because on GLES the Tessellation
>> Control stage varyings shouldn't be XFB candidates in any case. This
>> is addressed in the following patch.
>>
>> Fixes: 
>> KHR-GL4*.tessellation_shader.single.xfb_captures_data_from_correct_stage
>>
>> Cc: mesa-sta...@lists.freedesktop.org
>> ---
>>  src/compiler/glsl/ir.cpp| 1 +
>>  src/compiler/glsl/ir.h  | 6 ++
>>  src/compiler/glsl/link_uniforms.cpp | 6 --
>>  src/compiler/glsl/link_varyings.cpp | 8 +++-
>>  4 files changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
>> index 1d1a56ae9a5..582111d71f5 100644
>> --- a/src/compiler/glsl/ir.cpp
>> +++ b/src/compiler/glsl/ir.cpp
>> @@ -1750,6 +1750,7 @@ ir_variable::ir_variable(const struct glsl_type *type, 
>> const char *name,
>> this->data.fb_fetch_output = false;
>> this->data.bindless = false;
>> this->data.bound = false;
>> +   this->data.is_xfb_per_vertex_output = false;
>>
>> if (type != NULL) {
>>if (type->is_interface())
>> diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
>> index f478b29a6b5..e09f053b77c 100644
>> --- a/src/compiler/glsl/ir.h
>> +++ b/src/compiler/glsl/ir.h
>> @@ -766,6 +766,12 @@ public:
>> */
>>unsigned is_xfb_only:1;
>>
>> +  /**
>> +   * Is this varying a TSC per-vertex output candidate for transform
> 
> TCS?


Yes. I've fixed it locally at the commit summary too.


>> +   * feedback?
>> +   */
>> +  unsigned is_xfb_per_vertex_output:1;
>> +
>>/**
>> * Was a transfor feedback buffer set in the shader?
> 
> ugh, not your problem, but "transform" :(
> 
>> */
>> diff --git a/src/compiler/glsl/link_uniforms.cpp 
>> b/src/compiler/glsl/link_uniforms.cpp
>> index 63e688b19a7..547da68e216 100644
>> --- a/src/compiler/glsl/link_uniforms.cpp
>> +++ b/src/compiler/glsl/link_uniforms.cpp
>> @@ -72,8 +72,10 @@ program_resource_visitor::process(ir_variable *var, bool 
>> use_std430_as_default)
>>   get_internal_ifc_packing(use_std430_as_default) :
>>var->type->get_internal_ifc_packing(use_std430_as_default);
>>
>> -   const glsl_type *t =
>> -  var->data.from_named_ifc_block ? var->get_interface_type() : 
>> var->type;
>> +   const glsl_type *t = var->data.from_named_ifc_block ?
>> +  (var->data.is_xfb_per_vertex_output ?
>> +   var->get_interface_type()->without_array() :
>> +   var->get_interface_type()) : var->type;
>> const glsl_type *t_without_array = t->without_array();
>>
>> /* false is always passed for the row_major parameter to the other
>> diff --git a/src/compiler/glsl/link_varyings.cpp 
>> b/src/compiler/glsl/link_varyings.cpp
>> index 52e493cb599..1964dcc0a22 100644
>> --- a/src/compiler/glsl/link_varyings.cpp
>> +++ b/src/compiler/glsl/link_varyings.cpp
>> @@ -2150,7 +2150,10 @@ private:
>>tfeedback_candidate *candidate
>>   = rzalloc(this->mem_ctx, tfeedback_candidate);
>>candidate->toplevel_var = this->toplevel_var;
>> -  candidate->type = type;
>> +  if (this->toplevel_var->data.is_xfb_per_vertex_output)
>> + candidate->type = type->without_array();
>> +  else
>> + candidate->type = type;
>>candidate->offset = this->varying_floats;
>>_mesa_hash_table_insert(this->tfeedback_candidates,
>>ralloc_strdup(this->mem_ctx, name),
>> @@ -2499,6 +2502,9 @@ assign_varying_locations(struct gl_context *ctx,
>>
>>   if (num_tfeedback_decls > 0) {
>>  tfeedback_candidate_generator g(mem_ctx, tfeedback_candidates);
>> +if (producer->Stage == MESA_SHADER_TESS_CTRL &&
>> +

Re: [Mesa-dev] [PATCH 1/3] glsl: XFB TSC per-vertex output varyings match as not declared as arrays

[Ilia Mirkin]

On Wed, Nov 21, 2018 at 1:45 PM Jose Maria Casanova Crespo
 wrote:
>
> Recent change on OpenGL CTS ("Use non-arrayed varying name for TCS blocks")
> on KHR-GL*.tessellation_shader.single.xfb_captures_data_from_correct_stage
> tests changed how to name per-vertex Tessellation Control Shader output
> varyings in transform feedback using interface block as "BLOCK_INOUT.value"
> rather than "BLOCK_INOUT[0].value"
>
> So Tessellation control shader per-vertex output variables and blocks that
> are required to be declared as arrays, with each element representing output
> values for a single vertex of a multi-vertex primitive are expected to be
> named as they were not declared as arrays.
>
> This patch adds a new is_xfb_per_vertex_output flag at ir_variable level so
> we mark when an ir_variable is an per-vertex TCS output varying. So we
> treat it in terms on XFB its naming as a non array variable.
>
> As we don't support NV_gpu_shader5, so PATCHES mode is not accepted as
> primitiveMode parameter of BeginTransformFeedback the test expects a
> failure as we can use the XFB results.
>
> This patch uncovers that we were passing the GLES version of the tests
> because candidates naming didn't match, not because on GLES the Tessellation
> Control stage varyings shouldn't be XFB candidates in any case. This
> is addressed in the following patch.
>
> Fixes: 
> KHR-GL4*.tessellation_shader.single.xfb_captures_data_from_correct_stage
>
> Cc: mesa-sta...@lists.freedesktop.org
> ---
>  src/compiler/glsl/ir.cpp| 1 +
>  src/compiler/glsl/ir.h  | 6 ++
>  src/compiler/glsl/link_uniforms.cpp | 6 --
>  src/compiler/glsl/link_varyings.cpp | 8 +++-
>  4 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
> index 1d1a56ae9a5..582111d71f5 100644
> --- a/src/compiler/glsl/ir.cpp
> +++ b/src/compiler/glsl/ir.cpp
> @@ -1750,6 +1750,7 @@ ir_variable::ir_variable(const struct glsl_type *type, 
> const char *name,
> this->data.fb_fetch_output = false;
> this->data.bindless = false;
> this->data.bound = false;
> +   this->data.is_xfb_per_vertex_output = false;
>
> if (type != NULL) {
>if (type->is_interface())
> diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
> index f478b29a6b5..e09f053b77c 100644
> --- a/src/compiler/glsl/ir.h
> +++ b/src/compiler/glsl/ir.h
> @@ -766,6 +766,12 @@ public:
> */
>unsigned is_xfb_only:1;
>
> +  /**
> +   * Is this varying a TSC per-vertex output candidate for transform

TCS?

> +   * feedback?
> +   */
> +  unsigned is_xfb_per_vertex_output:1;
> +
>/**
> * Was a transfor feedback buffer set in the shader?

ugh, not your problem, but "transform" :(

> */
> diff --git a/src/compiler/glsl/link_uniforms.cpp 
> b/src/compiler/glsl/link_uniforms.cpp
> index 63e688b19a7..547da68e216 100644
> --- a/src/compiler/glsl/link_uniforms.cpp
> +++ b/src/compiler/glsl/link_uniforms.cpp
> @@ -72,8 +72,10 @@ program_resource_visitor::process(ir_variable *var, bool 
> use_std430_as_default)
>   get_internal_ifc_packing(use_std430_as_default) :
>var->type->get_internal_ifc_packing(use_std430_as_default);
>
> -   const glsl_type *t =
> -  var->data.from_named_ifc_block ? var->get_interface_type() : var->type;
> +   const glsl_type *t = var->data.from_named_ifc_block ?
> +  (var->data.is_xfb_per_vertex_output ?
> +   var->get_interface_type()->without_array() :
> +   var->get_interface_type()) : var->type;
> const glsl_type *t_without_array = t->without_array();
>
> /* false is always passed for the row_major parameter to the other
> diff --git a/src/compiler/glsl/link_varyings.cpp 
> b/src/compiler/glsl/link_varyings.cpp
> index 52e493cb599..1964dcc0a22 100644
> --- a/src/compiler/glsl/link_varyings.cpp
> +++ b/src/compiler/glsl/link_varyings.cpp
> @@ -2150,7 +2150,10 @@ private:
>tfeedback_candidate *candidate
>   = rzalloc(this->mem_ctx, tfeedback_candidate);
>candidate->toplevel_var = this->toplevel_var;
> -  candidate->type = type;
> +  if (this->toplevel_var->data.is_xfb_per_vertex_output)
> + candidate->type = type->without_array();
> +  else
> + candidate->type = type;
>candidate->offset = this->varying_floats;
>_mesa_hash_table_insert(this->tfeedback_candidates,
>ralloc_strdup(this->mem_ctx, name),
> @@ -2499,6 +2502,9 @@ assign_varying_locations(struct gl_context *ctx,
>
>   if (num_tfeedback_decls > 0) {
>  tfeedback_candidate_generator g(mem_ctx, tfeedback_candidates);
> +if (producer->Stage == MESA_SHADER_TESS_CTRL &&
> +!output_var->data.patch)
> +   output_var->data.is_xfb_per_vertex_output = true;
>  g.process(output_var);
>   }
>
> --
> 2.19.1
>
> ___
> 

[Mesa-dev] [PATCH 1/3] glsl: XFB TSC per-vertex output varyings match as not declared as arrays

[Jose Maria Casanova Crespo]

Recent change on OpenGL CTS ("Use non-arrayed varying name for TCS blocks")
on KHR-GL*.tessellation_shader.single.xfb_captures_data_from_correct_stage
tests changed how to name per-vertex Tessellation Control Shader output
varyings in transform feedback using interface block as "BLOCK_INOUT.value"
rather than "BLOCK_INOUT[0].value"

So Tessellation control shader per-vertex output variables and blocks that
are required to be declared as arrays, with each element representing output
values for a single vertex of a multi-vertex primitive are expected to be
named as they were not declared as arrays.

This patch adds a new is_xfb_per_vertex_output flag at ir_variable level so
we mark when an ir_variable is an per-vertex TCS output varying. So we
treat it in terms on XFB its naming as a non array variable.

As we don't support NV_gpu_shader5, so PATCHES mode is not accepted as
primitiveMode parameter of BeginTransformFeedback the test expects a
failure as we can use the XFB results.

This patch uncovers that we were passing the GLES version of the tests
because candidates naming didn't match, not because on GLES the Tessellation
Control stage varyings shouldn't be XFB candidates in any case. This
is addressed in the following patch.

Fixes: KHR-GL4*.tessellation_shader.single.xfb_captures_data_from_correct_stage

Cc: mesa-sta...@lists.freedesktop.org
---
 src/compiler/glsl/ir.cpp| 1 +
 src/compiler/glsl/ir.h  | 6 ++
 src/compiler/glsl/link_uniforms.cpp | 6 --
 src/compiler/glsl/link_varyings.cpp | 8 +++-
 4 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
index 1d1a56ae9a5..582111d71f5 100644
--- a/src/compiler/glsl/ir.cpp
+++ b/src/compiler/glsl/ir.cpp
@@ -1750,6 +1750,7 @@ ir_variable::ir_variable(const struct glsl_type *type, 
const char *name,
this->data.fb_fetch_output = false;
this->data.bindless = false;
this->data.bound = false;
+   this->data.is_xfb_per_vertex_output = false;
 
if (type != NULL) {
   if (type->is_interface())
diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
index f478b29a6b5..e09f053b77c 100644
--- a/src/compiler/glsl/ir.h
+++ b/src/compiler/glsl/ir.h
@@ -766,6 +766,12 @@ public:
*/
   unsigned is_xfb_only:1;
 
+  /**
+   * Is this varying a TSC per-vertex output candidate for transform
+   * feedback?
+   */
+  unsigned is_xfb_per_vertex_output:1;
+
   /**
* Was a transfor feedback buffer set in the shader?
*/
diff --git a/src/compiler/glsl/link_uniforms.cpp 
b/src/compiler/glsl/link_uniforms.cpp
index 63e688b19a7..547da68e216 100644
--- a/src/compiler/glsl/link_uniforms.cpp
+++ b/src/compiler/glsl/link_uniforms.cpp
@@ -72,8 +72,10 @@ program_resource_visitor::process(ir_variable *var, bool 
use_std430_as_default)
  get_internal_ifc_packing(use_std430_as_default) :
   var->type->get_internal_ifc_packing(use_std430_as_default);
 
-   const glsl_type *t =
-  var->data.from_named_ifc_block ? var->get_interface_type() : var->type;
+   const glsl_type *t = var->data.from_named_ifc_block ?
+  (var->data.is_xfb_per_vertex_output ?
+   var->get_interface_type()->without_array() :
+   var->get_interface_type()) : var->type;
const glsl_type *t_without_array = t->without_array();
 
/* false is always passed for the row_major parameter to the other
diff --git a/src/compiler/glsl/link_varyings.cpp 
b/src/compiler/glsl/link_varyings.cpp
index 52e493cb599..1964dcc0a22 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -2150,7 +2150,10 @@ private:
   tfeedback_candidate *candidate
  = rzalloc(this->mem_ctx, tfeedback_candidate);
   candidate->toplevel_var = this->toplevel_var;
-  candidate->type = type;
+  if (this->toplevel_var->data.is_xfb_per_vertex_output)
+ candidate->type = type->without_array();
+  else
+ candidate->type = type;
   candidate->offset = this->varying_floats;
   _mesa_hash_table_insert(this->tfeedback_candidates,
   ralloc_strdup(this->mem_ctx, name),
@@ -2499,6 +2502,9 @@ assign_varying_locations(struct gl_context *ctx,
 
  if (num_tfeedback_decls > 0) {
 tfeedback_candidate_generator g(mem_ctx, tfeedback_candidates);
+if (producer->Stage == MESA_SHADER_TESS_CTRL &&
+!output_var->data.patch)
+   output_var->data.is_xfb_per_vertex_output = true;
 g.process(output_var);
  }
 
-- 
2.19.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev