Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] First pass at mesh shader debugging for SPIR-V & Vulkan #3480

Merged
merged 1 commit into from
Nov 20, 2024

Conversation

Honeybunch
Copy link
Contributor

I needed to debug a mesh shader and figured someone might want this

Enabled the "Debug This Vertex" dialog for vertices in the meshlet debug view and tied that into the spirv debugger.

This is kind of messy right now, and I also had to do some work to get buffer textures read properly. Makes me think I'm doing something wrong. I also just skipped any implementation for the SetMeshOutputsEXT op. No idea if that has consequences.

Also making some pretty loose assumptions about the group/thread dimensions to use when debugging the mesh shader which are totally wrong right now

Maybe with some feedback and a little elbow grease this could be useful?

@baldurk
Copy link
Owner

baldurk commented Nov 11, 2024

Thanks for this PR, it definitely seems promising. I think the biggest challenge is similar to compute shader debugging but magnified - I would expect the large majority of mesh shaders to rely on cross-thread communication and work sharing (since that's part of the point of using mesh shaders as I understand it) which will fail under the current debugger model. It may still be useful though in the same way that compute shader debugging can be useful.

A couple of book-keeping things:

  • Can you please mark this as a normal PR? I personally find draft PRs on github pointless and I rather wish I could disable them on this repository as in my view they don't add any value at all and only cause confusion/annoyance.
  • As in the contributing docs please squash any formatting and compile fix changes into your commits as relevant, all commits should individually be correctly formatted and compile.

I'd also ask that you split out any fix to buffer view sampling into a separate PR as it doesn't seem mesh-shader related and can then be reviewed and merged potentially sooner as a bugfix. The VK_Shader_Debug_Zoo demo already does a test of sampling from a buffer view so it would be useful to modify/add to that test to see why that demo isn't covering whatever case needs your fixes - maybe the buffer format needs to be different?

I think from looking through this it mostly looks good but the main thing that would need to change is the thread selection that you mentioned. As you implied you can't assume that thread N outputs vertex N in a meshlet, the mapping is entirely user defined and not annotated with no realistic way to try and reverse engineer it. There may even be more or less threads than vertices.

I think instead what you should do is when debugging a meshlet pop up the compute shader thread-selection dialog and pre-populate the group with the calculated group ID, but set thread ID to 0,0,0. As a related note here you are currently assuming a 1D dispatch which out can't do, but since the order of outputs in the buffer is determined by us you can reverse the output index into a group ID using the dimensions.

As a related naming issue, the API function shouldn't be DebugMeshletVertex because it's not debugging a specific vertex, it's debugging a thread in the group that emitted a meshlet.

@Honeybunch
Copy link
Contributor Author

Thank you! This was exactly the feedback I was looking for. I'll try to find some time to chip away at this over the next week

qrenderdoc/Windows/BufferViewer.cpp Outdated Show resolved Hide resolved
qrenderdoc/Windows/BufferViewer.cpp Outdated Show resolved Hide resolved
qrenderdoc/Windows/BufferViewer.cpp Outdated Show resolved Hide resolved
qrenderdoc/Windows/BufferViewer.cpp Outdated Show resolved Hide resolved
qrenderdoc/Windows/ShaderMessageViewer.cpp Outdated Show resolved Hide resolved
renderdoc/driver/shaders/spirv/spirv_debug.cpp Outdated Show resolved Hide resolved
renderdoc/driver/vulkan/vk_shaderdebug.cpp Outdated Show resolved Hide resolved
renderdoc/api/replay/renderdoc_replay.h Outdated Show resolved Hide resolved
@Honeybunch Honeybunch force-pushed the vkMeshShaderDebugging branch 2 times, most recently from c71535e to d129574 Compare November 18, 2024 17:57
@Honeybunch Honeybunch requested a review from baldurk November 19, 2024 17:27
@Honeybunch Honeybunch force-pushed the vkMeshShaderDebugging branch from d129574 to 2c55167 Compare November 19, 2024 17:29
@baldurk baldurk merged commit 3ea9735 into baldurk:v1.x Nov 20, 2024
16 checks passed
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.

2 participants