Mesh shading example

[Firestar99]

1. Update documentation to reflect any user-facing changes - in this repository. - None

2. Make sure that the changes are covered by unit-tests. - None

3. Run cargo fmt on the changes.

4. Please put changelog entries in the description of this Pull Request
   if knowledge of this change could be valuable to users. No need to put the
   entries to the changelog directly, they will be transferred to the changelog
   file by maintainers right after the Pull Request merge.

   Please remove any items from the template below that are not applicable.

5. Describe in common words what is the purpose of this change, related
   Github Issues, and highlight important implementation aspects.

Changelog:

### Additions
- A `mesh-shader` example.

[marc0246]

Please be so kind as to rebase so Rua's commits aren't mixed in.

It can be done without any merge conflicts by rebasing onto Rua's branch first (for a remote named Rua):

git rebase Rua/mesh-shading

Then squashing her commits (pick her first commit and squash all her other commits into it):

git rebase -i b2bbe34896b0aa922dd904a0188e4288decaf6ce

Then you can rebase onto master without conflicts (making sure it's the upstream master not some outdated fork's):

git rebase master

There may be some more ergonomic way to go about it but this I've found works 100% of the time.

[Firestar99]

Sure, I've squashed Rua's PR. Otherwise it was already rebased on Rua's PR, which was already rebased on current master.

[Rua]

But now this PR still contains the same changes as my PR, which makes reviewing it more difficult, as it's hard to separate your changes from mine.

[marc0246]

You need to rebase onto master as well.

[marc0246]

As for the first step, that's needed because Rua's branch has had more commits added to it than your fork. If you didn't rebase onto Rua's updated branch, you would get a merge conflict along the way.

[Firestar99]

Ohhhh.. my remote for vulkano didn't update for some reason, I didn't notice you've merged ext_mesh_shaders already

[marc0246]

That's why I say to make sure to use upstream master ;) Because this happens every time. I suggest making your local master branch track upstream/master, that is, our remote, and have all the other branches track your remote.

[Firestar99]

That's already how my setup works, it's just that Intellij Idea's "update project" button seems to only fetch the origin of the checked out branch, not all branches...

[Firestar99]

I'm not submitting this PR because it is ready, but because there is a weird struct layout issue here.

vulkano/examples/mesh-shader/shader.mesh

Line 47 in 032f777

const bool LOAD_FROM_INSTANCE_BUFFER = true;

First turn on LOAD_FROM_INSTANCE_BUFFER, which it is on the current latest commit, to use instance data loaded from buffers instead of generating all the geometry dynamically in the mesh shader.

vulkano/examples/mesh-shader/shader.mesh

Lines 18 to 22 in 032f777

	// The per-instance data.
	struct Instance {
	vec2 position_offset;
	float scale;
	};

On the GPU side this Instance struct uses STD140 layout, which causes it's alignment to be 16 bytes, but on the CPU it's aligned to 12 bytes. In the main.rs the InstanceData struct is currently defined manually, but even if I define it as

// also move the shaders here
type InstanceData = mesh::Instance;

it still is misalignend, producing garbage output. Manually using repr(C, alignn(16)) also does not help.

[marc0246]

I see, that is the default behavior of git fetch I believe. I use lazygit which fetches every remote automatically, I can quite recommend it.

[marc0246]

I think the shaders should be named mesh.glsl and frag.glsl, for consistency with the other examples and easier syntax highlighting.

[Firestar99]

Wouldn't it be more consistent if we've used glslang's file endings?

$ glslang --help
[..]
'file' can end in .<stage> for auto-stage classification, where <stage> is:
    .conf   to provide a config file that replaces the default configuration
            (see -c option below for generating a template)
    .vert   for a vertex shader
    .tesc   for a tessellation control shader
    .tese   for a tessellation evaluation shader
    .geom   for a geometry shader
    .frag   for a fragment shader
    .comp   for a compute shader
    .mesh   for a mesh shader
    .task   for a task shader
    .rgen    for a ray generation shader
    .rint    for a ray intersection shader
    .rahit   for a ray any hit shader
    .rchit   for a ray closest hit shader
    .rmiss   for a ray miss shader
    .rcall   for a ray callable shader
    .glsl   for .vert.glsl, .tesc.glsl, ..., .comp.glsl compound suffixes
    .hlsl   for .vert.hlsl, .tesc.hlsl, ..., .comp.hlsl compound suffixes

The last two options could also be a good alternative.

[Firestar99]

I'm also not that happy with this if being semi-hidden in the shader code. It's hopefully obvious what it does once you read the shader.

[Rua]

I suppose it could be made into a specialization constant?

[Firestar99]

In practice you'd make it a SpecConstant, but I feel like for an example it's overkill and only complicates it further.
Instead I'd propose two possible solutions:

1. uniform bool toggled with a keyboard press: But that key would have to be known to the user, and the current state needs to be visualized. As we can't exactly render text easily, likely changing the color of the triangles.
2. Keep it as is: At the end of the day the if should switch between the two main use-cases of mesh shaders: reading vertex (& other) data manually and generating completely dynamic geometry. And those investigating mesh shaders will read the code and understand what the simple const bool does, and is likely less complicated than following where a uniform came from.

I'm currently leaning more on 2, especially with maybe some more docs.

[Firestar99]

I've implemented variant 2 as follows:

vulkano/examples/mesh-shader/mesh.glsl

Lines 54 to 61 in e89f419

	// There are two main use-cases for mesh shaders, switch in between them here.
	// They should both draw the same triangles, but with different colors.
	const bool LOAD_FROM_INSTANCE_BUFFER = false;
	if (LOAD_FROM_INSTANCE_BUFFER) {
	// Use-case 1: load instance data from buffers, similarly to doing an instanced draw
	// color triangles red
	color = vec4(1.0, 0.0, 0.0, 1.0);

vulkano/examples/mesh-shader/mesh.glsl

Lines 67 to 70 in e89f419

	} else {
	// Use-case 2: generate the geometry dynamically in the mesh shader
	// color triangles green
	color = vec4(0.0, 1.0, 0.0, 1.0);

[marc0246]

Those file endings are for automatic stage resolution, which we don't have, and I wouldn't call those endings standard at all. They don't even tell you what language it is. And as I already mentioned, syntax highlighting becomes harder. See for example how GitHub doesn't support the mesh extension yet. Granted, I use those extensions in my own projects, but I've had to do additional configuration in at least 2 editors I used. That's not exactly a quality I would be looking for in examples.

[marc0246]

Can you please name the file frag.glsl? I realize this is me being very nit-picky but it's for consistency with the other examples.

[marc0246]

Awesome, thanks. The only other thing that needs to be changed IMO is the formatting of the shaders. Namely, please use 4 spaces instead of the tabs like the other examples, and remove the multiple consecutive linefeeds.

[Firestar99]

I've also added an output variable to the mesh shader, simply passing the triangle color, so that pretty much everything you may want to do with mesh shaders is covered. Apart from task shaders, but those aren't super complicated to understand, and should easily be transferable from any other example or tutorial.

vulkano/examples/mesh-shader/mesh.glsl

Lines 38 to 39 in e89f419

	// As mesh shaders may emit multiple vertices, all outputs have to be an array. See below, when vertices are emitted.
	layout(location = 0) out vec4 out_color[];

[marc0246]

Actually something else I forgot, the Rust formatting is not like ours either. I assume this is rust-analyzer's formatting work. If you look in the .rustfmt.toml file and uncomment the lines, then run cargo +nightly fmt it should do it for you.

[marc0246]

Thank you for the very nice work! I really appreciate it ❤️

Let's keep the one comment unresolved for a possible future alteration. I think it's fine as is.