Or not. :( The patch no longer applies, and I wasn't able to trivially make it apply. I suspect one of Timothy's recent patches is to blame.
On 08/11/2016 10:06 AM, Ian Romanick wrote: > This seems okay... let me run it through our CI first. I should have > results later today. I'm going to step out for a bit. > > On 08/09/2016 05:43 PM, Ilia Mirkin wrote: >> 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
