Support custom vertex formats

[bonsairobo]

Right now, there is only one supported vertex format called Vertex that's hard-coded into the mesh provider system. The Mesh type already supports many more kinds of vertices, based on a list of VertexAttributeValues, but attempts to use them will not work as expected.

[GabCampbell]

Thanks for opening an issue @bonsairobo now we can begin a deeper discussion on where to go regarding that 😄

[bonsairobo]

#158

[nickbryan]

I am very interested in this functionality too. I am wondering if there is anything I can do to help move this one along or is it more of a case that this is waiting on other render stuff to be completed first (PBR comes to mind)?

[cart]

I think @bonsairobo did a close to ideal implementation of "100% reflected vertex formats that meshes coerce themselves into" in #158. I'm pretty sure we want something that is a bit more static (at least by default), where users define VertexBufferDescriptor formats ahead of time and those get mapped to shaders (which would use subsets of that descriptor). And we'd want a sane "default" vertex buffer descriptor that ideally gets assigned to shaders when no custom vertex configuration is specified.

We basically just need someone to come up with a design for that. If someone were to do one or more informal specs for ideas and post them here, that would help us move forward.

[bonsairobo]

I'll try to add a little context about where we left off with #158. I think that served as a good experiment for me to learn about the code and get some feedback about how to proceed, but I will probably close that PR once we have new path forward. The code, particularly the usage example, should still be useful in the future.

There were a few requirements that came up in feedback:

1. We want a single Mesh to be usable with many pipelines, even if their attribute layouts are different. For example, if a mesh has (position, normal, tangent, uv), then it should be compatible with shaders that need (position, normal) or (normal, position) or (normal, uv) or etc. Support custom vertex formats #158 satisfies this.
2. We want to avoid duplicating vertex buffers where possible. Support custom vertex formats #158 does not satisfy this.

Understanding the problem

I think (2) is the tricky part. I want to say that it should be possible to have a single vertex buffer per mesh that's usable by any pipelines that require a subset of the attributes, regardless of layout. Please double check my understanding here!

Just to be concrete, say we had these two shaders:

Shader/pipeline 1:

layout(location = 0) in vec3 position;
layout(location = 1) in vec4 color;
layout(location = 2) in vec3 normal;

// more code...

Shader/pipeline 2:

layout(location = 0) in vec3 normal;
layout(location = 1) in vec3 position;

// more code...

Right now, Bevy will always create interleaved vertex buffers for a given mesh. So let's assume that it creates a buffer with layout:

position, color, normal, position, color, normal, ...

This buffer should be usable by both shaders/pipelines.

For pipeline 1, the VertexBufferDescriptor would be:

// Assumes 16-byte attribute alignment and 64-byte vertex alignment.
{
    stride: 64,
    attributes: [
        { shader_location: 0, offset: 0, format: Float3 },
        { shader_location: 1, offset: 16, format: Float3 },
        { shader_location: 2, offset: 32, format: Float3 },
    ]
}

For pipeline 2:

// Assumes 16-byte attribute alignment and 64-byte vertex alignment.
{
    stride: 64,
    attributes: [
        { shader_location: 0, offset: 32, format: Float3 },
        { shader_location: 1, offset: 0, format: Float3 },
    ]
}

But this cannot be accomplished with the current shader reflection code, because it assumes that a vertex buffer will have a layout that matches the attribute locations. E.g. for pipeline 2, reflection would produce:

{
    stride: 32,
    attributes: [
        { shader_location: 0, offset: 0, format: Float3 },
        { shader_location: 1, offset: 16, format: Float3 },
    ]
}

which is not compatible with the vertex buffer we created for our mesh!

An ambitious plan

I believe that we could change the reflection code to avoid this problem. Rather than producing a VertexBufferDescriptor from reflection, we could produce something weaker, like:

struct VertexShaderDescriptor {
    attributes: Vec<VertexShaderAttribute>,
}

struct VertexShaderAttribute {
    name: String,
    format: VertexFormat,
    shader_location: u32,
}

Then, given a vertex buffer's full VertexBufferDescriptor, you could generate a subset VertexBufferDescriptor that's compatible with a VertexShaderDescriptor:

// Incomplete pseudo-code
fn vertex_buffer_descriptor_for_shader(
    buffer: &VertexBufferDescriptor,
    shader: &VertexShaderDescriptor
) -> Option<VertexBufferDescriptor> {
    let mut subset = VertexBufferDescriptor::new();
    for attribute in shader.attributes {
        if let Some(attribute) = buffer.get_attribute(attribute.name) {
            subset.attributes.push(attribute);
        } else {
            return None;
        }
    }

    Some(subset)
}

This could probably happen immediately after vertex buffers are generated for a mesh. It would use the RenderPipelines and it's compiled pipeline descriptors to access the VertexShaderDescriptors.

The difficult part is that we might have multiple meshes that are compatible with a single pipeline, and those meshes might produce different vertex buffer layouts. So you would need to standardize on some buffer layout that is compatible with both meshes. That sounds kind of hard, but possible.

After writing all of this out, I think it sounds like a nice future goal, but it's probably overkill for now. Let me know what you think!

Something simpler

Rather, we could do what @cart is suggesting as a stepping stone: make the user specify their own VertexBufferDescriptor for each pipeline, and use a default one if they don't.

Then we don't have to worry about reflecting a layout. (I would just delete that part of the reflection code, since I don't see a path forward that can use it). But we will rely on the user for specifying layouts that are compatible across pipelines, otherwise we would be forced to duplicate vertex buffers.

I think the global data structure that we need for creating and binding vertex buffers is:

struct VertexBufferMap {
    buffers: HashMap<Handle<Mesh>, Vec<VertexBuffer>>,
}

struct VertexBuffer {
    buffer: Buffer,
    descriptor: VertexBufferDescriptor,
}

This would be organized such that a single mesh would ideally only have one VertexBuffer, but there may be more than one if we have incompatible layouts for the same mesh.

When generating vertex buffers for (mesh, pipeline) combinations, we try to answer the question: "have we already generated a vertex buffer that we could use here?" Then we can consult the map by iterating over all of the VertexBuffers for the mesh, and if any of the descriptors have all of the required attributes at the correct offsets, we can use it. If a descriptor has all of the attributes we need, but at the wrong offsets, sadly, we would need to generate a new VertexBuffer and add it to the map. If none of the descriptors have all of the attributes we need, then we just give up on drawing that mesh with that pipeline. Maybe this case is deserving of a warning log message.

When binding the vertex buffers, we can consult the map with the same (mesh, pipeline) combination, and do the same compatibility search as before.

[bonsairobo]

BTW I would be willing to write another PR, but I can't promise it will happen in a timely manner. Anyone who wants to implement this, please let me know so I we don't step on each others' toes!

[nickbryan]

@cart @bonsairobo thank you both for taking the time to add additional information to this.

@bonsairobo fair play to you! That write up is really well thought out. I would absolutely love to get stuck in and contribute to this but I fear that I may be out of my depth after reading your proposal. I will have a look over this some more over the next couple of days and if I feel I may be able to contribute I will give you a shout.

Thanks again! I absolutely love this community so far, you have all been super helpful and it is incredible to see the level of thought and detail going into this project!

[nickbryan]

So I have tried my best to wrap my head around this to see if I could help out and put together an implementation but I think I am going to have to leave this one to the experts. Looking forward to seeing how this progresses though.

[cart]

Resolved by #599