[PATCH v63 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. -- v63: 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 | 18 +++++++++++++++++- dlls/d3d11/tests/d3d11.c | 18 +++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/dlls/d3d10core/tests/d3d10core.c b/dlls/d3d10core/tests/d3d10core.c index a949b14837e..cdac9210b47 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, *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,18 @@ 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 (bug #45364, MR #10567) */ + 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); + if (SUCCEEDED(hr)) + { + ID3D10Device_ClearRenderTargetView(device, backbuffer_0_rtv_srgb, grey); + color = get_texture_color(backbuffer_0, 320, 240); /* red */ + 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); @@ -11281,6 +11295,8 @@ 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); 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..74b98f240e8 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, *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,18 @@ 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 (bug #45364, MR #10567) */ + 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); + 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); @@ -16238,6 +16252,8 @@ 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); 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 | 15 +++++++++++++++ dlls/wined3d/wined3d_private.h | 7 +++++++ 3 files changed, 24 insertions(+) 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..9ba76c97d79 100644 --- a/dlls/wined3d/view.c +++ b/dlls/wined3d/view.c @@ -421,6 +421,21 @@ 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_swapchain_rendertarget_view *swap_view; + LIST_FOR_EACH_ENTRY(swap_view, &texture_from_resource(resource)->swapchain->back_buffer_rendertarget_views, struct wined3d_swapchain_rendertarget_view, entry) + { + if (swap_view->view != view) + continue; + + list_remove(&swap_view->entry); + free(swap_view); + break; + } + } + resource->device->adapter->adapter_ops->adapter_destroy_rendertarget_view(view); wined3d_mutex_unlock(); wined3d_resource_decref(resource); diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index 9537670f7c5..ca483dfb464 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -4093,6 +4093,12 @@ struct wined3d_swapchain_ops void (*swapchain_frontbuffer_updated)(struct wined3d_swapchain *swapchain); }; +struct wined3d_swapchain_rendertarget_view +{ + struct list entry; + struct wined3d_rendertarget_view *view; +}; + struct wined3d_swapchain { LONG ref; @@ -4103,6 +4109,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 | 46 +++++++++++++++++++++++++++++++-------- dlls/wined3d/wined3d_vk.h | 6 ++--- 3 files changed, 59 insertions(+), 14 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 9ba76c97d79..d5bc3492c1e 100644 --- a/dlls/wined3d/view.c +++ b/dlls/wined3d/view.c @@ -913,12 +913,15 @@ static void wined3d_render_target_view_vk_cs_init(void *object) { struct wined3d_rendertarget_view_vk *view_vk = object; struct wined3d_view_desc *desc = &view_vk->v.desc; + struct wined3d_swapchain_rendertarget_view *lview; 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); @@ -945,26 +948,51 @@ 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]) + { + ERR("Failed to create render target view for resource %p\n", resource); + /* TODO: cleanup? */ return; + } + + if (!(swapchain = texture_vk->t.swapchain)) + return; + + context = context_acquire(resource->device, NULL, 0); + for (i = 1; i < swapchain->state.desc.backbuffer_count; i++) + { + 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; + + ERR("Failed to create render target view for swapchain %p backbuffer[%u]\n", swapchain, i); + /* TODO: cleanup? */ + break; + } + + context_release(context); + + if (!(lview = calloc(1, sizeof(*lview)))) + { + ERR("Out of memory.\n"); + /* TODO: cleanup? */ + } + + list_add_head(&swapchain->back_buffer_rendertarget_views, &lview->entry); + lview->view = &view_vk->v; - 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 | 86 +++++++++++++++++++++++++++------------ dlls/wined3d/wined3d_gl.h | 2 +- 4 files changed, 88 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 d5bc3492c1e..3b0ca915ecd 100644 --- a/dlls/wined3d/view.c +++ b/dlls/wined3d/view.c @@ -605,45 +605,81 @@ 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_swapchain_rendertarget_view *lview; + struct wined3d_texture_gl *texture_gl; + struct wined3d_swapchain *swapchain; + unsigned int depth_or_layer_count; + GLenum resource_class, view_class; + struct wined3d_context *context; + 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; + context = context_acquire(resource->device, NULL, 0); + create_texture_view(&view_gl->gl_view[0], texture_gl->target, desc, texture_gl, view_gl->v.format); + context_release(context); - 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); + /* TODO: cleanup? */ + 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; + + context = context_acquire(resource->device, NULL, 0); + for (i = 1; i < swapchain->state.desc.backbuffer_count; i++) + { + texture_gl = wined3d_texture_gl(swapchain->back_buffers[i]); + create_texture_view(&view_gl->gl_view[i], texture_gl->target, desc, texture_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); + /* TODO: cleanup? */ + break; } + context_release(context); + + if (!(lview = calloc(1, sizeof(*lview)))) + { + ERR("Out of memory.\n"); + /* TODO: cleanup? */ + return; + } + + list_add_head(&swapchain->back_buffer_rendertarget_views, &lview->entry); + lview->view = &view_gl->v; } static HRESULT wined3d_rendertarget_view_init(struct wined3d_rendertarget_view *view, 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 | 2 +- dlls/d3d11/tests/d3d11.c | 2 +- dlls/wined3d/swapchain.c | 73 +++++++++++++++++++++++++++++++- 3 files changed, 73 insertions(+), 4 deletions(-) diff --git a/dlls/d3d10core/tests/d3d10core.c b/dlls/d3d10core/tests/d3d10core.c index cdac9210b47..39bbb5ef8d0 100644 --- a/dlls/d3d10core/tests/d3d10core.c +++ b/dlls/d3d10core/tests/d3d10core.c @@ -11238,7 +11238,7 @@ static void test_swapchain_flip(void) { ID3D10Device_ClearRenderTargetView(device, backbuffer_0_rtv_srgb, grey); color = get_texture_color(backbuffer_0, 320, 240); /* red */ - 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); diff --git a/dlls/d3d11/tests/d3d11.c b/dlls/d3d11/tests/d3d11.c index 74b98f240e8..52ae5019c4c 100644 --- a/dlls/d3d11/tests/d3d11.c +++ b/dlls/d3d11/tests/d3d11.c @@ -16194,7 +16194,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); diff --git a/dlls/wined3d/swapchain.c b/dlls/wined3d/swapchain.c index 2394f8d91ff..be04ad17305 100644 --- a/dlls/wined3d/swapchain.c +++ b/dlls/wined3d/swapchain.c @@ -543,6 +543,41 @@ 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_swapchain_rendertarget_view *swap_view; + struct wined3d_rendertarget_view_gl *view_gl; + GLuint name, name_prev; + unsigned int i; + + LIST_FOR_EACH_ENTRY(swap_view, &swapchain->back_buffer_rendertarget_views, + struct wined3d_swapchain_rendertarget_view, entry) + { + if (!swap_view->view) + { + FIXME("Skipping rotate for swap_view %p which should not occur.\n", swap_view); + continue; + } + + view_gl = wined3d_rendertarget_view_gl(swap_view->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 +618,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 +1240,40 @@ 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) +{ + struct wined3d_swapchain_rendertarget_view *swap_view; + struct wined3d_rendertarget_view_vk *view_vk; + VkImageView vk_image_view, vk_image_view_prev; + unsigned int i; + + LIST_FOR_EACH_ENTRY(swap_view, &swapchain->back_buffer_rendertarget_views, + struct wined3d_swapchain_rendertarget_view, entry) + { + if (!swap_view->view) + { + FIXME("Skipping rotate for swap_view %p which should not occur.\n", swap_view); + continue; + } + + view_vk = wined3d_rendertarget_view_vk(swap_view->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 +1322,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 Mon Jul 27 14:26:39 2026 +0000, Stian Low wrote:
Thanks for a quick review @hverbeet. No worries. I know you're busy with higher priorities like vkd3d. I minimized this merge as much as possible so to go further may require vulkan and gl as separate MRs if still considered too bulky. I'll restructure the MR based on your suggests to better support rollbacks if ever necessary in the meantime. SRV support still seems better handled via a separate MR to keep this one from growing more. Latest push removes todos for tests in same ending commit that adds rotate functions that supports them.
Please let me know if anything should be changed or split out to further. I'm working to push SRV rotate as separate MR in the meantime unless it should be included with this one. Sorry for such bloated merge request. I knew very little about wine at the start and had to go through a learning process and many iterations of different hacks and cleanup to finally arrive near a fix that passes tests. Thanks again for all the help and guidance. Hopefully future merges will be much cleaner now that I'm more familiar. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_147477
Hi, really sorry for the late review, I promise I wanted to get to it sooner. This looks structurally much closer to what it should, and is much easier to read, so thank you for that and for your patience. General comment: subject lines should be a grammatical sentence, and we don't need the MR number tag anywhere in the commit. I'd suggest titles like: 1/6: d3d10core/tests, d3d11/tests: Test toggling sRGB for swapchain views. 2/6: wined3d: Track swapchain views in a list. 3/6: wined3d/vk: Create views for all backbuffers. 4/6: wined3d/gl: Create views for all backbuffers. 5/6: wined3d/gl: Disable GL_FRAMEBUFFER_SRGB for drawable FBO blits. 6/6: wined3d: Rotate swapchain texture views. 1/6: ``` + /* Test SRGB rtv for UNORM swapchain common for Unity games for Win10 (bug #45364, MR #10567) */ ``` 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. ``` + hr = ID3D10Device_CreateRenderTargetView(device, (ID3D10Resource *)backbuffer_0, &rtv_desc, &backbuffer_0_rtv_srgb); + if (SUCCEEDED(hr)) ``` This needs ok(hr == S_OK). Or does it not actually work on native for some reason? Also, generally we split d3d10core/tests and d3d11/tests into their own commits, and label them accordingly, not using "wined3d/tests". 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). 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. I would also not assign the existing texture_vk variable as that creates a bit of a footgun. ``` + /* TODO: cleanup? */ ``` What do we need to clean up here? Note that Vulkan views will get deleted when the view object is destroyed. 4/6: ``` + context = context_acquire(resource->device, NULL, 0); ``` You're not using these contexts for anything? 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. 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? 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? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10567#note_147765
participants (3)
-
Elizabeth Figura (＠zfigura) -
Stian Low
-
Stian Low (＠stianlow)