[PATCH 1/3] wined3d: Remove poll list.

[Jan Sikorski]

Hi,

> On 23 Nov 2021, at 12:46, Henri Verbeet <hverbeet at gmail.com> wrote:
> 
> On Mon, 22 Nov 2021 at 13:46, Jan Sikorski <jsikorski at codeweavers.com> wrote:
>> 
>> Poll with callbacks from application threads instead.
>> 
>> Signed-off-by: Jan Sikorski <jsikorski at codeweavers.com>
>> ---
>> dlls/wined3d/cs.c              |  73 ++-------
>> dlls/wined3d/query.c           | 284 ++++++++++++++++++++++-----------
>> dlls/wined3d/wined3d_private.h |   6 +-
>> 3 files changed, 212 insertions(+), 151 deletions(-)
>> 
> I happen to know the background of this series of patches because of
> private conversations we've had, but for the benefit of both the
> commit message and the rest of the mailing list, please assume I
> don't. Why would we want to do this? What problem does this patch
> solve?

Right, I guess it could use some explanation.

> I also don't think it's ideal to lead with a patch removing the
> existing mechanism. The way we generally do these kinds of rewrites is
> to first introduce the new mechanism, then move users of the old
> mechanism over to the new mechanism, and only remove the old mechanism
> once it has become unused.

OK, I think this patch could be split into a couple more patches, so that removing the list goes after we can poll using the new way, with only a little intermediate mess.

>> -static void wined3d_cs_exec_query_issue(struct wined3d_cs *cs, const void *data)
>> +void wined3d_cs_run_priority_callback(struct wined3d_cs *cs, void (*callback)(void *object), void *object)
>> {
>> -    const struct wined3d_cs_query_issue *op = data;
>> -    struct wined3d_query *query = op->query;
>> -    BOOL poll;
>> +    struct wined3d_cs_callback *op;
>> 
>> -    poll = query->query_ops->query_issue(query, op->flags);
>> +    op = wined3d_device_context_require_space(&cs->c, sizeof(*op), WINED3D_CS_QUEUE_MAP);
>> +    op->opcode = WINED3D_CS_OP_CALLBACK;
>> +    op->callback = callback;
>> +    op->object = object;
>> 
>> -    if (!cs->thread)
>> -        return;
>> +    wined3d_device_context_submit(&cs->c, WINED3D_CS_QUEUE_MAP);
>> +    wined3d_cs_finish(cs, WINED3D_CS_QUEUE_MAP);
>> +}
>> 
> The wined3d_cs_finish() call is effectively going to make polling
> queries synchronous, at least as far as WINED3D_CS_QUEUE_MAP is
> concerned. That doesn't seem desirable.

If we can’t call directly into OpenGL on the application thread reasonably, I’m not sure how much better can this get. Just removing the wined3d_cs_finish() and getting the result asynchronously may actually make it worse, if a game knows the queries should be done by the time it is called, it will probably just busy wait for it by calling GetData() repeatedly.
That said, I don’t think it is so bad, because for occlusion queries, which are the bulk I guess, we probably use the buffer method; and other users of WINED3D_CS_QUEUE_MAP are synchronous too, so it shouldn't clog unless there’s more threads involved. It ran well with the couple games I tested, so there’s that too, but supposedly we could hit a pessimistic case where it’s worse.. Do you have a better way in mind?

Thanks,
- Jan