ping? do we want this? should i drop it?
On Wed, Jul 13, 2016 at 3:37 PM, Ilia Mirkin <imir...@alum.mit.edu> wrote: > Thanks for confirming, Corentin. > > Ian, do you have any opinions on this? Seems like a fairly innocuous > thing to be doing... > > On Fri, Jul 8, 2016 at 3:39 PM, Corentin Wallez <coren...@wallez.net> wrote: >> Not sure how reviews work in Mesa, but this patch LGTM. I also tested that >> it fixes the relevant tests failures it is supposed to address. >> >> On Wed, Jul 6, 2016 at 7:40 PM, Ilia Mirkin <imir...@alum.mit.edu> wrote: >>> >>> The GL spec is very unclear on this point. Apparently this is discussed >>> without resolution in the closed Khronos bugtracker at >>> https://cvs.khronos.org/bugzilla/show_bug.cgi?id=7829 . The >>> recommendation is to allow dropping the [0] for looking up the bindings. >>> >>> The approach taken in this patch is to instead tack on [0]'s for each >>> arrayness level of the output's type, and doing the lookup again. That >>> way, for >>> >>> out vec4 foo[2][2][2] >>> >>> we will end up looking for bindings for foo, foo[0], foo[0][0], and >>> foo[0][0][0], in that order of preference. >>> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96765 >>> Signed-off-by: Ilia Mirkin <imir...@alum.mit.edu> >>> --- >>> src/compiler/glsl/linker.cpp | 39 ++++++++++++++++++++++++++++----------- >>> 1 file changed, 28 insertions(+), 11 deletions(-) >>> >>> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp >>> index d963f54..9d54c2f 100644 >>> --- a/src/compiler/glsl/linker.cpp >>> +++ b/src/compiler/glsl/linker.cpp >>> @@ -2566,6 +2566,7 @@ find_available_slots(unsigned used_mask, unsigned >>> needed_count) >>> /** >>> * Assign locations for either VS inputs or FS outputs >>> * >>> + * \param mem_ctx Temporary ralloc context used for linking >>> * \param prog Shader program whose variables need locations >>> assigned >>> * \param constants Driver specific constant values for the program. >>> * \param target_index Selector for the program target to receive >>> location >>> @@ -2577,7 +2578,8 @@ find_available_slots(unsigned used_mask, unsigned >>> needed_count) >>> * error is emitted to the shader link log and false is returned. >>> */ >>> bool >>> -assign_attribute_or_color_locations(gl_shader_program *prog, >>> +assign_attribute_or_color_locations(void *mem_ctx, >>> + gl_shader_program *prog, >>> struct gl_constants *constants, >>> unsigned target_index) >>> { >>> @@ -2680,16 +2682,31 @@ >>> assign_attribute_or_color_locations(gl_shader_program *prog, >>> } else if (target_index == MESA_SHADER_FRAGMENT) { >>> unsigned binding; >>> unsigned index; >>> + const char *name = var->name; >>> + const glsl_type *type = var->type; >>> + >>> + while (type) { >>> + /* Check if there's a binding for the variable name */ >>> + if (prog->FragDataBindings->get(binding, name)) { >>> + assert(binding >= FRAG_RESULT_DATA0); >>> + var->data.location = binding; >>> + var->data.is_unmatched_generic_inout = 0; >>> + >>> + if (prog->FragDataIndexBindings->get(index, name)) { >>> + var->data.index = index; >>> + } >>> + break; >>> + } >>> >>> - if (prog->FragDataBindings->get(binding, var->name)) { >>> - assert(binding >= FRAG_RESULT_DATA0); >>> - var->data.location = binding; >>> - var->data.is_unmatched_generic_inout = 0; >>> + /* If not, but it's an array type, look for name[0] */ >>> + if (type->is_array()) { >>> + name = ralloc_asprintf(mem_ctx, "%s[0]", name); >>> + type = type->fields.array; >>> + continue; >>> + } >>> >>> - if (prog->FragDataIndexBindings->get(index, var->name)) { >>> - var->data.index = index; >>> - } >>> - } >>> + break; >>> + } >>> } >>> >>> /* From GL4.5 core spec, section 15.2 (Shader Execution): >>> @@ -4816,12 +4833,12 @@ link_shaders(struct gl_context *ctx, struct >>> gl_shader_program *prog) >>> prev = i; >>> } >>> >>> - if (!assign_attribute_or_color_locations(prog, &ctx->Const, >>> + if (!assign_attribute_or_color_locations(mem_ctx, prog, &ctx->Const, >>> MESA_SHADER_VERTEX)) { >>> goto done; >>> } >>> >>> - if (!assign_attribute_or_color_locations(prog, &ctx->Const, >>> + if (!assign_attribute_or_color_locations(mem_ctx, prog, &ctx->Const, >>> MESA_SHADER_FRAGMENT)) { >>> goto done; >>> } >>> -- >>> 2.7.3 >>> >> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
