[PATCH v3 0/3] MR4781: ddraw: Make sure that sysmem surfaces remain coherent when not locked.
-- v3: ddraw: Use the sysmem wined3d texture for sysmem surfaces if possible. ddraw: Sync to sysmem after performing a color fill. https://gitlab.winehq.org/wine/wine/-/merge_requests/4781
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ddraw/tests/ddraw1.c | 74 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/dlls/ddraw/tests/ddraw1.c b/dlls/ddraw/tests/ddraw1.c index 02d6adbb433..449d1873e5c 100644 --- a/dlls/ddraw/tests/ddraw1.c +++ b/dlls/ddraw/tests/ddraw1.c @@ -1403,6 +1403,16 @@ static BOOL compare_mode_rect(const DEVMODEW *mode1, const DEVMODEW *mode2) && mode1->dmPelsHeight == mode2->dmPelsHeight; } +static void init_format_b5g6r5(DDPIXELFORMAT *format) +{ + format->dwSize = sizeof(*format); + format->dwFlags = DDPF_RGB; + format->dwRGBBitCount = 16; + format->dwRBitMask = 0xf800; + format->dwGBitMask = 0x07e0; + format->dwBBitMask = 0x001f; +} + static ULONG get_refcount(IUnknown *test_iface) { IUnknown_AddRef(test_iface); @@ -15457,6 +15467,69 @@ static void test_enum_devices(void) ok(!refcount, "Device has %lu references left.\n", refcount); } +/* Emperor: Rise of the Middle Kingdom locks a sysmem surface and then accesses + * the pointer after unlocking it. This test roughly replicates the calls that + * it makes. */ +static void test_pinned_sysmem(void) +{ + DDBLTFX fx = {.dwSize = sizeof(fx), .dwFillColor = 0xface}; + IDirectDrawSurface *surface, *surface2; + DDSURFACEDESC surface_desc; + unsigned int color; + IDirectDraw *ddraw; + ULONG refcount; + HWND window; + HRESULT hr; + + window = create_window(); + ddraw = create_ddraw(); + hr = IDirectDraw_SetCooperativeLevel(ddraw, window, DDSCL_NORMAL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + memset(&surface_desc, 0, sizeof(surface_desc)); + surface_desc.dwSize = sizeof(surface_desc); + surface_desc.dwFlags = DDSD_CAPS | DDSD_WIDTH | DDSD_HEIGHT | DDSD_PIXELFORMAT; + surface_desc.ddsCaps.dwCaps = DDSCAPS_OFFSCREENPLAIN | DDSCAPS_SYSTEMMEMORY; + surface_desc.dwWidth = 32; + surface_desc.dwHeight = 32; + init_format_b5g6r5(&surface_desc.ddpfPixelFormat); + hr = IDirectDraw_CreateSurface(ddraw, &surface_desc, &surface, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IDirectDraw_CreateSurface(ddraw, &surface_desc, &surface2, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IDirectDrawSurface_Lock(surface, NULL, &surface_desc, DDLOCK_WAIT, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IDirectDrawSurface_Unlock(surface, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IDirectDrawSurface_Blt(surface, NULL, NULL, NULL, DDBLT_COLORFILL | DDBLT_WAIT, &fx); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + color = ((unsigned short *)surface_desc.lpSurface)[0]; + todo_wine ok(color == 0xface, "Got color %04x.\n", color); + + memset(surface_desc.lpSurface, 0x55, 32 * 16 * 2); + + hr = IDirectDrawSurface_BltFast(surface2, 0, 0, surface, NULL, DDBLTFAST_WAIT); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IDirectDrawSurface_Lock(surface2, NULL, &surface_desc, DDLOCK_WAIT, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + color = ((unsigned short *)surface_desc.lpSurface)[0]; + todo_wine ok(color == 0x5555, "Got color %04x.\n", color); + color = ((unsigned short *)surface_desc.lpSurface)[32 * 16]; + ok(color == 0xface, "Got color %04x.\n", color); + hr = IDirectDrawSurface_Unlock(surface2, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + IDirectDrawSurface_Release(surface2); + IDirectDrawSurface_Release(surface); + refcount = IDirectDraw_Release(ddraw); + ok(!refcount, "Device has %lu references left.\n", refcount); + DestroyWindow(window); +} + START_TEST(ddraw1) { DDDEVICEIDENTIFIER identifier; @@ -15576,4 +15649,5 @@ START_TEST(ddraw1) run_for_each_device_type(test_texture_wrong_caps); test_filling_convention(); test_enum_devices(); + test_pinned_sysmem(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4781
From: Zebediah Figura <zfigura(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56119 --- dlls/ddraw/surface.c | 32 +++++++++++++++++++++++++++++++- dlls/ddraw/tests/ddraw1.c | 4 ++-- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/dlls/ddraw/surface.c b/dlls/ddraw/surface.c index 5836a49f4fd..d0c4bf67cbc 100644 --- a/dlls/ddraw/surface.c +++ b/dlls/ddraw/surface.c @@ -1531,6 +1531,33 @@ static HRESULT WINAPI DECLSPEC_HOTPATCH ddraw_surface2_Flip(IDirectDrawSurface2 src_impl ? &src_impl->IDirectDrawSurface_iface : NULL, flags); } +/* Emperor: Rise of the Middle Kingdom accesses the map pointer outside of + * Lock()/Unlock(), and expects those updates to be propagated by a Blt(). + * It also blits to the surface, and color-fills it. + * + * This function is called after a color-fill that might update the GPU side. + * We need to make sure the sysmem surface is synchronized. */ +static void ddraw_surface_sync_pinned_sysmem(struct ddraw_surface *surface) +{ + RECT rect; + + if (!surface->draw_texture) + return; + + if (!(surface->surface_desc.ddsCaps.dwCaps & DDSCAPS_SYSTEMMEMORY)) + return; + + SetRect(&rect, 0, 0, surface->surface_desc.dwWidth, surface->surface_desc.dwHeight); + wined3d_device_context_blt(surface->ddraw->immediate_context, + surface->wined3d_texture, surface->sub_resource_idx, &rect, + surface->draw_texture, surface->sub_resource_idx, &rect, + WINED3D_BLT_SYNCHRONOUS, NULL, WINED3D_TEXF_POINT); + + /* The sysmem surface may be updated at any time, so we must invalidate the + * draw texture location here. */ + surface->texture_location = DDRAW_SURFACE_LOCATION_DEFAULT; +} + static HRESULT ddraw_surface_blt(struct ddraw_surface *dst_surface, const RECT *dst_rect, struct ddraw_surface *src_surface, const RECT *src_rect, DWORD flags, DWORD fill_colour, const struct wined3d_blt_fx *fx, enum wined3d_texture_filter_type filter) @@ -1539,6 +1566,7 @@ static HRESULT ddraw_surface_blt(struct ddraw_surface *dst_surface, const RECT * struct wined3d_device *wined3d_device = ddraw->wined3d_device; struct wined3d_color colour; DWORD wined3d_flags; + HRESULT hr; if (flags & DDBLT_COLORFILL) { @@ -1552,9 +1580,11 @@ static HRESULT ddraw_surface_blt(struct ddraw_surface *dst_surface, const RECT * wined3d_device_apply_stateblock(wined3d_device, ddraw->state); ddraw_surface_get_draw_texture(dst_surface, dst_rect ? DDRAW_SURFACE_RW : DDRAW_SURFACE_WRITE); - return wined3d_device_context_clear_rendertarget_view(ddraw->immediate_context, + hr = wined3d_device_context_clear_rendertarget_view(ddraw->immediate_context, ddraw_surface_get_rendertarget_view(dst_surface), dst_rect, wined3d_flags, &colour, 0.0f, 0); + ddraw_surface_sync_pinned_sysmem(dst_surface); + return hr; } if (flags & DDBLT_DEPTHFILL) diff --git a/dlls/ddraw/tests/ddraw1.c b/dlls/ddraw/tests/ddraw1.c index 449d1873e5c..d8fcb188c40 100644 --- a/dlls/ddraw/tests/ddraw1.c +++ b/dlls/ddraw/tests/ddraw1.c @@ -15507,7 +15507,7 @@ static void test_pinned_sysmem(void) ok(hr == S_OK, "Got hr %#lx.\n", hr); color = ((unsigned short *)surface_desc.lpSurface)[0]; - todo_wine ok(color == 0xface, "Got color %04x.\n", color); + ok(color == 0xface, "Got color %04x.\n", color); memset(surface_desc.lpSurface, 0x55, 32 * 16 * 2); @@ -15517,7 +15517,7 @@ static void test_pinned_sysmem(void) hr = IDirectDrawSurface_Lock(surface2, NULL, &surface_desc, DDLOCK_WAIT, NULL); ok(hr == S_OK, "Got hr %#lx.\n", hr); color = ((unsigned short *)surface_desc.lpSurface)[0]; - todo_wine ok(color == 0x5555, "Got color %04x.\n", color); + ok(color == 0x5555, "Got color %04x.\n", color); color = ((unsigned short *)surface_desc.lpSurface)[32 * 16]; ok(color == 0xface, "Got color %04x.\n", color); hr = IDirectDrawSurface_Unlock(surface2, NULL); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4781
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ddraw/ddraw_private.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dlls/ddraw/ddraw_private.h b/dlls/ddraw/ddraw_private.h index f40188f75ef..6fc93b91860 100644 --- a/dlls/ddraw/ddraw_private.h +++ b/dlls/ddraw/ddraw_private.h @@ -698,7 +698,8 @@ static inline struct wined3d_texture *ddraw_surface_get_draw_texture(struct ddra static inline struct wined3d_texture *ddraw_surface_get_any_texture(struct ddraw_surface *surface, unsigned int flags) { - if (surface->texture_location & DDRAW_SURFACE_LOCATION_DEFAULT) + if ((surface->texture_location & DDRAW_SURFACE_LOCATION_DEFAULT) + || (surface->surface_desc.ddsCaps.dwCaps & DDSCAPS_SYSTEMMEMORY)) return ddraw_surface_get_default_texture(surface, flags); assert(surface->texture_location & DDRAW_SURFACE_LOCATION_DRAW); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4781
On Thu Jan 11 10:34:35 2024 +0000, Jan Sikorski wrote:
I'm a little confused because the comment says it's called after a blit or color fill, but in `ddraw_surface_blt` it's apparently only called after a color fill, and not after a blit. That's a mistake; we don't need to call it after a blit because blits don't need to use the sysmem texture. Comment fixed in v2.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4781#note_57229
This merge request was approved by Jan Sikorski. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4781
participants (3)
-
Jan Sikorski (＠jsikorski) -
Zebediah Figura
-
Zebediah Figura (＠zfigura)