i965: Fix start/base_vertex_location for >1 prims but !BRW_NEW_VERTICES.
authorKenneth Graunke <kenneth@whitecape.org>
Thu, 18 Dec 2014 12:45:40 +0000 (04:45 -0800)
committerKenneth Graunke <kenneth@whitecape.org>
Thu, 1 Jan 2015 01:10:47 +0000 (17:10 -0800)
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:
"<gl_BaseVertexARB> holds the integer value passed to the <baseVertex>
 parameter to the command that resulted in the current shader
 invocation.  In the case where the command has no <baseVertex>
 parameter, the value of <gl_BaseVertexARB> 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" <mesa-stable@lists.freedesktop.org>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85529
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Acked-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Chris Forbes <chrisf@ijw.co.nz>
src/mesa/drivers/dri/i965/brw_context.h
src/mesa/drivers/dri/i965/brw_draw.c
src/mesa/drivers/dri/i965/brw_draw_upload.c

index a63c4834c26962fc5eb1684fccf6aecb87dc8aff..fde4177301895824b075ec9e5def20a1b5ce9c41 100644 (file)
@@ -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
index 5cd3f01cb1f6519e93b3aab703a0320b7a135346..b7e42cbf7d92ed380a70a79d087c16894e172bb2 100644 (file)
@@ -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);
 
index 6e0cf3e353f80c2b5ed6b7f8b3e49dd2810488ef..8123da8377cd86fd9536e8eee48fae4f0b35014f 100644 (file)
@@ -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);
    }