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 <jmcasan...@igalia.com> 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 > >> <jmcasan...@igalia.com> 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 && > >>> + !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 > >> > > _______________________________________________ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev