[PATCH] wined3d: Don't discard buffer data if buffer is not bound.
Fixes geometry flickering on some objects in AION. Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/d3d9/tests/visual.c | 162 +++++++++++++++++++++++++++++++++++++++ dlls/wined3d/buffer.c | 3 + 2 files changed, 165 insertions(+) diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c index 38d2875a266..a87f7003807 100644 --- a/dlls/d3d9/tests/visual.c +++ b/dlls/d3d9/tests/visual.c @@ -27171,6 +27171,167 @@ static void test_alpha_to_coverage(void) DestroyWindow(window); } +static void test_buffer_discard(void) +{ + unsigned int i, tri_count, size, quad_count; + IDirect3DVertexBuffer9 *buffer, *buffer2; + D3DADAPTER_IDENTIFIER9 identifier; + IDirect3DDevice9 *device; + IDirect3D9 *d3d; + ULONG refcount; + HWND window; + HRESULT hr; + + static const struct quad + { + struct + { + struct vec3 position; + DWORD diffuse; + } strip[4]; + } + quad1 = + { + { + {{-1.0f, -1.0f, 0.0f}, 0xffff0000}, + {{-1.0f, 1.0f, 0.0f}, 0xff00ff00}, + {{ 1.0f, -1.0f, 0.0f}, 0xff0000ff}, + {{ 1.0f, 1.0f, 0.0f}, 0xffffffff}, + } + }, + quad2 = + { + { + {{-1.0f, -1.0f, 0.0f}, 0xffffff00}, + {{-1.0f, 1.0f, 0.0f}, 0xffffff00}, + {{ 1.0f, -1.0f, 0.0f}, 0xffffff00}, + {{ 1.0f, 1.0f, 0.0f}, 0xffffff00}, + } + }; + struct quad *quads; + + window = create_window(); + ok(!!window, "Failed to create a window.\n"); + + d3d = Direct3DCreate9(D3D_SDK_VERSION); + ok(!!d3d, "Failed to create a D3D object.\n"); + if (!(device = create_device(d3d, window, window, TRUE))) + { + skip("Failed to create a D3D device, skipping tests.\n"); + IDirect3D9_Release(d3d); + DestroyWindow(window); + return; + } + + hr = IDirect3D9_GetAdapterIdentifier(d3d, D3DADAPTER_DEFAULT, 0, &identifier); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + if (adapter_is_warp(&identifier)) + { + skip("Running on WARP, skipping test.\n"); + goto done; + } + + tri_count = 0x100; + size = (tri_count + 2) * sizeof(*quad1.strip); + quad_count = size / sizeof(quad1); + + hr = IDirect3DDevice9_CreateVertexBuffer(device, size, + D3DUSAGE_DYNAMIC, 0, D3DPOOL_DEFAULT, &buffer, NULL); + hr = IDirect3DDevice9_CreateVertexBuffer(device, size, + D3DUSAGE_DYNAMIC | D3DUSAGE_WRITEONLY, 0, D3DPOOL_DEFAULT, &buffer2, NULL); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = IDirect3DVertexBuffer9_Lock(buffer, 0, size, (void **)&quads, D3DLOCK_DISCARD); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + for (i = 0; i < quad_count; ++i) + quads[i] = quad1; + + hr = IDirect3DVertexBuffer9_Unlock(buffer); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + hr = IDirect3DDevice9_SetRenderState(device, D3DRS_LIGHTING, FALSE); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = IDirect3DDevice9_SetFVF(device, D3DFVF_XYZ | D3DFVF_DIFFUSE); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + hr = IDirect3DDevice9_SetStreamSource(device, 0, buffer, 0, sizeof(*quads->strip)); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET, 0xff0000ff, 0.0f, 0); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = IDirect3DDevice9_BeginScene(device); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = IDirect3DDevice9_DrawPrimitive(device, D3DPT_TRIANGLESTRIP, 0, tri_count); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = IDirect3DDevice9_EndScene(device); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + hr = IDirect3DVertexBuffer9_Lock(buffer, (quad_count - 1) * sizeof(quad2), + sizeof(quad2), (void **)&quads, D3DLOCK_DISCARD); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + *quads = quad2; + hr = IDirect3DVertexBuffer9_Unlock(buffer); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + hr = IDirect3DVertexBuffer9_Lock(buffer, 0, size, (void **)&quads, 0); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + /* The data has been reset for the whole buffer. */ + for (i = 0; i < quad_count - 1; ++i) + ok(memcmp(&quads[i], &quad1, sizeof(quad1)), "Quads match, i %u.\n", i); + + ok(!memcmp(&quads[i], &quad2, sizeof(quad2)), "Quads do not match, i %u.\n", i); + + for (i = 0; i < quad_count; ++i) + quads[i] = quad1; + + hr = IDirect3DVertexBuffer9_Unlock(buffer); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + hr = IDirect3DDevice9_SetStreamSource(device, 0, buffer2, 0, sizeof(*quads->strip)); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET, 0xff0000ff, 0.0f, 0); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = IDirect3DDevice9_BeginScene(device); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = IDirect3DDevice9_DrawPrimitive(device, D3DPT_TRIANGLESTRIP, 0, tri_count); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = IDirect3DDevice9_EndScene(device); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + hr = IDirect3DVertexBuffer9_Lock(buffer, (quad_count - 1) * sizeof(quad2), + sizeof(quad2), (void **)&quads, D3DLOCK_DISCARD); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + *quads = quad2; + hr = IDirect3DVertexBuffer9_Unlock(buffer); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + hr = IDirect3DVertexBuffer9_Lock(buffer, 0, size, (void **)&quads, 0); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + /* The data are preserved when mapping with MAP_DISCARD on unbound buffer. */ + for (i = 0; i < quad_count - 1; ++i) + ok(!memcmp(&quads[i], &quad1, sizeof(quad1)), "Quads do not match, i %u.\n", i); + + ok(!memcmp(&quads[i], &quad2, sizeof(quad2)), "Quads do not match, i %u.\n", i); + + hr = IDirect3DVertexBuffer9_Unlock(buffer); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + hr = IDirect3DDevice9_Present(device, NULL, NULL, NULL, NULL); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + IDirect3DVertexBuffer9_Release(buffer); + IDirect3DVertexBuffer9_Release(buffer2); + +done: + refcount = IDirect3DDevice9_Release(device); + ok(!refcount, "Device has %u references left.\n", refcount); + IDirect3D9_Release(d3d); + DestroyWindow(window); +} + START_TEST(visual) { D3DADAPTER_IDENTIFIER9 identifier; @@ -27319,4 +27480,5 @@ START_TEST(visual) test_draw_mapped_buffer(); test_sample_attached_rendertarget(); test_alpha_to_coverage(); + test_buffer_discard(); } diff --git a/dlls/wined3d/buffer.c b/dlls/wined3d/buffer.c index b32d0160cc1..9d803efe758 100644 --- a/dlls/wined3d/buffer.c +++ b/dlls/wined3d/buffer.c @@ -1049,6 +1049,9 @@ static HRESULT buffer_resource_sub_resource_map(struct wined3d_resource *resourc { context = context_acquire(device, NULL, 0); + if (!buffer->resource.bind_count) + flags &= ~WINED3D_MAP_DISCARD; + if (flags & WINED3D_MAP_DISCARD) wined3d_buffer_validate_location(buffer, WINED3D_LOCATION_BUFFER); else -- 2.26.2
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=75057 Your paranoid android. === debiant (32 bit report) === d3d9: visual.c:26850: Test failed: Quads match, i 0. visual.c:26850: Test failed: Quads match, i 1. visual.c:26850: Test failed: Quads match, i 2. visual.c:26850: Test failed: Quads match, i 3. visual.c:26850: Test failed: Quads match, i 4. visual.c:26850: Test failed: Quads match, i 5. visual.c:26850: Test failed: Quads match, i 6. visual.c:26850: Test failed: Quads match, i 7. visual.c:26850: Test failed: Quads match, i 8. visual.c:26850: Test failed: Quads match, i 9. visual.c:26850: Test failed: Quads match, i 10. visual.c:26850: Test failed: Quads match, i 11. visual.c:26850: Test failed: Quads match, i 12. visual.c:26850: Test failed: Quads match, i 13. visual.c:26850: Test failed: Quads match, i 14. visual.c:26850: Test failed: Quads match, i 15. visual.c:26850: Test failed: Quads match, i 16. visual.c:26850: Test failed: Quads match, i 17. visual.c:26850: Test failed: Quads match, i 18. visual.c:26850: Test failed: Quads match, i 19. visual.c:26850: Test failed: Quads match, i 20. visual.c:26850: Test failed: Quads match, i 21. visual.c:26850: Test failed: Quads match, i 22. visual.c:26850: Test failed: Quads match, i 23. visual.c:26850: Test failed: Quads match, i 24. visual.c:26850: Test failed: Quads match, i 25. visual.c:26850: Test failed: Quads match, i 26. visual.c:26850: Test failed: Quads match, i 27. visual.c:26850: Test failed: Quads match, i 28. visual.c:26850: Test failed: Quads match, i 29. visual.c:26850: Test failed: Quads match, i 30. visual.c:26850: Test failed: Quads match, i 31. visual.c:26850: Test failed: Quads match, i 32. visual.c:26850: Test failed: Quads match, i 33. visual.c:26850: Test failed: Quads match, i 34. visual.c:26850: Test failed: Quads match, i 35. visual.c:26850: Test failed: Quads match, i 36. visual.c:26850: Test failed: Quads match, i 37. visual.c:26850: Test failed: Quads match, i 38. visual.c:26850: Test failed: Quads match, i 39. visual.c:26850: Test failed: Quads match, i 40. visual.c:26850: Test failed: Quads match, i 41. visual.c:26850: Test failed: Quads match, i 42. visual.c:26850: Test failed: Quads match, i 43. visual.c:26850: Test failed: Quads match, i 44. visual.c:26850: Test failed: Quads match, i 45. visual.c:26850: Test failed: Quads match, i 46. visual.c:26850: Test failed: Quads match, i 47. visual.c:26850: Test failed: Quads match, i 48. visual.c:26850: Test failed: Quads match, i 49. visual.c:26850: Test failed: Quads match, i 50. visual.c:26850: Test failed: Quads match, i 51. visual.c:26850: Test failed: Quads match, i 52. visual.c:26850: Test failed: Quads match, i 53. visual.c:26850: Test failed: Quads match, i 54. visual.c:26850: Test failed: Quads match, i 55. visual.c:26850: Test failed: Quads match, i 56. visual.c:26850: Test failed: Quads match, i 57. visual.c:26850: Test failed: Quads match, i 58. visual.c:26850: Test failed: Quads match, i 59. visual.c:26850: Test failed: Quads match, i 60. visual.c:26850: Test failed: Quads match, i 61. visual.c:26850: Test failed: Quads match, i 62. === debiant (32 bit French report) === d3d9: visual.c:26850: Test failed: Quads match, i 0. visual.c:26850: Test failed: Quads match, i 1. visual.c:26850: Test failed: Quads match, i 2. visual.c:26850: Test failed: Quads match, i 3. visual.c:26850: Test failed: Quads match, i 4. visual.c:26850: Test failed: Quads match, i 5. visual.c:26850: Test failed: Quads match, i 6. visual.c:26850: Test failed: Quads match, i 7. visual.c:26850: Test failed: Quads match, i 8. visual.c:26850: Test failed: Quads match, i 9. visual.c:26850: Test failed: Quads match, i 10. visual.c:26850: Test failed: Quads match, i 11. visual.c:26850: Test failed: Quads match, i 12. visual.c:26850: Test failed: Quads match, i 13. visual.c:26850: Test failed: Quads match, i 14. visual.c:26850: Test failed: Quads match, i 15. visual.c:26850: Test failed: Quads match, i 16. visual.c:26850: Test failed: Quads match, i 17. visual.c:26850: Test failed: Quads match, i 18. visual.c:26850: Test failed: Quads match, i 19. visual.c:26850: Test failed: Quads match, i 20. visual.c:26850: Test failed: Quads match, i 21. visual.c:26850: Test failed: Quads match, i 22. visual.c:26850: Test failed: Quads match, i 23. visual.c:26850: Test failed: Quads match, i 24. visual.c:26850: Test failed: Quads match, i 25. visual.c:26850: Test failed: Quads match, i 26. visual.c:26850: Test failed: Quads match, i 27. visual.c:26850: Test failed: Quads match, i 28. visual.c:26850: Test failed: Quads match, i 29. visual.c:26850: Test failed: Quads match, i 30. visual.c:26850: Test failed: Quads match, i 31. visual.c:26850: Test failed: Quads match, i 32. visual.c:26850: Test failed: Quads match, i 33. visual.c:26850: Test failed: Quads match, i 34. visual.c:26850: Test failed: Quads match, i 35. visual.c:26850: Test failed: Quads match, i 36. visual.c:26850: Test failed: Quads match, i 37. visual.c:26850: Test failed: Quads match, i 38. visual.c:26850: Test failed: Quads match, i 39. visual.c:26850: Test failed: Quads match, i 40. visual.c:26850: Test failed: Quads match, i 41. visual.c:26850: Test failed: Quads match, i 42. visual.c:26850: Test failed: Quads match, i 43. visual.c:26850: Test failed: Quads match, i 44. visual.c:26850: Test failed: Quads match, i 45. visual.c:26850: Test failed: Quads match, i 46. visual.c:26850: Test failed: Quads match, i 47. visual.c:26850: Test failed: Quads match, i 48. visual.c:26850: Test failed: Quads match, i 49. visual.c:26850: Test failed: Quads match, i 50. visual.c:26850: Test failed: Quads match, i 51. visual.c:26850: Test failed: Quads match, i 52. visual.c:26850: Test failed: Quads match, i 53. visual.c:26850: Test failed: Quads match, i 54. visual.c:26850: Test failed: Quads match, i 55. visual.c:26850: Test failed: Quads match, i 56. visual.c:26850: Test failed: Quads match, i 57. visual.c:26850: Test failed: Quads match, i 58. visual.c:26850: Test failed: Quads match, i 59. visual.c:26850: Test failed: Quads match, i 60. visual.c:26850: Test failed: Quads match, i 61. visual.c:26850: Test failed: Quads match, i 62. === debiant (32 bit Japanese:Japan report) === d3d9: visual.c:26850: Test failed: Quads match, i 0. visual.c:26850: Test failed: Quads match, i 1. visual.c:26850: Test failed: Quads match, i 2. visual.c:26850: Test failed: Quads match, i 3. visual.c:26850: Test failed: Quads match, i 4. visual.c:26850: Test failed: Quads match, i 5. visual.c:26850: Test failed: Quads match, i 6. visual.c:26850: Test failed: Quads match, i 7. visual.c:26850: Test failed: Quads match, i 8. visual.c:26850: Test failed: Quads match, i 9. visual.c:26850: Test failed: Quads match, i 10. visual.c:26850: Test failed: Quads match, i 11. visual.c:26850: Test failed: Quads match, i 12. visual.c:26850: Test failed: Quads match, i 13. visual.c:26850: Test failed: Quads match, i 14. visual.c:26850: Test failed: Quads match, i 15. visual.c:26850: Test failed: Quads match, i 16. visual.c:26850: Test failed: Quads match, i 17. visual.c:26850: Test failed: Quads match, i 18. visual.c:26850: Test failed: Quads match, i 19. visual.c:26850: Test failed: Quads match, i 20. visual.c:26850: Test failed: Quads match, i 21. visual.c:26850: Test failed: Quads match, i 22. visual.c:26850: Test failed: Quads match, i 23. visual.c:26850: Test failed: Quads match, i 24. visual.c:26850: Test failed: Quads match, i 25. visual.c:26850: Test failed: Quads match, i 26. visual.c:26850: Test failed: Quads match, i 27. visual.c:26850: Test failed: Quads match, i 28. visual.c:26850: Test failed: Quads match, i 29. visual.c:26850: Test failed: Quads match, i 30. visual.c:26850: Test failed: Quads match, i 31. visual.c:26850: Test failed: Quads match, i 32. visual.c:26850: Test failed: Quads match, i 33. visual.c:26850: Test failed: Quads match, i 34. visual.c:26850: Test failed: Quads match, i 35. visual.c:26850: Test failed: Quads match, i 36. visual.c:26850: Test failed: Quads match, i 37. visual.c:26850: Test failed: Quads match, i 38. visual.c:26850: Test failed: Quads match, i 39. visual.c:26850: Test failed: Quads match, i 40. visual.c:26850: Test failed: Quads match, i 41. visual.c:26850: Test failed: Quads match, i 42. visual.c:26850: Test failed: Quads match, i 43. visual.c:26850: Test failed: Quads match, i 44. visual.c:26850: Test failed: Quads match, i 45. visual.c:26850: Test failed: Quads match, i 46. visual.c:26850: Test failed: Quads match, i 47. visual.c:26850: Test failed: Quads match, i 48. visual.c:26850: Test failed: Quads match, i 49. visual.c:26850: Test failed: Quads match, i 50. visual.c:26850: Test failed: Quads match, i 51. visual.c:26850: Test failed: Quads match, i 52. visual.c:26850: Test failed: Quads match, i 53. visual.c:26850: Test failed: Quads match, i 54. visual.c:26850: Test failed: Quads match, i 55. visual.c:26850: Test failed: Quads match, i 56. visual.c:26850: Test failed: Quads match, i 57. visual.c:26850: Test failed: Quads match, i 58. visual.c:26850: Test failed: Quads match, i 59. visual.c:26850: Test failed: Quads match, i 60. visual.c:26850: Test failed: Quads match, i 61. visual.c:26850: Test failed: Quads match, i 62. === debiant (32 bit Chinese:China report) === d3d9: visual.c:26850: Test failed: Quads match, i 0. visual.c:26850: Test failed: Quads match, i 1. visual.c:26850: Test failed: Quads match, i 2. visual.c:26850: Test failed: Quads match, i 3. visual.c:26850: Test failed: Quads match, i 4. visual.c:26850: Test failed: Quads match, i 5. visual.c:26850: Test failed: Quads match, i 6. visual.c:26850: Test failed: Quads match, i 7. visual.c:26850: Test failed: Quads match, i 8. visual.c:26850: Test failed: Quads match, i 9. visual.c:26850: Test failed: Quads match, i 10. visual.c:26850: Test failed: Quads match, i 11. visual.c:26850: Test failed: Quads match, i 12. visual.c:26850: Test failed: Quads match, i 13. visual.c:26850: Test failed: Quads match, i 14. visual.c:26850: Test failed: Quads match, i 15. visual.c:26850: Test failed: Quads match, i 16. visual.c:26850: Test failed: Quads match, i 17. visual.c:26850: Test failed: Quads match, i 18. visual.c:26850: Test failed: Quads match, i 19. visual.c:26850: Test failed: Quads match, i 20. visual.c:26850: Test failed: Quads match, i 21. visual.c:26850: Test failed: Quads match, i 22. visual.c:26850: Test failed: Quads match, i 23. visual.c:26850: Test failed: Quads match, i 24. visual.c:26850: Test failed: Quads match, i 25. visual.c:26850: Test failed: Quads match, i 26. visual.c:26850: Test failed: Quads match, i 27. visual.c:26850: Test failed: Quads match, i 28. visual.c:26850: Test failed: Quads match, i 29. visual.c:26850: Test failed: Quads match, i 30. visual.c:26850: Test failed: Quads match, i 31. visual.c:26850: Test failed: Quads match, i 32. visual.c:26850: Test failed: Quads match, i 33. visual.c:26850: Test failed: Quads match, i 34. visual.c:26850: Test failed: Quads match, i 35. visual.c:26850: Test failed: Quads match, i 36. visual.c:26850: Test failed: Quads match, i 37. visual.c:26850: Test failed: Quads match, i 38. visual.c:26850: Test failed: Quads match, i 39. visual.c:26850: Test failed: Quads match, i 40. visual.c:26850: Test failed: Quads match, i 41. visual.c:26850: Test failed: Quads match, i 42. visual.c:26850: Test failed: Quads match, i 43. visual.c:26850: Test failed: Quads match, i 44. visual.c:26850: Test failed: Quads match, i 45. visual.c:26850: Test failed: Quads match, i 46. visual.c:26850: Test failed: Quads match, i 47. visual.c:26850: Test failed: Quads match, i 48. visual.c:26850: Test failed: Quads match, i 49. visual.c:26850: Test failed: Quads match, i 50. visual.c:26850: Test failed: Quads match, i 51. visual.c:26850: Test failed: Quads match, i 52. visual.c:26850: Test failed: Quads match, i 53. visual.c:26850: Test failed: Quads match, i 54. visual.c:26850: Test failed: Quads match, i 55. visual.c:26850: Test failed: Quads match, i 56. visual.c:26850: Test failed: Quads match, i 57. visual.c:26850: Test failed: Quads match, i 58. visual.c:26850: Test failed: Quads match, i 59. visual.c:26850: Test failed: Quads match, i 60. visual.c:26850: Test failed: Quads match, i 61. visual.c:26850: Test failed: Quads match, i 62. === debiant (32 bit WoW report) === d3d9: visual.c:26850: Test failed: Quads match, i 0. visual.c:26850: Test failed: Quads match, i 1. visual.c:26850: Test failed: Quads match, i 2. visual.c:26850: Test failed: Quads match, i 3. visual.c:26850: Test failed: Quads match, i 4. visual.c:26850: Test failed: Quads match, i 5. visual.c:26850: Test failed: Quads match, i 6. visual.c:26850: Test failed: Quads match, i 7. visual.c:26850: Test failed: Quads match, i 8. visual.c:26850: Test failed: Quads match, i 9. visual.c:26850: Test failed: Quads match, i 10. visual.c:26850: Test failed: Quads match, i 11. visual.c:26850: Test failed: Quads match, i 12. visual.c:26850: Test failed: Quads match, i 13. visual.c:26850: Test failed: Quads match, i 14. visual.c:26850: Test failed: Quads match, i 15. visual.c:26850: Test failed: Quads match, i 16. visual.c:26850: Test failed: Quads match, i 17. visual.c:26850: Test failed: Quads match, i 18. visual.c:26850: Test failed: Quads match, i 19. visual.c:26850: Test failed: Quads match, i 20. visual.c:26850: Test failed: Quads match, i 21. visual.c:26850: Test failed: Quads match, i 22. visual.c:26850: Test failed: Quads match, i 23. visual.c:26850: Test failed: Quads match, i 24. visual.c:26850: Test failed: Quads match, i 25. visual.c:26850: Test failed: Quads match, i 26. visual.c:26850: Test failed: Quads match, i 27. visual.c:26850: Test failed: Quads match, i 28. visual.c:26850: Test failed: Quads match, i 29. visual.c:26850: Test failed: Quads match, i 30. visual.c:26850: Test failed: Quads match, i 31. visual.c:26850: Test failed: Quads match, i 32. visual.c:26850: Test failed: Quads match, i 33. visual.c:26850: Test failed: Quads match, i 34. visual.c:26850: Test failed: Quads match, i 35. visual.c:26850: Test failed: Quads match, i 36. visual.c:26850: Test failed: Quads match, i 37. visual.c:26850: Test failed: Quads match, i 38. visual.c:26850: Test failed: Quads match, i 39. visual.c:26850: Test failed: Quads match, i 40. visual.c:26850: Test failed: Quads match, i 41. visual.c:26850: Test failed: Quads match, i 42. visual.c:26850: Test failed: Quads match, i 43. visual.c:26850: Test failed: Quads match, i 44. visual.c:26850: Test failed: Quads match, i 45. visual.c:26850: Test failed: Quads match, i 46. visual.c:26850: Test failed: Quads match, i 47. visual.c:26850: Test failed: Quads match, i 48. visual.c:26850: Test failed: Quads match, i 49. visual.c:26850: Test failed: Quads match, i 50. visual.c:26850: Test failed: Quads match, i 51. visual.c:26850: Test failed: Quads match, i 52. visual.c:26850: Test failed: Quads match, i 53. visual.c:26850: Test failed: Quads match, i 54. visual.c:26850: Test failed: Quads match, i 55. visual.c:26850: Test failed: Quads match, i 56. visual.c:26850: Test failed: Quads match, i 57. visual.c:26850: Test failed: Quads match, i 58. visual.c:26850: Test failed: Quads match, i 59. visual.c:26850: Test failed: Quads match, i 60. visual.c:26850: Test failed: Quads match, i 61. visual.c:26850: Test failed: Quads match, i 62. === debiant (64 bit WoW report) === d3d9: visual.c:26850: Test failed: Quads match, i 0. visual.c:26850: Test failed: Quads match, i 1. visual.c:26850: Test failed: Quads match, i 2. visual.c:26850: Test failed: Quads match, i 3. visual.c:26850: Test failed: Quads match, i 4. visual.c:26850: Test failed: Quads match, i 5. visual.c:26850: Test failed: Quads match, i 6. visual.c:26850: Test failed: Quads match, i 7. visual.c:26850: Test failed: Quads match, i 8. visual.c:26850: Test failed: Quads match, i 9. visual.c:26850: Test failed: Quads match, i 10. visual.c:26850: Test failed: Quads match, i 11. visual.c:26850: Test failed: Quads match, i 12. visual.c:26850: Test failed: Quads match, i 13. visual.c:26850: Test failed: Quads match, i 14. visual.c:26850: Test failed: Quads match, i 15. visual.c:26850: Test failed: Quads match, i 16. visual.c:26850: Test failed: Quads match, i 17. visual.c:26850: Test failed: Quads match, i 18. visual.c:26850: Test failed: Quads match, i 19. visual.c:26850: Test failed: Quads match, i 20. visual.c:26850: Test failed: Quads match, i 21. visual.c:26850: Test failed: Quads match, i 22. visual.c:26850: Test failed: Quads match, i 23. visual.c:26850: Test failed: Quads match, i 24. visual.c:26850: Test failed: Quads match, i 25. visual.c:26850: Test failed: Quads match, i 26. visual.c:26850: Test failed: Quads match, i 27. visual.c:26850: Test failed: Quads match, i 28. visual.c:26850: Test failed: Quads match, i 29. visual.c:26850: Test failed: Quads match, i 30. visual.c:26850: Test failed: Quads match, i 31. visual.c:26850: Test failed: Quads match, i 32. visual.c:26850: Test failed: Quads match, i 33. visual.c:26850: Test failed: Quads match, i 34. visual.c:26850: Test failed: Quads match, i 35. visual.c:26850: Test failed: Quads match, i 36. visual.c:26850: Test failed: Quads match, i 37. visual.c:26850: Test failed: Quads match, i 38. visual.c:26850: Test failed: Quads match, i 39. visual.c:26850: Test failed: Quads match, i 40. visual.c:26850: Test failed: Quads match, i 41. visual.c:26850: Test failed: Quads match, i 42. visual.c:26850: Test failed: Quads match, i 43. visual.c:26850: Test failed: Quads match, i 44. visual.c:26850: Test failed: Quads match, i 45. visual.c:26850: Test failed: Quads match, i 46. visual.c:26850: Test failed: Quads match, i 47. visual.c:26850: Test failed: Quads match, i 48. visual.c:26850: Test failed: Quads match, i 49. visual.c:26850: Test failed: Quads match, i 50. visual.c:26850: Test failed: Quads match, i 51. visual.c:26850: Test failed: Quads match, i 52. visual.c:26850: Test failed: Quads match, i 53. visual.c:26850: Test failed: Quads match, i 54. visual.c:26850: Test failed: Quads match, i 55. visual.c:26850: Test failed: Quads match, i 56. visual.c:26850: Test failed: Quads match, i 57. visual.c:26850: Test failed: Quads match, i 58. visual.c:26850: Test failed: Quads match, i 59. visual.c:26850: Test failed: Quads match, i 60. visual.c:26850: Test failed: Quads match, i 61. visual.c:26850: Test failed: Quads match, i 62.
Am 08.07.2020 um 22:44 schrieb Paul Gofman <pgofman(a)codeweavers.com>:
+ hr = IDirect3DVertexBuffer9_Lock(buffer, 0, size, (void **)&quads, 0); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + /* The data has been reset for the whole buffer. */ + for (i = 0; i < quad_count - 1; ++i) + ok(memcmp(&quads[i], &quad1, sizeof(quad1)), "Quads match, i %u.\n", i); + + ok(!memcmp(&quads[i], &quad2, sizeof(quad2)), "Quads do not match, i %u.\n", i); + + for (i = 0; i < quad_count; ++i) + quads[i] = quad1; + + hr = IDirect3DVertexBuffer9_Unlock(buffer); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + hr = IDirect3DDevice9_SetStreamSource(device, 0, buffer2, 0, sizeof(*quads->strip)); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
What happens if you draw something from 'buffer' between the Unlock and SetStreamSource? See test_vb_discard() in ddraw7.c, although I think Henri ported the test to d3d8/9 at some point. I couldn't find it on a quick search though. What happens if you bind the a to a stream that is not used by the vertex declaration?
On 7/9/20 01:36, Stefan Dösinger wrote:
Am 08.07.2020 um 22:44 schrieb Paul Gofman <pgofman(a)codeweavers.com <mailto:pgofman(a)codeweavers.com>>:
+ hr = IDirect3DVertexBuffer9_Lock(buffer, 0, size, (void **)&quads, 0); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + /* The data has been reset for the whole buffer. */ + for (i = 0; i < quad_count - 1; ++i) + ok(memcmp(&quads[i], &quad1, sizeof(quad1)), "Quads match, i %u.\n", i); + + ok(!memcmp(&quads[i], &quad2, sizeof(quad2)), "Quads do not match, i %u.\n", i); + + for (i = 0; i < quad_count; ++i) + quads[i] = quad1; + + hr = IDirect3DVertexBuffer9_Unlock(buffer); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + hr = IDirect3DDevice9_SetStreamSource(device, 0, buffer2, 0, sizeof(*quads->strip)); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr);
What happens if you draw something from 'buffer' between the Unlock and SetStreamSource?
That doesn't change anything on Windows. On Wine the result with my patch apparently depends on whether that draw is completed before the buffer is locked with MAP_DISCARD.
See test_vb_discard() in ddraw7.c, although I think Henri ported the test to d3d8/9 at some point. I couldn't find it on a quick search though.
That is a bit different case which is handled by WINED3D_BUFFER_DISCARD buffer flag in wined3d. If there are multiple discards not interleaved by buffer use only the first discard can actually discard the buffer data.
What happens if you bind the a to a stream that is not used by the vertex declaration?
That changes nothing on Windows for me but makes the buffer data discarded in Wine. That is probably harder to bring in line as it would need to track which buffers are actually used in the pipeline, but I don't know anything which depends on that yet.
On Wed, Jul 8, 2020 at 10:45 PM Paul Gofman <pgofman(a)codeweavers.com> wrote:
Fixes geometry flickering on some objects in AION.
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com>
Checking the discarded portion of the buffers (in the cases where the discard actually happens) is a bit questionable. I'm curious which GPUs (or, better, GPU vendors) did you test on Windows. I wouldn't be surprised if this turned out to be driver dependent.
On 08/07/2020 22:44, Paul Gofman wrote:
hr = IDirect3DVertexBuffer9_Lock(buffer, (quad_count - 1) * sizeof(quad2), + sizeof(quad2), (void **)&quads, D3DLOCK_DISCARD); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + *quads = quad2; + hr = IDirect3DVertexBuffer9_Unlock(buffer); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + hr = IDirect3DVertexBuffer9_Lock(buffer, 0, size, (void **)&quads, 0); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + /* The data has been reset for the whole buffer. */ + for (i = 0; i < quad_count - 1; ++i) + ok(memcmp(&quads[i], &quad1, sizeof(quad1)), "Quads match, i %u.\n", i); + + ok(!memcmp(&quads[i], &quad2, sizeof(quad2)), "Quads do not match, i %u.\n", i); + + for (i = 0; i < quad_count; ++i) + quads[i] = quad1; + + hr = IDirect3DVertexBuffer9_Unlock(buffer); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + hr = IDirect3DDevice9_SetStreamSource(device, 0, buffer2, 0, sizeof(*quads->strip)); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET, 0xff0000ff, 0.0f, 0); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = IDirect3DDevice9_BeginScene(device); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = IDirect3DDevice9_DrawPrimitive(device, D3DPT_TRIANGLESTRIP, 0, tri_count); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + hr = IDirect3DDevice9_EndScene(device); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + hr = IDirect3DVertexBuffer9_Lock(buffer, (quad_count - 1) * sizeof(quad2), + sizeof(quad2), (void **)&quads, D3DLOCK_DISCARD); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + *quads = quad2; + hr = IDirect3DVertexBuffer9_Unlock(buffer); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + hr = IDirect3DVertexBuffer9_Lock(buffer, 0, size, (void **)&quads, 0); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + /* The data are preserved when mapping with MAP_DISCARD on unbound buffer. */ + for (i = 0; i < quad_count - 1; ++i) + ok(!memcmp(&quads[i], &quad1, sizeof(quad1)), "Quads do not match, i %u.\n", i); + + ok(!memcmp(&quads[i], &quad2, sizeof(quad2)), "Quads do not match, i %u.\n", i);
Hi, I have another explanation for this behaviour: For the first DISCARD you try, the buffer backing is busy because it is doing to be used for rendering not yet finished. Thus the DISCARD does actually do something. Then when you map for reading the buffer it flushes everything and rendering has to finish. Thus for the next DISCARD, the driver finds that the driver backing is not busy, it decides to keep it. Definitely you should discard buffer data if the buffer is not bound. It will happen in a lot of games. A buffer not bound doesn't mean it is not used by the GPU. What you really want is to not discard when the buffer is not bound nor used in any pending rendering. That is of course much harder to detect. I would bet your application just needs the DISCARD to do nothing before the buffer is used for rendering for the first time. Yours, Axel Davy
On Thu, 9 Jul 2020 at 13:47, Axel Davy <davyaxel0(a)gmail.com> wrote:
I have another explanation for this behaviour:
For the first DISCARD you try, the buffer backing is busy because it is doing to be used for rendering not yet finished. Thus the DISCARD does actually do something.
Then when you map for reading the buffer it flushes everything and rendering has to finish. Thus for the next DISCARD, the driver finds that the driver backing is not busy, it decides to keep it.
Definitely you should discard buffer data if the buffer is not bound. It will happen in a lot of games. A buffer not bound doesn't mean it is not used by the GPU. What you really want is to not discard when the buffer is not bound nor used in any pending rendering. That is of course much harder to detect. I would bet your application just needs the DISCARD to do nothing before the buffer is used for rendering for the first time.
Yes, what Axel said. I suspect the application is essentially depending on an optimisation to not bother discarding the buffer if we wouldn't have to wait to map it. I.e., similar to the redundant discard filtering we do a bit further down in buffer_resource_sub_resource_map(), and similar to the Vulkan discard handling we do in adapter_vk_map_bo_address(). In its current form, this patch will introduce stalls when mapping buffers that have been unbound, but are still in use by the GPU.
On 7/9/20 13:11, Henri Verbeet wrote:
On Thu, 9 Jul 2020 at 13:47, Axel Davy <davyaxel0(a)gmail.com> wrote:
I have another explanation for this behaviour:
For the first DISCARD you try, the buffer backing is busy because it is doing to be used for rendering not yet finished. Thus the DISCARD does actually do something.
Then when you map for reading the buffer it flushes everything and rendering has to finish. Thus for the next DISCARD, the driver finds that the driver backing is not busy, it decides to keep it.
Definitely you should discard buffer data if the buffer is not bound. It will happen in a lot of games. A buffer not bound doesn't mean it is not used by the GPU. What you really want is to not discard when the buffer is not bound nor used in any pending rendering. That is of course much harder to detect. I would bet your application just needs the DISCARD to do nothing before the buffer is used for rendering for the first time.
Yes, what Axel said. I suspect the application is essentially depending on an optimisation to not bother discarding the buffer if we wouldn't have to wait to map it. I.e., similar to the redundant discard filtering we do a bit further down in buffer_resource_sub_resource_map(), and similar to the Vulkan discard handling we do in adapter_vk_map_bo_address(). In its current form, this patch will introduce stalls when mapping buffers that have been unbound, but are still in use by the GPU.
Yes, dropping discard flag for any unbound buffer maps was a bad idea. I don't see a good way to check if buffer is actually used in GL during mapping, so I think I should leave it for now until persistent buffer mapping changes are there.
On 09/07/2020 13:29, Paul Gofman wrote:
On 7/9/20 13:11, Henri Verbeet wrote:
On Thu, 9 Jul 2020 at 13:47, Axel Davy <davyaxel0(a)gmail.com> wrote:
I have another explanation for this behaviour:
For the first DISCARD you try, the buffer backing is busy because it is doing to be used for rendering not yet finished. Thus the DISCARD does actually do something.
Then when you map for reading the buffer it flushes everything and rendering has to finish. Thus for the next DISCARD, the driver finds that the driver backing is not busy, it decides to keep it.
Definitely you should discard buffer data if the buffer is not bound. It will happen in a lot of games. A buffer not bound doesn't mean it is not used by the GPU. What you really want is to not discard when the buffer is not bound nor used in any pending rendering. That is of course much harder to detect. I would bet your application just needs the DISCARD to do nothing before the buffer is used for rendering for the first time.
Yes, what Axel said. I suspect the application is essentially depending on an optimisation to not bother discarding the buffer if we wouldn't have to wait to map it. I.e., similar to the redundant discard filtering we do a bit further down in buffer_resource_sub_resource_map(), and similar to the Vulkan discard handling we do in adapter_vk_map_bo_address(). In its current form, this patch will introduce stalls when mapping buffers that have been unbound, but are still in use by the GPU. Yes, dropping discard flag for any unbound buffer maps was a bad idea. I don't see a good way to check if buffer is actually used in GL during mapping, so I think I should leave it for now until persistent buffer mapping changes are there.
One way would be to introduce a frame count and remember the last frame the buffer was bound. If the frame count difference with the current frame is more than, say, 4, it is definitely idle. Also mapping for read a buffer without RAM backing would mean all the other buffers become idle. Yours, Axel Davy
On 7/10/20 08:55, Axel Davy wrote:
On 09/07/2020 13:29, Paul Gofman wrote:
On 7/9/20 13:11, Henri Verbeet wrote:
On Thu, 9 Jul 2020 at 13:47, Axel Davy <davyaxel0(a)gmail.com> wrote:
I have another explanation for this behaviour:
For the first DISCARD you try, the buffer backing is busy because it is doing to be used for rendering not yet finished. Thus the DISCARD does actually do something.
Then when you map for reading the buffer it flushes everything and rendering has to finish. Thus for the next DISCARD, the driver finds that the driver backing is not busy, it decides to keep it.
Definitely you should discard buffer data if the buffer is not bound. It will happen in a lot of games. A buffer not bound doesn't mean it is not used by the GPU. What you really want is to not discard when the buffer is not bound nor used in any pending rendering. That is of course much harder to detect. I would bet your application just needs the DISCARD to do nothing before the buffer is used for rendering for the first time.
Yes, what Axel said. I suspect the application is essentially depending on an optimisation to not bother discarding the buffer if we wouldn't have to wait to map it. I.e., similar to the redundant discard filtering we do a bit further down in buffer_resource_sub_resource_map(), and similar to the Vulkan discard handling we do in adapter_vk_map_bo_address(). In its current form, this patch will introduce stalls when mapping buffers that have been unbound, but are still in use by the GPU. Yes, dropping discard flag for any unbound buffer maps was a bad idea. I don't see a good way to check if buffer is actually used in GL during mapping, so I think I should leave it for now until persistent buffer mapping changes are there.
One way would be to introduce a frame count and remember the last frame the buffer was bound. If the frame count difference with the current frame is more than, say, 4, it is definitely idle.
Also mapping for read a buffer without RAM backing would mean all the other buffers become idle.
Yes, sure, there should be some ways to introduce a workaround to make AION happy in the current buffer management logic, but probably a better solution is switching to persistent buffer mapping and overall buffer mapping logic redesign which AFAIK is being worked on and which I guess will obsolete such solutions. Also, the problem should probably not be present with the Vulkan backend.
On 10/07/2020 13:08, Paul Gofman wrote:
Yes, sure, there should be some ways to introduce a workaround to make AION happy in the current buffer management logic, but probably a better solution is switching to persistent buffer mapping and overall buffer mapping logic redesign which AFAIK is being worked on and which I guess will obsolete such solutions. Also, the problem should probably not be present with the Vulkan backend.
I'm not sure how persistent buffer mapping does help with this, but let me add another piece of information: The d3d9 vertex/index buffer memory management logic changed a lot between windows xp and vista+. It is also affected whether you have a single core or a multicore cpu (windows's d3d9 csmt equivalent seems to only activate if the app has access to at least 2 cores). I'm saying this to say that if the game relies on this behaviour, it is likely to be something the runtime does willingly since xp. In the case of buffer locks, if you check the driver ddi you will notice efforts to be able to process locks without needing to block or flush the csmt thread. In other words, the DISCARD can be processed before the internal gpu driver begins to process the calls needing the previous buffer data. Thus if you observe a behaviour where DISCARD doesn't change the internal buffer, it is quite likely this behaviour is handled in the d3d9 runtime, not the internal gpu driver. If so, the logic behind this optimization must be quite simple, and probably doesn't check the buffer is busy or not. In any case, I would suggest to check the behaviour on all vendors before going further. Yours, Axel Davy
participants (6)
-
Axel Davy -
Henri Verbeet -
Marvin -
Matteo Bruni -
Paul Gofman -
Stefan Dösinger