Re: [Mesa-dev] [PATCH] st/mesa: fix PRIMITIVES_GENERATED query after the "pipeline stat single" changes
On Wednesday, January 23, 2019 1:26:25 AM PST Erik Faye-Lund wrote:
> On Fri, 2019-01-18 at 11:27 -0500, Marek Olšák wrote:
> > From: Marek Olšák
> >
> > ---
> > src/mesa/state_tracker/st_cb_queryobj.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/mesa/state_tracker/st_cb_queryobj.c
> > b/src/mesa/state_tracker/st_cb_queryobj.c
> > index abb126547c9..642b901d05a 100644
> > --- a/src/mesa/state_tracker/st_cb_queryobj.c
> > +++ b/src/mesa/state_tracker/st_cb_queryobj.c
> > @@ -84,21 +84,22 @@ st_DeleteQuery(struct gl_context *ctx, struct
> > gl_query_object *q)
> > struct st_query_object *stq = st_query_object(q);
> >
> > free_queries(pipe, stq);
> >
> > free(stq);
> > }
> >
> > static int
> > target_to_index(const struct st_context *st, const struct
> > gl_query_object *q)
> > {
> > - if (q->Target == GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN ||
> > + if (q->Target == GL_PRIMITIVES_GENERATED ||
> > + q->Target == GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN ||
> > q->Target == GL_TRANSFORM_FEEDBACK_STREAM_OVERFLOW_ARB)
> >return q->Stream;
> >
> > if (st->has_single_pipe_stat) {
> >switch (q->Target) {
> >case GL_VERTICES_SUBMITTED_ARB:
> > return PIPE_STAT_QUERY_IA_VERTICES;
> >case GL_PRIMITIVES_SUBMITTED_ARB:
> > return PIPE_STAT_QUERY_IA_PRIMITIVES;
> >case GL_VERTEX_SHADER_INVOCATIONS_ARB:
>
> The change itself looks good to me.
>
> However, I think a commit message saying, well, *something* would be a
> good idea (and probably would have made it easier to find reviewers in
> the first place). Something like this, perhaps?
>
> "When this functionality was added, the PRIMITIVES_GENERATED query was
> accidentally omitted. This causes issues for drivers that support
> transform feedback."
>
> I also think this should have a Fixes tag:
>
> Fixes: d644698b443 ("gallium: Add the ability to query a single
> pipeline statistics counter")
>
> With those things changed:
>
> Reviewed-by: Erik Faye-Lund
>
> Also, I added Kenneth Grauke who wrote the commit in question to the CC
> list. Perhaps he has some thoughts.
Yikes. Sorry, Marek, not sure how I didn't catch this. :(
Reviewed-by: Kenneth Graunke
signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/mesa: fix PRIMITIVES_GENERATED query after the "pipeline stat single" changes
On Fri, 2019-01-18 at 11:27 -0500, Marek Olšák wrote:
> From: Marek Olšák
>
> ---
> src/mesa/state_tracker/st_cb_queryobj.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/state_tracker/st_cb_queryobj.c
> b/src/mesa/state_tracker/st_cb_queryobj.c
> index abb126547c9..642b901d05a 100644
> --- a/src/mesa/state_tracker/st_cb_queryobj.c
> +++ b/src/mesa/state_tracker/st_cb_queryobj.c
> @@ -84,21 +84,22 @@ st_DeleteQuery(struct gl_context *ctx, struct
> gl_query_object *q)
> struct st_query_object *stq = st_query_object(q);
>
> free_queries(pipe, stq);
>
> free(stq);
> }
>
> static int
> target_to_index(const struct st_context *st, const struct
> gl_query_object *q)
> {
> - if (q->Target == GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN ||
> + if (q->Target == GL_PRIMITIVES_GENERATED ||
> + q->Target == GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN ||
> q->Target == GL_TRANSFORM_FEEDBACK_STREAM_OVERFLOW_ARB)
>return q->Stream;
>
> if (st->has_single_pipe_stat) {
>switch (q->Target) {
>case GL_VERTICES_SUBMITTED_ARB:
> return PIPE_STAT_QUERY_IA_VERTICES;
>case GL_PRIMITIVES_SUBMITTED_ARB:
> return PIPE_STAT_QUERY_IA_PRIMITIVES;
>case GL_VERTEX_SHADER_INVOCATIONS_ARB:
The change itself looks good to me.
However, I think a commit message saying, well, *something* would be a
good idea (and probably would have made it easier to find reviewers in
the first place). Something like this, perhaps?
"When this functionality was added, the PRIMITIVES_GENERATED query was
accidentally omitted. This causes issues for drivers that support
transform feedback."
I also think this should have a Fixes tag:
Fixes: d644698b443 ("gallium: Add the ability to query a single
pipeline statistics counter")
With those things changed:
Reviewed-by: Erik Faye-Lund
Also, I added Kenneth Grauke who wrote the commit in question to the CC
list. Perhaps he has some thoughts.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/mesa: fix PRIMITIVES_GENERATED query after the "pipeline stat single" changes
ping
On Fri, Jan 18, 2019 at 11:27 AM Marek Olšák wrote:
> From: Marek Olšák
>
> ---
> src/mesa/state_tracker/st_cb_queryobj.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/state_tracker/st_cb_queryobj.c
> b/src/mesa/state_tracker/st_cb_queryobj.c
> index abb126547c9..642b901d05a 100644
> --- a/src/mesa/state_tracker/st_cb_queryobj.c
> +++ b/src/mesa/state_tracker/st_cb_queryobj.c
> @@ -84,21 +84,22 @@ st_DeleteQuery(struct gl_context *ctx, struct
> gl_query_object *q)
> struct st_query_object *stq = st_query_object(q);
>
> free_queries(pipe, stq);
>
> free(stq);
> }
>
> static int
> target_to_index(const struct st_context *st, const struct gl_query_object
> *q)
> {
> - if (q->Target == GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN ||
> + if (q->Target == GL_PRIMITIVES_GENERATED ||
> + q->Target == GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN ||
> q->Target == GL_TRANSFORM_FEEDBACK_STREAM_OVERFLOW_ARB)
>return q->Stream;
>
> if (st->has_single_pipe_stat) {
>switch (q->Target) {
>case GL_VERTICES_SUBMITTED_ARB:
> return PIPE_STAT_QUERY_IA_VERTICES;
>case GL_PRIMITIVES_SUBMITTED_ARB:
> return PIPE_STAT_QUERY_IA_PRIMITIVES;
>case GL_VERTEX_SHADER_INVOCATIONS_ARB:
> --
> 2.17.1
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
