[PATCH v74 0/6] MR10567: wined3d: fix SRGB RTV fail for UNORM swapchain backbuffers > 1 for d3d10/11
wined3d: fix vk swapchain rendering too dark by supporting UNORM to SRGB conversion for brightness similar to gl This merge request addresses: https://bugs.winehq.org/show_bug.cgi?id=45364 Frostpunk is very dark Very dark rendering issues for Frostpunk seem to match the same problem for Vulkan renderer for "Against the Storm": - https://bugs.winehq.org/show_bug.cgi?id=58632 - https://bugs.winehq.org/attachment.cgi?id=80656 Running both games with gl renderer seems to fix excessive darkness: - https://bugs.winehq.org/attachment.cgi?id=80654 - https://bugs.winehq.org/attachment.cgi?id=80690 Vulkan renderer seems to be missing SRGB conversion handling availiable for GL renderer. GL renderer can be forced to render darker the same as Vulkan by bypassing EXT_TEXTURE_SRGB support for ./dlls/wined3d/utils.c: ``` @@ -3133,7 +3146,7 @@ static BOOL init_format_texture_info(struct wined3d_adapter *adapter, struct win continue; copy_format(adapter, &srgb_format->f, &format->f); +#if 000 if (gl_info->supported[EXT_TEXTURE_SRGB] && !(adapter->d3d_info.wined3d_creation_flags & WINED3D_SRGB_READ_WRITE_CONTROL)) { @@ -3142,6 +3155,7 @@ static BOOL init_format_texture_info(struct wined3d_adapter *adapter, struct win format_set_caps(&srgb_format->f, WINED3D_FORMAT_CAP_SRGB_READ | WINED3D_FORMAT_CAP_SRGB_WRITE); query_internal_format(adapter, srgb_format, &format_texture_info[i], gl_info, TRUE); } +#endif } ``` This merge request patch allows Vulkan renderer to perform SRGB conversion for wined3d_swapchain_vk_select_vk_format() to mimic the GL capability. Changing VkSwapchainCreateInfoKHR imageColorSpace from VK_COLOR_SPACE_SRGB_NONLINEAR_KHR for `wined3d_swapchain_vk_create_vulkan_swapchain()` had no effect despite enabling VK_EXT_swapchain_colorspace for dlls/win32u/vulkan.c `convert_instance_create_info():` https://docs.vulkan.org/refpages/latest/refpages/source/VkColorSpaceKHR.html Therefore converting UNORM to SRGB was chosen similar to how the GL renderer seems to handle it: ``` static const struct wined3d_format_srgb_info format_srgb_info[] = { {WINED3DFMT_R8G8B8A8_UNORM_SRGB, WINED3DFMT_R8G8B8A8_UNORM}, {WINED3DFMT_BC1_UNORM_SRGB, WINED3DFMT_BC1_UNORM}, {WINED3DFMT_BC2_UNORM_SRGB, WINED3DFMT_BC2_UNORM}, {WINED3DFMT_BC3_UNORM_SRGB, WINED3DFMT_BC3_UNORM}, {WINED3DFMT_B8G8R8A8_UNORM_SRGB, WINED3DFMT_B8G8R8A8_UNORM}, {WINED3DFMT_B8G8R8X8_UNORM_SRGB, WINED3DFMT_B8G8R8X8_UNORM}, {WINED3DFMT_BC7_UNORM_SRGB, WINED3DFMT_BC7_UNORM}, }; ``` Its unclear whether SRGB conversion should always happen or only for particular games so the patch leaves the Vulkan renderer without SRGB conversion by default. Therefore the patch adds a new WINE_D3D_CONFIG flag named "vk_swap_srgb" to force the Vulkan renderer to perform SRGB conversion for the swapchain. By default vk_swap_srgb is set to FALSE or 0: `WINE_D3D_CONFIG=vk_swap_srgb=0,renderer=vulkan` To force vk_swap_srgb to TRUE or 1: `WINE_D3D_CONFIG=vk_swap_srgb=1,renderer=vulkan` For example, launching Frostpunk via the following after the patch fixes the dark rendering for Vulkan and appears brighter similar to the GL renderer: `WINEDEBUG=-all mangohud --dlsym WINE_D3D_CONFIG=csmt=0x1,renderer=vulkan,vk_swap_srgb=1 WINEPREFIX=/home/any/wine_stianlow_wow64_new_pfx ~/tmp/wine_stianlow_wow64_new_install/bin/wine /home/any/wine_stianlow_wow64_new_pfx/drive_c/GOG\ Games/Frostpunk/Frostpunk.exe` Some SRGB handling was added several years ago but not for mapping UNORM to SRGB that this patch adds: ``` 6f55c8d1c56 * wined3d: Use an sRGB fallback format for sRGB formats in wined3d_swapchain_vk_select_vk_format(). Author: Henri Verbeet <hverbeet@codeweavers.com> CommitDate: Wed Dec 9 16:29:28 2020 +0100 ``` If UNORM to SRGB should be enabled by default then this patch may be changed so that vk_swap_srgb defaults to TRUE. -- v74: wined3d: rotate wined3d_swapchain_rendertarget_view (MR-10567) wined3d: gl fix srgb texture2d_blt_fbo (MR-10567) wined3d: gl impl wined3d_swapchain_rendertarget_view (MR-10567) wined3d: vk impl wined3d_swapchain_rendertarget_view (MR-10567) wined3d: add wined3d_swapchain_rendertarget_view lists (MR-10567) wined3d/tests: todo srgb rtvs, unorm swapchain backbuf > 1 (MR-10567) https://gitlab.winehq.org/wine/wine/-/merge_requests/10567
From: Stian Low <wineryyyyy@gmail.com> --- dlls/d3d10core/tests/d3d10core.c | 30 +++++++++++++++++++++++++++++- dlls/d3d11/tests/d3d11.c | 31 ++++++++++++++++++++++++++++++- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/dlls/d3d10core/tests/d3d10core.c b/dlls/d3d10core/tests/d3d10core.c index a949b14837e..8c9d8c647f4 100644 --- a/dlls/d3d10core/tests/d3d10core.c +++ b/dlls/d3d10core/tests/d3d10core.c @@ -11061,9 +11061,10 @@ static void test_swapchain_views(void) static void test_swapchain_flip(void) { + ID3D10RenderTargetView *backbuffer_0_rtv, *backbuffer_0_rtv_srgb, *backbuffer_1_rtv_srgb, *offscreen_rtv; ID3D10Texture2D *backbuffer_0, *backbuffer_1, *backbuffer_2, *offscreen; ID3D10ShaderResourceView *backbuffer_0_srv, *backbuffer_1_srv; - ID3D10RenderTargetView *backbuffer_0_rtv, *offscreen_rtv; + D3D10_RENDER_TARGET_VIEW_DESC rtv_desc; unsigned int color, stride, offset; D3D10_TEXTURE2D_DESC texture_desc; ID3D10InputLayout *input_layout; @@ -11139,6 +11140,7 @@ static void test_swapchain_flip(void) static const float red[] = {1.0f, 0.0f, 0.0f, 0.5f}; static const float green[] = {0.0f, 1.0f, 0.0f, 0.5f}; static const float blue[] = {0.0f, 0.0f, 1.0f, 0.5f}; + static const float grey[] = {0.5, 0.5f, 0.5f, 0.5f}; struct swapchain_desc desc; if (!(device = create_device())) @@ -11227,6 +11229,19 @@ static void test_swapchain_flip(void) ok(SUCCEEDED(hr), "Failed to create pixel shader, hr %#lx.\n", hr); ID3D10Device_PSSetShader(device, ps); + /* Test SRGB rtv for UNORM swapchain common for Unity games for Win10 */ + rtv_desc.Format = DXGI_FORMAT_R8G8B8A8_UNORM_SRGB; + rtv_desc.ViewDimension = D3D10_RTV_DIMENSION_TEXTURE2D; + rtv_desc.Texture2D.MipSlice = 0; + hr = ID3D10Device_CreateRenderTargetView(device, (ID3D10Resource *)backbuffer_0, &rtv_desc, &backbuffer_0_rtv_srgb); + ok(hr == S_OK || broken(hr == E_INVALIDARG), "Failed to create render target view, hr %#lx.\n", hr); + if (SUCCEEDED(hr)) + { + ID3D10Device_ClearRenderTargetView(device, backbuffer_0_rtv_srgb, grey); + color = get_texture_color(backbuffer_0, 320, 240); /* grey */ + todo_wine ok(compare_color(color, 0x80bcbcbc, 1), "Got unexpected srgb color 0x%08x.\n", color); + } + ID3D10Device_ClearRenderTargetView(device, backbuffer_0_rtv, red); ID3D10Device_Draw(device, 4, 0); @@ -11244,6 +11259,15 @@ static void test_swapchain_flip(void) * for more than one buffer. */ IDXGISwapChain_Present(swapchain, 0, 0); + hr = ID3D10Device_CreateRenderTargetView(device, (ID3D10Resource *)backbuffer_0, &rtv_desc, &backbuffer_1_rtv_srgb); + ok(hr == S_OK || broken(hr == E_INVALIDARG), "Failed to create render target view, hr %#lx.\n", hr); + if (SUCCEEDED(hr)) + { + ID3D10Device_ClearRenderTargetView(device, backbuffer_1_rtv_srgb, grey); + color = get_texture_color(backbuffer_0, 320, 240); /* grey */ + todo_wine ok(compare_color(color, 0x80bcbcbc, 1), "Got unexpected srgb color 0x%08x.\n", color); + } + ID3D10Device_ClearRenderTargetView(device, backbuffer_0_rtv, green); ID3D10Device_Draw(device, 4, 0); @@ -11281,6 +11305,10 @@ static void test_swapchain_flip(void) ID3D10ShaderResourceView_Release(backbuffer_1_srv); ID3D10RenderTargetView_Release(backbuffer_0_rtv); ID3D10RenderTargetView_Release(offscreen_rtv); + if (backbuffer_0_rtv_srgb) + ID3D10RenderTargetView_Release(backbuffer_0_rtv_srgb); + if (backbuffer_1_rtv_srgb) + ID3D10RenderTargetView_Release(backbuffer_1_rtv_srgb); ID3D10Texture2D_Release(offscreen); ID3D10Texture2D_Release(backbuffer_0); ID3D10Texture2D_Release(backbuffer_1); diff --git a/dlls/d3d11/tests/d3d11.c b/dlls/d3d11/tests/d3d11.c index bd115a0f957..626cfa961c3 100644 --- a/dlls/d3d11/tests/d3d11.c +++ b/dlls/d3d11/tests/d3d11.c @@ -16013,9 +16013,10 @@ static void test_swapchain_views(void) static void test_swapchain_flip(void) { + ID3D11RenderTargetView *backbuffer_0_rtv, *backbuffer_0_rtv_srgb, *backbuffer_1_rtv_srgb, *offscreen_rtv; ID3D11Texture2D *backbuffer_0, *backbuffer_1, *backbuffer_2, *offscreen; ID3D11ShaderResourceView *backbuffer_0_srv, *backbuffer_1_srv; - ID3D11RenderTargetView *backbuffer_0_rtv, *offscreen_rtv; + D3D11_RENDER_TARGET_VIEW_DESC rtv_desc; D3D11_TEXTURE2D_DESC texture_desc; ID3D11InputLayout *input_layout; ID3D11DeviceContext *context; @@ -16094,6 +16095,7 @@ static void test_swapchain_flip(void) static const float red[] = {1.0f, 0.0f, 0.0f, 0.5f}; static const float green[] = {0.0f, 1.0f, 0.0f, 0.5f}; static const float blue[] = {0.0f, 0.0f, 1.0f, 0.5f}; + static const float grey[] = {0.5, 0.5f, 0.5f, 0.5f}; if (!(device = create_device(NULL))) { @@ -16183,6 +16185,19 @@ static void test_swapchain_flip(void) ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); ID3D11DeviceContext_PSSetShader(context, ps, NULL, 0); + /* Test SRGB rtv for UNORM swapchain common for Unity games for Win10 */ + rtv_desc.Format = DXGI_FORMAT_R8G8B8A8_UNORM_SRGB; + rtv_desc.ViewDimension = D3D11_RTV_DIMENSION_TEXTURE2D; + rtv_desc.Texture2D.MipSlice = 0; + hr = ID3D11Device_CreateRenderTargetView(device, (ID3D11Resource *)backbuffer_0, &rtv_desc, &backbuffer_0_rtv_srgb); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + if (SUCCEEDED(hr)) + { + ID3D11DeviceContext_ClearRenderTargetView(context, backbuffer_0_rtv_srgb, grey); + color = get_texture_color(backbuffer_0, 320, 240); /* grey */ + todo_wine ok(compare_color(color, 0x80bcbcbc, 1), "Got unexpected srgb color 0x%08lx.\n", color); + } + ID3D11DeviceContext_ClearRenderTargetView(context, backbuffer_0_rtv, red); ID3D11DeviceContext_Draw(context, 4, 0); @@ -16201,6 +16216,16 @@ static void test_swapchain_flip(void) * any productive use for more than one buffer. */ IDXGISwapChain_Present(swapchain, 0, 0); + /* Check RTVs created after present/rotate are properly handled/offset accordingly */ + hr = ID3D11Device_CreateRenderTargetView(device, (ID3D11Resource *)backbuffer_0, &rtv_desc, &backbuffer_1_rtv_srgb); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + if (SUCCEEDED(hr)) + { + ID3D11DeviceContext_ClearRenderTargetView(context, backbuffer_1_rtv_srgb, grey); + color = get_texture_color(backbuffer_0, 320, 240); /* grey */ + todo_wine ok(compare_color(color, 0x80bcbcbc, 1), "Got unexpected srgb color 0x%08lx.\n", color); + } + ID3D11DeviceContext_ClearRenderTargetView(context, backbuffer_0_rtv, green); ID3D11DeviceContext_Draw(context, 4, 0); @@ -16238,6 +16263,10 @@ static void test_swapchain_flip(void) ID3D11ShaderResourceView_Release(backbuffer_1_srv); ID3D11RenderTargetView_Release(backbuffer_0_rtv); ID3D11RenderTargetView_Release(offscreen_rtv); + if (backbuffer_0_rtv_srgb) + ID3D11RenderTargetView_Release(backbuffer_0_rtv_srgb); + if (backbuffer_1_rtv_srgb) + ID3D11RenderTargetView_Release(backbuffer_1_rtv_srgb); ID3D11Texture2D_Release(offscreen); ID3D11Texture2D_Release(backbuffer_0); ID3D11Texture2D_Release(backbuffer_1); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10567
From: Stian Low <wineryyyyy@gmail.com> --- dlls/wined3d/swapchain.c | 2 ++ dlls/wined3d/view.c | 18 +++++++++++++++++- dlls/wined3d/wined3d_private.h | 2 ++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/dlls/wined3d/swapchain.c b/dlls/wined3d/swapchain.c index 66092195233..2394f8d91ff 100644 --- a/dlls/wined3d/swapchain.c +++ b/dlls/wined3d/swapchain.c @@ -1813,6 +1813,7 @@ HRESULT wined3d_swapchain_gl_init(struct wined3d_swapchain_gl *swapchain_gl, str TRACE("swapchain_gl %p, device %p, desc %p, state_parent %p, parent %p, parent_ops %p.\n", swapchain_gl, device, desc, state_parent, parent, parent_ops); + list_init(&swapchain_gl->s.back_buffer_rendertarget_views); return wined3d_swapchain_init(&swapchain_gl->s, device, desc, state_parent, parent, parent_ops, &swapchain_gl_ops); } @@ -1826,6 +1827,7 @@ HRESULT wined3d_swapchain_vk_init(struct wined3d_swapchain_vk *swapchain_vk, str TRACE("swapchain_vk %p, device %p, desc %p, parent %p, parent_ops %p.\n", swapchain_vk, device, desc, parent, parent_ops); + list_init(&swapchain_vk->s.back_buffer_rendertarget_views); if (FAILED(hr = wined3d_swapchain_init(&swapchain_vk->s, device, desc, state_parent, parent, parent_ops, &swapchain_vk_ops))) return hr; diff --git a/dlls/wined3d/view.c b/dlls/wined3d/view.c index 8f9e60daf99..dd526faf973 100644 --- a/dlls/wined3d/view.c +++ b/dlls/wined3d/view.c @@ -421,6 +421,20 @@ ULONG CDECL wined3d_rendertarget_view_decref(struct wined3d_rendertarget_view *v /* Release the resource after destroying the view. * See wined3d_shader_resource_view_decref(). */ wined3d_mutex_lock(); + + if (resource->type == WINED3D_RTYPE_TEXTURE_2D && texture_from_resource(resource)->swapchain) + { + struct wined3d_rendertarget_view *swap_view; + LIST_FOR_EACH_ENTRY(swap_view, &texture_from_resource(resource)->swapchain->back_buffer_rendertarget_views, struct wined3d_rendertarget_view, entry) + { + if (swap_view != view) + continue; + + list_remove(&swap_view->entry); + break; + } + } + resource->device->adapter->adapter_ops->adapter_destroy_rendertarget_view(view); wined3d_mutex_unlock(); wined3d_resource_decref(resource); @@ -644,9 +658,11 @@ static HRESULT wined3d_rendertarget_view_init(struct wined3d_rendertarget_view * if (resource->type != WINED3D_RTYPE_BUFFER) { struct wined3d_texture *texture = texture_from_resource(resource); - if (texture->swapchain) + { allow_srgb_toggle = TRUE; + list_add_head(&texture->swapchain->back_buffer_rendertarget_views, &view->entry); + } } if (!(view->format = validate_resource_view(desc, resource, TRUE, allow_srgb_toggle))) return E_INVALIDARG; diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index 9537670f7c5..a11ca119734 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -3978,6 +3978,7 @@ struct wined3d_rendertarget_view unsigned int height; struct wined3d_view_desc desc; + struct list entry; }; void wined3d_rendertarget_view_cleanup(struct wined3d_rendertarget_view *view); @@ -4103,6 +4104,7 @@ struct wined3d_swapchain struct wined3d_texture **back_buffers; struct wined3d_texture *front_buffer; + struct list back_buffer_rendertarget_views; struct wined3d_gamma_ramp orig_gamma; bool reapply_mode; const struct wined3d_format *ds_format; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10567
From: Stian Low <wineryyyyy@gmail.com> --- dlls/wined3d/adapter_vk.c | 21 +++++++++++++++++++-- dlls/wined3d/view.c | 38 ++++++++++++++++++++++++++++---------- dlls/wined3d/wined3d_vk.h | 6 +++--- 3 files changed, 50 insertions(+), 15 deletions(-) diff --git a/dlls/wined3d/adapter_vk.c b/dlls/wined3d/adapter_vk.c index 7f68edb3651..e2a23374b4c 100644 --- a/dlls/wined3d/adapter_vk.c +++ b/dlls/wined3d/adapter_vk.c @@ -1393,12 +1393,17 @@ static HRESULT adapter_vk_create_rendertarget_view(const struct wined3d_view_des struct wined3d_rendertarget_view **view) { struct wined3d_rendertarget_view_vk *view_vk; + unsigned int count = 1; HRESULT hr; TRACE("desc %s, resource %p, parent %p, parent_ops %p, view %p.\n", wined3d_debug_view_desc(desc, resource), resource, parent, parent_ops, view); - if (!(view_vk = calloc(1, sizeof(*view_vk)))) + + if (resource->type == WINED3D_RTYPE_TEXTURE_2D && texture_from_resource(resource)->swapchain) + count = texture_from_resource(resource)->swapchain->state.desc.backbuffer_count; + + if (!(view_vk = calloc(1, offsetof(struct wined3d_rendertarget_view_vk, vk_image_view[count])))) return E_OUTOFMEMORY; if (FAILED(hr = wined3d_rendertarget_view_vk_init(view_vk, desc, resource, parent, parent_ops))) @@ -1522,7 +1527,19 @@ static void adapter_vk_destroy_rendertarget_view(struct wined3d_rendertarget_vie TRACE("view_vk %p.\n", view_vk); wined3d_rendertarget_view_cleanup(&view_vk->v); - wined3d_view_vk_destroy(resource->device, NULL, &view_vk->vk_image_view, + + + if (resource->type == WINED3D_RTYPE_TEXTURE_2D && texture_from_resource(resource)->swapchain) + { + unsigned int i; + for (i = 1; i < texture_from_resource(resource)->swapchain->state.desc.backbuffer_count; i++) + { + wined3d_view_vk_destroy(resource->device, NULL, &view_vk->vk_image_view[i], + NULL, NULL, NULL, &view_vk->command_buffer_id, NULL); + } + } + + wined3d_view_vk_destroy(resource->device, NULL, &view_vk->vk_image_view[0], NULL, NULL, NULL, &view_vk->command_buffer_id, view_vk); } diff --git a/dlls/wined3d/view.c b/dlls/wined3d/view.c index dd526faf973..968fff724be 100644 --- a/dlls/wined3d/view.c +++ b/dlls/wined3d/view.c @@ -916,10 +916,12 @@ static void wined3d_render_target_view_vk_cs_init(void *object) struct wined3d_view_desc *desc = &view_vk->v.desc; const struct wined3d_format_vk *format_vk; struct wined3d_texture_vk *texture_vk; + struct wined3d_swapchain *swapchain; struct wined3d_resource *resource; struct wined3d_context *context; VkImageUsageFlags vk_usage = 0; uint32_t default_flags = 0; + unsigned int i; TRACE("view_vk %p.\n", view_vk); @@ -946,26 +948,42 @@ static void wined3d_render_target_view_vk_cs_init(void *object) return; } - if (texture_vk->t.swapchain && texture_vk->t.swapchain->state.desc.backbuffer_count > 1) - { - FIXME("Swapchain views not supported.\n"); - return; - } - if (resource->bind_flags & WINED3D_BIND_RENDER_TARGET) vk_usage |= VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT; if (resource->bind_flags & WINED3D_BIND_DEPTH_STENCIL) vk_usage |= VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT; context = context_acquire(resource->device, NULL, 0); - view_vk->vk_image_view = wined3d_view_vk_create_vk_image_view(wined3d_context_vk(context), + view_vk->vk_image_view[0] = wined3d_view_vk_create_vk_image_view(wined3d_context_vk(context), desc, texture_vk, format_vk, COLOR_FIXUP_IDENTITY, true, vk_usage); - context_release(context); - if (!view_vk->vk_image_view) + if (!view_vk->vk_image_view[0]) + { + context_release(context); + ERR("Failed to create render target view for resource %p\n", resource); + return; + } + + if (!(swapchain = texture_vk->t.swapchain)) + { + context_release(context); return; + } + + for (i = 1; i < swapchain->state.desc.backbuffer_count; i++) + { + struct wined3d_texture_vk *tex_vk = wined3d_texture_vk(swapchain->back_buffers[i]); + if ((view_vk->vk_image_view[i] = wined3d_view_vk_create_vk_image_view(wined3d_context_vk(context), + desc, tex_vk, format_vk, COLOR_FIXUP_IDENTITY, true, vk_usage))) + continue; + + ERR("Failed to create render target view for swapchain %p backbuffer[%u]\n", swapchain, i); + break; + } + + context_release(context); - TRACE("Created image view 0x%s.\n", wine_dbgstr_longlong(view_vk->vk_image_view)); + TRACE("Created image view 0x%s.\n", wine_dbgstr_longlong(view_vk->vk_image_view[0])); } HRESULT wined3d_rendertarget_view_vk_init(struct wined3d_rendertarget_view_vk *view_vk, diff --git a/dlls/wined3d/wined3d_vk.h b/dlls/wined3d/wined3d_vk.h index 937a958760c..15015fda036 100644 --- a/dlls/wined3d/wined3d_vk.h +++ b/dlls/wined3d/wined3d_vk.h @@ -1035,8 +1035,8 @@ struct wined3d_rendertarget_view_vk { struct wined3d_rendertarget_view v; - VkImageView vk_image_view; uint64_t command_buffer_id; + VkImageView vk_image_view[1]; }; static inline struct wined3d_rendertarget_view_vk *wined3d_rendertarget_view_vk( @@ -1056,8 +1056,8 @@ static inline VkImageView wined3d_rendertarget_view_vk_get_image_view(struct win { struct wined3d_texture_vk *texture_vk; - if (rtv_vk->vk_image_view) - return rtv_vk->vk_image_view; + if (rtv_vk->vk_image_view[0]) + return rtv_vk->vk_image_view[0]; texture_vk = wined3d_texture_vk(wined3d_texture_from_resource(rtv_vk->v.resource)); return wined3d_texture_vk_get_default_image_info(texture_vk, context_vk)->imageView; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10567
From: Stian Low <wineryyyyy@gmail.com> --- dlls/wined3d/adapter_gl.c | 33 ++++++++++++------- dlls/wined3d/context_gl.c | 8 ++--- dlls/wined3d/view.c | 68 +++++++++++++++++++++++++-------------- dlls/wined3d/wined3d_gl.h | 2 +- 4 files changed, 70 insertions(+), 41 deletions(-) diff --git a/dlls/wined3d/adapter_gl.c b/dlls/wined3d/adapter_gl.c index 085c6ac48bb..60ce15e47b1 100644 --- a/dlls/wined3d/adapter_gl.c +++ b/dlls/wined3d/adapter_gl.c @@ -4564,12 +4564,16 @@ static HRESULT adapter_gl_create_rendertarget_view(const struct wined3d_view_des struct wined3d_rendertarget_view **view) { struct wined3d_rendertarget_view_gl *view_gl; + unsigned int count = 1; HRESULT hr; TRACE("desc %s, resource %p, parent %p, parent_ops %p, view %p.\n", wined3d_debug_view_desc(desc, resource), resource, parent, parent_ops, view); - if (!(view_gl = calloc(1, sizeof(*view_gl)))) + if (resource->type == WINED3D_RTYPE_TEXTURE_2D && texture_from_resource(resource)->swapchain) + count = texture_from_resource(resource)->swapchain->state.desc.backbuffer_count;; + + if (!(view_gl = calloc(1, offsetof(struct wined3d_rendertarget_view_gl, gl_view[count])))) return E_OUTOFMEMORY; if (FAILED(hr = wined3d_rendertarget_view_gl_init(view_gl, desc, resource, parent, parent_ops))) @@ -4588,7 +4592,7 @@ static HRESULT adapter_gl_create_rendertarget_view(const struct wined3d_view_des struct wined3d_view_gl_destroy_ctx { struct wined3d_device *device; - const struct wined3d_gl_view *gl_view; + GLuint name; struct wined3d_bo_user *bo_user; struct wined3d_bo_gl *counter_bo; void *object; @@ -4608,14 +4612,14 @@ static void wined3d_view_gl_destroy_object(void *object) device = ctx->device; counter_id = ctx->counter_bo ? ctx->counter_bo->id : 0; - if (ctx->gl_view->name || counter_id) + if (ctx->name || counter_id) { context = context_acquire(device, NULL, 0); gl_info = wined3d_context_gl(context)->gl_info; - if (ctx->gl_view->name) + if (ctx->name) { - context_gl_resource_released(device, ctx->gl_view->name, FALSE); - gl_info->gl_ops.gl.p_glDeleteTextures(1, &ctx->gl_view->name); + context_gl_resource_released(device, ctx->name, FALSE); + gl_info->gl_ops.gl.p_glDeleteTextures(1, &ctx->name); } if (counter_id) wined3d_context_gl_destroy_bo(wined3d_context_gl(context), ctx->counter_bo); @@ -4629,7 +4633,7 @@ static void wined3d_view_gl_destroy_object(void *object) free(ctx->free); } -static void wined3d_view_gl_destroy(struct wined3d_device *device, const struct wined3d_gl_view *gl_view, +static void wined3d_view_gl_destroy(struct wined3d_device *device, GLuint name, struct wined3d_bo_user *bo_user, struct wined3d_bo_gl *counter_bo, void *object) { struct wined3d_view_gl_destroy_ctx *ctx, c; @@ -4637,7 +4641,7 @@ static void wined3d_view_gl_destroy(struct wined3d_device *device, const struct if (!(ctx = malloc(sizeof(*ctx)))) ctx = &c; ctx->device = device; - ctx->gl_view = gl_view; + ctx->name = name; ctx->bo_user = bo_user; ctx->counter_bo = counter_bo; ctx->object = object; @@ -4656,7 +4660,14 @@ static void adapter_gl_destroy_rendertarget_view(struct wined3d_rendertarget_vie TRACE("view_gl %p.\n", view_gl); wined3d_rendertarget_view_cleanup(&view_gl->v); - wined3d_view_gl_destroy(resource->device, &view_gl->gl_view, NULL, NULL, view_gl); + if (resource->type == WINED3D_RTYPE_TEXTURE_2D && texture_from_resource(resource)->swapchain) + { + unsigned int i; + for (i = 1; i < texture_from_resource(resource)->swapchain->state.desc.backbuffer_count; i++) + wined3d_view_gl_destroy(resource->device, view_gl->gl_view[i].name, NULL, NULL, NULL); + } + + wined3d_view_gl_destroy(resource->device, view_gl->gl_view[0].name, NULL, NULL, view_gl); } static HRESULT adapter_gl_create_shader_resource_view(const struct wined3d_view_desc *desc, @@ -4693,7 +4704,7 @@ static void adapter_gl_destroy_shader_resource_view(struct wined3d_shader_resour TRACE("view_gl %p.\n", view_gl); wined3d_shader_resource_view_cleanup(&view_gl->v); - wined3d_view_gl_destroy(resource->device, &view_gl->gl_view, &view_gl->bo_user, NULL, view_gl); + wined3d_view_gl_destroy(resource->device, view_gl->gl_view.name, &view_gl->bo_user, NULL, view_gl); } static HRESULT adapter_gl_create_unordered_access_view(const struct wined3d_view_desc *desc, @@ -4730,7 +4741,7 @@ static void adapter_gl_destroy_unordered_access_view(struct wined3d_unordered_ac TRACE("view_gl %p.\n", view_gl); wined3d_unordered_access_view_cleanup(&view_gl->v); - wined3d_view_gl_destroy(resource->device, &view_gl->gl_view, &view_gl->bo_user, &view_gl->counter_bo, view_gl); + wined3d_view_gl_destroy(resource->device, view_gl->gl_view.name, &view_gl->bo_user, &view_gl->counter_bo, view_gl); } static HRESULT adapter_gl_create_video_decoder_output_view(const struct wined3d_view_desc *desc, diff --git a/dlls/wined3d/context_gl.c b/dlls/wined3d/context_gl.c index e5b41ffc783..33cf754475d 100644 --- a/dlls/wined3d/context_gl.c +++ b/dlls/wined3d/context_gl.c @@ -3235,7 +3235,7 @@ BOOL wined3d_context_gl_apply_clear_state(struct wined3d_context_gl *context_gl, if (rts[i]) { struct wined3d_rendertarget_view_gl *rtv_gl = wined3d_rendertarget_view_gl(rts[i]); - context_gl->blit_targets[i].gl_view = rtv_gl->gl_view; + context_gl->blit_targets[i].gl_view = rtv_gl->gl_view[0]; context_gl->blit_targets[i].resource = rtv_gl->v.resource; context_gl->blit_targets[i].sub_resource_idx = rtv_gl->v.sub_resource_idx; context_gl->blit_targets[i].layer_count = rtv_gl->v.layer_count; @@ -3247,7 +3247,7 @@ BOOL wined3d_context_gl_apply_clear_state(struct wined3d_context_gl *context_gl, if (dsv) { struct wined3d_rendertarget_view_gl *dsv_gl = wined3d_rendertarget_view_gl(dsv); - ds_info.gl_view = dsv_gl->gl_view; + ds_info.gl_view = dsv_gl->gl_view[0]; ds_info.resource = dsv_gl->v.resource; ds_info.sub_resource_idx = dsv_gl->v.sub_resource_idx; ds_info.layer_count = dsv_gl->v.layer_count; @@ -3400,7 +3400,7 @@ void context_state_fb(struct wined3d_context *context, const struct wined3d_stat continue; view_gl = wined3d_rendertarget_view_gl(fb->render_targets[i]); - context_gl->blit_targets[i].gl_view = view_gl->gl_view; + context_gl->blit_targets[i].gl_view = view_gl->gl_view[0]; context_gl->blit_targets[i].resource = view_gl->v.resource; context_gl->blit_targets[i].sub_resource_idx = view_gl->v.sub_resource_idx; context_gl->blit_targets[i].layer_count = view_gl->v.layer_count; @@ -3412,7 +3412,7 @@ void context_state_fb(struct wined3d_context *context, const struct wined3d_stat if (fb->depth_stencil) { view_gl = wined3d_rendertarget_view_gl(fb->depth_stencil); - ds_info.gl_view = view_gl->gl_view; + ds_info.gl_view = view_gl->gl_view[0]; ds_info.resource = view_gl->v.resource; ds_info.sub_resource_idx = view_gl->v.sub_resource_idx; ds_info.layer_count = view_gl->v.layer_count; diff --git a/dlls/wined3d/view.c b/dlls/wined3d/view.c index 968fff724be..ed91148e7ae 100644 --- a/dlls/wined3d/view.c +++ b/dlls/wined3d/view.c @@ -604,44 +604,62 @@ static void wined3d_render_target_view_gl_cs_init(void *object) struct wined3d_rendertarget_view_gl *view_gl = object; struct wined3d_resource *resource = view_gl->v.resource; const struct wined3d_view_desc *desc = &view_gl->v.desc; + struct wined3d_texture_gl *texture_gl; + struct wined3d_swapchain *swapchain; + unsigned int depth_or_layer_count; + GLenum resource_class, view_class; + unsigned int i; TRACE("view_gl %p.\n", view_gl); if (resource->type == WINED3D_RTYPE_BUFFER) { FIXME("Not implemented for resources %s.\n", debug_d3dresourcetype(resource->type)); + return; } + + texture_gl = wined3d_texture_gl(texture_from_resource(resource)); + if (resource->type == WINED3D_RTYPE_TEXTURE_3D) + depth_or_layer_count = wined3d_texture_get_level_depth(&texture_gl->t, desc->u.texture.level_idx); else + depth_or_layer_count = texture_gl->t.layer_count; + + if (resource->format->id == view_gl->v.format->id + && (view_gl->v.layer_count == 1 && view_gl->v.layer_count == depth_or_layer_count)) { - struct wined3d_texture_gl *texture_gl = wined3d_texture_gl(texture_from_resource(resource)); - unsigned int depth_or_layer_count; + TRACE("Skipping view init because format and layer_count matches underlying texture\n"); + return; + } - if (resource->type == WINED3D_RTYPE_TEXTURE_3D) - depth_or_layer_count = wined3d_texture_get_level_depth(&texture_gl->t, desc->u.texture.level_idx); - else - depth_or_layer_count = texture_gl->t.layer_count; + resource_class = wined3d_format_gl(resource->format)->view_class; + view_class = wined3d_format_gl(view_gl->v.format)->view_class; + if (resource_class != view_class) + { + FIXME("Render target view not supported, resource format %s, view format %s.\n", + debug_d3dformat(resource->format->id), debug_d3dformat(view_gl->v.format->id)); + return; + } - if (resource->format->id != view_gl->v.format->id - || (view_gl->v.layer_count != 1 && view_gl->v.layer_count != depth_or_layer_count)) - { - GLenum resource_class, view_class; + create_texture_view(&view_gl->gl_view[0], texture_gl->target, desc, texture_gl, view_gl->v.format); - resource_class = wined3d_format_gl(resource->format)->view_class; - view_class = wined3d_format_gl(view_gl->v.format)->view_class; - if (resource_class != view_class) - { - FIXME("Render target view not supported, resource format %s, view format %s.\n", - debug_d3dformat(resource->format->id), debug_d3dformat(view_gl->v.format->id)); - return; - } - if (texture_gl->t.swapchain && texture_gl->t.swapchain->state.desc.backbuffer_count > 1) - { - FIXME("Swapchain views not supported.\n"); - return; - } + if (!view_gl->gl_view[0].name) + { + FIXME("Failed to create render target view for resource %p\n", resource); + return; + } - create_texture_view(&view_gl->gl_view, texture_gl->target, desc, texture_gl, view_gl->v.format); - } + if (!(swapchain = texture_gl->t.swapchain)) + return; + + for (i = 1; i < swapchain->state.desc.backbuffer_count; i++) + { + struct wined3d_texture_gl *tex_gl = wined3d_texture_gl(swapchain->back_buffers[i]); + create_texture_view(&view_gl->gl_view[i], texture_gl->target, desc, tex_gl, view_gl->v.format); + if (view_gl->gl_view[i].name) + continue; + + ERR("Failed to create render target view for swapchain %p backbuffer[%u]\n", swapchain, i); + break; } } diff --git a/dlls/wined3d/wined3d_gl.h b/dlls/wined3d/wined3d_gl.h index c9c7118c59e..d4cf258d363 100644 --- a/dlls/wined3d/wined3d_gl.h +++ b/dlls/wined3d/wined3d_gl.h @@ -1081,7 +1081,7 @@ HRESULT wined3d_buffer_gl_init(struct wined3d_buffer_gl *buffer_gl, struct wined struct wined3d_rendertarget_view_gl { struct wined3d_rendertarget_view v; - struct wined3d_gl_view gl_view; + struct wined3d_gl_view gl_view[1]; }; static inline struct wined3d_rendertarget_view_gl *wined3d_rendertarget_view_gl( -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10567
From: Stian Low <wineryyyyy@gmail.com> --- dlls/wined3d/texture_gl.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dlls/wined3d/texture_gl.c b/dlls/wined3d/texture_gl.c index 311ee954c23..90e5d923dc9 100644 --- a/dlls/wined3d/texture_gl.c +++ b/dlls/wined3d/texture_gl.c @@ -913,6 +913,12 @@ static void texture2d_blt_fbo(struct wined3d_device *device, struct wined3d_cont gl_info->gl_ops.gl.p_glDisable(GL_SCISSOR_TEST); context_invalidate_state(context, STATE_RASTERIZER); + if (gl_info->supported[ARB_FRAMEBUFFER_SRGB] && (dst_location == WINED3D_LOCATION_DRAWABLE || src_location == WINED3D_LOCATION_DRAWABLE)) + { + gl_info->gl_ops.gl.p_glDisable(GL_FRAMEBUFFER_SRGB); + context_invalidate_state(context, STATE_SHADER(WINED3D_SHADER_TYPE_PIXEL)); + } + gl_info->fbo_ops.glBlitFramebuffer(src_rect->left, src_rect->top, src_rect->right, src_rect->bottom, dst_rect->left, dst_rect->top, dst_rect->right, dst_rect->bottom, GL_COLOR_BUFFER_BIT, gl_filter); checkGLcall("glBlitFramebuffer()"); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10567
From: Stian Low <wineryyyyy@gmail.com> --- dlls/d3d10core/tests/d3d10core.c | 4 +-- dlls/d3d11/tests/d3d11.c | 4 +-- dlls/wined3d/swapchain.c | 60 ++++++++++++++++++++++++++++++-- 3 files changed, 62 insertions(+), 6 deletions(-) diff --git a/dlls/d3d10core/tests/d3d10core.c b/dlls/d3d10core/tests/d3d10core.c index 8c9d8c647f4..fcf8f4f0509 100644 --- a/dlls/d3d10core/tests/d3d10core.c +++ b/dlls/d3d10core/tests/d3d10core.c @@ -11239,7 +11239,7 @@ static void test_swapchain_flip(void) { ID3D10Device_ClearRenderTargetView(device, backbuffer_0_rtv_srgb, grey); color = get_texture_color(backbuffer_0, 320, 240); /* grey */ - todo_wine ok(compare_color(color, 0x80bcbcbc, 1), "Got unexpected srgb color 0x%08x.\n", color); + ok(compare_color(color, 0x80bcbcbc, 1), "Got unexpected srgb color 0x%08x.\n", color); } ID3D10Device_ClearRenderTargetView(device, backbuffer_0_rtv, red); @@ -11265,7 +11265,7 @@ static void test_swapchain_flip(void) { ID3D10Device_ClearRenderTargetView(device, backbuffer_1_rtv_srgb, grey); color = get_texture_color(backbuffer_0, 320, 240); /* grey */ - todo_wine ok(compare_color(color, 0x80bcbcbc, 1), "Got unexpected srgb color 0x%08x.\n", color); + ok(compare_color(color, 0x80bcbcbc, 1), "Got unexpected srgb color 0x%08x.\n", color); } ID3D10Device_ClearRenderTargetView(device, backbuffer_0_rtv, green); diff --git a/dlls/d3d11/tests/d3d11.c b/dlls/d3d11/tests/d3d11.c index 626cfa961c3..7f60b9abb0f 100644 --- a/dlls/d3d11/tests/d3d11.c +++ b/dlls/d3d11/tests/d3d11.c @@ -16195,7 +16195,7 @@ static void test_swapchain_flip(void) { ID3D11DeviceContext_ClearRenderTargetView(context, backbuffer_0_rtv_srgb, grey); color = get_texture_color(backbuffer_0, 320, 240); /* grey */ - todo_wine ok(compare_color(color, 0x80bcbcbc, 1), "Got unexpected srgb color 0x%08lx.\n", color); + ok(compare_color(color, 0x80bcbcbc, 1), "Got unexpected srgb color 0x%08lx.\n", color); } ID3D11DeviceContext_ClearRenderTargetView(context, backbuffer_0_rtv, red); @@ -16223,7 +16223,7 @@ static void test_swapchain_flip(void) { ID3D11DeviceContext_ClearRenderTargetView(context, backbuffer_1_rtv_srgb, grey); color = get_texture_color(backbuffer_0, 320, 240); /* grey */ - todo_wine ok(compare_color(color, 0x80bcbcbc, 1), "Got unexpected srgb color 0x%08lx.\n", color); + ok(compare_color(color, 0x80bcbcbc, 1), "Got unexpected srgb color 0x%08lx.\n", color); } ID3D11DeviceContext_ClearRenderTargetView(context, backbuffer_0_rtv, green); diff --git a/dlls/wined3d/swapchain.c b/dlls/wined3d/swapchain.c index 2394f8d91ff..2392a86c355 100644 --- a/dlls/wined3d/swapchain.c +++ b/dlls/wined3d/swapchain.c @@ -543,6 +543,34 @@ static void swapchain_gl_set_swap_interval(struct wined3d_swapchain *swapchain, } } +static void wined3d_swapchain_rendertarget_view_gl_rotate(struct wined3d_swapchain *swapchain) +{ + struct wined3d_rendertarget_view_gl *view_gl; + struct wined3d_rendertarget_view *swap_view; + GLuint name, name_prev; + unsigned int i; + + LIST_FOR_EACH_ENTRY(swap_view, &swapchain->back_buffer_rendertarget_views, + struct wined3d_rendertarget_view, entry) + { + view_gl = wined3d_rendertarget_view_gl(swap_view); + if (!view_gl->gl_view[0].name) + { + FIXME("Skipping rotate for view_gl %p with gl view name 0 which should not occur.\n", view_gl); + continue; + } + + name_prev = view_gl->gl_view[0].name; + for (i = 1; i < swapchain->state.desc.backbuffer_count; i++) + { + name = view_gl->gl_view[i].name; + view_gl->gl_view[i].name = name_prev; + name_prev = name; + } + view_gl->gl_view[0].name = name_prev; + } +} + /* Context activation is done by the caller. */ static void wined3d_swapchain_gl_rotate(struct wined3d_swapchain *swapchain, struct wined3d_context *context) { @@ -583,7 +611,7 @@ static void wined3d_swapchain_gl_rotate(struct wined3d_swapchain *swapchain, str texture_prev->texture_rgb = tex0; texture_prev->rb_multisample = rb0; - + wined3d_swapchain_rendertarget_view_gl_rotate(swapchain); wined3d_texture_validate_location(&texture_prev->t, 0, locations0 & supported_locations); wined3d_texture_invalidate_location(&texture_prev->t, 0, ~(locations0 & supported_locations)); @@ -1205,6 +1233,34 @@ static VkResult wined3d_swapchain_vk_blit(struct wined3d_swapchain_vk *swapchain return vr; } +static void wined3d_swapchain_rendertarget_view_vk_rotate(struct wined3d_swapchain *swapchain) +{ + VkImageView vk_image_view, vk_image_view_prev; + struct wined3d_rendertarget_view_vk *view_vk; + struct wined3d_rendertarget_view *swap_view; + unsigned int i; + + LIST_FOR_EACH_ENTRY(swap_view, &swapchain->back_buffer_rendertarget_views, + struct wined3d_rendertarget_view, entry) + { + view_vk = wined3d_rendertarget_view_vk(swap_view); + if (!view_vk->vk_image_view[0]) + { + FIXME("Skipping rotate for view_vk %p with vk_image_view 0 which should not occur.\n", view_vk); + continue; + } + + vk_image_view_prev = view_vk->vk_image_view[0]; + for (i = 1; i < swapchain->state.desc.backbuffer_count; i++) + { + vk_image_view = view_vk->vk_image_view[i]; + view_vk->vk_image_view[i] = vk_image_view_prev; + vk_image_view_prev = vk_image_view; + } + view_vk->vk_image_view[0] = vk_image_view_prev; + } +} + static void wined3d_swapchain_vk_rotate(struct wined3d_swapchain *swapchain, struct wined3d_context_vk *context_vk) { struct wined3d_texture_sub_resource *sub_resource; @@ -1253,7 +1309,7 @@ static void wined3d_swapchain_vk_rotate(struct wined3d_swapchain *swapchain, str texture_prev->layout = vk_layout0; texture_prev->bind_mask = bind_mask0; texture_prev->default_image_info = vk_info0; - + wined3d_swapchain_rendertarget_view_vk_rotate(swapchain); wined3d_texture_validate_location(&texture_prev->t, 0, locations0 & supported_locations); wined3d_texture_invalidate_location(&texture_prev->t, 0, ~(locations0 & supported_locations)); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10567
On Thu Aug 6 15:36:23 2026 +0000, Stian Low wrote:
> > Hi, really sorry for the late review, I promise I wanted to get to it sooner.
> No worries. Thanks for review. It's a heftier merge and your review
> helped minimize further.
> Most of all your suggests have been pushed and I will cleanup commit
> messages soon after I wrap up testing latest push for Last Hope game for
> Unreal Engine:
> - https://bugs.winehq.org/show_bug.cgi?id=59067#c7
> MR-10567 has no effect for Last Hope which uses swap effect 0x2
> (FLIP_DISCARD) and `WINED3DFMT_R10G10B10A2_UNORM = 52` for both
> swapchain and rtv formats so the rotate logic should be skipped.
> ```
> 0264:fixme:d3d:wined3d_swapchain_rendertarget_view_gl_rotate swapchain
> backbuffer_count 2, format 52
> 014c:fixme:d3d:adapter_gl_create_rendertarget_view swap rtv format_id 52
> ```
> However rotate is still called and logs for condition that should never
> happen which will also be fixed for next push:
> `0264:fixme:d3d:wined3d_swapchain_rendertarget_view_gl_rotate Skipping
> rotate for view_gl 00007F63F9314E10 with gl view name 0 which should not occur.`
> Unnecessary rotate calls do not break anything and are practically
> skipped despite the logs but better to not call at all. The game bug
> otherwise seems unrelated to this merge request despite resembling it at
> first glance.
Maybe in the future splitting as separate merge requests each incremental improvement from original trivial patch vs combining all incremental improvements into this single merge request is a better approach in order to avoid what seems to have become an over-bloated mess.
> Hi, really sorry for the late review, I promise I wanted to get to it sooner.
Thanks also for patience and late reply. Most requests were addressed and resolved last week and with latest pushes.
Commit messages will be corrected after finalizing additional game tests that are failing with backtrace for `wined3d_swapchain_rendertarget_view_gl_rotate` because `wined3d_swapchain_resize_buffers` destroys previous underlying rtv textures while rotation happens so maybe some extra safety mechanisms are needed.
Skipping texture destroys at the end of `wined3d_swapchain_resize_buffers` fixes crashes but resize is not properly so applied.
Whatever gap that these game tests expose needs to be filled otherwise tests passing is misleading and will cause regressions.
Also the reason for vulkan tests failing has been determined:
`vk_blitter_clear_rendertargets` delays blitting for `ClearRenderTargetView` which may cause some order of operations discrepancy which leads to srgb correction not being applied properly.
Forcing `attachment_count > 0` for `vk_blitter_clear_rendertargets` forces immediate blitting and allows all vulkan tests to pass for srgb `ClearRenderTargetView`.
> Generally we don't put bug numbers in the code. Certainly I don't think there's a reason to put merge request numbers in the code.
Resolved. Will also remove for commit messages for next push once app tests are complete.
> This needs ok(hr == S_OK). Or does it not actually work on native for some reason?
Resolved.
d3d10core status check copied from `test_swapchain_views()` which seems to be the appropriate way of handling it:
`ok(hr == S_OK || broken(hr == E_INVALIDARG), "Failed to create render target view, hr %#lx.\n", hr);`
d3d11 passes on Windows 11 so `ok(hr == S_OK)` has been added for it as suggested.
> Also, generally we split d3d10core/tests and d3d11/tests into their own commits, and label them accordingly, not using "wined3d/tests".
Resolved. Git logs show some `wined3d/tests` messages but suggest seems much more common.
> 2/6: We don't need a separate wined3d_swapchain_rendertarget_view struct; we can just put the list entry directly in wined3d_rendertarget_view (and it doesn't need to be backend-specific either).
Resolved. I figured a wrapper struct only for swapchain rtvs might be more optimal vs adding extra memory per every rtvs but performance diffs are probably negligible and using `wined3d_rendertarget_view` as list directly may have wider use cases eventually.
> 3/6:
> + context = context_acquire(resource->device, NULL, 0);Better to just not release the context earlier.
> + texture_vk = wined3d_texture_vk(swapchain->back_buffers[i]);
> + if ((view_vk->vk_image_view[i] = wined3d_view_vk_create_vk_image_view(wined3d_context_vk(context),
> + desc, texture_vk, format_vk, COLOR_FIXUP_IDENTITY, true, vk_usage)))
> + continue;
> That's not quite right though; we need to offset the back buffer index by the index of this backbuffer.
Is the concern that views may be initialized after swapchain has already been rotated/offset?
If so then I added a test case that creates `backbuffer_1_rtv` right after initial present/rotate which passes testsfor the existing impl vs handling any extra offsets which seems consistent with Windows 11 test results.
```
+ hr = ID3D10Device_CreateRenderTargetView(device, (ID3D10Resource *)backbuffer_0, &rtv_desc, &backbuffer_1_rtv_srgb);
+ ok(hr == S_OK || broken(hr == E_INVALIDARG), "Failed to create render target view, hr %#lx.\n", hr);
+ if (SUCCEEDED(hr))
+ {
+ ID3D10Device_ClearRenderTargetView(device, backbuffer_1_rtv_srgb, grey);
+ color = get_texture_color(backbuffer_0, 320, 240); /* grey */
+ todo_wine ok(compare_color(color, 0x80bcbcbc, 1), "Got unexpected srgb color 0x%08x.\n", color);
+ }
```
> I would also not assign the existing texture_vk variable as that creates a bit of a footgun.
Resolved. Variable renamed and scoped within loop to reduce any risks of confusion.
> /* TODO: cleanup? */What do we need to clean up here? Note that Vulkan views will get deleted when the view object is destroyed.
Resolved. Comments left for review just in case problematic.
> You're not using these contexts for anything?
Nope. Thanks for catching. Copied from vk impl which uses contexts and accidentally overlooked maybe as wined3d sync related.
> 5/6: This one I need more time on, sorry. I've been staring at the spec all afternoon and it seems a bit hairier than we originally thought.
I'll keep testing more apps/games in the meantime but so far it still seems to be working and passing all tests.
> 6/6: Rotating views works, buuuut, what if we instead just store an index and increment it, and use that indexed view instead of using view 0?
Anything that helps wined3d and d3d12 better align always seems worthwhile to me but original impl kept consistent with existing rotate functions for now.
If indexing is more favorable then perhaps the existing rotate functions should also be changed for consistency between them all and to resemble how d3d12 already uses explicit indexing.
Unless its critical I've left it for a later merge request to handle wined3d and d3d12 alignment more specifically.
> Actually, for that matter, we could store the index in the swapchain, and then we wouldn't actually even have to make lists of views at all? Does this work or am I not thinking this through enough?
Swapchains support arbitrary number of rtv views per non-arbitray number of backbuffers with specific count so the list seems necessary unless I'm missing something.
However the iteration of the backbuffer indicies per rtv view may be handled via indices vs reassigning but whatever difference seems very negligible because only UINTs are reassigned which is consistent with how texture rotation is already handled.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_148492
Error log for vanilla 11.14-a37867ddf84 seems to indicate an existing bug that MR-10567 further exposes via crashes for some real world apps/games: `0024:err:d3d:wined3d_swapchain_resize_buffers Something's still holding back buffer 0 (00007FFFFE8851D0).` Test cases seem to be missing for this bug because they all pass and I've only been able to reproduce it via world world apps/games. Unclear if this existing bug for `wined3d_swapchain_resize_buffers` is better fixed via a separate merge request or as part of this merge which is dependent upon it. I will include the commit that fixes the bug with this merge for now but will separate if preferred. This may be the final bug to resolve before I consider this merge safe enough for others to consider it for merge. Original patch was simpler/safer and did not expose `wined3d_swapchain_resize_buffers` bug so latest crashes are related to latest more complex impl. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_148588
Original patch has been part of staging for 3 months now which makes up the majority of this merge's duration, during which staging users have only reported games fixed and none bugged. Maybe a better approach to this merge would have been to marked the few tests that failed as todo to be resolved later vs combining the entire effort into 1 merge request which again, has become so bloated that gitlab struggles to load the comments anymore. I realize staging probably represents a more narrow user test pool overall but proably a higher proportionality of gamers for real world apps/games most relevant to the original patch so maybe it should have been merged despite a few extra test cases failing which seem much more edge case vs real world cases. Just throwing out suggestions to manage other future bloat problems. Some of the bloat is my fault for being new but most of the bloat seems only avoidable by more effective methods of splitting. If other devs are given more leniency for incremental iterative improvements via separate merge requests then please consider applying it for me as well so that the overall effort is more easily trackable/manageable. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_148592
Also the reason for vulkan tests failing has been determined: `vk_blitter_clear_rendertargets` delays blitting for `ClearRenderTargetView` which may cause some order of operations discrepancy which leads to srgb correction not being applied properly. I'll have to take a closer look how to properly fix it.
Forcing `attachment_count > 0` for `vk_blitter_clear_rendertargets` forces immediate blitting and allows all vulkan tests to pass for srgb `ClearRenderTargetView`.
Right, we need to be rotating locations as well, which is a bit annoying. Forcing backbuffers into TEXTURE_RGB on a present and invalidating !TEXTURE_RGB is probably best. Though forcing an immediate clear for a swapchain texture seems good enough for a stopgap.
d3d10core status check copied from `test_swapchain_views()` which seems to be the appropriate way of handling it: `ok(hr == S_OK || broken(hr == E_INVALIDARG), "Failed to create render target view, hr %#lx.\n", hr);`
Ah, it works on 7 and newer then presumably. Copying the comment would be helpful there. I think a broken() should always be annotated with some sort of explanation, personally.
Resolved. Git logs show some `wined3d/tests` messages but suggest seems much more common.
Hrm, where? I checked `git log --grep` and didn't see any.
Resolved. I figured a wrapper struct only for swapchain rtvs might be more optimal vs adding extra memory per every rtvs but performance diffs are probably negligible and using `wined3d_rendertarget_view` as list directly may have wider use cases eventually.
That's a fair concern. I don't think there'll be enough RTVs for it to really matter, though, yeah.
That's not quite right though; we need to offset the back buffer index by the index of this backbuffer.
Is the concern that views may be initialized after swapchain has already been rotated/offset?
If so then I added a test case that creates `backbuffer_1_rtv` right after initial present/rotate which passes testsfor the existing impl vs handling any extra offsets which seems consistent with Windows 11 test results.
Not per se—as you observe, that already works because the textures are rotated. My comment was about creating a view on anything but the first backbuffer. E.g. the view list for backbuffer 0 goes [0, 1, 2, 3]; the view list for backbuffer 1 goes [1, 2, 3, 0], where each view is the index of texture we want to create a view of.
/* TODO: cleanup? */What do we need to clean up here? Note that Vulkan views will get deleted when the view object is destroyed.
Resolved. Comments left for review just in case problematic.
Since there's nothing to do please go ahead and delete the comment.
5/6: This one I need more time on, sorry. I've been staring at the spec all afternoon and it seems a bit hairier than we originally thought.
I'll keep testing more apps/games in the meantime but so far it still seems to be working and passing all tests.
Thanks for the patience. Also, thanks a lot to Henri for helping me understand this. I believe that this patch is correct as-is, no further review needed.
6/6: Rotating views works, buuuut, what if we instead just store an index and increment it, and use that indexed view instead of using view 0?
Anything that helps wined3d and d3d12 better align always seems worthwhile to me but original impl kept consistent with existing rotate functions for now.
If indexing is more favorable then perhaps the existing rotate functions should also be changed for consistency between them all and to resemble how d3d12 already uses explicit indexing.
Unless its critical I've left it for a later merge request to handle wined3d and d3d12 alignment more specifically.
Sure, we can leave it like this for now.
Actually, for that matter, we could store the index in the swapchain, and then we wouldn't actually even have to make lists of views at all? Does this work or am I not thinking this through enough?
Swapchains support arbitrary number of rtv views per non-arbitray number of backbuffers with specific count so the list seems necessary unless I'm missing something.
Well, with this scheme, any usage of the views would go through the swapchain's index. We don't need to touch the views because we're instead touching the thing that the views use.
Original patch has been part of staging for 3 months now which makes up the majority of this merge's duration, during which staging users have only reported games fixed and none bugged.
Maybe a better approach to this merge would have been to marked the few tests that failed as todo to be resolved later vs combining the entire effort into 1 merge request which again, has become so bloated that gitlab struggles to load the comments anymore.
I realize staging probably represents a more narrow user test pool overall but proably a higher proportionality of gamers for real world apps/games most relevant to the original patch so maybe it should have been merged despite a few extra test cases failing which seem much more edge case vs real world cases.
I understand the thought, and I recognize the value of pushing a hack now and fixing the bug properly later. It's something we avoid, though, not just in d3d but in broader Wine, and it's for the reason that these hacks make things a lot harder for maintainers (and d3d is a hard beast to maintain even under the best of conditions). It's also common for contributors to get their hack committed and then disappear, never actualizing the "fix it later" part; it takes at least an amount of trust to do that. I apologize that it's been open so long; ultimaately there hasn't actually been that many rounds of review, and the delay is mostly my fault. I am sorry for that; I have been dealing with some health issues which certainly make it harder, although I don't like to use that as an excuse. FWIW, it's possible that gitlab simply struggles due to the large number of pushes. I've seen merge requests with more comments load fine. I don't know if you're pushing for the sake of CI, but you can run CI by pushing to a local fork, which also avoids generating lots of notifications for reviewers. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_148680
On Wed Aug 12 04:16:09 2026 +0000, Elizabeth Figura wrote:
Also the reason for vulkan tests failing has been determined: `vk_blitter_clear_rendertargets` delays blitting for `ClearRenderTargetView` which may cause some order of operations discrepancy which leads to srgb correction not being applied properly. I'll have to take a closer look how to properly fix it.
Forcing `attachment_count > 0` for `vk_blitter_clear_rendertargets` forces immediate blitting and allows all vulkan tests to pass for srgb `ClearRenderTargetView`. Right, we need to be rotating locations as well, which is a bit annoying. Forcing backbuffers into TEXTURE_RGB on a present and invalidating !TEXTURE_RGB is probably best. Though forcing an immediate clear for a swapchain texture seems good enough for a stopgap. d3d10core status check copied from `test_swapchain_views()` which seems to be the appropriate way of handling it: `ok(hr == S_OK || broken(hr == E_INVALIDARG), "Failed to create render target view, hr %#lx.\n", hr);` Ah, it works on 7 and newer then presumably. Copying the comment would be helpful there. I think a broken() should always be annotated with some sort of explanation, personally. Resolved. Git logs show some `wined3d/tests` messages but suggest seems much more common. Hrm, where? I checked `git log --grep` and didn't see any. Resolved. I figured a wrapper struct only for swapchain rtvs might be more optimal vs adding extra memory per every rtvs but performance diffs are probably negligible and using `wined3d_rendertarget_view` as list directly may have wider use cases eventually. That's a fair concern. I don't think there'll be enough RTVs for it to really matter, though, yeah.
That's not quite right though; we need to offset the back buffer index by the index of this backbuffer.
Is the concern that views may be initialized after swapchain has already been rotated/offset?
If so then I added a test case that creates `backbuffer_1_rtv` right after initial present/rotate which passes testsfor the existing impl vs handling any extra offsets which seems consistent with Windows 11 test results. Not per se—as you observe, that already works because the textures are rotated. My comment was about creating a view on anything but the first backbuffer. E.g. the view list for backbuffer 0 goes [0, 1, 2, 3]; the view list for backbuffer 1 goes [1, 2, 3, 0], where each view is the index of texture we want to create a view of.
/* TODO: cleanup? */What do we need to clean up here? Note that Vulkan views will get deleted when the view object is destroyed.
Resolved. Comments left for review just in case problematic. Since there's nothing to do please go ahead and delete the comment.
5/6: This one I need more time on, sorry. I've been staring at the spec all afternoon and it seems a bit hairier than we originally thought.
I'll keep testing more apps/games in the meantime but so far it still seems to be working and passing all tests. Thanks for the patience. Also, thanks a lot to Henri for helping me understand this. I believe that this patch is correct as-is, no further review needed.
6/6: Rotating views works, buuuut, what if we instead just store an index and increment it, and use that indexed view instead of using view 0?
Anything that helps wined3d and d3d12 better align always seems worthwhile to me but original impl kept consistent with existing rotate functions for now.
If indexing is more favorable then perhaps the existing rotate functions should also be changed for consistency between them all and to resemble how d3d12 already uses explicit indexing.
Unless its critical I've left it for a later merge request to handle wined3d and d3d12 alignment more specifically. Sure, we can leave it like this for now.
Actually, for that matter, we could store the index in the swapchain, and then we wouldn't actually even have to make lists of views at all? Does this work or am I not thinking this through enough?
Swapchains support arbitrary number of rtv views per non-arbitray number of backbuffers with specific count so the list seems necessary unless I'm missing something. Well, with this scheme, any usage of the views would go through the swapchain's index. We don't need to touch the views because we're instead touching the thing that the views use. Original patch has been part of staging for 3 months now which makes up the majority of this merge's duration, during which staging users have only reported games fixed and none bugged.
Maybe a better approach to this merge would have been to marked the few tests that failed as todo to be resolved later vs combining the entire effort into 1 merge request which again, has become so bloated that gitlab struggles to load the comments anymore.
I realize staging probably represents a more narrow user test pool overall but proably a higher proportionality of gamers for real world apps/games most relevant to the original patch so maybe it should have been merged despite a few extra test cases failing which seem much more edge case vs real world cases. I understand the thought, and I recognize the value of pushing a hack now and fixing the bug properly later. It's something we avoid, though, not just in d3d but in broader Wine, and it's for the reason that these hacks make things a lot harder for maintainers (and d3d is a hard beast to maintain even under the best of conditions). It's also common for contributors to get their hack committed and then disappear, never actualizing the "fix it later" part; it takes at least an amount of trust to do that. I apologize that it's been open so long; ultimaately there hasn't actually been that many rounds of review, and the delay is mostly my fault. I am sorry for that; I have been dealing with some health issues which certainly make it harder, although I don't like to use that as an excuse. FWIW, it's possible that gitlab simply struggles due to the large number of pushes. I've seen merge requests with more comments load fine. I don't know if you're pushing for the sake of CI, but you can run CI by pushing to a local fork, which also avoids generating lots of notifications for reviewers. Thanks for review. I'll address whatever remains that needs resolving this week for next review.
(and d3d is a hard beast to maintain even under the best of conditions).
Tis a hard beast to tame.
It's also common for contributors to get their hack committed and then disappear, never actualizing the "fix it later" part; it takes at least an amount of trust to do that.
Understood. Maybe someday I'll earn trust for some leniency privileges.
I apologize that it's been open so long; ultimaately there hasn't actually been that many rounds of review,
Agreed review of latest complex/complete solution has only actually been under review for about a month because the orig simpler patch was left unchanged for most of the duration while its brewed in staging.
and the delay is mostly my fault. I am sorry for that; I have been dealing with some health issues which certainly make it harder, although I don't like to use that as an excuse.
Sounds perfectly reasonable/legit to me. Please be/stay well.
FWIW, it's possible that gitlab simply struggles due to the large number of pushes. I've seen merge requests with more comments load fine. I don't know if you're pushing for the sake of CI, but you can run CI by pushing to a local fork, which also avoids generating lots of notifications for reviewers.
Thanks for insights. I indeed have partly been pushing excessively to set CI expectations for me and to compare diffs between my local tests vs server side. I'll ease up more now that expectations seem steadier and diffs between local and server side seem mostly resolved. Apologies for spam it may have caused. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_148692
Nitpick: Can this list_init be moved into wined3d_swapchain_init? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_149085
I think swapchain_rtv_entry or something like it would be a better name -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_149086
Does the framebuffer state need to be invalidated if a backbuffer view is currently bound to any render target slot? The answer might well be "no" or "it is already invalidated if there is more than 1 backbuffer". I just don't know from the top of my head. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_149089
The two newlines (rather than one) look unintentional -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_149087
I think this can be improved a bit: Like in adapter_vk_create_rendertarget_view, figure out the total view count, then run a for loop. There's no need to make vk_image_view[0] special I think. The nasty thing will be error handling: If a view creation fails you'll have to iterate from 0 to i (the successfully created views) and destroy them -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_149088
I read over the patches. First of all a big thumbs up to Stian for sticking with this and following up on review. What looked originally like a one line bugfix turned into a complicated topic. I read through Henri's and Zeb's comments. If any of my comments conflict with theirs listen to them, not me :-) I saw a few opportunities for simplifying the code. Nothing major, I think the general structure of the patches is OK. I have pointed them out in the diffs itself Regarding delayed clears and swaps: The locations are already swapped by wined3d_swapchain_gl_rotate and wined3d_swapchain_vk_rotate. I think what is missing is rotating the clear color. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_149090
Regarding delayed clears and swaps: The locations are already swapped by wined3d_swapchain_gl_rotate and wined3d_swapchain_vk_rotate. I think what is missing is rotating the clear color.
Actually I think I am wrong on that one. It only rotates locations explicitly mentioned in supported_locations, and LOCATION_CLEARED is not one of them. So it should clear the texture copy before rotating. In either case, I don't think delayed clear handling should hold up this merge request. I'll have a look at it myself and see if I can find out what's wrong there. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_149093
On Mon Aug 17 18:50:23 2026 +0000, Stefan Dösinger wrote:
Regarding delayed clears and swaps: The locations are already swapped by wined3d_swapchain_gl_rotate and wined3d_swapchain_vk_rotate. I think what is missing is rotating the clear color. Actually I think I am wrong on that one. It only rotates locations explicitly mentioned in supported_locations, and LOCATION_CLEARED is not one of them. So it should clear the texture copy before rotating. In either case, I don't think delayed clear handling should hold up this merge request. I'll have a look at it myself and see if I can find out what's wrong there. I think the reason for the (past?) test failure with delayed clears is that e.g. a sRGB view is used to clear the texture, the clear color gets stored in the texture itself, then a RGB view is used to draw to the texture. Now the vulkan pass setup uses the RGB view to interpret the load color and things break.
We don't do delayed clears on typeless formats for this reason (see vk_blitter_clear_rendertargets), but backbuffer textures are an exception: They allow a sRGB mismatched view attached to a typed format - the cause of the bug Stian set out to fix. The check in vk_blitter_clear_rendertargets either needs to treat swapchain buffers as typeless or both vk_blitter_clear_rendertargets and the renderpass setup need to convert between texture and view RGB/sRGB formats. In either case this is not something this MR needs to be concerned about. I'll write a test and send an MR to address that -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_149097
The check in vk_blitter_clear_rendertargets either needs to treat swapchain buffers as typeless or both vk_blitter_clear_rendertargets and the renderpass setup need to convert between texture and view RGB/sRGB formats.
Ah, in that case forcing immediate clear is probably what we should do. I doubt it's worth trying to handle delayed typeless clears. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_149099
On Mon Aug 17 19:42:37 2026 +0000, Elizabeth Figura wrote:
The check in vk_blitter_clear_rendertargets either needs to treat swapchain buffers as typeless or both vk_blitter_clear_rendertargets and the renderpass setup need to convert between texture and view RGB/sRGB formats. Ah, in that case forcing immediate clear is probably what we should do. I doubt it's worth trying to handle delayed typeless clears. !11690 should take care of this particular side quest
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_149104
@stefan thanks for review. Some of your suggests have already been included for latest patches along with @zfigura suggests from previous review which have not been pushed yet pending more game tests. I will also include your other suggests as well and test MR-11690 for compatibility with MR-10567 to fix vulkan tests failing. NieR Automata crashes for latest patch so I am working to understand what may be bugged. NieR Automata does not crash for original simple patch still part of staging so crash is related to extra complexity added for latest solution. NieR Automata uses UAVs for swapchains which I hoped would not need to be handled by this merge request since it is already considered too bloated: `0178:fixme:d3d:wined3d_unordered_access_view_gl_cs_init texture swap 00007FFFFE890310` Hopefully the crash is unrelated to UAVs so that they may be handled by a separate merge request if needed eventually. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_149154
On Tue Aug 18 12:07:01 2026 +0000, Stian Low wrote:
@stefan thanks for review. Some of your suggests have already been included for latest patches along with @zfigura suggests from previous review which have not been pushed yet pending more game tests. I will also include your other suggests as well and test MR-11690 for compatibility with MR-10567 to fix vulkan tests failing. NieR Automata crashes for latest patch so I am working to understand what may be bugged. NieR Automata does not crash for original simple patch still part of staging so crash is related to extra complexity added for latest solution. NieR Automata uses UAVs for swapchains which I hoped would not need to be handled by this merge request since it is already considered too bloated: `0178:fixme:d3d:wined3d_unordered_access_view_gl_cs_init texture swap 00007FFFFE890310` Hopefully the crash is unrelated to UAVs so that they may be handled by a separate merge request if needed eventually. NieR Automata no longer crashes after skipping `wined3d_swapchain_rendertarget_view_gl_rotate` so it seems UAVs are not involved and may still be handled separately.
NieR seems to expose a bug for latest patch that none of the tests nor any of the many games tested seem to catch. I will resolve NieR crashes before pushing any more changes. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_149155
On Tue Aug 18 12:07:01 2026 +0000, Stian Low wrote:
NieR Automata no longer crashes after skipping `wined3d_swapchain_rendertarget_view_gl_rotate` so it seems UAVs are not involved and may still be handled separately. NieR seems to expose a bug for latest patch that none of the tests nor any of the many games tested seem to catch. I will resolve NieR crashes before pushing any more changes. Yeah, leave UAVs and SRVs out of this patch - lets get what you have over the finish line.
Did you investigate Elizabeth's swapchain rotate index? I'll try to look into it myself later today. As I understand it the idea is like instead of using `view_gl->gl_view[0]`and rotating the gl_view pointer array do (pseudo code, I don't have the wined3d source code open right now) ``` static inline struct gl_view * wined3d_current_gl_view(const struct wined3d_rendertarget_view_gl *rtv_gl) { if (!rtv_gl->texture->swapchain) return &rtv_gl->gl_view[0]; return &rtv_gl->gl_view[rtv_gl->texture->swapchain->rotate_count]; } ``` then swapchain_view_rotate becomes a simple wrapping increment: ``` swapchain->rotate_count++; if (swapchain->rotate_count == swapchain->back_buffer_count) swapchain->rotate_count = 0; ``` This keeps the data structures simpler and no need for complicated pointer rotation. There are probably some details missing: e.g. before looking up texture->swapchain a check is needed if the view is actually targeting a 2D texture. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_149160
Did you investigate Elizabeth's swapchain rotate index? I'll try to look into it myself later today. As I understand it the idea is like
I think that could work in principle, although it would depend on wined3d_swapchain_resize_buffers() not being possible to call while any swapchain views exist. I think that's true, but we'd want to make sure.
``` swapchain->rotate_count++; if (swapchain->rotate_count == swapchain->back_buffer_count) swapchain->rotate_count = 0; ```
For what it's worth, in dxgi's struct d3d12_swapchain, the equivalent field is called "current_buffer_index". -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_149165
On Tue Aug 18 14:52:24 2026 +0000, Henri Verbeet wrote:
Did you investigate Elizabeth's swapchain rotate index? I'll try to look into it myself later today. As I understand it the idea is like I think that could work in principle, although it would depend on wined3d_swapchain_resize_buffers() not being possible to call while any swapchain views exist. I think that's true, but we'd want to make sure. ``` swapchain->rotate_count++; if (swapchain->rotate_count == swapchain->back_buffer_count) swapchain->rotate_count = 0; ``` For what it's worth, in dxgi's struct d3d12_swapchain, the equivalent field is called "current_buffer_index". I was trying to minimize risks of breaking changes by keeping as much consistent as possible and reduce review efforts to expedite a fix but since it seems brought up multiple times I'll add alignment with d3d12 as part of this effort instead of splitting.
`wined3d_swapchain_resize_buffers` seems to be what causes NieR to crash unlike all the other games so it may need extra work regardless so I'll account for it for `current_buffer_index` solution. Unclear if its related to swapchain use-after-free but I continue to keep it in mind during this debugging effort: - https://bugs.winehq.org/show_bug.cgi?id=58325 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_149167
On Tue Aug 18 14:52:24 2026 +0000, Stian Low wrote:
I was trying to minimize risks of breaking changes by keeping as much consistent as possible and reduce review efforts to expedite a fix but since it seems brought up multiple times I'll add alignment with d3d12 as part of this effort instead of splitting. `wined3d_swapchain_resize_buffers` seems to be what causes NieR to crash unlike all the other games so it may need extra work regardless so I'll account for it for `current_buffer_index` solution. Unclear if its related to swapchain use-after-free but I continue to keep it in mind during this debugging effort: - https://bugs.winehq.org/show_bug.cgi?id=58325 I wasn't aware IDXGISwapChain::ResizeBuffers can change the backbuffer count and not just the size.
MSDN says a call to ResizeBuffers is not allowed when references to the swapchain's backbuffers are held, and that includes existing views. We do check the d3d11 refcounts of backbuffers, but it is possible that views can slip through. I am a bit rusty on d3d11 refcounting, but if it is like d3d9, a view could be bound as current render target, have a non-zero external refcount and be "alive". -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_149446
On Thu Aug 20 20:36:21 2026 +0000, Stefan Dösinger wrote:
I wasn't aware IDXGISwapChain::ResizeBuffers can change the backbuffer count and not just the size. MSDN says a call to ResizeBuffers is not allowed when references to the swapchain's backbuffers are held, and that includes existing views. We do check the d3d11 refcounts of backbuffers, but it is possible that views can slip through. I am a bit rusty on d3d11 refcounting, but if it is like d3d9, a view could be bound as current render target, have a non-zero external refcount and be "alive". These logs seem to be part of the problem for rtv views rotations only which happen for master without any of my changes so it seems this merge may be exposing another discrepancy similar to test fails that may be fixed via !11690:
`0024:err:d3d:wined3d_swapchain_resize_buffers Something's still holding back buffer 0 (00007FFFFE88E5D0), swapchain 00007FFFFE88DBE0.` Skipping `swapchain->back_buffers[i]->swapchain = NULL;` for `wined3d_swapchain_resize_buffers` prevents NieR from crashing with backtrace. It creates two swapchains with one as the parent of the other which seems less common unless I've been overlooking that scenario for other games. Still unclear if its actually related to whatever is causing the crash. I added additional wined3d_lock/unlocks but it seems to just be adding redundancies of what is already wrapped so whatever is holding back buffer 0 seems fairly subtle which again seems related to bug-58325. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_149488
On Thu Aug 20 23:39:42 2026 +0000, Stian Low wrote:
These logs seem to be part of the problem for rtv views rotations only which happen for master without any of my changes so it seems this merge may be exposing another discrepancy similar to test fails that may be fixed via !11690: `0024:err:d3d:wined3d_swapchain_resize_buffers Something's still holding back buffer 0 (00007FFFFE88E5D0), swapchain 00007FFFFE88DBE0.` Skipping `swapchain->back_buffers[i]->swapchain = NULL;` for `wined3d_swapchain_resize_buffers` prevents NieR from crashing with backtrace. It creates two swapchains with one as the parent of the other which seems less common unless I've been overlooking that scenario for other games. Still unclear if its actually related to whatever is causing the crash. I added additional wined3d_lock/unlocks but it seems to just be adding redundancies of what is already wrapped so whatever is holding back buffer 0 seems fairly subtle which again seems related to bug-58325. ERR logs for back buffer 0 being held back for swapchain resize was introduced a few years ago:
``` ebaa0a9426864cfa6e36955f26ff4c66c1d5af76 Author: Elizabeth Figura <zfigura@codeweavers.com> CommitDate: Thu Jun 29 12:04:35 2023 +0200 wined3d: Recreate swapchain textures in wined3d_swapchain_resize_buffers(). ``` Originally swapchain backbuffers were reused with `wined3d_texture_update_desc` vs deref/destroyed and recreated. Unclear if back buffers still being held for resize was a regression but the commit seems to have correctly added an ERR message whenever it happens as @stefan referenced from MSDN: - https://learn.microsoft.com/en-us/windows/win32/api/dxgi/nf-dxgi-idxgiswapch... However according to MSDN, if that ERR is ever logged then it is expected to return an error code instead of S_OK. Unclear if returning error code for ERR log was skipped to reduce risks of breaking apps but it definitely breaks NieR without whatever is holding back buffer back is fixed which seems like a bug. I added some extra logs and confirmed that a UAV is involved for incrementing back buffer 0 on init +1 greater than the other 2 backbuffers for NieR but still unclear if this means UAVs may also need to be handled and rotated by this merge. I will try to add some basic UAV rotation handling since it may need to be handled eventually anyways to see if resolves the ERR log. If not then I'll preserve the UAV logic to be split as a separate merge later. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_149531
On Fri Aug 21 14:01:32 2026 +0000, Stian Low wrote:
ERR logs for back buffer 0 being held back for swapchain resize was introduced a few years ago: ``` ebaa0a9426864cfa6e36955f26ff4c66c1d5af76 Author: Elizabeth Figura <zfigura@codeweavers.com> CommitDate: Thu Jun 29 12:04:35 2023 +0200 wined3d: Recreate swapchain textures in wined3d_swapchain_resize_buffers(). ``` Originally swapchain backbuffers were reused with `wined3d_texture_update_desc` vs deref/destroyed and recreated. Unclear if back buffers still being held for resize was a regression but the commit seems to have correctly added an ERR message whenever it happens as @stefan referenced from MSDN: - https://learn.microsoft.com/en-us/windows/win32/api/dxgi/nf-dxgi-idxgiswapch... However according to MSDN, if that ERR is ever logged then it is expected to return an error code instead of S_OK. Unclear if returning error code for ERR log was skipped to reduce risks of breaking apps but it definitely breaks NieR without whatever is holding back buffer back is fixed which seems like a bug. I added some extra logs and confirmed that a UAV is involved for incrementing back buffer 0 on init +1 greater than the other 2 backbuffers for NieR but still unclear if this means UAVs may also need to be handled and rotated by this merge. I will try to add some basic UAV rotation handling since it may need to be handled eventually anyways to see if resolves the ERR log. If not then I'll preserve the UAV logic to be split as a separate merge later. [swapchain.diff](/uploads/c86cb078cfe086fd505327fd4461b800/swapchain.diff)
I played around a bit with swapchain RTVs on Windows. The attached swapchain.diff is a way to invoke ResizeBuffers() while having a live RTV on a backbuffer. It creates a RTV, sets it on the (d3d10) device, then releases the reference to both the RTV and back buffer. It doesn't get destroyed - the "Pre resize buf ok" runs successfully with heap validation enabled. ResizeBuffers returns success. The post resize buf runs into a use-after-free heap issue. Running it with heap validation enabled (set via gflags from the Windows SDK), it crashes after ResizeBuffers. So I guess that ResizeBuffer succeeds is a bug in native. I am pretty sure we have the same bug, considering that we are checking if a texture or view is alive via refcount. If this is indeed what Nier is running into that would be annoying. It would mean we have to replicate the bug and tiptoe around the consequences well enough to keep Nier working. It is of course possible that Nier has an entirely different problem. I'll try to download the game over night. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_149604
On Sat Aug 22 15:50:29 2026 +0000, Stefan Dösinger wrote:
[swapchain.diff](/uploads/c86cb078cfe086fd505327fd4461b800/swapchain.diff) I played around a bit with swapchain RTVs on Windows. The attached swapchain.diff is a way to invoke ResizeBuffers() while having a live RTV on a backbuffer. It creates a RTV, sets it on the (d3d10) device, then releases the reference to both the RTV and back buffer. It doesn't get destroyed - the "Pre resize buf ok" runs successfully with heap validation enabled. ResizeBuffers returns success. The post resize buf runs into a use-after-free heap issue. Running it with heap validation enabled (set via gflags from the Windows SDK), it crashes after ResizeBuffers. So I guess that ResizeBuffer succeeds is a bug in native. I am pretty sure we have the same bug, considering that we are checking if a texture or view is alive via refcount. If this is indeed what Nier is running into that would be annoying. It would mean we have to replicate the bug and tiptoe around the consequences well enough to keep Nier working. It is of course possible that Nier has an entirely different problem. I'll try to download the game over night. Here is the output of my test hack:
``` dxgi.c:4615: Test failed: d3d10: rtv ref 0 dxgi.c:4617: Test failed: d3d10: tex ref 1 dxgi.c:4631: Test failed: d3d10: Pre resize buf ok dxgi.c:4641: Test failed: d3d10: post resize buf ok 4f9c:dxgi: 307 tests executed (0 marked as todo, 0 as flaky, 4 failures), 0 skipped. ``` The "test failed" are my ok(0, ...) debug lines. With heap validation: ``` dxgi.c:4615: Test failed: d3d10: rtv ref 0 dxgi.c:4617: Test failed: d3d10: tex ref 1 dxgi.c:4631: Test failed: d3d10: Pre resize buf ok dxgi.c:4635: this is the last test seen before the exception 0e68:dxgi: unhandled exception c0000005 at 00007FF622E73E91 ``` So an assigned RTV with ref=0 stays alive ("pre resize buf" addref/release cycle succeeds), ResizeBuffers does its thing and returns success and frees the texture object. I see the following possibilities: 1) My guess is just wrong and something different is going on 2) Nier tries to resize while objects are active but gets an error on Windows 3) Nier tries to resize and expects it to succeed and is (un)lucky not to run into a crash with the defective textures/views afterwards 4) Something before the ResizeBuffers call goes wrong and Nier enters a codepath it does not enter on Windows -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_149605
participants (5)
-
Elizabeth Figura (＠zfigura) -
Henri Verbeet (＠hverbeet)
-
Stefan Dösinger (＠stefan)
-
Stian Low
-
Stian Low (＠stianlow)