[PATCH v7 0/3] MR10874: wined3d: Disallow locking of multisampled back buffers.
-- v7: wined3d: Disallow locking of multisampled back buffers. d3d9/tests: Test multisampled back buffer locking. d3d8/tests: Test multisampled back buffer locking. https://gitlab.winehq.org/wine/wine/-/merge_requests/10874
From: Conor McCarthy <cmccarthy@codeweavers.com> --- dlls/d3d8/tests/device.c | 56 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/dlls/d3d8/tests/device.c b/dlls/d3d8/tests/device.c index 02383233cff..0c8b5f1e3ee 100644 --- a/dlls/d3d8/tests/device.c +++ b/dlls/d3d8/tests/device.c @@ -9341,6 +9341,7 @@ static void test_lockable_backbuffer(void) { D3DPRESENT_PARAMETERS present_parameters = {0}; struct device_desc device_desc; + IDirect3DSwapChain8 *swapchain; IDirect3DSurface8 *surface; IDirect3DDevice8 *device; D3DLOCKED_RECT lockrect; @@ -9415,6 +9416,42 @@ static void test_lockable_backbuffer(void) IDirect3DSurface8_Release(surface); refcount = IDirect3DDevice8_Release(device); ok(!refcount, "Device has %lu references left.\n", refcount); + + /* Multisampling is allowed. */ + present_parameters.MultiSampleType = D3DMULTISAMPLE_2_SAMPLES; + hr = IDirect3D8_CreateDevice(d3d, D3DADAPTER_DEFAULT, D3DDEVTYPE_HAL, window, + D3DCREATE_SOFTWARE_VERTEXPROCESSING, &present_parameters, &device); + ok(SUCCEEDED(hr), "Failed to create device, hr %#lx.\n", hr); + + hr = IDirect3DDevice8_GetBackBuffer(device, 0, D3DBACKBUFFER_TYPE_MONO, &surface); + ok(SUCCEEDED(hr), "Failed to get backbuffer, hr %#lx.\n", hr); + + /* But locking multisampled is not allowed. */ + hr = IDirect3DSurface8_LockRect(surface, &lockrect, NULL, D3DLOCK_READONLY); + todo_wine + ok(hr == D3DERR_INVALIDCALL, "Unexpected hr %#lx.\n", hr); + if (SUCCEEDED(hr)) + IDirect3DSurface8_UnlockRect(surface); + + IDirect3DSurface8_Release(surface); + + present_parameters.hDeviceWindow = NULL; + hr = IDirect3DDevice8_CreateAdditionalSwapChain(device, &present_parameters, &swapchain); + ok(hr == D3D_OK, "Got hr %#lx.\n", hr); + + hr = IDirect3DSwapChain8_GetBackBuffer(swapchain, 0, 0, &surface); + ok(SUCCEEDED(hr), "Failed to get backbuffer, hr %#lx.\n", hr); + + hr = IDirect3DSurface8_LockRect(surface, &lockrect, NULL, D3DLOCK_READONLY); + todo_wine + ok(hr == D3DERR_INVALIDCALL, "Unexpected hr %#lx.\n", hr); + if (SUCCEEDED(hr)) + IDirect3DSurface8_UnlockRect(surface); + + IDirect3DSurface8_Release(surface); + IDirect3DSwapChain8_Release(swapchain); + + IDirect3DDevice8_Release(device); IDirect3D8_Release(d3d); DestroyWindow(window); } @@ -9507,7 +9544,9 @@ static void test_clip_planes_limits(void) static void test_swapchain_multisample_reset(void) { D3DPRESENT_PARAMETERS present_parameters; + IDirect3DSurface8 *surface; IDirect3DDevice8 *device; + D3DLOCKED_RECT lr; IDirect3D8 *d3d; ULONG refcount; HWND window; @@ -9552,6 +9591,23 @@ static void test_swapchain_multisample_reset(void) hr = IDirect3DDevice8_Clear(device, 0, NULL, D3DCLEAR_TARGET, 0xffffffff, 0.0f, 0); ok(hr == D3D_OK, "Failed to clear, hr %#lx.\n", hr); + /* Lockable back buffer flag is allowed. */ + present_parameters.Flags = D3DPRESENTFLAG_LOCKABLE_BACKBUFFER; + hr = IDirect3DDevice8_Reset(device, &present_parameters); + ok(hr == D3D_OK, "Failed to reset device, hr %#lx.\n", hr); + + hr = IDirect3DDevice8_GetBackBuffer(device, 0, D3DBACKBUFFER_TYPE_MONO, &surface); + ok(SUCCEEDED(hr), "GetBackBuffer failed, hr %#lx.\n", hr); + + /* But locking is not allowed. */ + hr = IDirect3DSurface8_LockRect(surface, &lr, NULL, 0); + todo_wine + ok(hr == D3DERR_INVALIDCALL, "Unexpected hr %#lx.\n", hr); + if (SUCCEEDED(hr)) + IDirect3DSurface8_UnlockRect(surface); + + IDirect3DSurface8_Release(surface); + refcount = IDirect3DDevice8_Release(device); ok(!refcount, "Device has %lu references left.\n", refcount); IDirect3D8_Release(d3d); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10874
From: Conor McCarthy <cmccarthy@codeweavers.com> --- dlls/d3d9/tests/device.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/dlls/d3d9/tests/device.c b/dlls/d3d9/tests/device.c index 3dc9bff4dc2..95a08943752 100644 --- a/dlls/d3d9/tests/device.c +++ b/dlls/d3d9/tests/device.c @@ -13047,6 +13047,7 @@ static void test_lockable_backbuffer(void) { D3DPRESENT_PARAMETERS present_parameters = {0}; struct device_desc device_desc; + IDirect3DSwapChain9 *swapchain; IDirect3DSurface9 *surface; IDirect3DDevice9 *device; D3DLOCKED_RECT lockrect; @@ -13125,8 +13126,26 @@ static void test_lockable_backbuffer(void) ok(SUCCEEDED(hr), "Failed to unlock rect, hr %#lx.\n", hr); IDirect3DSurface9_Release(surface); + + present_parameters.hDeviceWindow = NULL; + present_parameters.MultiSampleType = D3DMULTISAMPLE_2_SAMPLES; + hr = IDirect3DDevice9_CreateAdditionalSwapChain(device, &present_parameters, &swapchain); + todo_wine + ok(hr == D3DERR_INVALIDCALL, "Unexpected hr %#lx.\n", hr); + if (SUCCEEDED(hr)) + IDirect3DSwapChain9_Release(swapchain); + refcount = IDirect3DDevice9_Release(device); ok(!refcount, "Device has %lu references left.\n", refcount); + + present_parameters.hDeviceWindow = window; + hr = IDirect3D9_CreateDevice(d3d, D3DADAPTER_DEFAULT, D3DDEVTYPE_HAL, window, + D3DCREATE_SOFTWARE_VERTEXPROCESSING, &present_parameters, &device); + todo_wine + ok(hr == D3DERR_INVALIDCALL, "Unexpected hr %#lx.\n", hr); + if (SUCCEEDED(hr)) + IDirect3DDevice9_Release(device); + IDirect3D9_Release(d3d); DestroyWindow(window); } @@ -13269,6 +13288,15 @@ static void test_swapchain_multisample_reset(void) hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET | D3DCLEAR_ZBUFFER, 0xffffffff, 1.0f, 0); ok(hr == D3D_OK, "Failed to clear, hr %#lx.\n", hr); + /* Lockable back buffer flag is not allowed. */ + d3dpp.Flags = D3DPRESENTFLAG_LOCKABLE_BACKBUFFER; + hr = IDirect3DDevice9_Reset(device, &d3dpp); + todo_wine + ok(hr == D3DERR_INVALIDCALL, "Unexpected hr %#lx.\n", hr); + hr = IDirect3DDevice9_TestCooperativeLevel(device); + todo_wine + ok(hr == D3DERR_DEVICENOTRESET, "TestCooperativeLevel returned hr %#lx.\n", hr); + refcount = IDirect3DDevice9_Release(device); ok(!refcount, "Device has %lu references left.\n", refcount); IDirect3D9_Release(d3d); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10874
From: Conor McCarthy <cmccarthy@codeweavers.com> --- dlls/d3d8/device.c | 3 ++- dlls/d3d8/tests/device.c | 9 --------- dlls/d3d9/tests/device.c | 8 -------- dlls/wined3d/device.c | 5 ++++- dlls/wined3d/swapchain.c | 15 ++++++++++++++- dlls/wined3d/wined3d_private.h | 2 ++ include/wine/wined3d.h | 2 ++ 7 files changed, 24 insertions(+), 20 deletions(-) diff --git a/dlls/d3d8/device.c b/dlls/d3d8/device.c index 036c60edc37..c04283c1ad4 100644 --- a/dlls/d3d8/device.c +++ b/dlls/d3d8/device.c @@ -308,7 +308,8 @@ static BOOL wined3d_swapchain_desc_from_d3d8(struct wined3d_swapchain_desc *swap swapchain_desc->auto_depth_stencil_format = wined3dformat_from_d3dformat(present_parameters->AutoDepthStencilFormat); swapchain_desc->flags - = (present_parameters->Flags & D3DPRESENTFLAGS_MASK) | WINED3D_SWAPCHAIN_ALLOW_MODE_SWITCH; + = (present_parameters->Flags & D3DPRESENTFLAGS_MASK) | WINED3D_SWAPCHAIN_ALLOW_MODE_SWITCH + | WINED3D_SWAPCHAIN_ALLOW_MS_LOCKABLE_BACKBUFFER; swapchain_desc->refresh_rate = present_parameters->FullScreen_RefreshRateInHz; swapchain_desc->auto_restore_display_mode = TRUE; diff --git a/dlls/d3d8/tests/device.c b/dlls/d3d8/tests/device.c index 0c8b5f1e3ee..54cc1e9ffde 100644 --- a/dlls/d3d8/tests/device.c +++ b/dlls/d3d8/tests/device.c @@ -9428,10 +9428,7 @@ static void test_lockable_backbuffer(void) /* But locking multisampled is not allowed. */ hr = IDirect3DSurface8_LockRect(surface, &lockrect, NULL, D3DLOCK_READONLY); - todo_wine ok(hr == D3DERR_INVALIDCALL, "Unexpected hr %#lx.\n", hr); - if (SUCCEEDED(hr)) - IDirect3DSurface8_UnlockRect(surface); IDirect3DSurface8_Release(surface); @@ -9443,10 +9440,7 @@ static void test_lockable_backbuffer(void) ok(SUCCEEDED(hr), "Failed to get backbuffer, hr %#lx.\n", hr); hr = IDirect3DSurface8_LockRect(surface, &lockrect, NULL, D3DLOCK_READONLY); - todo_wine ok(hr == D3DERR_INVALIDCALL, "Unexpected hr %#lx.\n", hr); - if (SUCCEEDED(hr)) - IDirect3DSurface8_UnlockRect(surface); IDirect3DSurface8_Release(surface); IDirect3DSwapChain8_Release(swapchain); @@ -9601,10 +9595,7 @@ static void test_swapchain_multisample_reset(void) /* But locking is not allowed. */ hr = IDirect3DSurface8_LockRect(surface, &lr, NULL, 0); - todo_wine ok(hr == D3DERR_INVALIDCALL, "Unexpected hr %#lx.\n", hr); - if (SUCCEEDED(hr)) - IDirect3DSurface8_UnlockRect(surface); IDirect3DSurface8_Release(surface); diff --git a/dlls/d3d9/tests/device.c b/dlls/d3d9/tests/device.c index 95a08943752..51d8e15f2f0 100644 --- a/dlls/d3d9/tests/device.c +++ b/dlls/d3d9/tests/device.c @@ -13130,10 +13130,7 @@ static void test_lockable_backbuffer(void) present_parameters.hDeviceWindow = NULL; present_parameters.MultiSampleType = D3DMULTISAMPLE_2_SAMPLES; hr = IDirect3DDevice9_CreateAdditionalSwapChain(device, &present_parameters, &swapchain); - todo_wine ok(hr == D3DERR_INVALIDCALL, "Unexpected hr %#lx.\n", hr); - if (SUCCEEDED(hr)) - IDirect3DSwapChain9_Release(swapchain); refcount = IDirect3DDevice9_Release(device); ok(!refcount, "Device has %lu references left.\n", refcount); @@ -13141,10 +13138,7 @@ static void test_lockable_backbuffer(void) present_parameters.hDeviceWindow = window; hr = IDirect3D9_CreateDevice(d3d, D3DADAPTER_DEFAULT, D3DDEVTYPE_HAL, window, D3DCREATE_SOFTWARE_VERTEXPROCESSING, &present_parameters, &device); - todo_wine ok(hr == D3DERR_INVALIDCALL, "Unexpected hr %#lx.\n", hr); - if (SUCCEEDED(hr)) - IDirect3DDevice9_Release(device); IDirect3D9_Release(d3d); DestroyWindow(window); @@ -13291,10 +13285,8 @@ static void test_swapchain_multisample_reset(void) /* Lockable back buffer flag is not allowed. */ d3dpp.Flags = D3DPRESENTFLAG_LOCKABLE_BACKBUFFER; hr = IDirect3DDevice9_Reset(device, &d3dpp); - todo_wine ok(hr == D3DERR_INVALIDCALL, "Unexpected hr %#lx.\n", hr); hr = IDirect3DDevice9_TestCooperativeLevel(device); - todo_wine ok(hr == D3DERR_DEVICENOTRESET, "TestCooperativeLevel returned hr %#lx.\n", hr); refcount = IDirect3DDevice9_Release(device); diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c index e754cc633b7..8f195c6dee4 100644 --- a/dlls/wined3d/device.c +++ b/dlls/wined3d/device.c @@ -4987,7 +4987,7 @@ static void update_swapchain_flags(struct wined3d_texture *texture) { unsigned int flags = texture->swapchain->state.desc.flags; - if (flags & WINED3D_SWAPCHAIN_LOCKABLE_BACKBUFFER) + if (flags & WINED3D_SWAPCHAIN_LOCKABLE_BACKBUFFER && !texture->swapchain->state.desc.multisample_type) texture->resource.access |= WINED3D_RESOURCE_ACCESS_MAP_R | WINED3D_RESOURCE_ACCESS_MAP_W; else texture->resource.access &= ~(WINED3D_RESOURCE_ACCESS_MAP_R | WINED3D_RESOURCE_ACCESS_MAP_W); @@ -5082,6 +5082,9 @@ HRESULT CDECL wined3d_device_reset(struct wined3d_device *device, TRACE("refresh_rate %u\n", swapchain_desc->refresh_rate); TRACE("auto_restore_display_mode %#x\n", swapchain_desc->auto_restore_display_mode); + if (FAILED(hr = wined3d_swapchain_desc_validate_flags(swapchain_desc))) + return hr; + if (swapchain_desc->backbuffer_bind_flags && swapchain_desc->backbuffer_bind_flags != WINED3D_BIND_RENDER_TARGET) FIXME("Got unexpected backbuffer bind flags %#x.\n", swapchain_desc->backbuffer_bind_flags); diff --git a/dlls/wined3d/swapchain.c b/dlls/wined3d/swapchain.c index 2bc544c1213..c26df770cdb 100644 --- a/dlls/wined3d/swapchain.c +++ b/dlls/wined3d/swapchain.c @@ -1506,7 +1506,7 @@ static HRESULT swapchain_create_texture(struct wined3d_swapchain *swapchain, texture_desc.access = WINED3D_RESOURCE_ACCESS_CPU; else texture_desc.access = WINED3D_RESOURCE_ACCESS_GPU; - if (!depth && (swapchain_desc->flags & WINED3D_SWAPCHAIN_LOCKABLE_BACKBUFFER)) + if (!depth && (swapchain_desc->flags & WINED3D_SWAPCHAIN_LOCKABLE_BACKBUFFER) && !swapchain_desc->multisample_type) texture_desc.access |= WINED3D_RESOURCE_ACCESS_MAP_R | WINED3D_RESOURCE_ACCESS_MAP_W; texture_desc.width = swapchain_desc->backbuffer_width; texture_desc.height = swapchain_desc->backbuffer_height; @@ -1536,6 +1536,16 @@ static HRESULT swapchain_create_texture(struct wined3d_swapchain *swapchain, return S_OK; } +HRESULT wined3d_swapchain_desc_validate_flags(const struct wined3d_swapchain_desc *desc) +{ + /* d3d8 allows the lockable flag even though the backbuffer is not lockable. */ + if ((desc->flags & WINED3D_SWAPCHAIN_LOCKABLE_BACKBUFFER) && desc->multisample_type + && !(desc->flags & WINED3D_SWAPCHAIN_ALLOW_MS_LOCKABLE_BACKBUFFER)) + return WINED3DERR_INVALIDCALL; + + return WINED3D_OK; +} + static HRESULT wined3d_swapchain_init(struct wined3d_swapchain *swapchain, struct wined3d_device *device, const struct wined3d_swapchain_desc *desc, struct wined3d_swapchain_state_parent *state_parent, void *parent, const struct wined3d_parent_ops *parent_ops, @@ -1554,6 +1564,9 @@ static HRESULT wined3d_swapchain_init(struct wined3d_swapchain *swapchain, struc && desc->swap_effect != WINED3D_SWAP_EFFECT_COPY) FIXME("Unimplemented swap effect %#x.\n", desc->swap_effect); + if (FAILED(hr = wined3d_swapchain_desc_validate_flags(desc))) + return hr; + window = desc->device_window ? desc->device_window : device->create_parms.focus_window; TRACE("Using target window %p.\n", window); diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index fa758c53cc1..f8e367b2637 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -4062,6 +4062,8 @@ struct wined3d_decoder_output_view void wined3d_decoder_output_view_cleanup(struct wined3d_decoder_output_view *view); +HRESULT wined3d_swapchain_desc_validate_flags(const struct wined3d_swapchain_desc *desc); + struct wined3d_swapchain_state { struct wined3d *wined3d; diff --git a/include/wine/wined3d.h b/include/wine/wined3d.h index cfb098adfb0..355140043db 100644 --- a/include/wine/wined3d.h +++ b/include/wine/wined3d.h @@ -920,6 +920,8 @@ enum wined3d_memory_segment_group #define WINED3D_SWAPCHAIN_NO_WINDOW_CHANGES 0x00040000u #define WINED3D_SWAPCHAIN_RESTORE_WINDOW_STATE 0x00080000u #define WINED3D_SWAPCHAIN_REGISTER_TOPMOST_TIMER 0x00100000u +/* Allow the swapchain flag, but not actual locking */ +#define WINED3D_SWAPCHAIN_ALLOW_MS_LOCKABLE_BACKBUFFER 0x00200000u #define WINED3DDP_MAXTEXCOORD 8 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10874
On Thu May 28 05:18:16 2026 +0000, Henri Verbeet wrote:
```diff @@ -957,7 +957,7 @@ static HRESULT WINAPI
d3d8_device_Reset(IDirect3DDevice8 *iface,
if (!wined3d_swapchain_desc_from_d3d8(&swapchain_desc, device->d3d_parent->wined3d_outputs[output_idx], present_parameters)) return D3DERR_INVALIDCALL; - swapchain_desc.flags |= WINED3D_SWAPCHAIN_IMPLICIT; + swapchain_desc.flags |= WINED3D_SWAPCHAIN_IMPLICIT | WINED3D_SWAPCHAIN_ALLOW_MS_LOCKABLE_BACKBUFFER;
wined3d_mutex_lock();
@@ -3758,7 +3758,7 @@ HRESULT device_init(struct d3d8_device *device,
struct d3d8 *parent, struct wine
free(device->handle_table.entries); return D3DERR_INVALIDCALL; } - swapchain_desc.flags |= WINED3D_SWAPCHAIN_IMPLICIT; + swapchain_desc.flags |= WINED3D_SWAPCHAIN_IMPLICIT | WINED3D_SWAPCHAIN_ALLOW_MS_LOCKABLE_BACKBUFFER;
if (FAILED(hr = d3d8_swapchain_create(device, &swapchain_desc,
wined3dswapinterval_from_d3d(parameters->FullScreen_PresentationInterval), &d3d_swapchain)))
```
Should this apply to d3d8_device_CreateAdditionalSwapChain() as well? Or put a different way, would it make sense to set this flag in wined3d_swapchain_desc_from_d3d8()? Yes, I added `CreateAdditionalSwapChain()` tests and the suggested change.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10874#note_141484
```diff + /* Multisampling is allowed. */ + present_parameters.MultiSampleType = D3DMULTISAMPLE_2_SAMPLES; + hr = IDirect3D8_CreateDevice(d3d, D3DADAPTER_DEFAULT, D3DDEVTYPE_HAL, window, + D3DCREATE_SOFTWARE_VERTEXPROCESSING, &present_parameters, &device); + ok(SUCCEEDED(hr), "Failed to create device, hr %#lx.\n", hr); ```
This will need to check for support for D3DMULTISAMPLE_2_SAMPLES first, similar to what e.g. test_swapchain_multisample_reset() does; currently the test is failing on the CI. Note that for the CI it may be better to use D3DMULTISAMPLE_4_SAMPLES instead of D3DMULTISAMPLE_2_SAMPLES here; IIRC llvmpipe supports 4xMSAA but not 2xMSAA. In principle that applies to the d3d9 test as well, although we're expecting things to fail there regardless. Since we're looking at the details at this point, I notice you're adding both ```diff ok(SUCCEEDED(hr), "Failed to create device, hr %#lx.\n", hr); ``` and ```diff ok(hr == D3D_OK, "Got hr %#lx.\n", hr); ``` style tests here. I think we've generally settled on preferring the latter style, but either way it would be good to pick one or the other. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10874#note_141520
participants (3)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Henri Verbeet (＠hverbeet)