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.
