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.

Sign up for GitHub

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

Clean up render bundle index buffer tracking. #2743

Merged
merged 2 commits into from
Jun 7, 2022
+55 −51
Merged

Clean up render bundle index buffer tracking. #2743

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3abf33e
Remove unused field `bundle::IndexState::pipeline_format`.
jimblandy May 25, 2022
7593b61
Clean up render bundle index buffer tracking.
jimblandy May 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 55 additions & 51 deletions wgpu-core/src/command/bundle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ impl RenderBundleEncoder {
&*pipeline_guard,
&*query_set_guard,
),
index: IndexState::new(),
index: None,
vertex: (0..hal::MAX_VERTEX_BUFFERS)
.map(|_| VertexState::new())
.collect(),
Expand Down Expand Up @@ -355,7 +355,6 @@ impl RenderBundleEncoder {
pipeline_layout_id = Some(pipeline.layout_id.value);

state.set_pipeline(
pipeline.strip_index_format,
&pipeline.vertex_strides,
&layout.bind_group_layout_ids,
&layout.push_constant_ranges,
Expand Down Expand Up @@ -391,8 +390,7 @@ impl RenderBundleEncoder {
offset..end,
MemoryInitKind::NeedsInitializedMemory,
));
state.index.set_format(index_format);
state.index.set_buffer(buffer_id, offset..end);
state.set_index_buffer(buffer_id, index_format, offset..end);
}
RenderCommand::SetVertexBuffer {
slot,
Expand Down Expand Up @@ -488,9 +486,13 @@ impl RenderBundleEncoder {
indirect: false,
pipeline: state.pipeline,
};
let index = match state.index {
Some(ref index) => index,
None => return Err(DrawError::MissingIndexBuffer).map_pass_err(scope),
};
//TODO: validate that base_vertex + max_index() is within the provided range
let vertex_limits = state.vertex_limits();
let index_limit = state.index.limit();
let index_limit = index.limit();
let last_index = first_index + index_count;
if last_index > index_limit {
return Err(DrawError::IndexBeyondLimit {
Expand All @@ -508,7 +510,7 @@ impl RenderBundleEncoder {
})
.map_pass_err(scope);
}
commands.extend(state.index.flush());
commands.extend(state.flush_index());
commands.extend(state.flush_vertices());
commands.extend(state.flush_binds(base.dynamic_offsets));
commands.push(command);
Expand Down Expand Up @@ -579,7 +581,12 @@ impl RenderBundleEncoder {
MemoryInitKind::NeedsInitializedMemory,
));

commands.extend(state.index.flush());
let index = match state.index {
Some(ref mut index) => index,
None => return Err(DrawError::MissingIndexBuffer).map_pass_err(scope),
};

commands.extend(index.flush());
commands.extend(state.flush_vertices());
commands.extend(state.flush_binds(base.dynamic_offsets));
commands.push(command);
Expand Down Expand Up @@ -907,53 +914,36 @@ impl<A: HalApi> Resource for RenderBundle<A> {

/// A render bundle's current index buffer state.
///
/// [`RenderBundleEncoder::finish`] uses this to drop redundant
/// `SetIndexBuffer` commands from the final [`RenderBundle`]. It
/// records index buffer state changes here, and then calls this
/// type's [`flush`] method before any indexed draw command to produce
/// [`RenderBundleEncoder::finish`] records the currently set index buffer here,
/// and calls [`State::flush_index`] before any indexed draw command to produce
/// a `SetIndexBuffer` command if one is necessary.
///
/// [`flush`]: IndexState::flush
#[derive(Debug)]
struct IndexState {
buffer: Option<id::BufferId>,
buffer: id::BufferId,
format: wgt::IndexFormat,
pipeline_format: Option<wgt::IndexFormat>,
range: Range<wgt::BufferAddress>,
is_dirty: bool,
}

impl IndexState {
/// Return a fresh state: no index buffer has been set yet.
fn new() -> Self {
Self {
buffer: None,
format: wgt::IndexFormat::default(),
pipeline_format: None,
range: 0..0,
is_dirty: false,
}
}

/// Return the number of entries in the current index buffer.
///
/// Panic if no index buffer has been set.
fn limit(&self) -> u32 {
assert!(self.buffer.is_some());
let bytes_per_index = match self.format {
wgt::IndexFormat::Uint16 => 2,
wgt::IndexFormat::Uint32 => 4,
};
((self.range.end - self.range.start) / bytes_per_index) as u32
}

/// Prepare for an indexed draw, producing a `SetIndexBuffer`
/// command if necessary.
/// Generate a `SetIndexBuffer` command to prepare for an indexed draw
/// command, if needed.
fn flush(&mut self) -> Option<RenderCommand> {
if self.is_dirty {
self.is_dirty = false;
Some(RenderCommand::SetIndexBuffer {
buffer_id: self.buffer.unwrap(),
buffer_id: self.buffer,
index_format: self.format,
offset: self.range.start,
size: wgt::BufferSize::new(self.range.end - self.range.start),
Expand All @@ -962,21 +952,6 @@ impl IndexState {
None
}
}

/// Set the current index buffer's format.
fn set_format(&mut self, format: wgt::IndexFormat) {
if self.format != format {
self.format = format;
self.is_dirty = true;
}
}

/// Set the current index buffer.
fn set_buffer(&mut self, id: id::BufferId, range: Range<wgt::BufferAddress>) {
self.buffer = Some(id);
self.range = range;
self.is_dirty = true;
}
}

/// The state of a single vertex buffer slot during render bundle encoding.
Expand Down Expand Up @@ -1100,9 +1075,9 @@ struct State<A: HalApi> {
/// Resources used by this bundle. This will become [`RenderBundle::used`].
trackers: RenderBundleScope<A>,

/// The current index buffer. We flush this state before indexed
/// draw commands.
index: IndexState,
/// The current index buffer, if one has been set. We flush this state
/// before indexed draw commands.
index: Option<IndexState>,

/// The state of each vertex buffer slot.
vertex: ArrayVec<VertexState, { hal::MAX_VERTEX_BUFFERS }>,
Expand Down Expand Up @@ -1195,13 +1170,10 @@ impl<A: HalApi> State<A> {

fn set_pipeline(
&mut self,
index_format: Option<wgt::IndexFormat>,
vertex_strides: &[(wgt::BufferAddress, wgt::VertexStepMode)],
layout_ids: &[id::Valid<id::BindGroupLayoutId>],
push_constant_layouts: &[wgt::PushConstantRange],
) {
self.index.pipeline_format = index_format;

for (vs, &(stride, step_mode)) in self.vertex.iter_mut().zip(vertex_strides) {
if vs.stride != stride || vs.rate != step_mode {
vs.stride = stride;
Expand Down Expand Up @@ -1231,6 +1203,38 @@ impl<A: HalApi> State<A> {
}
}

/// Set the bundle's current index buffer and its associated parameters.
fn set_index_buffer(
&mut self,
buffer: id::BufferId,
format: wgt::IndexFormat,
range: Range<wgt::BufferAddress>,
) {
match self.index {
Some(ref current)
cwfitzgerald marked this conversation as resolved.
Show resolved Hide resolved
if current.buffer == buffer
&& current.format == format
&& current.range == range =>
{
return
}
_ => (),
}

self.index = Some(IndexState {
buffer,
format,
range,
is_dirty: true,
});
}

/// Generate a `SetIndexBuffer` command to prepare for an indexed draw
/// command, if needed.
fn flush_index(&mut self) -> Option<RenderCommand> {
self.index.as_mut().and_then(|index| index.flush())
}

fn flush_push_constants(&mut self) -> Option<impl Iterator<Item = RenderCommand>> {
let is_dirty = self.push_constant_ranges.is_dirty;

Expand Down