[2/5] WineD3D: Add an internal event query finish function

[Henri Verbeet]

On 26 January 2010 20:22, Stefan Dösinger <stefandoesinger at gmx.at> wrote:
> Patch 2 moves the event query faking out of the way. That allows me to remove all GL extension checks, and the buffer code just checks if it can create a d3d event query. (Patch 1 removes some dead code I ran into). Once Mesa supports ARB_sync we can kill this code entirely and tell people to update their drivers.
>
I'm not sure if it's an improvement over the previous patches or not.

> The only deeper change in patch 3 is that I am passing GL_SYNC_FLUSH_COMMANDS_BIT. I think this is reasonable when we're waiting for drawing to complete.
>
You shouldn't need that. It only flushes for the current context anyway.

>> > +HRESULT wined3d_event_query_finish(IWineD3DQuery* iface) {
>> Please don't create function prototypes (or other code) by copy/paste.
> Mind elaborating? If you're talking about the placing of the {, I changed this. I think we should at some point change the style of the remaining files(or at least low hanging fruits) to get rid of the existing formatting vs new wined3d style issues. I'm getting tired of those formatting problems.
>
You clearly copied this from some other prototype in query.c, the
placement of the brace and the * for example are inconsistent with the
rest of the code. The same goes for e.g. some of the debug prints that
are consistent with the places they were copied from, instead of with
each other. Although those would be minor style issues that could be
fixed, I think the deeper issue is that it is a bad way to write code.

> The other patches aren't really changed. Patch 5 uses CreateQuery to find out if event queries are supported, which brings a few GL locking changes to avoid calling CreateQuery with the lock held.
>
It also points out another problem with using IWineD3DQuery, you don't
have a real parent object to pass to CreateQuery(). Queries probably
don't really need a parent, but it does highlight that the interface
was never meant for wined3d internal use. It's also the kind of thing
you should be able to spot yourself when you write something like
"IWineD3DDevice_CreateQuery(iface, WINED3DQUERYTYPE_EVENT,
&cur->query, NULL);".

> +/* The caller provides a context and GL locking and binds the buffer */
> +static void buffer_sync_apple(struct wined3d_buffer *This, DWORD flags)
> +{
...
> +    LEAVE_GL();
> +    hr = wined3d_event_query_finish(This->query);
> +    ENTER_GL();
...
> +        LEAVE_GL();
> +        IWineD3DQuery_Release(This->query);
> +        ENTER_GL();
...
> +}
Don't do that. Patch 6 adds another one of those.