From cdffa3e11403ab92620aa66f69e07ba6bce72704 Mon Sep 17 00:00:00 2001 From: Kenneth Graunke Date: Thu, 4 Mar 2021 12:51:20 -0800 Subject: [PATCH] vbo: Fix vbo_sw_primitive_restart for start > 0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit e99e7aa4 began passing start > 0 to indexed draw calls rather than keeping start at 0 and manually advancing ib->ptr. This should work fine, however, there have been instances of software fallbacks not handling things right. vbo_sw_primitive_restart had a bug where it was ignoring "start" and always calling find_sub_primitives with start = 0 and end = ib->count. This meant that when start > 0, it was analyzing the wrong part of the index buffer when finding subprimitives. In theory, each _mesa_prim can have a different "start" value. But the code only calls find_sub_primitives once, because it wants to map, analyze, and unmap the index buffer before calling ctx->Draw, as some drivers don't support drawing with the index buffer mapped. To handle this, we break vbo_sw_primitive_restart calls into sections where "start" matches across all the primitives, similar to how I handled the issue in tnl in commit bd6120f562d57e150aa2071f9108. In the common case, start matches and we handle it in one pass anyway. Fixes Piglit's primitive-restart VBO_COMBINED_VERTEX_AND_INDEX test and KHR-GL33.pipeline_statistics_query_tests_ARB.functional_primitives_vertices_submitted_and_clipping_input_output_primitives on Intel Ivybridge and older (which don't do arbitrary cut indices). Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/4052 Reviewed-by: Marek Olšák Part-of: --- src/mesa/vbo/vbo_primitive_restart.c | 56 ++++++++++++++++++++++------ 1 file changed, 44 insertions(+), 12 deletions(-) diff --git a/src/mesa/vbo/vbo_primitive_restart.c b/src/mesa/vbo/vbo_primitive_restart.c index 08df39b0711..da62fafdf38 100644 --- a/src/mesa/vbo/vbo_primitive_restart.c +++ b/src/mesa/vbo/vbo_primitive_restart.c @@ -159,16 +159,17 @@ find_sub_primitives(const void *elements, unsigned element_size, * This function breaks up calls into the driver so primitive restart * support is not required in the driver. */ -void -vbo_sw_primitive_restart(struct gl_context *ctx, - const struct _mesa_prim *prims, - GLuint nr_prims, - const struct _mesa_index_buffer *ib, - GLuint num_instances, GLuint base_instance, - struct gl_buffer_object *indirect, - GLsizeiptr indirect_offset, - bool primitive_restart, - unsigned restart_index) +static void +vbo_sw_primitive_restart_common_start(struct gl_context *ctx, + const struct _mesa_prim *prims, + GLuint nr_prims, + const struct _mesa_index_buffer *ib, + GLuint num_instances, + GLuint base_instance, + struct gl_buffer_object *indirect, + GLsizeiptr indirect_offset, + bool primitive_restart, + unsigned restart_index) { GLuint prim_num; struct _mesa_prim new_prim; @@ -231,8 +232,8 @@ vbo_sw_primitive_restart(struct gl_context *ctx, ptr = ib->ptr; sub_prims = find_sub_primitives(ptr, 1 << ib->index_size_shift, - 0, ib->count, restart_index, - &num_sub_prims); + prims[0].start, prims[0].start + ib->count, + restart_index, &num_sub_prims); if (map_ib) { ctx->Driver.UnmapBuffer(ctx, ib->obj, MAP_INTERNAL); @@ -271,3 +272,34 @@ vbo_sw_primitive_restart(struct gl_context *ctx, free(sub_prims); } +void +vbo_sw_primitive_restart(struct gl_context *ctx, + const struct _mesa_prim *prims, + GLuint nr_prims, + const struct _mesa_index_buffer *ib, + GLuint num_instances, + GLuint base_instance, + struct gl_buffer_object *indirect, + GLsizeiptr indirect_offset, + bool primitive_restart, + unsigned restart_index) +{ + unsigned i; + for (i = 1; i < nr_prims; i++) { + if (prims[i].start != prims[0].start) + break; + } + + vbo_sw_primitive_restart_common_start(ctx, &prims[0], i, ib, + num_instances, base_instance, + indirect, indirect_offset, + primitive_restart, + restart_index); + if (i != nr_prims) { + vbo_sw_primitive_restart(ctx, &prims[i], nr_prims - i, ib, + num_instances, base_instance, + indirect, indirect_offset, + primitive_restart, + restart_index); + } +}