[PATCH 0/3] MR4781: ddraw: Make sure that sysmem surfaces remain coherent when not locked.
From: Elizabeth 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: Elizabeth 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..052a49a4eef 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 blit or 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: Elizabeth 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
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=141576 Your paranoid android. === w864 (64 bit report) === ddraw: ddraw1.c:14602: Test failed: Expect window rect (0,0)-(1024,768), got (-32000,-32000)-(-31840,-31972). ddraw1.c:14609: Test failed: Expect window rect (0,0)-(1024,768), got (-32000,-32000)-(-31840,-31972).
participants (3)
-
Elizabeth Figura -
Marvin -
Zebediah Figura (＠zfigura)