Skip to content

Make vertex buffer slots optional#9351

Open
teoxoy wants to merge 5 commits intogfx-rs:trunkfrom
teoxoy:null-vertex-buffers
Open

Make vertex buffer slots optional#9351
teoxoy wants to merge 5 commits intogfx-rs:trunkfrom
teoxoy:null-vertex-buffers

Conversation

@teoxoy
Copy link
Copy Markdown
Member

@teoxoy teoxoy commented Mar 31, 2026

Connections
Fixes #7354.
Fixes #8832.
Fixes #7912.

Description
Makes vertex buffer slots optional.

Testing
Enabled more CTS tests.

Squash or Rebase?
Rebase.

Copy link
Copy Markdown
Contributor

@andyleiserson andyleiserson left a comment

Choose a reason for hiding this comment

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

Nice! Just a few nits.


// Determine how many vertex buffers have already been bound
let vertex_buffer_count = self
// Check all needed vertex buffers have been bound
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I took a stab at deduplicating this for passes and bundles. It needs a bunch of internal types from command so possibly it's worth creating a new module command::validation rather than putting this in crate::validation.

pub(crate) fn validate_vertex_buffers(
    pipeline: &Arc<crate::pipeline::RenderPipeline>,
    slot_mask: BitVec,
    binder: &crate::command::bind::Binder,
) -> Result<(), DrawError> {
    // Check all needed vertex buffers have been bound
    for index in pipeline.vertex_steps
        .iter()
        .enumerate()
        .filter_map(|(index, step)| step.map(|_| index))
    {
        if slot_mask.get(index) != Some(true) {
            return Err(DrawError::MissingVertexBuffer {
                pipeline: pipeline.error_ident(),
                index,
            });
        }
    }

    let bind_group_space_used = binder.last_assigned_index().map_or(0, |i| i + 1);
    let vertex_buffer_space_used = slot_mask
        .iter()
        .enumerate()
        .filter_map(|(i, size)| size.then_some(i))
        .next_back()
        .map_or(0, |i| i + 1);

    let bind_groups_plus_vertex_buffers =
        u32::try_from(bind_group_space_used + vertex_buffer_space_used).unwrap();
    if bind_groups_plus_vertex_buffers
        > pipeline.device.limits.max_bind_groups_plus_vertex_buffers
    {
        return Err(DrawError::TooManyBindGroupsPlusVertexBuffers {
            given: bind_groups_plus_vertex_buffers,
            limit: pipeline.device.limits.max_bind_groups_plus_vertex_buffers,
        });
    }

    Ok(())
}

// Pass
validation::validate_vertex_buffers(
    &pipeline,
    BitVec::from_iter(self.vertex.buffer_sizes.iter().map(|size| size.is_some())),
    &self.pass.binder,
)?;

// Bundle
validation::validate_vertex_buffers(
    &pipeline.pipeline,
    BitVec::from_iter(self.vertex.iter().map(|vbs| vbs.is_some())),
    &self.binder,
)?;

size,
} => {
let buffer = buffer.try_raw(snatch_guard)?;
let buffer = buffer.as_ref().unwrap().try_raw(snatch_guard)?;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It might be worth using expect here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There are quite a few unwraps in this function, mainly because we are reusing the same RenderCommand type which has variants with optional fields. I think we should probably use a different type for the render bundle commands after they have been validated. It would be in line with our recent talks about splitting internal from external types.

@teoxoy teoxoy force-pushed the null-vertex-buffers branch from bd87a84 to dbecf92 Compare April 13, 2026 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants