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

Stefan Dösinger stefandoesinger at gmx.at
Wed Jan 27 06:07:24 CST 2010

Am 27.01.2010 um 11:19 schrieb Henri Verbeet:
>> 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.
From section 5.2.2 in the extension:
>    If the sync object being blocked upon will not be signaled in finite
>    time (for example, by an associated fence command issued previously,
>    but not yet flushed to the graphics pipeline), then ClientWaitSync
>    may hang forever.
I don't know if there is any driver that waits forever for more commands before flushing at some point, but I noticed that with single buffering and no glFlush or glFinish fglrx keeps queuing commands until it runs out of memory. So I think we need it.

For multiple contexts we need a glFlush after draws, you're working on that for different reasons anyway.

Related, but subject for a different patch: We don't handle D3DGETDATA_FLUSH in either event or occlusion queries. Afaics this only matters for NV_fence and ARB_sync since APPLE_fence and occlusion queries already take care of that on the GL side.

> 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);".
I did of course notice that but didn't consider it much of an issue since the Query parent is entirely unused, except for Query_GetParent, which is never called. The parents cause issues whenever there is a mismatch between the client side object structure and the wined3d structure. E.g. ddraw has to create dummy parent objects because there's no swapchain or texture object(yeah, I know I run into hot water whenever I bring up ddraw).

I'd say we should get rid of the parent objects entirely and client libs can use SetPrivateData/GetPrivateData to find their interfaces in places like GetRenderTarget, GetTexture, etc. That way we don't make any assumptions about client library objects. That's something for a different patch though - I am happy with the NULL for now, or start by removing the query parent.

>> +/* 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.
I disliked the idea of having a LEAVE_G(); buffer_sync(); ENTER_GL() in the caller, when the two most common cases(NOOVERWRITE and DISCARD) don't need them, but ok.

More information about the wine-devel mailing list