[Mesa-dev] [PATCH 1/2] gallium: make get_query_result return union* and not void*

[Brian Paul]

On 03/27/2012 02:51 PM, Marek Olšák wrote:
> This replaces the cryptic void* parameter with a union.
> (based on union r600_query_result)
>
> Users of this can still pass uint64* in it, but that cannot work for every
> query type, obviously. Most importantly, the code now documents what should
> be expected from get_query_result.
>
> This also adds pipe_query_data_pipeline_statistics as per the D3D11 docs.

In the future, an addition like that should probably go into a 
separate commit.  But it's OK here.


> ---
>   src/gallium/include/pipe/p_context.h |    3 +-
>   src/gallium/include/pipe/p_defines.h |   41 ++++++++++++++++++++++++++++++++++
>   2 files changed, 43 insertions(+), 1 deletions(-)
>
> diff --git a/src/gallium/include/pipe/p_context.h b/src/gallium/include/pipe/p_context.h
> index aaeeb81..8b4a158 100644
> --- a/src/gallium/include/pipe/p_context.h
> +++ b/src/gallium/include/pipe/p_context.h
> @@ -64,6 +64,7 @@ struct pipe_video_buffer;
>   struct pipe_video_decoder;
>   struct pipe_viewport_state;
>   union pipe_color_union;
> +union pipe_query_result;
>
>   /**
>    * Gallium rendering context.  Basically:
> @@ -117,7 +118,7 @@ struct pipe_context {
>      boolean (*get_query_result)(struct pipe_context *pipe,
>                                  struct pipe_query *q,
>                                  boolean wait,
> -                               void *result);
> +                               union pipe_query_result *result);
>      /*@}*/
>
>      /**
> diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h
> index 889fc99..a0a0c53 100644
> --- a/src/gallium/include/pipe/p_defines.h
> +++ b/src/gallium/include/pipe/p_defines.h
> @@ -528,12 +528,53 @@ struct pipe_query_data_so_statistics
>      uint64_t num_primitives_written;
>      uint64_t primitives_storage_needed;
>   };
> +
>   struct pipe_query_data_timestamp_disjoint

The existing query-related struct should probably have comments 
indicating which PIPE_QUERY_x they correspond to.


>   {
>      uint64_t frequency;
>      boolean  disjoint;
>   };
>
> +struct pipe_query_data_pipeline_statistics {

To be consistent with other structs, the opening brace should be on a 
new line.

> +   uint64_t ia_vertices;    /* Num vertices read by the vertex fetcher. */
> +   uint64_t ia_primitives;  /* Num primitives read by the vertex fetcher. */
> +   uint64_t vs_invocations; /* Num vertex shader invocations. */
> +   uint64_t gs_invocations; /* Num geometry shader invocations. */
> +   uint64_t gs_primitives;  /* Num primitives output by a geometry shader. */
> +   uint64_t c_invocations;  /* Num primitives sent to the rasterizer. */
> +   uint64_t c_primitives;   /* Num primitives that were rendered. */
> +   uint64_t ps_invocations; /* Num pixel shader invocations. */
> +   uint64_t hs_invocations; /* Num hull shader invocations. */
> +   uint64_t ds_invocations; /* Num domain shader invocations. */
> +   uint64_t cs_invocations; /* Num compute shader invocations. */

Other field comments use the doxygen /**< comment */ style.


> +};
> +
> +/**
> + * Query result (returned by pipe_context::get_query_result).
> + */
> +union pipe_query_result {

Brace on next line for consistency.


> +   /* PIPE_QUERY_OCCLUSION_PREDICATE */
> +   /* PIPE_QUERY_SO_OVERFLOW_PREDICATE */
> +   /* PIPE_QUERY_GPU_FINISHED */
> +   boolean b;
> +
> +   /* PIPE_QUERY_OCCLUSION_COUNTER */
> +   /* PIPE_QUERY_TIMESTAMP */
> +   /* PIPE_QUERY_TIME_ELAPSED */
> +   /* PIPE_QUERY_PRIMITIVES_GENERATED */
> +   /* PIPE_QUERY_PRIMITIVES_EMITTED */
> +   uint64_t u64;
> +
> +   /* PIPE_QUERY_SO_STATISTICS */
> +   struct pipe_query_data_so_statistics so_statistics;
> +
> +   /* PIPE_QUERY_TIMESTAMP_DISJOINT */
> +   struct pipe_query_data_timestamp_disjoint timestamp_disjoint;
> +
> +   /* PIPE_QUERY_PIPELINE_STATISTICS */
> +   struct pipe_query_data_pipeline_statistics pipeline_statistics;
> +};
> +
>   union pipe_color_union
>   {
>      float f[4];

For both, Reviewed-by: Brian Paul <brianp at vmware.com>