WIP: Initial stub for mesh shaders #3236

MatusT · 2020-04-28T16:11:25Z

This is my attempt at adding support for mesh shaders.

This (first) PR only stubs the API. From my investigation, the extension is relatively simple(it effectively only adds new shaders and 3 draw commands mirroring the original ones) and seems to be the same across Direct3D and Vulkan.

Even though the Vulkan only has NV extension so far, based on announcements of RDNA2 and DirectX Ultimate we can expect KHR version and I don't see how it could diverge in any way. Thus, both Vulkan and DX12 implementations should be trivial.

I know nothing about Metal and I am not aware of information about support for mesh shaders. At worst, they could be emulated using compute shaders but It would probably be so slow that It would be practically worthless.

Tasks:

Stub the API
Implement empty backend
Implement non-functional Vulkan backend
Implement non-functional DirectX 12 backend
Implement non-functional DirectX 11 backend
Implement non-functional Metal backend
Implement non-functional OpenGL backend
Fix examples compilation errors
Fix warden compilation errors

PR checklist:

Resources:
Vulkan specification of mesh shaders
DirectX specification of mesh shader

monocodus · 2020-04-28T16:11:31Z

The .monocodus config file is invalid. The code review has not been performed.
Check config documentation here

Click to expand the list of errors ocurred.

.monocodus
Invalid config version: None

monocodus · 2020-04-28T16:13:21Z

The .monocodus config file is invalid. The code review has not been performed.
Check config documentation here

Click to expand the list of errors ocurred.

.monocodus
Invalid config version: None

monocodus · 2020-04-28T16:13:43Z

The .monocodus config file is invalid. The code review has not been performed.
Check config documentation here

Click to expand the list of errors ocurred.

.monocodus
Invalid config version: None

kvark · 2020-04-28T17:04:18Z

Thank you, that's amazing to see! I'll do a proper review shortly.
Just wanted to comment on Metal and platforms without the mesh shaders.

IIRC from WebGPU discussions, the whole idea of the mesh shaders was kicked off by some research work of doing them on compute, long before it became available in hardware/drivers. Therefore, compute path should be possible, even if not as efficient. I'd like us to have that path available at some point. We have Hints specifically to tell the users whether something is natively supported versus emulated.

kvark · 2020-04-28T17:20:03Z

there be MeshPipelineDesc as counterpart to GraphicsPipelineDesc?

Only if Vulkan or D3D12 define a separate pipeline kind for this. As far as I see from a brief look, it's all the same graphics pipeline, just a bunch of stages are replaced. So let's keep it GraphicsPipelineDesc

GraphicsPipelineDesc silently ignores input_assembler and contents of vertex_buffers and attributes

Can we have an enum?

enum PrimitiveAssemer {
  Vertex(InputAssemer, VertexBuffers),
  Mesh(...),
}

MatusT · 2020-04-28T18:19:36Z

Can we have an enum?

enum PrimitiveAssemer {
  Vertex(InputAssemer, VertexBuffers),
  Mesh(...),
}

I will try to summarise here what options I see.

Firstly, just the struct itself:

Follow what Vulkan/DX12 does and silently ignore unnecessary attributes.

Pros:
- Backwards compatible API, since members are public. I see this as a very important attribute.
Cons:
- Vec<VertexBufferDesc> and Vec<AttributeDesc> must be set empty. Not that annoying.
- InputAssemblerDesc must be filled with some values, which does not really make sense and seems annoying. However, I would personally circumvent this by implementing Default for it + Default for GraphicsPipelineDesc itself so that a user could create the struct with only necessary attributes and just .. Default::default() the rest.

Make an enum

Pros:
- You now have only one member and it kind of makes sense.
Cons:
- Breaking change.
- Enum must contain 3 whole members inside one variant and absolutely none in the second:
```
enum PrimitiveAssembler {
  Vertex(Vec<VertexBufferDesc>, Vec<AttributeDesc>, InputAssemblerDesc),
  Mesh, // literally nothing
}
```
You can of course have any variation: make those 3 members optional or make only InputAssemblerDesc optional

Secondly, there is a question of GraphicsPipelineDesc::new() function. Similar questions with respect to API apply as above:

Make primitive and rasterizer optional? Remove them altogether from the new function?
Make a new new_mesh function? I am personally in favour of this option.

Since I have started writing about breakage of the API, I have realised that the API of GraphicsShaderSet will be broken in any case because vertex becomes optional and task+mesh must be filled. I don't know how many projects rely on the stability of gfx, so I am unable to asses the potential disruption in the ecosystem.

As you are the maintainer, you should choose the course of action and I will modify the code accordingly.

kvark · 2020-04-28T19:07:55Z

the API of GraphicsShaderSet will be broken in any case because vertex becomes optional

Right, I don't think we should try too hard to avoid API breakage here. Consider this to be experimental, we'll adjust as we go, and master is where experimentation happens.

Mesh, // literally nothing

I think we can do better than that. We can exclude the task and mesh stages from the graphics shader set and put them here:

enum PrimitiveAssember<'a, B> {
  Vertex {
    buffers: Vec<VertexBufferDesc>,
    attributes: Vec<AttributeDesc>,
    input_assember: InputAssemblerDesc,
  },
  Mesh {
    task_shader: EntryPoint<'a, B>,
    mesh_shader: EntryPoint<'a, B>,
  },
}

msiglreith · 2020-04-28T22:02:09Z

+1 for the enum variant - totally replacing the GraphicsShaderSet might be an option here and move all old pipeline stages into the Vertex variant. Fragment shader entrypoint could live then as field in the GraphicsPipelineDesc or be part of some other sub desc.

MatusT · 2020-04-29T21:10:07Z

I updated the API with the Enum idea. I must admit that the solution is slowly growing on me.

I also added 2 new errors to the pipeline CreationError, one for when the mesh shader is not supported and the other one for when the vertex stage is missing in vertex primitive assembly.

kvark

Thank you! I believe this is great amount of good work here.
Just a few notes on how we should polish this more before landing.

examples/colour-uniform/main.rs

src/backend/dx11/src/conv.rs

src/backend/dx11/src/device.rs

src/backend/gl/src/command.rs

src/backend/gl/src/device.rs

src/backend/gl/src/queue.rs

src/backend/vulkan/src/device.rs

kvark

Ok, looks like we are almost there :)

src/backend/dx11/src/device.rs

src/hal/src/lib.rs

src/hal/src/pso/graphics.rs

src/hal/src/pso/mod.rs

src/backend/dx11/src/conv.rs

MatusT · 2020-05-02T12:15:57Z

We have one final big problem to resolve that I just realized.

There is one major difference between DX12 and Vulkan.
Vulkan only allows 1D Tasks https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/vkCmdDrawMeshTasksNV.html.
While DX12 allows specifying all 3 dimensions https://microsoft.github.io/DirectX-Specs/d3d/MeshShader.html#dispatchmesh-api. In addition, It does not have offsets(first_task).

I modelled the API according to Vulkan, which is currently the lowest common denominator.

Alternative could be:

allowing all 3 dimensions and reporting maximum sizes of Y, Z as 1 on the Vulkan backend
maybe patch the shader to transform 1D to 3D?

Maybe this will change with KHR version of the extension and we should wait until then. Unless, NVIDIA/Khronos/AMD could give use some more formation.

kvark · 2020-05-02T15:36:55Z

Let's model this after Vulkan for now, and then reconsider when KHR extension comes out.
Are you in Khronos by any chance? We could use some early insights on what the API will be, if it happens at all :)

MatusT · 2020-05-02T17:10:27Z

Let's model this after Vulkan for now, and then reconsider when KHR extension comes out.

I will leave It as is It is then.

Are you in Khronos by any chance? We could use some early insights on what the API will be, if it happens at all :)

I thought you were part of Khronos on behalf of Mozilla :D. What I would give for being part of Khronos. No, I am merely a master's student who is using WebGPU for my diploma thesis and I wanted to use mesh shaders for that.

kvark · 2020-05-02T22:37:28Z

I am a part of Khronos, just having more than I can chew on my plate. I can dive and check the archives though to see where things are going, at least.
CI mac is experiencing strange failures - https://travis-ci.org/github/gfx-rs/gfx/jobs/682350838#L3665

kvark · 2020-05-04T14:05:46Z

bors try

bors · 2020-05-04T14:40:23Z

try

Build failed:

continuous-integration/travis-ci/push

kvark · 2020-05-04T14:41:56Z

Could you rebase and squash please?

MatusT · 2020-05-04T15:15:12Z

Could you rebase and squash please?

Done.

kvark · 2020-05-04T15:27:09Z

bors try

bors · 2020-05-04T15:55:51Z

try

Build failed:

continuous-integration/travis-ci/push

kvark · 2020-05-04T17:31:18Z

I'm confused as to why some PRs are landing fine, and others aren't :/
Anyhow, this nightly error appears to be fixed upstream, just going to wait for the next nightly.

Initial stub for mesh shaders to flesh out the API changes. Stub mesh shaders address some details DX12: remove mesh shaders from the indented block Improve the code based on review fix metal backend

MatusT · 2020-05-07T10:25:08Z

@kvark CI finally succeeded!

kvark

Sweet!
bors r+

bors · 2020-05-07T12:42:50Z

Build succeeded:

continuous-integration/travis-ci/push

kvark approved these changes Apr 28, 2020

View reviewed changes

monocodus bot approved these changes Apr 28, 2020

View reviewed changes

monocodus bot approved these changes Apr 29, 2020

View reviewed changes