Looks like it all checks out. Someone refactored a bunch of the code, and it just had to move a tiny bit, hence the conflicts. Updated patch here:
https://github.com/imirkin/mesa/commit/56bae8a1c06aaf3b5be408ad76708de48c482909.patch which is basically identical to the original. On Thu, Aug 11, 2016 at 1:44 PM, Ilia Mirkin <imir...@alum.mit.edu> wrote: > I'll rebase and feed it through Jenkins (I was given a branch of late). > > On Thu, Aug 11, 2016 at 1:38 PM, Ian Romanick <i...@freedesktop.org> wrote: >> 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
