Skip to content

Conversation

@beicause
Copy link

@beicause beicause commented Dec 5, 2025

Description
Noted this when I was checking #8162

The offset argument of draw_indexed_indirect in hal/vulkan seems incorrect. It should + i * stride.
Also multi draw indirect should respect limits.maxDrawIndirectCount.

Actually, vkCmdDrawIndirectCount vkCmdDrawIndexedIndirectCount vkCmdDrawMeshTasksIndirectCountEXT also require that the count in the countBuffer must be <= maxDrawIndirectCount, altough on many devices it's u32::MAX(vulkan gpuinfo)

Testing
I assume the code is correct, I don't have a device to test this.

Squash or Rebase?
Squash

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy --tests. If applicable, add:
    • --target wasm32-unknown-unknown
  • Run cargo xtask test to run tests.
  • If this contains user-facing changes, add a CHANGELOG.md entry.

@cwfitzgerald cwfitzgerald self-assigned this Dec 10, 2025
Copy link
Member

@ErichDonGubler ErichDonGubler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not @cwfitzgerald (the current assignee), but: We definitely need test coverage for this. This is fundamental enough to these specific APIs that I'd be shocked if there wasn't a CTS test path we could simply drop into cts_runner/test.lst it's necessary.

EDIT: This isn't covered by CTS, because it ain't WebGPU. 😅

@cwfitzgerald
Copy link
Member

cwfitzgerald commented Dec 11, 2025

if there wasn't a CTS test path

MDI currently isn't in the spec, though there are plans to add it at some point.

We definitely need test coverage for this

+1

@ErichDonGubler
Copy link
Member

Welp, I'm shocked, and it's my fault. 🤦🏻‍♂️😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants