Re: [PATCH 3/5] wined3d: Add support for timestamp queries. (try 2)

[Henri Verbeet]

On 14 April 2014 22:52, Matteo Bruni  wrote:
> - D3DQUERYTYPE type;

...
> - type = wined3d_query_get_type(query->wined3d_query);
> - if (type == D3DQUERYTYPE_TIMESTAMPDISJOINT)
> -
>    ret = sizeof(BOOL);
> - else
> -
>    ret = wined3d_query_get_data_size(query->wined3d_query);

Strictly speaking wined3d_query_get_type() returns an enum wined3d_query_type.
> - if (type == D3DQUERYTYPE_TIMESTAMPDISJOINT && data && size == sizeof(BOOL))
> - {
> -
>    struct wined3d_query_data_timestamp_disjoint data_disjoint;
> -
> -
>    hr = wined3d_query_get_data(query->wined3d_query, &data_disjoint, sizeof(data_disjoint), flags);
> -
>    *(BOOL *)data = data_disjoint.disjoint;
> - }
> - else
> - {
> -
>    hr = wined3d_query_get_data(query->wined3d_query, data, size, flags);
> - }

Does this do the right thing if an application were to pass
sizeof(data_disjoint) as size?
> -
>    if (gl_info->supported[ARB_TIMER_QUERY])
> -
>    {
> -
>        GL_EXTCALL(glGenQueriesARB(1, &query->id));
> -
>        checkGLcall("glGenQueriesARB");
> -
> -
>        TRACE("Allocated timestamp query %u in context %p.\n", query->id, context);
> -
>    }
> -
>    else
> -
>    {
> -
>        WARN("Timestamp queries not supported, not allocating query id.\n");
> -
>        query->id = 0;
> -
>    }

This should never happen, so this should either be an ERR(), or just
be removed. Though I'll grant that e.g. the occlusion query code has
some room for improvement as well in that regard.
> @@ -969,6 +1024,9 @@ static void context_destroy_gl_resources(struct wined3d_context *context)
>              GL_EXTCALL(glDeleteProgramsARB(1, &context->dummy_arbfp_prog));
>          }
> -
>    if (gl_info->supported[ARB_TIMER_QUERY])
> -
>        GL_EXTCALL(glDeleteQueriesARB(context->free_timestamp_query_count, context->free_timestamp_queries));
> -

What about queries that are still on the context->timestamp_queries list?
> +static HRESULT wined3d_timestamp_query_ops_get_data(struct wined3d_query *query,

...
> - if (!gl_info->supported[ARB_TIMER_QUERY])
> - {
> -
>    WARN("%p timestamp queries not supported. Returning 0.\n", query);
> -
>    if (u64data)
> -
>        *u64data = 0;
> -
>    return S_OK;
> - }

...
> +static HRESULT wined3d_timestamp_query_ops_issue(struct wined3d_query *query, DWORD flags)

...
> - if (gl_info->supported[ARB_TIMER_QUERY])
> - {

...
> - }
> - else
> - {
> -
>    FIXME("%p timestamp queries not supported.\n", query);
> - }

Similar to above, the query should never have been created in these cases.