d3d9 does not do instancing for not-indexed draw calls on Windows
Original issue in DXVK repo: https://github.com/doitsujin/dxvk/issues/3157
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54570
-- v6: d3d9: Always draw only a single instance for non-indexed draws. d3d9/tests: Add test for not-indexed instancing drawing
From: Illia Polishchuk illia.a.polishchuk@globallogic.com
Windows d3d9 always draws only a single instance for non-indexed draws --- dlls/d3d9/tests/visual.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c index 230f7deabb2..b69868bc992 100644 --- a/dlls/d3d9/tests/visual.c +++ b/dlls/d3d9/tests/visual.c @@ -13090,6 +13090,34 @@ static void stream_test(void) hr = IDirect3DDevice9_SetStreamSource(device, act.strInstance, vb3, 0, sizeof(instancepos[0])); ok(SUCCEEDED(hr), "Failed to set stream source, hr %#lx.\n", hr);
+ /* Not indexed draw test. Only single instance should be drawed */ + hr = IDirect3DDevice9_DrawPrimitive(device, D3DPT_TRIANGLELIST, 0, 1); + ok(SUCCEEDED(hr), "Failed to draw, hr %#lx.\n", hr); + + hr = IDirect3DDevice9_EndScene(device); + ok(SUCCEEDED(hr), "Failed to end scene, hr %#lx.\n", hr); + + /* Check that only single triangle instance has beed drawed with not-indexed draw */ + color = getPixelColor(device, 200, 340); + ok(color == act.color1, "has color 0x%08x, expected 0x%08x (case %i)\n", color, act.color1, i); + color = getPixelColor(device, 400, 340); + ok(color == 0x00ffffff, "has color 0x%08x, expected 0x%08x (case %i)\n", color, 0x00ffffff, i); + color = getPixelColor(device, 400, 180); + ok(color == 0x00ffffff, "has color 0x%08x, expected 0x%08x (case %i)\n", color, 0x00ffffff, i); + color = getPixelColor(device, 200, 180); + ok(color == 0x00ffffff, "has color 0x%08x, expected 0x%08x (case %i)\n", color, 0x00ffffff, i); + + + hr = IDirect3DDevice9_Present(device, NULL, NULL, NULL, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET | D3DCLEAR_ZBUFFER, 0xffffffff, 1.0f, 0); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IDirect3DDevice9_BeginScene(device); + ok(SUCCEEDED(hr), "Failed to begin scene, hr %#lx.\n", hr); + + /* Indexed draw test. More then signle instance allowed */ hr = IDirect3DDevice9_DrawIndexedPrimitive(device, D3DPT_TRIANGLELIST, 0, 0, 4, 0, 2); ok(SUCCEEDED(hr), "Failed to draw, hr %#lx.\n", hr); hr = IDirect3DDevice9_EndScene(device);
From: Illia Polishchuk illia.a.polishchuk@globallogic.com
Windows d3d9 always draws only a single instance for non-indexed draws
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54570 --- dlls/d3d9/device.c | 18 ++++++++++++++---- dlls/wined3d/adapter_vk.c | 2 -- dlls/wined3d/context_gl.c | 6 ------ dlls/wined3d/wined3d_private.h | 1 - 4 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/dlls/d3d9/device.c b/dlls/d3d9/device.c index eddb2a0ebdd..bd75f9eb3a9 100644 --- a/dlls/d3d9/device.c +++ b/dlls/d3d9/device.c @@ -3052,7 +3052,11 @@ static HRESULT WINAPI d3d9_device_DrawPrimitive(IDirect3DDevice9Ex *iface, d3d9_generate_auto_mipmaps(device); wined3d_device_context_set_primitive_type(device->immediate_context, wined3d_primitive_type_from_d3d(primitive_type), 0); - wined3d_device_context_draw(device->immediate_context, start_vertex, vertex_count, 0, 0); + + /* Windows d3d9 always draws only a single instance for non-indexed draws + * Set instance count to 1 for the not indexed draw */ + wined3d_device_context_draw(device->immediate_context, start_vertex, vertex_count, 0, 1); + d3d9_rts_flag_auto_gen_mipmap(device); wined3d_mutex_unlock();
@@ -3092,7 +3096,8 @@ static HRESULT WINAPI d3d9_device_DrawIndexedPrimitive(IDirect3DDevice9Ex *iface wined3d_primitive_type_from_d3d(primitive_type), 0); wined3d_device_apply_stateblock(device->wined3d_device, device->state); d3d9_device_upload_sysmem_index_buffer(device, &start_idx, index_count); - wined3d_device_context_draw_indexed(device->immediate_context, base_vertex_idx, start_idx, index_count, 0, 0); + wined3d_device_context_draw_indexed(device->immediate_context, base_vertex_idx, start_idx, index_count, 0, + device->stateblock_state->streams[0].frequency); d3d9_rts_flag_auto_gen_mipmap(device); wined3d_mutex_unlock();
@@ -3144,7 +3149,11 @@ static HRESULT WINAPI d3d9_device_DrawPrimitiveUP(IDirect3DDevice9Ex *iface, wined3d_device_context_set_primitive_type(device->immediate_context, wined3d_primitive_type_from_d3d(primitive_type), 0); wined3d_device_apply_stateblock(device->wined3d_device, device->state); - wined3d_device_context_draw(device->immediate_context, vb_pos / stride, vtx_count, 0, 0); + + /* Windows d3d9 always draws only a single instance for non-indexed draws + * Set instance count to 1 for the not indexed draw */ + wined3d_device_context_draw(device->immediate_context, vb_pos / stride, vtx_count, 0, 1); + wined3d_stateblock_set_stream_source(device->state, 0, NULL, 0, 0); d3d9_rts_flag_auto_gen_mipmap(device);
@@ -3211,7 +3220,8 @@ static HRESULT WINAPI d3d9_device_DrawIndexedPrimitiveUP(IDirect3DDevice9Ex *ifa wined3d_device_context_set_primitive_type(device->immediate_context, wined3d_primitive_type_from_d3d(primitive_type), 0); wined3d_device_context_draw_indexed(device->immediate_context, - vb_pos / vertex_stride - min_vertex_idx, ib_pos / idx_fmt_size, idx_count, 0, 0); + vb_pos / vertex_stride - min_vertex_idx, ib_pos / idx_fmt_size, idx_count, 0, + device->stateblock_state->streams[0].frequency);
wined3d_stateblock_set_stream_source(device->state, 0, NULL, 0, 0); wined3d_stateblock_set_index_buffer(device->state, NULL, WINED3DFMT_UNKNOWN); diff --git a/dlls/wined3d/adapter_vk.c b/dlls/wined3d/adapter_vk.c index cf8a7aec6dc..64f2ca369ee 100644 --- a/dlls/wined3d/adapter_vk.c +++ b/dlls/wined3d/adapter_vk.c @@ -1813,8 +1813,6 @@ static void adapter_vk_draw_primitive(struct wined3d_device *device, else { instance_count = parameters->u.direct.instance_count; - if (context_vk->c.instance_count) - instance_count = context_vk->c.instance_count; if (!instance_count) instance_count = 1;
diff --git a/dlls/wined3d/context_gl.c b/dlls/wined3d/context_gl.c index 5574997bf05..27969f0b5c9 100644 --- a/dlls/wined3d/context_gl.c +++ b/dlls/wined3d/context_gl.c @@ -5321,8 +5321,6 @@ void draw_primitive(struct wined3d_device *device, const struct wined3d_state *s else { unsigned int instance_count = parameters->u.direct.instance_count; - if (context->instance_count) - instance_count = context->instance_count;
if (context->use_immediate_mode_draw || emulation) draw_primitive_immediate_mode(wined3d_context_gl(context), state, stream_info, idx_data, @@ -5466,7 +5464,6 @@ static void wined3d_context_gl_load_vertex_data(struct wined3d_context_gl *conte
/* This is used for the fixed-function pipeline only, and the * fixed-function pipeline doesn't do instancing. */ - context_gl->c.instance_count = 0; current_bo = gl_info->supported[ARB_VERTEX_BUFFER_OBJECT] ? ~0u : 0;
/* Blend data */ @@ -5680,7 +5677,6 @@ static void wined3d_context_gl_load_numbered_arrays(struct wined3d_context_gl *c unsigned int i;
/* Default to no instancing. */ - context->instance_count = 0; current_bo = gl_info->supported[ARB_VERTEX_BUFFER_OBJECT] ? ~0u : 0;
if (stream_info->use_map & ~wined3d_mask_from_size(gl_info->limits.vertex_attribs)) @@ -5726,8 +5722,6 @@ static void wined3d_context_gl_load_numbered_arrays(struct wined3d_context_gl *c stream = &state->streams[element->stream_idx]; stream->buffer->bo_user.valid = true;
- if ((stream->flags & WINED3DSTREAMSOURCE_INSTANCEDATA) && !context->instance_count) - context->instance_count = state->streams[0].frequency;
if (gl_info->supported[ARB_INSTANCED_ARRAYS]) { diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index 459ac7a19c8..c0a190cbc5c 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -2307,7 +2307,6 @@ struct wined3d_context DWORD numbered_array_mask; enum fogsource fog_source;
- UINT instance_count;
void *shader_backend_data; void *fragment_pipe_data;
On Tue Feb 28 21:17:22 2023 +0000, Henri Verbeet wrote:
There are some minor style and formatting issues in this series that I'm going to ignore for the moment. The test either doesn't test the changes in patch 2/2, or it fails before that patch. The larger issue with this series though is that it doesn't seem quite ideal to potentially set and restore the stream source frequency on each DrawPrimitive()/DrawPrimitiveUP() call, because of the resulting state invalidation. It may be time to move the d3d9-style instance count handling out of wined3d and track that entirely in d3d9.
The changes in d3d9\test is clearly covers changes in my patch, it fails if instancing happens for not indexed d3d9 draws, but it is disabled in pipeline now
tools/gitlab/test.yml ``` test-linux-32: extends: .wine-test variables: EXCLUDE_TESTS: "d3d10core:d3d10core d3d11:d3d11 d3d8:device d3d8:visual d3d9:d3d9ex d3d9:device d3d9:visual" ```
On Tue Feb 28 21:17:54 2023 +0000, Henri Verbeet wrote:
Note that this will still respect D3DSTREAMSOURCE_INSTANCEDATA for DrawPrimitive()/DrawPrimitiveUP(). That may very well be the correct behaviour, but something like "d3d9: Always draw only a single instance for non-indexed draws." would be a more accurate description of the commit.
- wined3d_device_context_draw_indexed(device->immediate_context,base_vertex_idx, start_idx, index_count, 0, 0);
- wined3d_device_context_draw_indexed(device->immediate_context,
base_vertex_idx, start_idx, index_count, 0,
wined3d_stateblock_get_state(device->state)->streams[0].frequency);I think you should be able to use "device->stateblock_state" instead of "wined3d_stateblock_get_state(device->state)" here, as well as in d3d9_device_DrawIndexedPrimitiveUP(). There are a couple of places with trailing spaces in this series.
Makes sense. Thank you for the tips, updated MR
Zebediah Figura (@zfigura) commented about dlls/d3d9/tests/visual.c:
hr = IDirect3DDevice9_SetStreamSource(device, act.strInstance, vb3, 0, sizeof(instancepos[0])); ok(SUCCEEDED(hr), "Failed to set stream source, hr %#lx.\n", hr);
/* Not indexed draw test. Only single instance should be drawed */
```suggestion:-0+0 /* Non-indexed draw test. Only a single instance should be drawn. */ ```
Zebediah Figura (@zfigura) commented about dlls/d3d9/tests/visual.c:
color = getPixelColor(device, 400, 180);ok(color == 0x00ffffff, "has color 0x%08x, expected 0x%08x (case %i)\n", color, 0x00ffffff, i);color = getPixelColor(device, 200, 180);ok(color == 0x00ffffff, "has color 0x%08x, expected 0x%08x (case %i)\n", color, 0x00ffffff, i);hr = IDirect3DDevice9_Present(device, NULL, NULL, NULL, NULL);ok(hr == S_OK, "Got hr %#lx.\n", hr);hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET | D3DCLEAR_ZBUFFER, 0xffffffff, 1.0f, 0);ok(hr == S_OK, "Got hr %#lx.\n", hr);hr = IDirect3DDevice9_BeginScene(device);ok(SUCCEEDED(hr), "Failed to begin scene, hr %#lx.\n", hr);/* Indexed draw test. More then signle instance allowed */
```suggestion:-0+0 /* Indexed draw test. */ ```
Zebediah Figura (@zfigura) commented about dlls/d3d9/device.c:
d3d9_generate_auto_mipmaps(device); wined3d_device_context_set_primitive_type(device->immediate_context, wined3d_primitive_type_from_d3d(primitive_type), 0);
- wined3d_device_context_draw(device->immediate_context, start_vertex, vertex_count, 0, 0);
- /* Windows d3d9 always draws only a single instance for non-indexed draws
* Set instance count to 1 for the not indexed draw */
```suggestion:-1+0 /* Instancing is ignored for non-indexed draws. */ ```
Zebediah Figura (@zfigura) commented about dlls/d3d9/device.c:
wined3d_device_context_set_primitive_type(device->immediate_context, wined3d_primitive_type_from_d3d(primitive_type), 0); wined3d_device_apply_stateblock(device->wined3d_device, device->state);
- wined3d_device_context_draw(device->immediate_context, vb_pos / stride, vtx_count, 0, 0);
- /* Windows d3d9 always draws only a single instance for non-indexed draws
* Set instance count to 1 for the not indexed draw */
```suggestion:-1+0 /* Instancing is ignored for non-indexed draws. */ ```
Thanks for the patch!
We need the tests to pass after each commit, so the tests introduced in patch 1/2 should be marked todo_wine where appropriate, and then those todo_wine annotations should be removed in patch 2/2.
I also have made a couple of inline suggestions, in order to keep our comments concise and grammatical.
-
Illia Polishchuk -
Illia Polishchuk (＠illia.a.polishchuk)
-
Zebediah Figura (＠zfigura)