[Mesa-dev] [PATCH V3 12/13] vbo: map indirect buffer and extract params if doing sw primitive restart

[Kenneth Graunke]

On 11/09/2013 01:02 AM, Chris Forbes wrote:
> V2: Check for mapping failure (thanks Brian)
> V3: - Change error on mapping failure to OUT_OF_MEMORY (Brian)
>     - Unconst; remove casting away of const.
> 
> Signed-off-by: Chris Forbes <chrisf at ijw.co.nz>
> ---
>  src/mesa/vbo/vbo_primitive_restart.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/src/mesa/vbo/vbo_primitive_restart.c b/src/mesa/vbo/vbo_primitive_restart.c
> index 75e760f..e0bd233 100644
> --- a/src/mesa/vbo/vbo_primitive_restart.c
> +++ b/src/mesa/vbo/vbo_primitive_restart.c
> @@ -180,6 +180,39 @@ vbo_sw_primitive_restart(struct gl_context *ctx,
>     GLboolean map_ib = ib->obj->Name && !ib->obj->Pointer;
>     void *ptr;
>  
> +   /* If there is an indirect buffer, map it and extract the draw params */
> +   if (indirect && prims[0].is_indirect) {
> +      struct _mesa_prim new_prim = *prims;
> +      struct _mesa_index_buffer new_ib = *ib;
> +      const uint32_t *indirect_params;
> +      if (!ctx->Driver.MapBufferRange(ctx, 0, indirect->Size, GL_MAP_READ_BIT,
> +            indirect)) {
> +
> +         /* something went wrong with mapping, give up */
> +         _mesa_error(ctx, GL_OUT_OF_MEMORY,
> +                     "failed to map indirect buffer for sw primitive restart");
> +         return;

I had to think about this a bit:

If the current batch writes data into the indirect BO, and the
application immediately tries to DrawIndirect from it without flushing...

We should actually be okay, since intel_bufferobj_map_range will flush
the batch for us:

   if (!(access & GL_MAP_UNSYNCHRONIZED_BIT)) {
      if (drm_intel_bo_references(brw->batch.bo, intel_obj->buffer)) {
         if (access & GL_MAP_INVALIDATE_BUFFER_BIT) {
            ...
         } else {
            perf_debug("Stalling on the GPU for mapping a busy buffer "
                       "object\n");
            intel_batchbuffer_flush(brw);
         }
      } else {
         ...
      }
   }

Nice.

Have you at least done touch testing of SW primitive restart + draw
indirect?  SW primitive restart is pretty broken in a lot of ways
anyway, so as long as you tried /something/ and it worked I won't object.

Patches 12 and 13 (with the relnotes fix) are also:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

> +      }
> +
> +      assert(nr_prims == 1);
> +      indirect_params = (const uint32_t *) ADD_POINTERS(indirect->Pointer,
> +            new_prim.indirect_offset);
> +
> +      new_prim.is_indirect = 0;
> +      new_prim.count = indirect_params[0];
> +      new_prim.num_instances = indirect_params[1];
> +      new_prim.start = indirect_params[2];
> +      new_prim.basevertex = indirect_params[3];
> +      new_prim.base_instance = indirect_params[4];
> +
> +      new_ib.count = new_prim.count;
> +
> +      prims = &new_prim;
> +      ib = &new_ib;
> +
> +      ctx->Driver.UnmapBuffer(ctx, indirect);
> +   }
> +
>     /* Find the sub-primitives. These are regions in the index buffer which
>      * are split based on the primitive restart index value.
>      */
>