<div dir="auto"><div>Hello Lukas,</div><div dir="auto"><br></div><div dir="auto">I'm not a llmpipe developer, but I think the place for this discussion is on the Mesa gitlab. Feel free to open an issue about this there or better yet, submit a merge request with your proposed fix.</div><div dir="auto"><br></div><div dir="auto">Best regards,</div><div dir="auto">Timur<br><br><div class="gmail_quote" dir="auto"><div dir="ltr" class="gmail_attr"> <<a href="mailto:lists@luckyxxl.de">lists@luckyxxl.de</a>> ezt írta (időpont: 2023. febr. 19., Vas 12:10):<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hello everyone,<br>
<br>
newbie to the Mesa codebase here.<br>
<br>
I found that in the current main branch the llvmpipe Vulkan driver does <br>
not produce correct results for `vkCmdDrawIndexedIndirect` when the <br>
preceding call to `vkCmdBindIndexBuffer` passed a non-zero value for <br>
`offset`. I poked around in the code a little bit, found that the <br>
`offset` parameter is just not used at all for indirect draws and came <br>
up with the following two solutions to fix my usecase:<br>
<br>
<br>
1) <br>
<a href="https://gitlab.freedesktop.org/luckyxxl/mesa/-/commits/fix_indirect_draw_ib_offset1" rel="noreferrer noreferrer" target="_blank">https://gitlab.freedesktop.org/luckyxxl/mesa/-/commits/fix_indirect_draw_ib_offset1</a><br>
<br>
While this is a pretty minimal change I don't think it's a good one. <br>
Adding `index_offset` to `struct pipe_draw_indirect_info` feels like a <br>
hack. Also it will fail silently when `offset` is not a multiple of the <br>
index type size, which is valid according to the Vulkan specification.<br>
<br>
2) <br>
<a href="https://gitlab.freedesktop.org/luckyxxl/mesa/-/commits/fix_indirect_draw_ib_offset2" rel="noreferrer noreferrer" target="_blank">https://gitlab.freedesktop.org/luckyxxl/mesa/-/commits/fix_indirect_draw_ib_offset2</a><br>
<br>
While this one adheres the Vulkan specification in that any value for <br>
`offset` is supported, I feel like it is a fairly large/risky change. <br>
`struct pipe_draw_info` is used throughout the codebase in various <br>
utility functions and drivers, and I can't judge how big the impact of <br>
the suggested change would be. Also, it seems to me that the code of <br>
many drivers should be updated to handle the new parameter, though that <br>
might not be necessary given that the new behavior cannot be triggered <br>
via the OpenGL API (see below). How would such a global change be <br>
handled if it's worth proceeding-on?<br>
<br>
<br>
The issue can only be triggered using the Vulkan API, given that there's <br>
no way (at least none that I know of / could find) to use an offset into <br>
the index buffer for indirect draws with the OpenGL API. Therefore I <br>
think that some changes to core data-structures/algorithms are necessary <br>
in order to be able to implement the fix, unless I oversaw something. I <br>
would be glad if someone could advise on how to proceed: Does it look <br>
worth to you to try to upstream one of the two suggested fixes? In that <br>
case I'd be happy to write a proper PR. Or is the issue better worked-on <br>
by a developer who is experienced with the code, in that case I'll <br>
gladly file an issue and provide a repro. Also, as a final feedback, is <br>
this mailing-list the correct place to discuss such a matter, or would <br>
that better be done in a PR or via IRC?<br>
<br>
Thank you for your time :)<br>
<br>
Lukas<br>
<br>
</blockquote></div></div></div>