glsl: make the xfb varying sort stable

qsort is not guaranteed to produce a stable sort, and indeed
in MSVC CRT does not. The xfb varying sort functions were
relying on undefined behavior (that qsort would be stable).

Signed-off-by: Eric R. Smith <eric.smith@collabora.com>
Reviewed-by: Erik Faye-Lund <erik.faye-lund@collabora.com>
Reviewed-by: Jesse Natalie <jenatali@microsoft.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/29178>
This commit is contained in:
Eric R. Smith
2024-05-14 21:55:38 -03:00
committed by Marge Bot
parent 5102a922e7
commit 43f9b3b986

View File

@@ -2341,6 +2341,12 @@ struct match {
* value 0.
*/
unsigned generic_location;
/**
* Original index, used as a fallback sorting key to ensure
* a stable sort
*/
unsigned original_index;
};
/**
@@ -2413,7 +2419,9 @@ varying_matches_match_comparator(const void *x_generic, const void *y_generic)
if (x->packing_class != y->packing_class)
return x->packing_class - y->packing_class;
return x->packing_order - y->packing_order;
if (x->packing_order != y->packing_order)
return x->packing_order - y->packing_order;
return x->original_index - y->original_index;
}
/**
@@ -2436,17 +2444,8 @@ varying_matches_xfb_comparator(const void *x_generic, const void *y_generic)
return -1;
}
/* FIXME: When the comparator returns 0 it means the elements being
* compared are equivalent. However the qsort documentation says:
*
* "The order of equivalent elements is undefined."
*
* In practice the sort ends up reversing the order of the varyings which
* means locations are also assigned in this reversed order and happens to
* be what we want. This is also whats happening in
* varying_matches_match_comparator().
*/
return 0;
/* otherwise leave the order alone */
return x->original_index - y->original_index;
}
/**
@@ -2464,17 +2463,8 @@ varying_matches_not_xfb_comparator(const void *x_generic, const void *y_generic)
/* if both are non-xfb, then sort them */
return varying_matches_match_comparator(x_generic, y_generic);
/* FIXME: When the comparator returns 0 it means the elements being
* compared are equivalent. However the qsort documentation says:
*
* "The order of equivalent elements is undefined."
*
* In practice the sort ends up reversing the order of the varyings which
* means locations are also assigned in this reversed order and happens to
* be what we want. This is also whats happening in
* varying_matches_match_comparator().
*/
return 0;
/* otherwise, leave the order alone */
return x->original_index - y->original_index;
}
static bool
@@ -2759,19 +2749,22 @@ varying_matches_assign_locations(struct varying_matches *vm,
struct gl_shader_program *prog,
uint8_t components[], uint64_t reserved_slots)
{
/* Establish the original order of the varying_matches array; our
* sorts will use this for sorting when the varyings do not have
* xfb qualifiers
*/
for (unsigned i = 0; i < vm->num_matches; i++)
vm->matches[i].original_index = i;
/* If packing has been disabled then we cannot safely sort the varyings by
* class as it may mean we are using a version of OpenGL where
* interpolation qualifiers are not guaranteed to be matching across
* shaders, sorting in this case could result in mismatching shader
* interfaces.
* When packing is disabled the sort orders varyings used by transform
* feedback first, but also depends on *undefined behaviour* of qsort to
* reverse the order of the varyings. See: xfb_comparator().
* interfaces. So we sort only the varyings used by transform feedback.
*
* If packing is only disabled for xfb varyings (mutually exclusive with
* disable_varying_packing), we then group varyings depending on if they
* are captured for transform feedback. The same *undefined behaviour* is
* taken advantage of.
* are captured for transform feedback.
*/
if (vm->disable_varying_packing) {
/* Only sort varyings that are only used by transform feedback. */