[Mesa-stable] [Mesa-dev] [PATCH 09/11] st/nine: Rework queries

[Ilia Mirkin]

On Sun, Nov 23, 2014 at 5:40 PM, David Heidelberg <david at ixit.cz> wrote:
> From: Axel Davy <axel.davy at ens.fr>
>
> From this moment we should handle errors same way as Wine does.
>
> Original patch from John Ettedgui <john.ettedgui at gmail.com>
>
> Cc: "10.4" <mesa-stable at lists.freedesktop.org>
> Tested-by: David Heidelberg <david at ixit.cz>
> Signed-off-by: Axel Davy <axel.davy at ens.fr>
> ---
>  src/gallium/state_trackers/nine/query9.c | 66 +++++++++++++++++++++-----------
>  1 file changed, 44 insertions(+), 22 deletions(-)
>
> diff --git a/src/gallium/state_trackers/nine/query9.c b/src/gallium/state_trackers/nine/query9.c
> index 908420c..34dfec7 100644
> --- a/src/gallium/state_trackers/nine/query9.c
> +++ b/src/gallium/state_trackers/nine/query9.c
> @@ -123,6 +123,15 @@ NineQuery9_ctor( struct NineQuery9 *This,
>          if (!This->pq)
>              return E_OUTOFMEMORY;
>      } else {
> +        /* we have a fallback when app create a query that is
> +           not supported. Wine has different behaviour. It won't fill the
> +           pointer with a valid NineQuery9, but let it NULL and return error.
> +           However even if driver doesn't support D3DQUERYTYPE_EVENT, it
> +           will say it is supported and have a fallback for it. Since we
> +           support more queries than wine we may hit different rendering paths
> +           than it, so perhaps these fallbacks are required.
> +           TODO: someone with a lot of different games should try to see
> +           if these dummy queries are needed. */
>          DBG("Returning dummy NineQuery9 for %s.\n",
>              nine_D3DQUERYTYPE_to_str(Type));
>      }
> @@ -174,10 +183,15 @@ NineQuery9_Issue( struct NineQuery9 *This,
>
>      DBG("This=%p dwIssueFlags=%d\n", This, dwIssueFlags);
>
> -    user_assert((dwIssueFlags == D3DISSUE_BEGIN && !This->instant) ||
> +    user_assert((dwIssueFlags == D3DISSUE_BEGIN) ||
>                  (dwIssueFlags == 0) ||
>                  (dwIssueFlags == D3DISSUE_END), D3DERR_INVALIDCALL);
>
> +    /* Wine tests: always return D3D_OK on D3DISSUE_BEGIN
> +     * even when the call is supposed to be forbidden */
> +    if (dwIssueFlags == D3DISSUE_BEGIN && This->instant)
> +        return D3D_OK;
> +
>      if (!This->pq) {
>          DBG("Issued dummy query.\n");
>          return D3D_OK;
> @@ -185,15 +199,17 @@ NineQuery9_Issue( struct NineQuery9 *This,
>
>      if (dwIssueFlags == D3DISSUE_BEGIN) {
>          if (This->state == NINE_QUERY_STATE_RUNNING) {
> -           pipe->end_query(pipe, This->pq);
> -       }
> +            pipe->end_query(pipe, This->pq);
> +        }
>          pipe->begin_query(pipe, This->pq);
>          This->state = NINE_QUERY_STATE_RUNNING;
>      } else {
> -        if (This->state == NINE_QUERY_STATE_RUNNING) {
> -            pipe->end_query(pipe, This->pq);
> -            This->state = NINE_QUERY_STATE_ENDED;
> -       }
> +        if (This->state != NINE_QUERY_STATE_RUNNING &&
> +            This->type != D3DQUERYTYPE_EVENT &&
> +            This->type != D3DQUERYTYPE_TIMESTAMP)
> +            pipe->begin_query(pipe, This->pq);
> +        pipe->end_query(pipe, This->pq);
> +        This->state = NINE_QUERY_STATE_ENDED;
>      }
>      return D3D_OK;
>  }
> @@ -220,7 +236,7 @@ NineQuery9_GetData( struct NineQuery9 *This,
>                      DWORD dwGetDataFlags )
>  {
>      struct pipe_context *pipe = This->base.device->pipe;
> -    boolean ok = !This->pq;
> +    boolean ok, should_flush, should_wait;
>      unsigned i;
>      union pipe_query_result presult;
>      union nine_query_result nresult;
> @@ -235,22 +251,28 @@ NineQuery9_GetData( struct NineQuery9 *This,
>
>      if (!This->pq) {
>          DBG("No pipe query available.\n");
> -        if (!dwSize)
> -           return S_OK;
> -    }
> -    if (This->state == NINE_QUERY_STATE_FRESH)
> -        return S_OK;
> +    } else {
> +        should_flush = dwGetDataFlags && This->state != NINE_QUERY_STATE_FLUSHED;
> +        /* Wine tests: D3DQUERYTYPE_TIMESTAMP always succeeds
> +         * directly when flushed */
> +        should_wait = dwGetDataFlags && This->type == D3DQUERYTYPE_TIMESTAMP;
> +
> +        if (This->state == NINE_QUERY_STATE_FRESH) {
> +            /* App forgot issue the request. Be nice and issue it. */
> +            (void) NineQuery9_Issue(This, D3DISSUE_END);
> +            /* Wine tests: we have to succeed. */
> +            should_flush = TRUE;
> +            should_wait = TRUE;
> +        }
>
> -    if (!ok) {
> -        ok = pipe->get_query_result(pipe, This->pq, FALSE, &presult);
> -        if (!ok) {
> -            if (dwGetDataFlags) {
> -                if (This->state != NINE_QUERY_STATE_FLUSHED)
> -                    pipe->flush(pipe, NULL, 0);
> -                This->state = NINE_QUERY_STATE_FLUSHED;
> -            }
> -            return S_FALSE;
> +        if (should_flush) {
> +            pipe->flush(pipe, NULL, 0);
> +            This->state = NINE_QUERY_STATE_FLUSHED;

This flips the logic around considerably -- instead of flushing on
get_query_result failure, you're flushing the pipe preemptively. A
pipe flush is a fairly heavy operation, and not at all necessary to do
here. It's just as likely that the query will have completed by then.
You should only do the flush if the query isn't ready yet [as
indicated by get_query_result returning false].

>          }
> +
> +        ok = pipe->get_query_result(pipe, This->pq, should_wait, &presult);
> +        if (!ok)
> +            return S_FALSE;
>      }
>      if (!dwSize)
>          return S_OK;
> --
> 2.1.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev