[PATCH 0/1] MR754: wined3d: Pass a draw count of 1 to Vulkan indirect draw commands.

[Zebediah Figura (＠zfigura)]

Spotted by Philip Rebohle.

This fixes rendering in "Discovery Tour by Assassin's Creed: Ancient Egypt".

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/754

[PATCH 1/1] wined3d: Pass a draw count of 1 to Vulkan indirect draw commands.

[Zebediah Figura]

From: Zebediah Figura <zfigura(a)codeweavers.com>

Spotted by Philip Rebohle.

This fixes rendering in "Discovery Tour by Assassin's Creed: Ancient Egypt".
---
 dlls/wined3d/adapter_vk.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/dlls/wined3d/adapter_vk.c b/dlls/wined3d/adapter_vk.c
index 58671cfb5f9..52f4da858a5 100644
--- a/dlls/wined3d/adapter_vk.c
+++ b/dlls/wined3d/adapter_vk.c
@@ -1813,23 +1813,14 @@ static void adapter_vk_draw_primitive(struct wined3d_device *device,
     if (parameters->indirect)
     {
         struct wined3d_bo_vk *bo = wined3d_bo_vk(indirect_vk->b.buffer_object);
-        uint32_t stride, size;
 
         wined3d_context_vk_reference_bo(context_vk, bo);
-        size = indirect_vk->b.resource.size - parameters->u.indirect.offset;
-
         if (parameters->indexed)
-        {
-            stride = sizeof(VkDrawIndexedIndirectCommand);
             VK_CALL(vkCmdDrawIndexedIndirect(vk_command_buffer, bo->vk_buffer,
-                    bo->b.buffer_offset + parameters->u.indirect.offset, size / stride, stride));
-        }
+                    bo->b.buffer_offset + parameters->u.indirect.offset, 1, sizeof(VkDrawIndexedIndirectCommand)));
         else
-        {
-            stride = sizeof(VkDrawIndirectCommand);
             VK_CALL(vkCmdDrawIndirect(vk_command_buffer, bo->vk_buffer,
-                    bo->b.buffer_offset + parameters->u.indirect.offset, size / stride, stride));
-        }
+                    bo->b.buffer_offset + parameters->u.indirect.offset, 1, sizeof(VkDrawIndirectCommand)));
     }
     else
     {
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/754

[Jan Sikorski (＠jsikorski)]

Maybe, but I don't see why..?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/754#note_8313

[Henri Verbeet (＠hverbeet)]

> Maybe, but I don't see why..?

What I imagine happens is that the application uses a single indirect buffer containing information for multiple draws, and then uses the offset argument to select between them. The current code would execute draws for the indirect draw selected by the application, but also for any subsequent entries in the buffer. The subsequent entries may then either contain uninitialised data themselves, or refer to data that's uninitialised/stale/not ready.

Still, while I don't doubt the change is correct, the commit message could probably be slightly more elaborate.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/754#note_8319

[Zebediah Figura]

On 9/13/22 06:56, Henri Verbeet (@hverbeet) wrote:
> > Maybe, but I don't see why..?
>
> What I imagine happens is that the application uses a single indirect buffer containing information for multiple draws, and then uses the offset argument to select between them. The current code would execute draws for the indirect draw selected by the application, but also for any subsequent entries in the buffer. The subsequent entries may then either contain uninitialised data themselves, or refer to data that's uninitialised/stale/not ready.
>
> Still, while I don't doubt the change is correct, the commit message could probably be slightly more elaborate.

I didn't look at what the application actually did, although Philip 
might have, but that seems like the most likely explanation.

[Zebediah Figura]

On 9/13/22 06:56, Henri Verbeet (@hverbeet) wrote:
> > Maybe, but I don't see why..?
>
> What I imagine happens is that the application uses a single indirect buffer containing information for multiple draws, and then uses the offset argument to select between them. The current code would execute draws for the indirect draw selected by the application, but also for any subsequent entries in the buffer. The subsequent entries may then either contain uninitialised data themselves, or refer to data that's uninitialised/stale/not ready.
>
> Still, while I don't doubt the change is correct, the commit message could probably be slightly more elaborate.

I didn't look at what the application actually did, although Philip 
might have, but that seems like the most likely explanation.

[Jan Sikorski (＠jsikorski)]

Oh right, D3D only does one indirect draw at a time, somehow I didn't notice it then. Makes sense. I'd still write this in the commit message.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/754#note_8321

[PATCH 0/1] MR754: wined3d: Pass a draw count of 1 to Vulkan indirect draw commands. - approved

[Jan Sikorski (＠jsikorski)]

This merge request was approved by Jan Sikorski.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/754