[PATCH v3 0/3] MR2404: ddraw: Two improvements for Prince of Persia 3D.
This fixes a regression introduced by 15d53761a5fbfc12fc5f9974c029dace00eab33d. -- v3: ddraw/tests: Test GetVertexBufferDesc(). https://gitlab.winehq.org/wine/wine/-/merge_requests/2404
From: Zebediah Figura <zfigura(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=44863 --- dlls/ddraw/ddraw_private.h | 1 + dlls/ddraw/device.c | 4 ++-- dlls/ddraw/vertexbuffer.c | 15 ++++++++++++++- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/dlls/ddraw/ddraw_private.h b/dlls/ddraw/ddraw_private.h index 1f0c94a9c6e..7d08ffea361 100644 --- a/dlls/ddraw/ddraw_private.h +++ b/dlls/ddraw/ddraw_private.h @@ -595,6 +595,7 @@ struct d3d_vertex_buffer DWORD size; BOOL dynamic; bool discarded; + bool sysmem; }; HRESULT d3d_vertex_buffer_create(struct d3d_vertex_buffer **buffer, struct ddraw *ddraw, diff --git a/dlls/ddraw/device.c b/dlls/ddraw/device.c index f64db3aa633..a744d361bd7 100644 --- a/dlls/ddraw/device.c +++ b/dlls/ddraw/device.c @@ -4128,7 +4128,7 @@ static HRESULT d3d_device7_DrawPrimitiveVB(IDirect3DDevice7 *iface, D3DPRIMITIVE stride = get_flexible_vertex_size(vb_impl->fvf); - if (vb_impl->Caps & D3DVBCAPS_SYSTEMMEMORY) + if (vb_impl->sysmem) { TRACE("Drawing from D3DVBCAPS_SYSTEMMEMORY vertex buffer, forwarding to DrawPrimitive().\n"); wined3d_mutex_lock(); @@ -4246,7 +4246,7 @@ static HRESULT d3d_device7_DrawIndexedPrimitiveVB(IDirect3DDevice7 *iface, vb_impl->discarded = false; - if (vb_impl->Caps & D3DVBCAPS_SYSTEMMEMORY) + if (vb_impl->sysmem) { TRACE("Drawing from D3DVBCAPS_SYSTEMMEMORY vertex buffer, forwarding to DrawIndexedPrimitive().\n"); wined3d_mutex_lock(); diff --git a/dlls/ddraw/vertexbuffer.c b/dlls/ddraw/vertexbuffer.c index 73cf0a37991..c02d7e6e354 100644 --- a/dlls/ddraw/vertexbuffer.c +++ b/dlls/ddraw/vertexbuffer.c @@ -120,7 +120,7 @@ static HRESULT d3d_vertex_buffer_create_wined3d_buffer(struct d3d_vertex_buffer if (dynamic) desc.usage |= WINED3DUSAGE_DYNAMIC; desc.bind_flags = WINED3D_BIND_VERTEX_BUFFER; - if (buffer->Caps & D3DVBCAPS_SYSTEMMEMORY) + if (buffer->sysmem) desc.access = WINED3D_RESOURCE_ACCESS_CPU | WINED3D_RESOURCE_ACCESS_MAP_R | WINED3D_RESOURCE_ACCESS_MAP_W; else desc.access = WINED3D_RESOURCE_ACCESS_GPU | WINED3D_RESOURCE_ACCESS_MAP_R | WINED3D_RESOURCE_ACCESS_MAP_W; @@ -466,6 +466,19 @@ HRESULT d3d_vertex_buffer_create(struct d3d_vertex_buffer **vertex_buf, buffer->fvf = desc->dwFVF; buffer->size = get_flexible_vertex_size(desc->dwFVF) * desc->dwNumVertices; + /* ddraw4 vertex buffers ignore DISCARD and NOOVERWRITE, even on + * pretransformed geometry, which means that a GPU-based buffer cannot + * perform well. + * + * While at least one contemporaneous card (Geforce 4) does seem to show a + * difference in its performance characteristics based on whether + * D3DVBCAPS_SYSTEMMEMORY is set, it also doesn't *improve* performance to + * use a non-SYSTEMMEMORY buffer with ddraw4. For wined3d it should always + * be better to use sysmem. + * + * This improves performance in Prince of Persia 3D. */ + buffer->sysmem = ((buffer->Caps & D3DVBCAPS_SYSTEMMEMORY) || buffer->version < 7); + wined3d_mutex_lock(); if (FAILED(hr = d3d_vertex_buffer_create_wined3d_buffer(buffer, FALSE, &buffer->wined3d_buffer))) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2404
From: Zebediah Figura <zfigura(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=44863 --- dlls/ddraw/device.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/dlls/ddraw/device.c b/dlls/ddraw/device.c index a744d361bd7..eb1ad90ee0f 100644 --- a/dlls/ddraw/device.c +++ b/dlls/ddraw/device.c @@ -3582,9 +3582,11 @@ static HRESULT d3d_device7_DrawIndexedPrimitive(IDirect3DDevice7 *iface, WORD *indices, DWORD index_count, DWORD flags) { struct d3d_device *device = impl_from_IDirect3DDevice7(iface); + unsigned int idx_size = index_count * sizeof(*indices); + unsigned short min_index = USHRT_MAX, max_index = 0; + unsigned int i; HRESULT hr; UINT stride = get_flexible_vertex_size(fvf); - UINT vtx_size = stride * vertex_count, idx_size = index_count * sizeof(*indices); UINT vb_pos, ib_pos; TRACE("iface %p, primitive_type %#x, fvf %#lx, vertices %p, vertex_count %lu, " @@ -3597,11 +3599,19 @@ static HRESULT d3d_device7_DrawIndexedPrimitive(IDirect3DDevice7 *iface, return D3D_OK; } + /* Prince of Persia 3D creates large vertex buffers but only actually uses + * a few vertices from them. This improves performance dramatically. */ + for (i = 0; i < index_count; ++i) + { + min_index = min(min_index, indices[i]); + max_index = max(max_index, indices[i]); + } + /* Set the D3DDevice's FVF */ wined3d_mutex_lock(); - if (FAILED(hr = wined3d_streaming_buffer_upload(device->wined3d_device, - &device->vertex_buffer, vertices, vtx_size, stride, &vb_pos))) + if (FAILED(hr = wined3d_streaming_buffer_upload(device->wined3d_device, &device->vertex_buffer, + (char *)vertices + (min_index * stride), (max_index + 1 - min_index) * stride, stride, &vb_pos))) goto done; if (FAILED(hr = wined3d_streaming_buffer_upload(device->wined3d_device, @@ -3618,7 +3628,7 @@ static HRESULT d3d_device7_DrawIndexedPrimitive(IDirect3DDevice7 *iface, wined3d_primitive_type_from_ddraw(primitive_type), 0); wined3d_device_apply_stateblock(device->wined3d_device, device->state); d3d_device_sync_surfaces(device); - wined3d_device_context_draw_indexed(device->immediate_context, vb_pos / stride, + wined3d_device_context_draw_indexed(device->immediate_context, (int)(vb_pos / stride) - min_index, ib_pos / sizeof(*indices), index_count, 0, 0); done: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2404
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ddraw/tests/ddraw4.c | 71 +++++++++++++++++++++++++++++++++++++++ dlls/ddraw/tests/ddraw7.c | 71 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 142 insertions(+) diff --git a/dlls/ddraw/tests/ddraw4.c b/dlls/ddraw/tests/ddraw4.c index 6cf58c3b29a..a0e4fe18c31 100644 --- a/dlls/ddraw/tests/ddraw4.c +++ b/dlls/ddraw/tests/ddraw4.c @@ -19663,6 +19663,76 @@ static void test_multiple_devices(void) DestroyWindow(window); } +static void test_vb_desc(void) +{ + IDirect3DVertexBuffer *vb; + D3DVERTEXBUFFERDESC desc; + IDirectDraw4 *ddraw; + IDirect3D3 *d3d; + ULONG refcount; + HWND window; + HRESULT hr; + + static const DWORD caps_tests[] = + { + 0, + D3DVBCAPS_WRITEONLY, + D3DVBCAPS_SYSTEMMEMORY, + D3DVBCAPS_SYSTEMMEMORY | D3DVBCAPS_WRITEONLY + }; + + static const DWORD fvf_tests[] = {D3DFVF_XYZ, D3DFVF_XYZRHW}; + + ddraw = create_ddraw(); + ok(!!ddraw, "Failed to create a ddraw object.\n"); + window = create_window(); + ok(!!window, "Failed to create a window.\n"); + hr = IDirectDraw4_SetCooperativeLevel(ddraw, window, DDSCL_EXCLUSIVE | DDSCL_FULLSCREEN); + ok(hr == DD_OK, "SetCooperativeLevel failed, hr %#lx.\n", hr); + + hr = IDirectDraw4_QueryInterface(ddraw, &IID_IDirect3D3, (void **)&d3d); + if (FAILED(hr)) + { + skip("D3D interface is not available, skipping test.\n"); + IDirectDraw4_Release(ddraw); + return; + } + + for (unsigned int i = 0; i < ARRAY_SIZE(caps_tests); ++i) + { + for (unsigned int j = 0; j < ARRAY_SIZE(fvf_tests); ++j) + { + winetest_push_context("caps %#lx, fvf %#lx", caps_tests[i], fvf_tests[j]); + + desc.dwSize = sizeof(desc); + desc.dwCaps = caps_tests[i]; + desc.dwFVF = fvf_tests[j]; + desc.dwNumVertices = 256; + + hr = IDirect3D3_CreateVertexBuffer(d3d, &desc, &vb, 0, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + memset(&desc, 0, sizeof(desc)); + hr = IDirect3DVertexBuffer_GetVertexBufferDesc(vb, &desc); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + ok(!desc.dwSize, "Got size %lu.\n", desc.dwSize); + ok(desc.dwCaps == caps_tests[i], "Got caps %#lx.\n", desc.dwCaps); + ok(desc.dwFVF == fvf_tests[j], "Got FVF %#lx.\n", desc.dwFVF); + ok(desc.dwNumVertices == 256, "Got %lu vertices.\n", desc.dwNumVertices); + + IDirect3DVertexBuffer_Release(vb); + + winetest_pop_context(); + } + } + + IDirect3D3_Release(d3d); + DestroyWindow(window); + refcount = IDirectDraw4_Release(ddraw); + ok(!refcount, "Device has %lu references left.\n", refcount); +} + START_TEST(ddraw4) { DDDEVICEIDENTIFIER identifier; @@ -19805,4 +19875,5 @@ START_TEST(ddraw4) test_filling_convention(); test_enum_devices(); test_multiple_devices(); + test_vb_desc(); } diff --git a/dlls/ddraw/tests/ddraw7.c b/dlls/ddraw/tests/ddraw7.c index 3b2bc281328..e222e798ffe 100644 --- a/dlls/ddraw/tests/ddraw7.c +++ b/dlls/ddraw/tests/ddraw7.c @@ -20119,6 +20119,76 @@ static void test_multiple_devices(void) DestroyWindow(window); } +static void test_vb_desc(void) +{ + IDirect3DVertexBuffer7 *vb; + D3DVERTEXBUFFERDESC desc; + IDirectDraw7 *ddraw; + IDirect3D7 *d3d; + ULONG refcount; + HWND window; + HRESULT hr; + + static const DWORD caps_tests[] = + { + 0, + D3DVBCAPS_WRITEONLY, + D3DVBCAPS_SYSTEMMEMORY, + D3DVBCAPS_SYSTEMMEMORY | D3DVBCAPS_WRITEONLY + }; + + static const DWORD fvf_tests[] = {D3DFVF_XYZ, D3DFVF_XYZRHW}; + + ddraw = create_ddraw(); + ok(!!ddraw, "Failed to create a ddraw object.\n"); + window = create_window(); + ok(!!window, "Failed to create a window.\n"); + hr = IDirectDraw7_SetCooperativeLevel(ddraw, window, DDSCL_EXCLUSIVE | DDSCL_FULLSCREEN); + ok(hr == DD_OK, "SetCooperativeLevel failed, hr %#lx.\n", hr); + + hr = IDirectDraw7_QueryInterface(ddraw, &IID_IDirect3D7, (void **)&d3d); + if (FAILED(hr)) + { + skip("D3D interface is not available, skipping test.\n"); + IDirectDraw7_Release(ddraw); + return; + } + + for (unsigned int i = 0; i < ARRAY_SIZE(caps_tests); ++i) + { + for (unsigned int j = 0; j < ARRAY_SIZE(fvf_tests); ++j) + { + winetest_push_context("caps %#lx, fvf %#lx", caps_tests[i], fvf_tests[j]); + + desc.dwSize = sizeof(desc); + desc.dwCaps = caps_tests[i]; + desc.dwFVF = fvf_tests[j]; + desc.dwNumVertices = 256; + + hr = IDirect3D7_CreateVertexBuffer(d3d, &desc, &vb, 0); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + memset(&desc, 0, sizeof(desc)); + hr = IDirect3DVertexBuffer7_GetVertexBufferDesc(vb, &desc); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + ok(!desc.dwSize, "Got size %lu.\n", desc.dwSize); + ok(desc.dwCaps == caps_tests[i], "Got caps %#lx.\n", desc.dwCaps); + ok(desc.dwFVF == fvf_tests[j], "Got FVF %#lx.\n", desc.dwFVF); + ok(desc.dwNumVertices == 256, "Got %lu vertices.\n", desc.dwNumVertices); + + IDirect3DVertexBuffer7_Release(vb); + + winetest_pop_context(); + } + } + + IDirect3D7_Release(d3d); + DestroyWindow(window); + refcount = IDirectDraw7_Release(ddraw); + ok(!refcount, "Device has %lu references left.\n", refcount); +} + START_TEST(ddraw7) { DDDEVICEIDENTIFIER2 identifier; @@ -20295,4 +20365,5 @@ START_TEST(ddraw7) run_for_each_device_type(test_user_memory); test_flip_3d(); test_multiple_devices(); + test_vb_desc(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2404
Looks good to me. I'll send a patch reducing dwMaxVertexCount when I have time and check what my WinXP does. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2404#note_68276
This merge request was approved by Stefan Dösinger. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2404
It may be worth including a call to GetVertexBufferDesc() in the tests. As Matteo mentioned in an earlier comment, the docs seem to suggest that omitting D3DVBCAPS_SYSTEMMEMORY allows the driver/runtime to place the buffer as it sees fit.
I mentioned earlier that GetVertexBufferDesc() never returns anything other than the flags used to create the buffer. That holds for both cards. I'll add a test the the suite.
Actually, those tests are probably helpful separate from this bug as well; we don't seem to have a lot of test coverage for GetVertexBufferDesc()... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2404#note_68609
This merge request was approved by Jan Sikorski. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2404
participants (5)
-
Henri Verbeet (＠hverbeet) -
Jan Sikorski (＠jsikorski)
-
Stefan Dösinger
-
Zebediah Figura
-
Zebediah Figura (＠zfigura)