[Mesa-dev] [PATCH 1/3] st/mesa: Make an enum for pipeline statistics query result indices.

[Kenneth Graunke]

That seems like a reasonable interface to me.

But, I don't think it's backwards compatible.  Today, don't state
trackers set index = 0 and expect all 11 to be returned?  We could
easily change the in-tree state trackers, but not sure about the
other ones.

We could always encode the index differently, but at that point, I
wonder if it would be cleaner to just add a new query type like Ilia
suggested.

Marek, what would you prefer?

--Ken

On Friday, December 21, 2018 1:24:32 PM PST Marek Olšák wrote:
> The series looks good. Another way to distinguish between return one and
> return all is to use "index". index <= 11 returns one. index == ~0 returns
> all. This is the least intrusive.
> 
> st/mesa and gallium/hud always want to get one.
> st/nine and util/u_helpers always want to get all.
> 
> Marek
> 
> On Sat, Dec 15, 2018 at 4:45 AM Kenneth Graunke <kenneth at whitecape.org>
> wrote:
> 
> > Gallium handles pipeline statistics queries as a single query
> > (PIPE_QUERY_PIPELINE_STATISTICS) which returns a struct with 11 values.
> > Sometimes it's useful to refer to each of those values individually,
> > rather than as a group.  To avoid hardcoding numbers, we define a new
> > enum for each value.  Here, the name and enum value correspond to the
> > index in the struct pipe_query_data_pipeline_statistics result.
> >
> > Cc: Roland Scheidegger <sroland at vmware.com>
> > ---
> >  src/gallium/include/pipe/p_defines.h    | 17 +++++++++++++++++
> >  src/mesa/state_tracker/st_cb_queryobj.c | 22 +++++++++++-----------
> >  2 files changed, 28 insertions(+), 11 deletions(-)
> >
> > diff --git a/src/gallium/include/pipe/p_defines.h
> > b/src/gallium/include/pipe/p_defines.h
> > index 6d96f1ccb5b..21005955a36 100644
> > --- a/src/gallium/include/pipe/p_defines.h
> > +++ b/src/gallium/include/pipe/p_defines.h
> > @@ -568,6 +568,23 @@ enum pipe_query_type {
> >     PIPE_QUERY_DRIVER_SPECIFIC = 256,
> >  };
> >
> > +/**
> > + * Index for PIPE_QUERY_PIPELINE_STATISTICS subqueries.
> > + */
> > +enum pipe_statistics_query_index {
> > +   PIPE_STAT_QUERY_IA_VERTICES,
> > +   PIPE_STAT_QUERY_IA_PRIMITIVES,
> > +   PIPE_STAT_QUERY_VS_INVOCATIONS,
> > +   PIPE_STAT_QUERY_GS_INVOCATIONS,
> > +   PIPE_STAT_QUERY_GS_PRIMITIVES,
> > +   PIPE_STAT_QUERY_C_INVOCATIONS,
> > +   PIPE_STAT_QUERY_C_PRIMITIVES,
> > +   PIPE_STAT_QUERY_PS_INVOCATIONS,
> > +   PIPE_STAT_QUERY_HS_INVOCATIONS,
> > +   PIPE_STAT_QUERY_DS_INVOCATIONS,
> > +   PIPE_STAT_QUERY_CS_INVOCATIONS,
> > +};
> > +
> >  /**
> >   * Conditional rendering modes
> >   */
> > diff --git a/src/mesa/state_tracker/st_cb_queryobj.c
> > b/src/mesa/state_tracker/st_cb_queryobj.c
> > index 69e6004c3f1..82f53243336 100644
> > --- a/src/mesa/state_tracker/st_cb_queryobj.c
> > +++ b/src/mesa/state_tracker/st_cb_queryobj.c
> > @@ -386,37 +386,37 @@ st_StoreQueryResult(struct gl_context *ctx, struct
> > gl_query_object *q,
> >     } else if (stq->type == PIPE_QUERY_PIPELINE_STATISTICS) {
> >        switch (q->Target) {
> >        case GL_VERTICES_SUBMITTED_ARB:
> > -         index = 0;
> > +         index = PIPE_STAT_QUERY_IA_VERTICES;
> >           break;
> >        case GL_PRIMITIVES_SUBMITTED_ARB:
> > -         index = 1;
> > +         index = PIPE_STAT_QUERY_IA_PRIMITIVES;
> >           break;
> >        case GL_VERTEX_SHADER_INVOCATIONS_ARB:
> > -         index = 2;
> > +         index = PIPE_STAT_QUERY_VS_INVOCATIONS;
> >           break;
> >        case GL_GEOMETRY_SHADER_INVOCATIONS:
> > -         index = 3;
> > +         index = PIPE_STAT_QUERY_GS_INVOCATIONS;
> >           break;
> >        case GL_GEOMETRY_SHADER_PRIMITIVES_EMITTED_ARB:
> > -         index = 4;
> > +         index = PIPE_STAT_QUERY_GS_PRIMITIVES;
> >           break;
> >        case GL_CLIPPING_INPUT_PRIMITIVES_ARB:
> > -         index = 5;
> > +         index = PIPE_STAT_QUERY_C_INVOCATIONS;
> >           break;
> >        case GL_CLIPPING_OUTPUT_PRIMITIVES_ARB:
> > -         index = 6;
> > +         index = PIPE_STAT_QUERY_C_PRIMITIVES;
> >           break;
> >        case GL_FRAGMENT_SHADER_INVOCATIONS_ARB:
> > -         index = 7;
> > +         index = PIPE_STAT_QUERY_PS_INVOCATIONS;
> >           break;
> >        case GL_TESS_CONTROL_SHADER_PATCHES_ARB:
> > -         index = 8;
> > +         index = PIPE_STAT_QUERY_HS_INVOCATIONS;
> >           break;
> >        case GL_TESS_EVALUATION_SHADER_INVOCATIONS_ARB:
> > -         index = 9;
> > +         index = PIPE_STAT_QUERY_DS_INVOCATIONS;
> >           break;
> >        case GL_COMPUTE_SHADER_INVOCATIONS_ARB:
> > -         index = 10;
> > +         index = PIPE_STAT_QUERY_CS_INVOCATIONS;
> >           break;
> >        default:
> >           unreachable("Unexpected target");
> > --
> > 2.19.1
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181221/30840946/attachment.sig>