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

[Marek Olšák]

On Fri, Dec 21, 2018, 6:28 PM Kenneth Graunke <kenneth at whitecape.org wrote:

> 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?
>

Backward compatibility is not required. Gallium is not a stable API. In
tree state trackers can be fixed easily. We shouldn't worry too much about
closed source state trackers.

Marek


> --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 --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181221/9ac26452/attachment-0001.html>