winex11: Moved child window buffer copy to vkQueuePresentKHR

[pac85]

Normally the copy is done in vkAcquireNextImageKHR ( relevant line ) but it would make more sense to do it during a call to vkQueuePresentKHR. This causes a bug in SeriousEditor (and potentially other programs) where the ui is displayed with a 1 event delay making it unusable (as well as showing uninitialized buffers whenever they are reinitialized).

[rbernon]

Hi, thanks for having a look at this.

If you remove the copy from Acquire, you probably need to remove the fencing and everything that's been added for it, it seems unnecessary to keep it.

Then note that it's how the vulkan child window patch series was initially doing, and it was suffering from a lot of image tearing. The newer version, that is in Proton, and which copies the image in Acquire also waits on the host Acquire to complete, using the fence, and I believe it helped with the tearing.

[pac85]

Hi, I updated my PR. To recap the issue I have, let's say an app does the following:

• Render offscreen
• vkAcquireNextImageKHR
• Copy offscreen image to acquired image
• vkQueuePresentKHR

Then doing the copy in vkAcquireNextImageKHR will introduce a 1 frame delay (or 1 event delay in the case of Serious Editor breaking everything). It seems far from ideal.

I don't have any good ideas on how to solve both problems at once, storing fences in the wine_vk_surface struct during the call to Acquire and then wait for it in QueuePresent could cause a deadlock if the application calls vkResetFences between the calls, it might also not prevent tearing.

[rbernon]

This feature is only supposed to solve issues when rendering to a child window, or when creating multiple surfaces on the same window. How is offscreen rendering done in this context? Maybe it triggers that code path incorrectly?

[pac85]

Serious Editor uses child windows to show it's panels. Unfortunately being a proprietary program I can't tell exactly how it's going wrong, the bullet points I gave where just an example of a potentially problematic pattern, in the case of Serious Editor, being event driven, it could also look like this:

• Wait some event
• vkAcquireNextImageKHR
• Render ui
• vkQueuePresentKHR

So an event comes in and the result gets rendered but the copy only happens when it loops back to the second point which doesn't happen until more events come in.

[rbernon]

Then maybe, in the cases where we want to prevent tearing, we could perhaps instead move the call to the host Acquire to the vkQueuePresentKHR call, waiting on the Acquire fence there, and keeping the acquired image somewhere to pass it back to the app on the next vkAcquireNextImageKHR call instead.

When tearing doesn't matter we just call host Acquire on win32 Acquire normally.

Ideally maybe it should be asynchronously done, so that vkQeuePresent doesn't block (as I believe it's not supposed to), but that'd require much more work and potential issues.

[pac85]

This sounds good, I'll try implement it and update my PR

[pac85]

Then maybe, in the cases where we want to prevent tearing, we could perhaps instead move the call to the host Acquire to the vkQueuePresentKHR call, waiting on the Acquire fence there, and keeping the acquired image somewhere to pass it back to the app on the next vkAcquireNextImageKHR call instead.

When tearing doesn't matter we just call host Acquire on win32 Acquire normally.

Ideally maybe it should be asynchronously done, so that vkQeuePresent doesn't block (as I believe it's not supposed to), but that'd require much more work and potential issues.

Hi, I've implemented this and updated the PR. There is still one issue: at least with my driver (latest mesa on archlinux, rx580) forcing VK_PRESENT_MODE_FIFO (that is, overriding the present mode in the wine wrapper) in Serious Editor still causes the 1 event delay. I think this is due to the fact that waiting the fence is not enough to guarantee that the new frame is visible to X (this is supported by the fact that forcing the new behavior where host vkAcquireNextImageKHR is called in win32 QueuePresent but passing VK_PRESENT_MODE_IMMEDIATE to the host driver during swapchain creation doesn't present the issue). In any case Serious Editor offers no way of using VK_PRESENT_MODE_FIFO without hacks as far as I know and tearing is gone from apps that use it so this PR should still be an improvement. (aside from some dirty hacks like the way I get a queue from the device, but I can look into improving thay if the PR is worth merging).

Also regarding wether it should be zone asynchronously, the spec states "Calls to vkQueuePresentKHR may block, but must return in finite time." so it shouldn't be necessary I think.

[pac85]

If there is really no way of syncing the buffer swap by QueuePresent with the read from XCopyArea (and of it really is the source of the problem) it might be worth considering to just wait the fence and do the copy in Acquire if MAILBOX or FIFO are used and do it in QueuePresent if IMEDIATE is used. To be honest I don't like any of those solutions. I was thinking whether the swapchain could be completely emulated using vulkan for child windows so that the copy could be done with vulkan and synced properly but I'm afraid this would be far more complex than the current solution.