On Mon Jun 17 14:33:56 2024 +0000, Giovanni Mascellani wrote:
I don't understand whether this fixes an actual bug. In the description and commit messages there is no reference, but in !5830 you mentioned that some glitches are fixed and this is an alternative solution. On the changes themselves: introducing an API like yours was I think the first solution I tried when I had (what I presume to be) a similar problem with The Medium. I got some pushback from other vkd3d developers and in the end I convinced myself that it's undesirable for a few reasons, both philosophical (vkd3d is not concerned with presentation in any other way, so this kind of violates some "separation of concerns" principle) and practical (coordinating changes between vkd3d and DXGI is hard, so we'd like to keep the interface as generic as possible; by moving presentation to vkd3d we already know we'll have to eventually change something, for example to handle the latency waitable). Also, I echo what I wrote in https://gitlab.winehq.org/wine/wine/-/merge_requests/5830#note_73417: I think the right way to implement the behavior you need shouldn't depend on `vkAcquireNextImageKHR()` blocking (or not), independently of how then you propagate the block to the caller.
However, this occurs in a worker thread to avoid deadlock, so an
arbitrary number of additional commands may be enqueued before vkQueuePresentKHR(). I haven't found a specific scenario where this is a problem, but it is not desirable if we can avoid it. Those commands are ending up in a queue anyway. I agree that ideally we'd do with as few queues as possible, but as I said we need a relatively fine control on how the DXGI present requests are handled, and I think that trying to retain the correct behavior across a public API that we'd like to keep as general as possible is going to be rather painful.
It does fix the vsync bug mentioned in !5830, yes. Also, a wait could be enqueued between the signal added for presentation and when the worker thread adds the `vkQueuePresentKHR()`, which could cause frame pacing issues.
vkd3d is not concerned with presentation in any other way, so this kind of violates some "separation of concerns" principle
But present commands must be added to a command queue, which is vkd3d's concern, so there's an argument that it should be concerned with presentation. Having an API which allows external clients to add anything the choose to the current queue tail doesn't separate concerns very well either.
coordinating changes between vkd3d and DXGI is hard, so we'd like to keep the interface as generic as possible
We could query for the PFN and use it if available, until the new API has been around for long enough.
by moving presentation to vkd3d we already know we'll have to eventually change something, for example to handle the latency waitable
It should be possible to handle this entirely on the DXGI side using the frame latency fence and event. We just need to always create them.