[Mesa-dev] [PATCH] st/mesa: cleanup and fix primitive restart for indirect draws

[Marek Olšák]

Reviewed-by: Marek Olšák <marek.olsak at amd.com>

Marek

On Fri, Oct 21, 2016 at 10:18 AM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
> There are three intended functional changes here:
>
> 1. OpenGL 4.5 clarifies that primitive restart should only apply with index
>    buffers, so make that change explicit in the indirect draw path.
>
> 2. Make PrimitiveRestartFixedIndex work with indirect draws.
>
> 3. The change where primitive_restart is only set when the restart index can
>    actually have an effect (based on the size of indices) is also applied for
>    indirect draws.
>
> Cc: 13.0 <mesa-stable at lists.freedesktop.org>
> ---
>  src/mesa/state_tracker/st_draw.c | 45 +++++++++++++++++++++++++---------------
>  1 file changed, 28 insertions(+), 17 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_draw.c b/src/mesa/state_tracker/st_draw.c
> index 5dcaff0..e9f25b6 100644
> --- a/src/mesa/state_tracker/st_draw.c
> +++ b/src/mesa/state_tracker/st_draw.c
> @@ -120,20 +120,44 @@ setup_index_buffer(struct st_context *st,
>        /* indices are in user space memory */
>        ibuffer->user_buffer = ib->ptr;
>     }
>
>     cso_set_index_buffer(st->cso_context, ibuffer);
>     return TRUE;
>  }
>
>
>  /**
> + * Set the restart index.
> + */
> +static void
> +setup_primitive_restart(struct gl_context *ctx,
> +                        const struct _mesa_index_buffer *ib,
> +                        struct pipe_draw_info *info)
> +{
> +   if (ctx->Array._PrimitiveRestart) {
> +      info->restart_index = _mesa_primitive_restart_index(ctx, ib->type);
> +
> +      /* Enable primitive restart only when the restart index can have an
> +       * effect. This is required for correctness in radeonsi VI support.
> +       * Other hardware may also benefit from taking a faster, non-restart path
> +       * when possible.
> +       */
> +      if ((ib->type == GL_UNSIGNED_INT) ||
> +          (ib->type == GL_UNSIGNED_SHORT && info->restart_index <= 0xffff) ||
> +          (ib->type == GL_UNSIGNED_BYTE && info->restart_index <= 0xff))
> +         info->primitive_restart = true;
> +   }
> +}
> +
> +
> +/**
>   * Translate OpenGL primtive type (GL_POINTS, GL_TRIANGLE_STRIP, etc) to
>   * the corresponding Gallium type.
>   */
>  static unsigned
>  translate_prim(const struct gl_context *ctx, unsigned prim)
>  {
>     /* GL prims should match Gallium prims, spot-check a few */
>     STATIC_ASSERT(GL_POINTS == PIPE_PRIM_POINTS);
>     STATIC_ASSERT(GL_QUADS == PIPE_PRIM_QUADS);
>     STATIC_ASSERT(GL_TRIANGLE_STRIP_ADJACENCY == PIPE_PRIM_TRIANGLE_STRIP_ADJACENCY);
> @@ -198,33 +222,21 @@ st_draw_vbo(struct gl_context *ctx,
>
>        info.indexed = TRUE;
>        if (min_index != ~0U && max_index != ~0U) {
>           info.min_index = min_index;
>           info.max_index = max_index;
>        }
>
>        /* The VBO module handles restart for the non-indexed GLDrawArrays
>         * so we only set these fields for indexed drawing:
>         */
> -      if (ctx->Array._PrimitiveRestart) {
> -         info.restart_index = _mesa_primitive_restart_index(ctx, ib->type);
> -
> -         /* Enable primitive restart only when the restart index can have an
> -          * effect. This is required for correctness in radeonsi VI support,
> -          * though other hardware may also benefit from taking a faster,
> -          * non-restart path when possible.
> -          */
> -         if ((ibuffer.index_size >= 4) ||
> -             (ibuffer.index_size >= 2 && info.restart_index <= 0xffff) ||
> -             (info.restart_index <= 0xff))
> -            info.primitive_restart = true;
> -      }
> +      setup_primitive_restart(ctx, ib, &info);
>     }
>     else {
>        /* Transform feedback drawing is always non-indexed. */
>        /* Set info.count_from_stream_output. */
>        if (tfb_vertcount) {
>           if (!st_transform_feedback_draw_init(tfb_vertcount, stream, &info))
>              return;
>        }
>     }
>
> @@ -303,31 +315,30 @@ st_indirect_draw_vbo(struct gl_context *ctx,
>
>     if (ib) {
>        if (!setup_index_buffer(st, ib, &ibuffer)) {
>           _mesa_error(ctx, GL_OUT_OF_MEMORY, "gl%sDrawElementsIndirect%s",
>                       (draw_count > 1) ? "Multi" : "",
>                       indirect_params ? "CountARB" : "");
>           return;
>        }
>
>        info.indexed = TRUE;
> +
> +      /* Primitive restart is not handled by the VBO module in this case. */
> +      setup_primitive_restart(ctx, ib, &info);
>     }
>
>     info.mode = translate_prim(ctx, mode);
>     info.vertices_per_patch = ctx->TessCtrlProgram.patch_vertices;
>     info.indirect = st_buffer_object(indirect_data)->buffer;
>     info.indirect_offset = indirect_offset;
>
> -   /* Primitive restart is not handled by the VBO module in this case. */
> -   info.primitive_restart = ctx->Array._PrimitiveRestart;
> -   info.restart_index = ctx->Array.RestartIndex;
> -
>     if (ST_DEBUG & DEBUG_DRAW) {
>        debug_printf("st/draw indirect: mode %s drawcount %d indexed %d\n",
>                     u_prim_name(info.mode),
>                     draw_count,
>                     info.indexed);
>     }
>
>     if (!st->has_multi_draw_indirect) {
>        int i;
>
> --
> 2.7.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev