[PATCH v6 0/3] MR10874: wined3d: Disallow locking of multisampled back buffers.
-- v6: 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 | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/dlls/d3d8/tests/device.c b/dlls/d3d8/tests/device.c index 02383233cff..6b8bf475c60 100644 --- a/dlls/d3d8/tests/device.c +++ b/dlls/d3d8/tests/device.c @@ -9415,6 +9415,25 @@ 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); + IDirect3DDevice8_Release(device); IDirect3D8_Release(d3d); DestroyWindow(window); } @@ -9507,7 +9526,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 +9573,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 | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/dlls/d3d9/tests/device.c b/dlls/d3d9/tests/device.c index 3dc9bff4dc2..7ec91c072f5 100644 --- a/dlls/d3d9/tests/device.c +++ b/dlls/d3d9/tests/device.c @@ -13127,6 +13127,15 @@ static void test_lockable_backbuffer(void) IDirect3DSurface9_Release(surface); refcount = IDirect3DDevice9_Release(device); ok(!refcount, "Device has %lu references left.\n", refcount); + + present_parameters.MultiSampleType = D3DMULTISAMPLE_2_SAMPLES; + 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 +13278,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 | 4 ++-- dlls/d3d8/tests/device.c | 6 ------ dlls/d3d9/tests/device.c | 5 ----- 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(+), 15 deletions(-) diff --git a/dlls/d3d8/device.c b/dlls/d3d8/device.c index 036c60edc37..5c72eccb44a 100644 --- a/dlls/d3d8/device.c +++ b/dlls/d3d8/device.c @@ -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))) diff --git a/dlls/d3d8/tests/device.c b/dlls/d3d8/tests/device.c index 6b8bf475c60..38279549b46 100644 --- a/dlls/d3d8/tests/device.c +++ b/dlls/d3d8/tests/device.c @@ -9427,10 +9427,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); IDirect3DDevice8_Release(device); @@ -9583,10 +9580,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 7ec91c072f5..e1bd665dc37 100644 --- a/dlls/d3d9/tests/device.c +++ b/dlls/d3d9/tests/device.c @@ -13131,10 +13131,7 @@ static void test_lockable_backbuffer(void) present_parameters.MultiSampleType = D3DMULTISAMPLE_2_SAMPLES; 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); @@ -13281,10 +13278,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 Tue May 26 04:43:30 2026 +0000, Henri Verbeet wrote:
Sadly it seems these tests get skipped on the testbot, but it seems surprising that d3d8:device passes for you; I don't think WINED3D_SWAPCHAIN_ALLOW_MS_LOCKABLE_BACKBUFFER gets set anywhere? There may be some value in having test coverage for swapchain creation as well.
```diff + /* d3d8 allows the lockable flag even though the backbuffer is
not lockable. */
+ if (swapchain_desc->multisample_type && !(swapchain_desc->flags & WINED3D_SWAPCHAIN_ALLOW_MS_LOCKABLE_BACKBUFFER) + && (swapchain_desc->flags & WINED3D_SWAPCHAIN_LOCKABLE_BACKBUFFER)) + return WINED3DERR_INVALIDCALL; ``` I'd be inclined to put the test for WINED3D_SWAPCHAIN_LOCKABLE_BACKBUFFER first; it's the main thing we're concerned with here, and it seems like the most likely test to fail. ```diff +#define WINED3D_SWAPCHAIN_ALLOW_MS_LOCKABLE_BACKBUFFER 0x00200000u /* Allow the swapchain flag, but not actual locking */ ``` I'd put the comment on a separate line. Broadly I think this approach works for me though. It passed because I built 64-bit Wine but ran the 32-bit d3d8 test.
The `WINED3D_SWAPCHAIN_ALLOW_MS_LOCKABLE_BACKBUFFER` is now added in d3d8, I rearranged the flag checks and moved them to a helper to allow use in `wined3d_swapchain_init()`, and I added `CreateDevice()` tests. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10874#note_141296
```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()? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10874#note_141312
participants (3)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Henri Verbeet (＠hverbeet)