[PATCH v3 0/3] MR11690: wined3d: Don't delay-clear backbuffers except to black.
This was found by Stian Low in !10567. Backbuffers are special: They can have a view of a different format even though they are typed. In particular, they can be RGB and have sRGB views or vice versa. Like typeless textures we can still clear them to vec4(0.0/0.0/0.0/0.0) as 0 is interpreted the same way in RGB and sRGB. I did not run the full d3d testsuite locally as I am on an Nvidia GPU at the moment. Lets see what gitlab's bot finds -- v3: d3d10/tests: Synchronize test_swapchain_views with d3d11. d3d11/tests: Test delayed clear RGB/sRGB mismatch. https://gitlab.winehq.org/wine/wine/-/merge_requests/11690
From: Stefan Dösinger <stefan@codeweavers.com> This was found by Stian Low in !10567. Backbuffers are special: They can have a view of a different format even though they are typed. In particular, they can be RGB and have sRGB views or vice versa. Like typeless textures we can still clear them to vec4(0.0/0.0/0.0/0.0) as 0 is interpreted the same way in RGB and sRGB. --- dlls/wined3d/texture_vk.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/dlls/wined3d/texture_vk.c b/dlls/wined3d/texture_vk.c index 9a0862e738c..c0469bcd05c 100644 --- a/dlls/wined3d/texture_vk.c +++ b/dlls/wined3d/texture_vk.c @@ -1203,21 +1203,26 @@ static void vk_blitter_clear_rendertargets(struct wined3d_context_vk *context_vk for (i = 0, attachment_count = 0, layer_count = 1; i < rt_count; ++i) { + struct wined3d_texture *texture; + bool view_dependent; + if (!(view = fb->render_targets[i])) continue; + texture = texture_from_resource(view->resource); /* Don't delay typeless clears because the data written into the resource depends on the - * view format. Except all-zero clears, those should result in zeros in either case. + * view format. Except all-zero clears, those should result in zeros in either case. The + * same applies to swapchain backbuffers as they may have views with an RGB/sRGB mismatch. * * We could store the clear format along with the clear value, but then we'd have to * create a matching RTV at draw time, which would need its own render pass, thus mooting * the point of the delayed clear. (Unless we are lucky enough that the application * draws with the same RTV as it clears.) */ + view_dependent = wined3d_format_is_typeless(texture->resource.format) || texture->swapchain; + if (wined3d_rendertarget_view_is_full_clear(view, draw_rect, clear_rects) - && (!wined3d_format_is_typeless(view->resource->format) || (!colour->r && !colour->g - && !colour->b && !colour->a))) + && (!view_dependent || (!colour->r && !colour->g && !colour->b && !colour->a))) { - struct wined3d_texture *texture = texture_from_resource(view->resource); wined3d_rendertarget_view_validate_location(view, WINED3D_LOCATION_CLEARED); wined3d_rendertarget_view_invalidate_location(view, ~WINED3D_LOCATION_CLEARED); texture->sub_resources[view->sub_resource_idx].clear_value.colour = *colour; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11690
From: Stefan Dösinger <stefan@codeweavers.com> --- dlls/d3d11/tests/d3d11.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/dlls/d3d11/tests/d3d11.c b/dlls/d3d11/tests/d3d11.c index bd115a0f957..c7e584e40b8 100644 --- a/dlls/d3d11/tests/d3d11.c +++ b/dlls/d3d11/tests/d3d11.c @@ -15961,17 +15961,19 @@ static void test_swapchain_formats(const D3D_FEATURE_LEVEL feature_level) static void test_swapchain_views(void) { + ID3D11RenderTargetView *rtv_rgb, *rtv_srgb; D3D11_SHADER_RESOURCE_VIEW_DESC srv_desc; struct d3d11_test_context test_context; D3D11_RENDER_TARGET_VIEW_DESC rtv_desc; ID3D11ShaderResourceView *srv; ID3D11DeviceContext *context; - ID3D11RenderTargetView *rtv; ID3D11Device *device; ULONG refcount; HRESULT hr; static const struct vec4 color = {0.2f, 0.3f, 0.5f, 1.0f}; + static const RECT r1 = {15, 15, 16, 16}; + static const RECT r2 = {5, 5, 6, 6}; if (!init_test_context(&test_context, NULL)) return; @@ -15985,12 +15987,19 @@ static void test_swapchain_views(void) draw_color_quad(&test_context, &color); check_texture_color(test_context.backbuffer, 0xff7f4c33, 1); - rtv_desc.Format = DXGI_FORMAT_R8G8B8A8_UNORM_SRGB; + rtv_desc.Format = DXGI_FORMAT_R8G8B8A8_UNORM; rtv_desc.ViewDimension = D3D11_RTV_DIMENSION_TEXTURE2D; rtv_desc.Texture2D.MipSlice = 0; - hr = ID3D11Device_CreateRenderTargetView(device, (ID3D11Resource *)test_context.backbuffer, &rtv_desc, &rtv); + hr = ID3D11Device_CreateRenderTargetView(device, (ID3D11Resource *)test_context.backbuffer, + &rtv_desc, &rtv_rgb); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - ID3D11DeviceContext_OMSetRenderTargets(context, 1, &rtv, NULL); + + rtv_desc.Format = DXGI_FORMAT_R8G8B8A8_UNORM_SRGB; + hr = ID3D11Device_CreateRenderTargetView(device, (ID3D11Resource *)test_context.backbuffer, + &rtv_desc, &rtv_srgb); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + + ID3D11DeviceContext_OMSetRenderTargets(context, 1, &rtv_srgb, NULL); refcount = get_refcount(test_context.backbuffer); ok(refcount == 1, "Got unexpected refcount %lu.\n", refcount); @@ -16007,7 +16016,16 @@ static void test_swapchain_views(void) if (SUCCEEDED(hr)) ID3D11ShaderResourceView_Release(srv); - ID3D11RenderTargetView_Release(rtv); + /* Clear with an RGB view, draw with sRGB view and make sure the clear color has no sRGB + * correction applied. */ + ID3D11DeviceContext_ClearRenderTargetView(context, rtv_rgb, (const float *)&color); + set_viewport(context, 0.0f, 0.0f, 10.0f, 10.0f, 0.0f, 1.0f); + draw_color_quad(&test_context, &color); + check_texture_sub_resource_color(test_context.backbuffer, 0, &r1, 0xff7f4c33, 1); + check_texture_sub_resource_color(test_context.backbuffer, 0, &r2, 0xffbc957c, 1); + + ID3D11RenderTargetView_Release(rtv_srgb); + ID3D11RenderTargetView_Release(rtv_rgb); release_test_context(&test_context); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11690
From: Stefan Dösinger <stefan@codeweavers.com> It fell behind its d3d11 counterpart. I did not do a survey of Windows versions to figure out which versions the old comment was referring to. I suspect Windows Vista, as the d3d11 version of the test unconditionally expects success. --- dlls/d3d10core/tests/d3d10core.c | 42 ++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/dlls/d3d10core/tests/d3d10core.c b/dlls/d3d10core/tests/d3d10core.c index a949b14837e..a68d92b179b 100644 --- a/dlls/d3d10core/tests/d3d10core.c +++ b/dlls/d3d10core/tests/d3d10core.c @@ -11021,15 +11021,19 @@ static void test_swapchain_formats(void) static void test_swapchain_views(void) { + ID3D10RenderTargetView *rtv_rgb, *rtv_srgb; struct d3d10core_test_context test_context; D3D10_SHADER_RESOURCE_VIEW_DESC srv_desc; D3D10_RENDER_TARGET_VIEW_DESC rtv_desc; ID3D10ShaderResourceView *srv; - ID3D10RenderTargetView *rtv; ID3D10Device *device; ULONG refcount; HRESULT hr; + static const struct vec4 color = {0.2f, 0.3f, 0.5f, 1.0f}; + static const RECT r1 = {15, 15, 16, 16}; + static const RECT r2 = {5, 5, 6, 6}; + if (!init_test_context(&test_context)) return; @@ -11038,14 +11042,31 @@ static void test_swapchain_views(void) refcount = get_refcount(test_context.backbuffer); ok(refcount == 1, "Got refcount %lu.\n", refcount); + draw_color_quad(&test_context, &color); + check_texture_color(test_context.backbuffer, 0xff7f4c33, 1); + 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 *)test_context.backbuffer, &rtv_desc, &rtv); - /* This seems to work only on Windows 7. */ + hr = ID3D10Device_CreateRenderTargetView(device, (ID3D10Resource *)test_context.backbuffer, + &rtv_desc, &rtv_srgb); + /* This does not work on some Windows versions. */ ok(hr == S_OK || broken(hr == E_INVALIDARG), "Failed to create render target view, hr %#lx.\n", hr); - if (SUCCEEDED(hr)) - ID3D10RenderTargetView_Release(rtv); + if (FAILED(hr)) + goto done; + + rtv_desc.Format = DXGI_FORMAT_R8G8B8A8_UNORM; + hr = ID3D10Device_CreateRenderTargetView(device, (ID3D10Resource *)test_context.backbuffer, + &rtv_desc, &rtv_rgb); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + + ID3D10Device_OMSetRenderTargets(device, 1, &rtv_srgb, NULL); + + refcount = get_refcount(test_context.backbuffer); + ok(refcount == 1, "Got unexpected refcount %lu.\n", refcount); + + draw_color_quad(&test_context, &color); + check_texture_color(test_context.backbuffer, 0xffbc957c, 1); srv_desc.Format = DXGI_FORMAT_R8G8B8A8_UNORM_SRGB; srv_desc.ViewDimension = D3D10_SRV_DIMENSION_TEXTURE2D; @@ -11056,6 +11077,17 @@ static void test_swapchain_views(void) if (SUCCEEDED(hr)) ID3D10ShaderResourceView_Release(srv); + /* Clear with an RGB view, draw with sRGB view and make sure the clear color has no sRGB + * correction applied. */ + ID3D10Device_ClearRenderTargetView(device, rtv_rgb, (const float *)&color); + set_viewport(device, 0.0f, 0.0f, 10.0f, 10.0f, 0.0f, 1.0f); + draw_color_quad(&test_context, &color); + check_texture_sub_resource_color(test_context.backbuffer, 0, &r1, 0xff7f4c33, 1); + check_texture_sub_resource_color(test_context.backbuffer, 0, &r2, 0xffbc957c, 1); + + ID3D10RenderTargetView_Release(rtv_srgb); + ID3D10RenderTargetView_Release(rtv_rgb); +done: release_test_context(&test_context); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11690
`texture->swapchain` check for `vk_blitter_clear_rendertargets` aligns with my latest unpushed patches for MR-10567 to fix the vulkan tests so I'll exclude from MR-10567 since this merge already handles separately. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11690#note_149158
This merge request was approved by Elizabeth Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11690
participants (4)
-
Elizabeth Figura (＠zfigura) -
Stefan Dösinger
-
Stefan Dösinger (＠stefan)
-
Stian Low (＠stianlow)