[Mesa-dev] [PATCH v2 5/8] mesa: Add a new GetTransformFeedbackVertexCount() driver hook.

[Marek Olšák]

This looks good. I recommend porting all_varyings_in_vbos from
st_draw.c to vbo_all_varyings_in_vbos, because it also takes instanced
arrays into account.

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

Marek

On Sat, Oct 26, 2013 at 7:35 AM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> DrawTransformFeedback() needs to obtain the number of vertices written
> to a particular stream during the last Begin/EndTransformFeedback block.
> The new driver hook returns exactly that information.
>
> Gallium drivers already implement this by passing the transform feedback
> object to the drawing function, counting the number of vertices written
> on the GPU, and using draw indirect.  This is efficient, but doesn't
> always work:
>
> If vertex data comes from user arrays, then the VBO module needs to
> know how many vertices to upload, so we need to synchronously count.
> Gallium drivers are currently broken in this case.
>
> It also doesn't work if primitive restart is done in software.  For
> normal drawing, vbo_draw_arrays() performs software primitive restart,
> splitting the draw call in two.  vbo_draw_transform_feedback() currently
> doesn't because it has no idea how many vertices need to be drawn.
>
> The new driver hook gives it that information, allowing us to reuse
> the existing vbo_draw_arrays() code to do everything right.
>
> On Intel hardware (at least Ivybridge), using the draw indirect approach
> is difficult since the hardware counts primitives, rather than vertices,
> which requires doing some simple math.  So we always use this hook.
>
> Gallium drivers will likely want to use this hook in some cases, but
> want to use the existing draw indirect approach where possible.  Hence,
> I've added a flag to allow drivers to opt-in to this call.
>
> v2: Make it possible to implement this hook but only use this path
>     when necessary (suggested by Marek).
>
> Cc: Marek Olšák <maraeo at gmail.com>
> Cc: Eric Anholt <eric at anholt.net>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_context.c |  2 ++
>  src/mesa/main/dd.h                      |  8 ++++++++
>  src/mesa/main/mtypes.h                  |  6 ++++++
>  src/mesa/vbo/vbo_exec_array.c           | 10 ++++++++++
>  4 files changed, 26 insertions(+)
>
> Marek,
>
> Does this look like what you wanted?  I feel a bit silly adding all of this
> seeing as the later conditions are totally untested - i965 sets the "always
> use this hook" flag, so it short-circuits them, and Gallium drivers don't
> yet implement the hook, so they don't hit it either. :)
>
> But I think this is probably roughly what you're going to want...
>
> Eric, does this look reasonable?
>
> Thanks for everything!
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> index 90d9be4..623273c 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -329,6 +329,8 @@ brw_initialize_context_constants(struct brw_context *brw)
>     ctx->Const.MaxTransformFeedbackSeparateComponents =
>        BRW_MAX_SOL_BINDINGS / BRW_MAX_SOL_BUFFERS;
>
> +   ctx->Const.AlwaysUseGetTransformFeedbackVertexCount = true;
> +
>     if (brw->gen == 6) {
>        ctx->Const.MaxSamples = 4;
>        ctx->Const.MaxColorTextureSamples = 4;
> diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
> index 29469ce..11d5a9e 100644
> --- a/src/mesa/main/dd.h
> +++ b/src/mesa/main/dd.h
> @@ -843,6 +843,14 @@ struct dd_function_table {
>                                     struct gl_transform_feedback_object *obj);
>
>     /**
> +    * Return the number of vertices written to a stream during the last
> +    * Begin/EndTransformFeedback block.
> +    */
> +   GLsizei (*GetTransformFeedbackVertexCount)(struct gl_context *ctx,
> +                                              struct gl_transform_feedback_object *obj,
> +                                              GLuint stream);
> +
> +   /**
>      * \name GL_NV_texture_barrier interface
>      */
>     void (*TextureBarrier)(struct gl_context *ctx);
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 97ed1bd..f5e1f01 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -3131,6 +3131,12 @@ struct gl_constants
>      */
>     GLboolean PrimitiveRestartInSoftware;
>
> +   /**
> +    * Always use the GetTransformFeedbackVertexCount() driver hook, rather
> +    * than passing the transform feedback object to the drawing function.
> +    */
> +   GLboolean AlwaysUseGetTransformFeedbackVertexCount;
> +
>     /** GL_ARB_map_buffer_alignment */
>     GLuint MinMapBufferAlignment;
>
> diff --git a/src/mesa/vbo/vbo_exec_array.c b/src/mesa/vbo/vbo_exec_array.c
> index 1670409..f25a9de 100644
> --- a/src/mesa/vbo/vbo_exec_array.c
> +++ b/src/mesa/vbo/vbo_exec_array.c
> @@ -1464,6 +1464,16 @@ vbo_draw_transform_feedback(struct gl_context *ctx, GLenum mode,
>        return;
>     }
>
> +   if (ctx->Driver.GetTransformFeedbackVertexCount &&
> +       (ctx->Const.AlwaysUseGetTransformFeedbackVertexCount ||
> +        (ctx->Const.PrimitiveRestartInSoftware &&
> +         ctx->Array._PrimitiveRestart) ||
> +        !vbo_all_varyings_in_vbos(exec->array.inputs))) {
> +      GLsizei n = ctx->Driver.GetTransformFeedbackVertexCount(ctx, obj, stream);
> +      vbo_draw_arrays(ctx, mode, 0, n, numInstances, 0);
> +      return;
> +   }
> +
>     vbo_bind_arrays(ctx);
>
>     /* init most fields to zero */
> --
> 1.8.3.2
>