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

[Henri Verbeet]

On 14 April 2014 22:52, Matteo Bruni <mbruni(a)codeweavers.com> 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.