From c633528cbac007a73a066f269b3c9a25daf1e21a Mon Sep 17 00:00:00 2001 From: Kenneth Graunke Date: Thu, 18 Dec 2014 04:45:40 -0800 Subject: [PATCH] i965: Fix start/base_vertex_location for >1 prims but !BRW_NEW_VERTICES. This is a partial revert of c89306983c07e5a88c0d636267e5ccf263cb4213. It split the {start,base}_vertex_location handling into several steps: 1. Set brw->draw.start_vertex_location = prim[i].start and brw->draw.base_vertex_location = prim[i].basevertex. (This happened once per _mesa_prim, in the main drawing loop.) 2. Add brw->vb.start_vertex_bias and brw->ib.start_vertex_offset appropriately. (This happened in brw_prepare_shader_draw_parameters, which was called just after brw_prepare_vertices, as part of state upload, and only happened when BRW_NEW_VERTICES was flagged.) 3. Use those values when emitting 3DPRIMITIVE (once per _mesa_prim). If we drew multiple _mesa_prims, but didn't flag BRW_NEW_VERTICES on the second (or later) primitives, we would do step #1, but not #2. The first _mesa_prim would get correct values, but subsequent ones would only get the first half of the summation. The reason I originally did this was because I needed the value of gl_BaseVertexARB to exist in a buffer object prior to uploading 3DSTATE_VERTEX_BUFFERS. I believed I wanted to upload the value of 3DPRIMITIVE's "Base Vertex Location" field, which was computed as: (prims[i].indexed ? prims[i].start : prims[i].basevertex) + brw->vb.start_vertex_bias. The latter value wasn't available until after brw_prepare_vertices, and the former weren't available in the state upload code at all. Hence the awkward split. However, I believe that including brw->vb.start_vertex_bias was a mistake. It's an extra bias we apply when uploading vertex data into VBOs, to move [min_index, max_index] to [0, max_index - min_index]. >From the GL_ARB_shader_draw_parameters specification: " holds the integer value passed to the parameter to the command that resulted in the current shader invocation. In the case where the command has no parameter, the value of is zero." I conclude that gl_BaseVertexARB should only include the baseVertex parameter from glDraw*Elements*, not any internal biases we add for optimization purposes. With that in mind, gl_BaseVertexARB only needs prim[i].start or prim[i].basevertex. We can simply store that, and go back to computing start_vertex_location and base_vertex_location in brw_emit_prim(), like we used to. This is much simpler, and should actually fix two bugs. Fixes missing geometry in Unvanquished. Cc: "10.4 10.3" Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85529 Signed-off-by: Kenneth Graunke Acked-by: Ian Romanick Reviewed-by: Chris Forbes --- src/mesa/drivers/dri/i965/brw_context.h | 7 ++----- src/mesa/drivers/dri/i965/brw_draw.c | 15 ++++++++++----- src/mesa/drivers/dri/i965/brw_draw_upload.c | 12 +----------- 3 files changed, 13 insertions(+), 21 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h index a63c4834c26..fde41773018 100644 --- a/src/mesa/drivers/dri/i965/brw_context.h +++ b/src/mesa/drivers/dri/i965/brw_context.h @@ -1110,11 +1110,8 @@ struct brw_context uint32_t pma_stall_bits; struct { - /** Does the current draw use the index buffer? */ - bool indexed; - - int start_vertex_location; - int base_vertex_location; + /** The value of gl_BaseVertex for the current _mesa_prim. */ + int gl_basevertex; /** * Buffer and offset used for GL_ARB_shader_draw_parameters diff --git a/src/mesa/drivers/dri/i965/brw_draw.c b/src/mesa/drivers/dri/i965/brw_draw.c index 5cd3f01cb1f..b7e42cbf7d9 100644 --- a/src/mesa/drivers/dri/i965/brw_draw.c +++ b/src/mesa/drivers/dri/i965/brw_draw.c @@ -182,14 +182,20 @@ static void brw_emit_prim(struct brw_context *brw, DBG("PRIM: %s %d %d\n", _mesa_lookup_enum_by_nr(prim->mode), prim->start, prim->count); + int start_vertex_location = prim->start; + int base_vertex_location = prim->basevertex; + if (prim->indexed) { vertex_access_type = brw->gen >= 7 ? GEN7_3DPRIM_VERTEXBUFFER_ACCESS_RANDOM : GEN4_3DPRIM_VERTEXBUFFER_ACCESS_RANDOM; + start_vertex_location += brw->ib.start_vertex_offset; + base_vertex_location += brw->vb.start_vertex_bias; } else { vertex_access_type = brw->gen >= 7 ? GEN7_3DPRIM_VERTEXBUFFER_ACCESS_SEQUENTIAL : GEN4_3DPRIM_VERTEXBUFFER_ACCESS_SEQUENTIAL; + start_vertex_location += brw->vb.start_vertex_bias; } /* We only need to trim the primitive count on pre-Gen6. */ @@ -264,10 +270,10 @@ static void brw_emit_prim(struct brw_context *brw, vertex_access_type); } OUT_BATCH(verts_per_instance); - OUT_BATCH(brw->draw.start_vertex_location); + OUT_BATCH(start_vertex_location); OUT_BATCH(prim->num_instances); OUT_BATCH(prim->base_instance); - OUT_BATCH(brw->draw.base_vertex_location); + OUT_BATCH(base_vertex_location); ADVANCE_BATCH(); /* Only used on Sandybridge; harmless to set elsewhere. */ @@ -471,9 +477,8 @@ static void brw_try_draw_prims( struct gl_context *ctx, } } - brw->draw.indexed = prims[i].indexed; - brw->draw.start_vertex_location = prims[i].start; - brw->draw.base_vertex_location = prims[i].basevertex; + brw->draw.gl_basevertex = + prims[i].indexed ? prims[i].basevertex : prims[i].start; drm_intel_bo_unreference(brw->draw.draw_params_bo); diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c b/src/mesa/drivers/dri/i965/brw_draw_upload.c index 6e0cf3e353f..8123da8377c 100644 --- a/src/mesa/drivers/dri/i965/brw_draw_upload.c +++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c @@ -604,19 +604,9 @@ brw_prepare_vertices(struct brw_context *brw) void brw_prepare_shader_draw_parameters(struct brw_context *brw) { - int *gl_basevertex_value; - if (brw->draw.indexed) { - brw->draw.start_vertex_location += brw->ib.start_vertex_offset; - brw->draw.base_vertex_location += brw->vb.start_vertex_bias; - gl_basevertex_value = &brw->draw.base_vertex_location; - } else { - brw->draw.start_vertex_location += brw->vb.start_vertex_bias; - gl_basevertex_value = &brw->draw.start_vertex_location; - } - /* For non-indirect draws, upload gl_BaseVertex. */ if (brw->vs.prog_data->uses_vertexid && brw->draw.draw_params_bo == NULL) { - intel_upload_data(brw, gl_basevertex_value, 4, 4, + intel_upload_data(brw, &brw->draw.gl_basevertex, 4, 4, &brw->draw.draw_params_bo, &brw->draw.draw_params_offset); } -- 2.30.2