[PATCH 0/2] MR5851: Draft: dxgi: Handle D3D12 swapchain Present() commands by calling a dedicated present function in vkd3d.

[Conor McCarthy (＠cmccarthy)]

The current implementation enqueues the vkQueuePresentKHR() command after waiting on a fence which was enqueued where this command should go. 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.

This new implementation is just an RFC; it won't build because the vkd3d function is missing. That can be found [here](https://gitlab.winehq.org/cmccarthy/vkd3d/-/blob/present/libs/vkd3d/command....) but I haven't raised an MR. The DXGI side meets all requirements: vkAcquireNextImageKHR() blocks there, but the driver unblocks it so deadlock is not possible. The vkd3d function waits only for the command queue op mutex and cannot deadlock either. The overall implementation should be less fragile, and doesn't need a worker thread.

A few days ago I saw a performance increase in HZD from reverting DXGI to before the worker was added, but have never seen this again. Performance differences are minimal between old, current and this one.

[PATCH 1/2] dxgi: Do not wait on a fence after calling vkAcquireNextImageKHR().

[Conor McCarthy]

From: Conor McCarthy

If the timeout is UINT64_MAX, vkAcquireNextImageKHR() blocks until the
next image is acquired. Waiting on a fence is unnecessary.
---
 dlls/dxgi/swapchain.c | 27 +--------------------------
 1 file changed, 1 insertion(+), 26 deletions(-)

diff --git a/dlls/dxgi/swapchain.c b/dlls/dxgi/swapchain.c
index 4797fcecd8d..a23aed604e8 100644
--- a/dlls/dxgi/swapchain.c
+++ b/dlls/dxgi/swapchain.c
@@ -1138,7 +1138,6 @@ struct d3d12_swapchain
     struct wined3d_swapchain_state_parent state_parent;

VkSurfaceKHR vk_surface;
-    VkFence vk_fence;
     VkInstance vk_instance;
     VkDevice vk_device;
     VkPhysicalDevice vk_physical_device;
@@ -1753,25 +1752,15 @@ static VkResult d3d12_swapchain_acquire_next_vulkan_image(struct d3d12_swapchain
 {
     const struct dxgi_vk_funcs *vk_funcs = &swapchain->vk_funcs;
     VkDevice vk_device = swapchain->vk_device;
-    VkFence vk_fence = swapchain->vk_fence;
     VkResult vr;

swapchain->vk_image_index = INVALID_VK_IMAGE_INDEX;

if ((vr = vk_funcs->p_vkAcquireNextImageKHR(vk_device, swapchain->vk_swapchain, UINT64_MAX,
-            VK_NULL_HANDLE, vk_fence, &swapchain->vk_image_index)) < 0)
+            VK_NULL_HANDLE, VK_NULL_HANDLE, &swapchain->vk_image_index)) < 0)
     {
         WARN("Failed to acquire next Vulkan image, vr %d.\n", vr);
-        return vr;
-    }
-
-    if ((vr = vk_funcs->p_vkWaitForFences(vk_device, 1, &vk_fence, VK_TRUE, UINT64_MAX)) != VK_SUCCESS)
-    {
-        ERR("Failed to wait for fence, vr %d.\n", vr);
-        return vr;
     }
-    if ((vr = vk_funcs->p_vkResetFences(vk_device, 1, &vk_fence)) < 0)
-        ERR("Failed to reset fence, vr %d.\n", vr);

return vr;
 }
@@ -2067,7 +2056,6 @@ static void d3d12_swapchain_destroy(struct d3d12_swapchain *swapchain)

if (swapchain->vk_device)
     {
-        vk_funcs->p_vkDestroyFence(swapchain->vk_device, swapchain->vk_fence, NULL);
         vk_funcs->p_vkDestroySwapchainKHR(swapchain->vk_device, swapchain->vk_swapchain, NULL);
     }

@@ -3182,14 +3170,12 @@ static HRESULT d3d12_swapchain_init(struct d3d12_swapchain *swapchain, IWineDXGI
     VkWin32SurfaceCreateInfoKHR surface_desc;
     VkPhysicalDevice vk_physical_device;
     struct dxgi_factory *dxgi_factory;
-    VkFenceCreateInfo fence_desc;
     uint32_t queue_family_index;
     VkSurfaceKHR vk_surface;
     VkInstance vk_instance;
     IDXGIOutput *output;
     VkBool32 supported;
     VkDevice vk_device;
-    VkFence vk_fence;
     bool fullscreen;
     VkResult vr;
     HRESULT hr;
@@ -3341,17 +3327,6 @@ static HRESULT d3d12_swapchain_init(struct d3d12_swapchain *swapchain, IWineDXGI
         return hr;
     }

-    fence_desc.sType = VK_STRUCTURE_TYPE_FENCE_CREATE_INFO;
-    fence_desc.pNext = NULL;
-    fence_desc.flags = 0;
-    if ((vr = vk_funcs->p_vkCreateFence(vk_device, &fence_desc, NULL, &vk_fence)) < 0)
-    {
-        WARN("Failed to create Vulkan fence, vr %d.\n", vr);
-        d3d12_swapchain_destroy(swapchain);
-        return hresult_from_vk_result(vr);
-    }
-    swapchain->vk_fence = vk_fence;
-
     swapchain->current_buffer_index = 0;

if (swapchain_desc->Flags & DXGI_SWAP_CHAIN_FLAG_FRAME_LATENCY_WAITABLE_OBJECT)

[PATCH 2/2] dxgi: Call vkd3d_present_vk_swapchain() to present D3D12 swapchains.

[Conor McCarthy]

From: Conor McCarthy

---
 dlls/dxgi/swapchain.c | 387 +++++++-----------------------------------
 1 file changed, 59 insertions(+), 328 deletions(-)

diff --git a/dlls/dxgi/swapchain.c b/dlls/dxgi/swapchain.c
index a23aed604e8..66ac3f312ce 100644
--- a/dlls/dxgi/swapchain.c
+++ b/dlls/dxgi/swapchain.c
@@ -1142,12 +1142,6 @@ struct d3d12_swapchain
     VkDevice vk_device;
     VkPhysicalDevice vk_physical_device;

-    HANDLE worker_thread;
-    CRITICAL_SECTION worker_cs;
-    CONDITION_VARIABLE worker_cv;
-    bool worker_running;
-    struct list worker_ops;
-
     /* D3D12 side of the swapchain (frontend): these objects are
      * visible to the IDXGISwapChain client, so they must never be
      * recreated, except when ResizeBuffers*() is called. */
@@ -1174,8 +1168,6 @@ struct d3d12_swapchain
     VkPresentModeKHR present_mode;
     DXGI_SWAP_CHAIN_DESC1 backend_desc;

-    ID3D12Fence *present_fence;
-
     uint32_t vk_image_index;

struct dxgi_vk_funcs vk_funcs;
@@ -1197,101 +1189,6 @@ struct d3d12_swapchain
     uint32_t frame_latency;
 };

-enum d3d12_swapchain_op_type
-{
-    D3D12_SWAPCHAIN_OP_PRESENT,
-    D3D12_SWAPCHAIN_OP_RESIZE_BUFFERS,
-};
-
-struct d3d12_swapchain_op
-{
-    struct list entry;
-    enum d3d12_swapchain_op_type type;
-    union
-    {
-        struct
-        {
-            unsigned int sync_interval;
-            VkImage vk_image;
-            unsigned int frame_number;
-        } present;
-        struct
-        {
-            /* The resize buffers op takes ownership of the memory and
-             * images used to back the previous ID3D12Resource
-             * objects, so that they're only released once the worker
-             * thread is done with them. */
-            VkDeviceMemory vk_memory;
-            VkImage vk_images[DXGI_MAX_SWAP_CHAIN_BUFFERS];
-            DXGI_SWAP_CHAIN_DESC1 desc;
-        } resize_buffers;
-    };
-};
-
-static void d3d12_swapchain_op_destroy(struct d3d12_swapchain *swapchain, struct d3d12_swapchain_op *op)
-{
-    const struct dxgi_vk_funcs *vk_funcs = &swapchain->vk_funcs;
-    unsigned int i;
-
-    if (op->type == D3D12_SWAPCHAIN_OP_RESIZE_BUFFERS)
-    {
-        assert(swapchain->vk_device);
-
-        for (i = 0; i < DXGI_MAX_SWAP_CHAIN_BUFFERS; ++i)
-            vk_funcs->p_vkDestroyImage(swapchain->vk_device, op->resize_buffers.vk_images[i], NULL);
-
-        vk_funcs->p_vkFreeMemory(swapchain->vk_device, op->resize_buffers.vk_memory, NULL);
-    }
-
-    free(op);
-}
-
-static HRESULT d3d12_swapchain_op_present_execute(struct d3d12_swapchain *swapchain, struct d3d12_swapchain_op *op);
-static HRESULT d3d12_swapchain_op_resize_buffers_execute(struct d3d12_swapchain *swapchain, struct d3d12_swapchain_op *op);
-
-static DWORD WINAPI d3d12_swapchain_worker_proc(void *data)
-{
-    struct d3d12_swapchain *swapchain = data;
-
-    EnterCriticalSection(&swapchain->worker_cs);
-
-    while (swapchain->worker_running)
-    {
-        if (!list_empty(&swapchain->worker_ops))
-        {
-            struct d3d12_swapchain_op *op = LIST_ENTRY(list_head(&swapchain->worker_ops), struct d3d12_swapchain_op, entry);
-
-            list_remove(&op->entry);
-
-            LeaveCriticalSection(&swapchain->worker_cs);
-
-            switch (op->type)
-            {
-                case D3D12_SWAPCHAIN_OP_PRESENT:
-                    d3d12_swapchain_op_present_execute(swapchain, op);
-                    break;
-
-                case D3D12_SWAPCHAIN_OP_RESIZE_BUFFERS:
-                    d3d12_swapchain_op_resize_buffers_execute(swapchain, op);
-                    break;
-            }
-
-            d3d12_swapchain_op_destroy(swapchain, op);
-
-            EnterCriticalSection(&swapchain->worker_cs);
-        }
-        else if (!SleepConditionVariableCS(&swapchain->worker_cv, &swapchain->worker_cs, INFINITE))
-        {
-            ERR("Cannot sleep on condition variable, last error %ld.\n", GetLastError());
-            break;
-        }
-    }
-
-    LeaveCriticalSection(&swapchain->worker_cs);
-
-    return 0;
-}
-
 static DXGI_FORMAT dxgi_format_from_vk_format(VkFormat vk_format)
 {
     switch (vk_format)
@@ -1757,7 +1654,7 @@ static VkResult d3d12_swapchain_acquire_next_vulkan_image(struct d3d12_swapchain
     swapchain->vk_image_index = INVALID_VK_IMAGE_INDEX;

if ((vr = vk_funcs->p_vkAcquireNextImageKHR(vk_device, swapchain->vk_swapchain, UINT64_MAX,
-            VK_NULL_HANDLE, VK_NULL_HANDLE, &swapchain->vk_image_index)) < 0)
+            VK_NULL_HANDLE, VK_NULL_HANDLE, &swapchain->vk_image_index)) < 0 && vr != VK_ERROR_OUT_OF_DATE_KHR)
     {
         WARN("Failed to acquire next Vulkan image, vr %d.\n", vr);
     }
@@ -2013,36 +1910,10 @@ static void d3d12_swapchain_destroy(struct d3d12_swapchain *swapchain)
 {
     const struct dxgi_vk_funcs *vk_funcs = &swapchain->vk_funcs;
     void *vulkan_module = vk_funcs->vulkan_module;
-    struct d3d12_swapchain_op *op, *op2;
-    DWORD ret;
-
-    EnterCriticalSection(&swapchain->worker_cs);
-    swapchain->worker_running = false;
-    WakeAllConditionVariable(&swapchain->worker_cv);
-    LeaveCriticalSection(&swapchain->worker_cs);
-
-    if (swapchain->worker_thread)
-    {
-        if ((ret = WaitForSingleObject(swapchain->worker_thread, INFINITE)) != WAIT_OBJECT_0)
-            ERR("Failed to wait for worker thread, return value %ld.\n", ret);
-
-        if (!CloseHandle(swapchain->worker_thread))
-            ERR("Failed to close worker thread, last error %ld.\n", GetLastError());
-    }
-
-    DeleteCriticalSection(&swapchain->worker_cs);
-
-    LIST_FOR_EACH_ENTRY_SAFE(op, op2, &swapchain->worker_ops, struct d3d12_swapchain_op, entry)
-    {
-        d3d12_swapchain_op_destroy(swapchain, op);
-    }

d3d12_swapchain_destroy_vulkan_resources(swapchain);
     d3d12_swapchain_destroy_d3d12_resources(swapchain);

-    if (swapchain->present_fence)
-        ID3D12Fence_Release(swapchain->present_fence);
-
     if (swapchain->frame_latency_event)
         CloseHandle(swapchain->frame_latency_event);

@@ -2180,182 +2051,88 @@ static HRESULT d3d12_swapchain_set_sync_interval(struct d3d12_swapchain *swapcha
     return d3d12_swapchain_create_vulkan_resources(swapchain);
 }

-static VkResult d3d12_swapchain_queue_present(struct d3d12_swapchain *swapchain, VkImage vk_src_image,
-        uint64_t frame_number)
+static HRESULT d3d12_swapchain_present(struct d3d12_swapchain *swapchain,
+        unsigned int sync_interval, unsigned int flags)
 {
     const struct dxgi_vk_funcs *vk_funcs = &swapchain->vk_funcs;
-    VkPresentInfoKHR present_info;
+    struct vkd3d_vk_swapchain_present_info present_info;
     VkCommandBuffer vk_cmd_buffer;
-    VkSubmitInfo submit_info;
     VkImage vk_dst_image;
-    VkQueue vk_queue;
     VkResult vr;
     HRESULT hr;

-    if (swapchain->vk_image_index == INVALID_VK_IMAGE_INDEX)
-    {
-        if ((vr = d3d12_swapchain_acquire_next_vulkan_image(swapchain)) < 0)
-            return vr;
-    }
-
-    assert(swapchain->vk_image_index < swapchain->buffer_count);
-
-    vk_cmd_buffer = swapchain->vk_cmd_buffers[swapchain->vk_image_index];
-    vk_dst_image = swapchain->vk_swapchain_images[swapchain->vk_image_index];
-
-    if ((vr = vk_funcs->p_vkResetCommandBuffer(vk_cmd_buffer, 0)) < 0)
-    {
-        ERR("Failed to reset command buffer, vr %d.\n", vr);
-        return vr;
-    }
-
-    if ((vr = d3d12_swapchain_record_swapchain_blit(swapchain,
-            vk_cmd_buffer, vk_dst_image, vk_src_image)) < 0 )
-        return vr;
-
-    if (FAILED(hr = ID3D12Fence_SetEventOnCompletion(swapchain->present_fence, frame_number + 1, NULL)))
+    if (sync_interval > 4)
     {
-        ERR("Failed to wait for present event, hr %#lx.\n", hr);
-        return VK_ERROR_UNKNOWN;
+        WARN("Invalid sync interval %u.\n", sync_interval);
+        return DXGI_ERROR_INVALID_CALL;
     }

-    vk_queue = vkd3d_acquire_vk_queue(swapchain->command_queue);
-
-    submit_info.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO;
-    submit_info.pNext = NULL;
-    submit_info.waitSemaphoreCount = 0;
-    submit_info.pWaitSemaphores = NULL;
-    submit_info.pWaitDstStageMask = NULL;
-    submit_info.commandBufferCount = 1;
-    submit_info.pCommandBuffers = &vk_cmd_buffer;
-    submit_info.signalSemaphoreCount = 1;
-    submit_info.pSignalSemaphores = &swapchain->vk_semaphores[swapchain->vk_image_index];
-
-    if ((vr = vk_funcs->p_vkQueueSubmit(vk_queue, 1, &submit_info, VK_NULL_HANDLE)) < 0)
+    if (flags & ~DXGI_PRESENT_TEST)
+        FIXME("Unimplemented flags %#x.\n", flags);
+    if (flags & DXGI_PRESENT_TEST)
     {
-        ERR("Failed to blit swapchain buffer, vr %d.\n", vr);
-        vkd3d_release_vk_queue(swapchain->command_queue);
-        return vr;
+        WARN("Returning S_OK for DXGI_PRESENT_TEST.\n");
+        return S_OK;
     }

-    present_info.sType = VK_STRUCTURE_TYPE_PRESENT_INFO_KHR;
-    present_info.pNext = NULL;
-    present_info.waitSemaphoreCount = 1;
-    present_info.pWaitSemaphores = &swapchain->vk_semaphores[swapchain->vk_image_index];
-    present_info.swapchainCount = 1;
-    present_info.pSwapchains = &swapchain->vk_swapchain;
-    present_info.pImageIndices = &swapchain->vk_image_index;
-    present_info.pResults = NULL;
-
-    if ((vr = vk_funcs->p_vkQueuePresentKHR(vk_queue, &present_info)) >= 0)
-        swapchain->vk_image_index = INVALID_VK_IMAGE_INDEX;
-
-    vkd3d_release_vk_queue(swapchain->command_queue);
-
-    return vr;
-}
-
-static HRESULT d3d12_swapchain_op_present_execute(struct d3d12_swapchain *swapchain, struct d3d12_swapchain_op *op)
-{
-    VkResult vr;
-    HRESULT hr;
-
-    if (FAILED(hr = d3d12_swapchain_set_sync_interval(swapchain, op->present.sync_interval)))
+    if (FAILED(hr = d3d12_swapchain_set_sync_interval(swapchain, sync_interval)))
         return hr;

-    vr = d3d12_swapchain_queue_present(swapchain, op->present.vk_image, op->present.frame_number);
-    if (vr == VK_ERROR_OUT_OF_DATE_KHR)
-    {
-        TRACE("Recreating Vulkan swapchain.\n");
-
-        d3d12_swapchain_destroy_vulkan_resources(swapchain);
-        if (FAILED(hr = d3d12_swapchain_create_vulkan_resources(swapchain)))
-            return hr;
-
-        if ((vr = d3d12_swapchain_queue_present(swapchain, op->present.vk_image, op->present.frame_number)) < 0)
-            ERR("Failed to present after recreating swapchain, vr %d.\n", vr);
-    }
-
-    if (vr < 0)
-    {
-        ERR("Failed to queue present, vr %d.\n", vr);
-        return hresult_from_vk_result(vr);
-    }
-
-    if (swapchain->frame_latency_fence)
+    if (swapchain->vk_image_index == INVALID_VK_IMAGE_INDEX)
     {
-        /* Use the same bias as d3d12_swapchain_present(). Add one to
-         * account for the "++swapchain->frame_number" there. */
-        uint64_t number = op->present.frame_number + DXGI_MAX_SWAP_CHAIN_BUFFERS + 1;
-
-        if (FAILED(hr = ID3D12CommandQueue_Signal(swapchain->command_queue,
-                swapchain->frame_latency_fence, number)))
+        if ((vr = d3d12_swapchain_acquire_next_vulkan_image(swapchain)) == VK_ERROR_OUT_OF_DATE_KHR)
         {
-            ERR("Failed to signal frame latency fence, hr %#lx.\n", hr);
-            return hr;
-        }
-    }
+            TRACE("Recreating Vulkan swapchain.\n");

-    return S_OK;
-}
+            d3d12_swapchain_destroy_vulkan_resources(swapchain);
+            if (FAILED(hr = d3d12_swapchain_create_vulkan_resources(swapchain)))
+                return hr;

-static HRESULT d3d12_swapchain_present(struct d3d12_swapchain *swapchain,
-        unsigned int sync_interval, unsigned int flags)
-{
-    struct d3d12_swapchain_op *op;
-    HANDLE frame_latency_event;
-    HRESULT hr;
+            if ((vr = d3d12_swapchain_acquire_next_vulkan_image(swapchain)) < 0)
+                ERR("Failed to acquire image after recreating swapchain, vr %d.\n", vr);
+        }

-    if (sync_interval > 4)
-    {
-        WARN("Invalid sync interval %u.\n", sync_interval);
-        return DXGI_ERROR_INVALID_CALL;
+        if (vr < 0)
+            return hresult_from_vk_result(vr);
     }

-    if (flags & ~DXGI_PRESENT_TEST)
-        FIXME("Unimplemented flags %#x.\n", flags);
-    if (flags & DXGI_PRESENT_TEST)
-    {
-        WARN("Returning S_OK for DXGI_PRESENT_TEST.\n");
-        return S_OK;
-    }
+    assert(swapchain->vk_image_index < swapchain->buffer_count);

-    if (!(op = calloc(1, sizeof(*op))))
+    vk_cmd_buffer = swapchain->vk_cmd_buffers[swapchain->vk_image_index];
+    vk_dst_image = swapchain->vk_swapchain_images[swapchain->vk_image_index];
+
+    if ((vr = vk_funcs->p_vkResetCommandBuffer(vk_cmd_buffer, 0)) < 0)
     {
-        WARN("Cannot allocate memory.\n");
-        return E_OUTOFMEMORY;
+        ERR("Failed to reset command buffer, vr %d.\n", vr);
+        return hresult_from_vk_result(vr);
     }

-    op->type = D3D12_SWAPCHAIN_OP_PRESENT;
-    op->present.sync_interval = sync_interval;
-    op->present.vk_image = swapchain->vk_images[swapchain->current_buffer_index];
-    op->present.frame_number = swapchain->frame_number;
-
-    EnterCriticalSection(&swapchain->worker_cs);
-    list_add_tail(&swapchain->worker_ops, &op->entry);
-    WakeAllConditionVariable(&swapchain->worker_cv);
-    LeaveCriticalSection(&swapchain->worker_cs);
+    if ((vr = d3d12_swapchain_record_swapchain_blit(swapchain, vk_cmd_buffer,
+            vk_dst_image, swapchain->vk_images[swapchain->current_buffer_index])) < 0 )
+        return hresult_from_vk_result(vr);

++swapchain->frame_number;
-    if ((frame_latency_event = swapchain->frame_latency_event))
-    {
-        /* Bias the frame number to avoid underflowing in
-         * SetEventOnCompletion(). */
-        uint64_t number = swapchain->frame_number + DXGI_MAX_SWAP_CHAIN_BUFFERS;

-        if (FAILED(hr = ID3D12Fence_SetEventOnCompletion(swapchain->frame_latency_fence,
-                number - swapchain->frame_latency, frame_latency_event)))
-        {
-            ERR("Failed to enqueue frame latency event, hr %#lx.\n", hr);
-            return hr;
-        }
+    present_info.type = VKD3D_STRUCTURE_TYPE_VK_SWAPCHAIN_PRESENT_INFO;
+    present_info.next = NULL;
+    present_info.vk_swapchain = swapchain->vk_swapchain;
+    present_info.vk_image_index = swapchain->vk_image_index;
+    present_info.vk_blit_cmd_buffer = vk_cmd_buffer;
+    present_info.vk_semaphore = swapchain->vk_semaphores[swapchain->vk_image_index];
+    present_info.frame_latency_fence = swapchain->frame_latency_fence;
+    present_info.fence_signal_value = swapchain->frame_number + DXGI_MAX_SWAP_CHAIN_BUFFERS;
+    if (FAILED(hr = vkd3d_present_vk_swapchain(swapchain->command_queue, &present_info)))
+    {
+        ERR("Failed to present swapchain, hr %#lx.\n", hr);
+        return hr;
     }

-    if (FAILED(hr = ID3D12CommandQueue_Signal(swapchain->command_queue,
-            swapchain->present_fence, swapchain->frame_number)))
+    swapchain->vk_image_index = INVALID_VK_IMAGE_INDEX;
+
+    if (swapchain->frame_latency_event)
     {
-        ERR("Failed to signal present fence, hf %#lx.\n", hr);
-        return hr;
+        ID3D12Fence_SetEventOnCompletion(swapchain->frame_latency_fence,
+                present_info.fence_signal_value - swapchain->frame_latency, swapchain->frame_latency_event);
     }

swapchain->current_buffer_index = (swapchain->current_buffer_index + 1) % swapchain->desc.BufferCount;
@@ -2537,22 +2314,13 @@ static HRESULT STDMETHODCALLTYPE d3d12_swapchain_GetDesc(IDXGISwapChain4 *iface,
     return S_OK;
 }

-static HRESULT d3d12_swapchain_op_resize_buffers_execute(struct d3d12_swapchain *swapchain, struct d3d12_swapchain_op *op)
-{
-    d3d12_swapchain_destroy_vulkan_resources(swapchain);
-
-    swapchain->backend_desc = op->resize_buffers.desc;
-
-    return d3d12_swapchain_create_vulkan_resources(swapchain);
-}
-
 static HRESULT d3d12_swapchain_resize_buffers(struct d3d12_swapchain *swapchain,
         UINT buffer_count, UINT width, UINT height, DXGI_FORMAT format, UINT flags)
 {
     DXGI_SWAP_CHAIN_DESC1 *desc, new_desc;
-    struct d3d12_swapchain_op *op;
     unsigned int i;
     ULONG refcount;
+    HRESULT hr;

if (flags)
         FIXME("Ignoring flags %#x.\n", flags);
@@ -2596,38 +2364,22 @@ static HRESULT d3d12_swapchain_resize_buffers(struct d3d12_swapchain *swapchain,
     if (!dxgi_validate_swapchain_desc(&new_desc))
         return DXGI_ERROR_INVALID_CALL;

+    swapchain->current_buffer_index = 0;
+
     if (desc->Width == new_desc.Width && desc->Height == new_desc.Height
             && desc->Format == new_desc.Format && desc->BufferCount == new_desc.BufferCount)
     {
-        swapchain->current_buffer_index = 0;
         return S_OK;
     }

-    if (!(op = calloc(1, sizeof(*op))))
-    {
-        WARN("Cannot allocate memory.\n");
-        return E_OUTOFMEMORY;
-    }
-
-    op->type = D3D12_SWAPCHAIN_OP_RESIZE_BUFFERS;
-    op->resize_buffers.vk_memory = swapchain->vk_memory;
-    swapchain->vk_memory = VK_NULL_HANDLE;
-    memcpy(&op->resize_buffers.vk_images, &swapchain->vk_images, sizeof(swapchain->vk_images));
-    memset(&swapchain->vk_images, 0, sizeof(swapchain->vk_images));
-    op->resize_buffers.desc = new_desc;
-
-    EnterCriticalSection(&swapchain->worker_cs);
-    list_add_tail(&swapchain->worker_ops, &op->entry);
-    WakeAllConditionVariable(&swapchain->worker_cv);
-    LeaveCriticalSection(&swapchain->worker_cs);
-
-    swapchain->current_buffer_index = 0;
-
+    d3d12_swapchain_destroy_vulkan_resources(swapchain);
     d3d12_swapchain_destroy_d3d12_resources(swapchain);
-
     swapchain->desc = new_desc;

-    return d3d12_swapchain_create_d3d12_resources(swapchain);
+    if (FAILED(hr = d3d12_swapchain_create_d3d12_resources(swapchain)))
+        return hr;
+
+    return d3d12_swapchain_create_vulkan_resources(swapchain);
 }

static HRESULT STDMETHODCALLTYPE d3d12_swapchain_ResizeBuffers(IDXGISwapChain4 *iface,
@@ -3285,11 +3037,6 @@ static HRESULT d3d12_swapchain_init(struct d3d12_swapchain *swapchain, IWineDXGI

wined3d_private_store_init(&swapchain->private_store);

-    InitializeCriticalSection(&swapchain->worker_cs);
-    InitializeConditionVariable(&swapchain->worker_cv);
-    swapchain->worker_running = true;
-    list_init(&swapchain->worker_ops);
-
     surface_desc.sType = VK_STRUCTURE_TYPE_WIN32_SURFACE_CREATE_INFO_KHR;
     surface_desc.pNext = NULL;
     surface_desc.flags = 0;
@@ -3350,24 +3097,8 @@ static HRESULT d3d12_swapchain_init(struct d3d12_swapchain *swapchain, IWineDXGI
         }
     }

-    if (FAILED(hr = ID3D12Device_CreateFence(device, 0, 0,
-            &IID_ID3D12Fence, (void **)&swapchain->present_fence)))
-    {
-        WARN("Failed to create present fence, hr %#lx.\n", hr);
-        d3d12_swapchain_destroy(swapchain);
-        return hr;
-    }
-
     IWineDXGIFactory_AddRef(swapchain->factory = factory);

-    if (!(swapchain->worker_thread = CreateThread(NULL, 0, d3d12_swapchain_worker_proc, swapchain, 0, NULL)))
-    {
-        hr = HRESULT_FROM_WIN32(GetLastError());
-        WARN("Failed to create worker thread, hr %#lx.\n", hr);
-        d3d12_swapchain_destroy(swapchain);
-        return hr;
-    }
-
     return S_OK;
 }

[PATCH 2/2] dxgi: Call vkd3d_present_vk_swapchain() to present D3D12 swapchains.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=146300

Your paranoid android.

=== debian11 (build log) ===

error: patch failed: dlls/dxgi/swapchain.c:1757
Task: Patch failed to apply

=== debian11b (build log) ===

error: patch failed: dlls/dxgi/swapchain.c:1757
Task: Patch failed to apply

[Giovanni Mascellani (＠giomasce)]

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.

[Conor McCarthy (＠cmccarthy)]

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.

[PATCH 0/2] MR5851: Draft: dxgi: Handle D3D12 swapchain Present() commands by calling a dedicated present function in vkd3d. - closed

[Conor McCarthy (＠cmccarthy)]

This merge request was closed by Conor McCarthy.