From 885c78801785701fdca0217a7b0f992f26115ee4 Mon Sep 17 00:00:00 2001 From: Ilia Mirkin Date: Fri, 4 Nov 2016 21:01:18 -0400 Subject: [PATCH] glsl: fix slot_end calculations and simplify reserved_slots check The previous code was confused about whether slot_end was inclusive or exclusive. Make it so that it is inclusive consistently, and use it for setting the new location. This also avoids discrepancies in how num_components is calculated vs the more manual approach taken for the former reserved_slots check. Signed-off-by: Ilia Mirkin Reviewed-by: Timothy Arceri --- src/compiler/glsl/link_varyings.cpp | 46 +++++++++++++---------------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp index e622b3e46bc..43204a7c3f2 100644 --- a/src/compiler/glsl/link_varyings.cpp +++ b/src/compiler/glsl/link_varyings.cpp @@ -1546,14 +1546,16 @@ varying_matches::assign_locations(struct gl_shader_program *prog, previous_var_xfb_only = var->data.is_xfb_only; - unsigned num_elements = type->count_attribute_slots(is_vertex_input); - unsigned slot_end; - if (this->disable_varying_packing && - !is_varying_packing_safe(type, var)) - slot_end = 4; - else - slot_end = type->without_array()->vector_elements; - slot_end += *location - 1; + /* The number of components taken up by this variable. For vertex shader + * inputs, we use the number of slots * 4, as they have different + * counting rules. + */ + unsigned num_components = is_vertex_input ? + type->count_attribute_slots(is_vertex_input) * 4 : + this->matches[i].num_components; + + /* The last slot for this variable, inclusive. */ + unsigned slot_end = *location + num_components - 1; /* FIXME: We could be smarter in the below code and loop back over * trying to fill any locations that we skipped because we couldn't pack @@ -1561,29 +1563,21 @@ varying_matches::assign_locations(struct gl_shader_program *prog, * hit the linking error if we run out of room and suggest they use * explicit locations. */ - for (unsigned j = 0; j < num_elements; j++) { - while ((slot_end < MAX_VARYING * 4u) && - ((reserved_slots & (UINT64_C(1) << *location / 4u) || - (reserved_slots & (UINT64_C(1) << slot_end / 4u))))) { + while (slot_end < MAX_VARYING * 4u) { + const unsigned slots = (slot_end / 4u) - (*location / 4u) + 1; + const uint64_t slot_mask = ((1ull << slots) - 1) << (*location / 4u); + assert(slots > 0); + if (reserved_slots & slot_mask) { *location = ALIGN(*location + 1, 4); - slot_end = *location; - - /* reset the counter and try again */ - j = 0; + slot_end = *location + num_components - 1; + continue; } - /* Increase the slot to make sure there is enough room for next - * array element. - */ - if (this->disable_varying_packing && - !is_varying_packing_safe(type, var)) - slot_end += 4; - else - slot_end += type->without_array()->vector_elements; + break; } - if (!var->data.patch && *location >= MAX_VARYING * 4u) { + if (!var->data.patch && slot_end >= MAX_VARYING * 4u) { linker_error(prog, "insufficient contiguous locations available for " "%s it is possible an array or struct could not be " "packed between varyings with explicit locations. Try " @@ -1593,7 +1587,7 @@ varying_matches::assign_locations(struct gl_shader_program *prog, this->matches[i].generic_location = *location; - *location += this->matches[i].num_components; + *location = slot_end + 1; } return (generic_location + 3) / 4; -- 2.30.2