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.