From f9c16024ddbd9f0583a457fc2004513746f7548f Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Fri, 20 May 2022 22:27:12 -0700 Subject: [PATCH 1/4] Document some parts of wgpu_core's render bundle code. --- wgpu-core/src/command/bundle.rs | 61 +++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index fc51927858..4d8eccd538 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/wgpu-core/src/command/bundle.rs @@ -167,6 +167,16 @@ impl RenderBundleEncoder { self.parent_id } + /// Convert this encoder's commands into a [`RenderBundle`]. + /// + /// We want executing a [`RenderBundle`] to be quick, so we take + /// this opportunity to clean up the [`RenderBundleEncoder`]'s + /// command stream and gather metadata about it that will help + /// keep [`ExecuteBundle`] simple and fast. We remove redundant + /// commands (along with their side data), note resource usage, + /// and accumulate buffer and texture initialization actions. + /// + /// [`ExecuteBundle`]: RenderCommand::ExecuteBundle pub(crate) fn finish( self, desc: &RenderBundleDescriptor, @@ -823,6 +833,15 @@ impl Resource for RenderBundle { } } +/// 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 +/// a `SetIndexBuffer` command if one is necessary. +/// +/// [`flush`]: IndexState::flush #[derive(Debug)] struct IndexState { buffer: Option, @@ -833,6 +852,7 @@ struct IndexState { } impl IndexState { + /// Return a fresh state: no index buffer has been set yet. fn new() -> Self { Self { buffer: None, @@ -843,6 +863,9 @@ impl IndexState { } } + /// 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 { @@ -852,6 +875,8 @@ impl IndexState { ((self.range.end - self.range.start) / bytes_per_index) as u32 } + /// Prepare for an indexed draw, producing a `SetIndexBuffer` + /// command if necessary. fn flush(&mut self) -> Option { if self.is_dirty { self.is_dirty = false; @@ -866,6 +891,7 @@ impl IndexState { } } + /// Set the current index buffer's format. fn set_format(&mut self, format: wgt::IndexFormat) { if self.format != format { self.format = format; @@ -873,6 +899,7 @@ impl IndexState { } } + /// Set the current index buffer. fn set_buffer(&mut self, id: id::BufferId, range: Range) { self.buffer = Some(id); self.range = range; @@ -880,6 +907,15 @@ impl IndexState { } } +/// The state of a single vertex buffer slot during render bundle encoding. +/// +/// [`RenderBundleEncoder::finish`] uses this to drop redundant +/// `SetVertexBuffer` commands from the final [`RenderBundle`]. It +/// records one vertex buffer slot's state changes here, and then +/// calls this type's [`flush`] method just before any draw command to +/// produce a `SetVertexBuffer` commands if one is necessary. +/// +/// [`flush`]: IndexState::flush #[derive(Debug)] struct VertexState { buffer: Option, @@ -890,6 +926,8 @@ struct VertexState { } impl VertexState { + /// Construct a fresh `VertexState`: no buffer has been set for + /// this slot. fn new() -> Self { Self { buffer: None, @@ -900,12 +938,16 @@ impl VertexState { } } + /// Set this slot's vertex buffer. fn set_buffer(&mut self, buffer_id: id::BufferId, range: Range) { self.buffer = Some(buffer_id); self.range = range; self.is_dirty = true; } + /// Generate a `SetVertexBuffer` command for this slot, if necessary. + /// + /// `slot` is the index of the vertex buffer slot that `self` tracks. fn flush(&mut self, slot: u32) -> Option { if self.is_dirty { self.is_dirty = false; @@ -992,14 +1034,33 @@ struct VertexLimitState { instance_limit_slot: u32, } +/// State for analyzing and cleaning up bundle command streams. +/// +/// To minimize state updates, [`RenderBundleEncoder::finish`] +/// actually just applies commands like [`SetBindGroup`] and +/// [`SetIndexBuffer`] to the simulated state stored here, and then +/// calls the `flush_foo` methods before draw calls to produce the +/// update commands we actually need. #[derive(Debug)] struct State { + /// Resources used by this bundle. This will become `RenderBundle::used`. trackers: TrackerSet, + + /// The current index buffer. We flush this state before indexed + /// draw commands. index: IndexState, + + /// The state of each vertex buffer slot. vertex: ArrayVec, bind: ArrayVec, push_constant_ranges: PushConstantState, + + /// The accumulated dynamic offsets for all `SetBindGroup` commands seen. + /// + /// Each occupied entry of `bind` has a `dynamic_offsets` range that says + /// which elements of this vector it owns. raw_dynamic_offsets: Vec, + flat_dynamic_offsets: Vec, used_bind_groups: usize, pipeline: Option, From f80f9af382047ffbf8e51c199b543336011bca45 Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Fri, 20 May 2022 23:18:53 -0700 Subject: [PATCH 2/4] Clean up render bundle bind group tracking. The `dynamic_offsets` and `is_dirty` flags only make sense when the slot is occupied, so they should be inside the `Option`. This makes `State::bind` into an `ArrayVec>`, and cleans up various other bits. --- wgpu-core/src/command/bundle.rs | 141 +++++++++++++++++--------------- 1 file changed, 77 insertions(+), 64 deletions(-) diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index 4d8eccd538..4ac8e2c3fe 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/wgpu-core/src/command/bundle.rs @@ -195,9 +195,7 @@ impl RenderBundleEncoder { vertex: (0..hal::MAX_VERTEX_BUFFERS) .map(|_| VertexState::new()) .collect(), - bind: (0..hal::MAX_BIND_GROUPS) - .map(|_| BindState::new()) - .collect(), + bind: (0..hal::MAX_BIND_GROUPS).map(|_| None).collect(), push_constant_ranges: PushConstantState::new(), raw_dynamic_offsets: Vec::new(), flat_dynamic_offsets: Vec::new(), @@ -228,6 +226,8 @@ impl RenderBundleEncoder { .map_pass_err(scope); } + // Peel off the front `num_dynamic_offsets` entries from + // `base.dynamic_offsets`. let offsets = &base.dynamic_offsets[..num_dynamic_offsets as usize]; base.dynamic_offsets = &base.dynamic_offsets[num_dynamic_offsets as usize..]; @@ -963,39 +963,22 @@ impl VertexState { } } +/// A bind group that has been set at a particular index during render bundle encoding. #[derive(Debug)] struct BindState { - bind_group: Option<(id::BindGroupId, id::BindGroupLayoutId)>, - dynamic_offsets: Range, - is_dirty: bool, -} + /// The id of the bind group set at this index. + bind_group_id: id::BindGroupId, -impl BindState { - fn new() -> Self { - Self { - bind_group: None, - dynamic_offsets: 0..0, - is_dirty: false, - } - } + /// The layout of `group`. + layout_id: id::Valid, - fn set_group( - &mut self, - bind_group_id: id::BindGroupId, - layout_id: id::BindGroupLayoutId, - dyn_offset: usize, - dyn_count: usize, - ) -> bool { - match self.bind_group { - Some((bg_id, _)) if bg_id == bind_group_id && dyn_count == 0 => false, - _ => { - self.bind_group = Some((bind_group_id, layout_id)); - self.dynamic_offsets = dyn_offset..dyn_offset + dyn_count; - self.is_dirty = true; - true - } - } - } + /// The range of dynamic offsets in `State::raw_dynamic_offsets` + /// for this bind group. + dynamic_offsets: Range, + + /// True if this index's contents have been changed since the last time we + /// generated a `SetBindGroup` command. + is_dirty: bool, } #[derive(Debug)] @@ -1052,16 +1035,24 @@ struct State { /// The state of each vertex buffer slot. vertex: ArrayVec, - bind: ArrayVec, + + /// The bind group set at each index, if any. + bind: ArrayVec, { hal::MAX_BIND_GROUPS }>, + push_constant_ranges: PushConstantState, - /// The accumulated dynamic offsets for all `SetBindGroup` commands seen. + /// The dynamic offsets for all `SetBindGroup` commands we've seen so far. /// /// Each occupied entry of `bind` has a `dynamic_offsets` range that says /// which elements of this vector it owns. raw_dynamic_offsets: Vec, + /// Dynamic offset values used by the cleaned-up command sequence. + /// + /// These end up in the final `RenderBundle`. Each `SetBindGroup` command + /// consumes the next `num_dynamic_offsets` entries off the front. flat_dynamic_offsets: Vec, + used_bind_groups: usize, pipeline: Option, } @@ -1097,11 +1088,10 @@ impl State { vert_state } - fn invalidate_group_from(&mut self, slot: usize) { - for bind in self.bind[slot..].iter_mut() { - if bind.bind_group.is_some() { - bind.is_dirty = true; - } + /// Mark all non-empty bind group table entries from `index` onwards as dirty. + fn invalidate_group_from(&mut self, index: usize) { + for contents in self.bind[index..].iter_mut().flatten() { + contents.is_dirty = true; } } @@ -1112,15 +1102,33 @@ impl State { layout_id: id::Valid, offsets: &[wgt::DynamicOffset], ) { - if self.bind[slot as usize].set_group( - bind_group_id, - layout_id.0, - self.raw_dynamic_offsets.len(), - offsets.len(), - ) { - self.invalidate_group_from(slot as usize + 1); + // If this call wouldn't actually change this index's state, we can + // return early. (If there are dynamic offsets, the range will always + // be different.) + if offsets.is_empty() { + if let Some(ref contents) = self.bind[slot as usize] { + if contents.bind_group_id == bind_group_id { + return; + } + } } + + // Save `offsets` in the side array, and note where they landed. + let raw_start = self.raw_dynamic_offsets.len(); self.raw_dynamic_offsets.extend(offsets); + let raw_end = self.raw_dynamic_offsets.len(); + + // Record the index's new state. + self.bind[slot as usize] = Some(BindState { + bind_group_id, + layout_id, + dynamic_offsets: raw_start..raw_end, + is_dirty: true, + }); + + // Once we've changed the bind group at a particular index, all + // subsequent indices need to be rewritten. + self.invalidate_group_from(slot as usize + 1); } fn set_pipeline( @@ -1151,8 +1159,8 @@ impl State { self.bind .iter() .zip(layout_ids) - .position(|(bs, layout_id)| match bs.bind_group { - Some((_, bgl_id)) => bgl_id != layout_id.0, + .position(|(entry, &layout_id)| match *entry { + Some(ref contents) => contents.layout_id != layout_id, None => false, }) }; @@ -1190,29 +1198,34 @@ impl State { .flat_map(|(i, vs)| vs.flush(i as u32)) } + /// Generate `SetBindGroup` commands for any bind groups that need to be updated. fn flush_binds(&mut self) -> impl Iterator + '_ { - for bs in self.bind[..self.used_bind_groups].iter() { - if bs.is_dirty { + // Append each dirty bind group's dynamic offsets to `flat_dynamic_offsets`. + for contents in self.bind[..self.used_bind_groups].iter().flatten() { + if contents.is_dirty { self.flat_dynamic_offsets - .extend_from_slice(&self.raw_dynamic_offsets[bs.dynamic_offsets.clone()]); + .extend_from_slice(&self.raw_dynamic_offsets[contents.dynamic_offsets.clone()]); } } - self.bind + + // Then, generate `SetBindGroup` commands to update the dirty bind + // groups. After this, all entries are clean. + self.bind[..self.used_bind_groups] .iter_mut() - .take(self.used_bind_groups) .enumerate() - .flat_map(|(i, bs)| { - if bs.is_dirty { - bs.is_dirty = false; - Some(RenderCommand::SetBindGroup { - index: i as u8, - bind_group_id: bs.bind_group.unwrap().0, - num_dynamic_offsets: (bs.dynamic_offsets.end - bs.dynamic_offsets.start) - as u8, - }) - } else { - None + .flat_map(|(i, entry)| { + if let Some(ref mut contents) = *entry { + if contents.is_dirty { + contents.is_dirty = false; + let offsets = &contents.dynamic_offsets; + return Some(RenderCommand::SetBindGroup { + index: i as u8, + bind_group_id: contents.bind_group_id, + num_dynamic_offsets: (offsets.end - offsets.start) as u8, + }); + } } + None }) } } From 1ad45484910fde2378169eaa900ef764f93cd2fa Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Sat, 21 May 2022 15:11:27 -0700 Subject: [PATCH 3/4] Document command::BasePass. --- wgpu-core/src/command/mod.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index a4e7967307..f16704c53f 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -219,6 +219,17 @@ pub struct BasePassRef<'a, C> { pub push_constant_data: &'a [u32], } +/// A stream of commands for a render pass or compute pass. +/// +/// This also contains side tables referred to by certain commands, +/// like dynamic offsets for [`SetBindGroup`] or string data for +/// [`InsertDebugMarker`]. +/// +/// Render passes use `BasePass`, whereas compute +/// passes use `BasePass`. +/// +/// [`SetBindGroup`]: RenderCommand::SetBindGroup +/// [`InsertDebugMarker`]: RenderCommand::InsertDebugMarker #[doc(hidden)] #[derive(Debug)] #[cfg_attr( @@ -231,9 +242,22 @@ pub struct BasePassRef<'a, C> { )] pub struct BasePass { pub label: Option, + + /// The stream of commands. pub commands: Vec, + + /// Dynamic offsets consumed by [`SetBindGroup`] commands in `commands`. + /// + /// Each successive `SetBindGroup` consumes the next + /// [`num_dynamic_offsets`] values from this list. pub dynamic_offsets: Vec, + + /// Strings used by debug instructions. + /// + /// Each successive [`PushDebugGroup`] or [`InsertDebugMarker`] + /// instruction consumes the next `len` bytes from this vector. pub string_data: Vec, + pub push_constant_data: Vec, } From 4dd2caecb2b02e263cf400be790e7902abfc0d9a Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Sat, 21 May 2022 15:13:08 -0700 Subject: [PATCH 4/4] Eliminate wgpu_core::commands::bundle::State::raw_dynamic_offsets. This vector's contents always ended up identical to the `RenderBundleEncoder`'s `BasePass`'s `dynamic_offsets` vector, so we can just take values from there instead of copying them. --- wgpu-core/src/command/bundle.rs | 63 ++++++++++++++++----------------- 1 file changed, 30 insertions(+), 33 deletions(-) diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index 4ac8e2c3fe..dc0eaf733e 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/wgpu-core/src/command/bundle.rs @@ -197,17 +197,18 @@ impl RenderBundleEncoder { .collect(), bind: (0..hal::MAX_BIND_GROUPS).map(|_| None).collect(), push_constant_ranges: PushConstantState::new(), - raw_dynamic_offsets: Vec::new(), flat_dynamic_offsets: Vec::new(), used_bind_groups: 0, pipeline: None, }; let mut commands = Vec::new(); - let mut base = self.base.as_ref(); let mut pipeline_layout_id = None::>; let mut buffer_memory_init_actions = Vec::new(); let mut texture_memory_init_actions = Vec::new(); + let base = self.base.as_ref(); + let mut next_dynamic_offset = 0; + for &command in base.commands { match command { RenderCommand::SetBindGroup { @@ -226,10 +227,12 @@ impl RenderBundleEncoder { .map_pass_err(scope); } - // Peel off the front `num_dynamic_offsets` entries from - // `base.dynamic_offsets`. - let offsets = &base.dynamic_offsets[..num_dynamic_offsets as usize]; - base.dynamic_offsets = &base.dynamic_offsets[num_dynamic_offsets as usize..]; + // Identify the next `num_dynamic_offsets` entries from `base.dynamic_offsets`. + let num_dynamic_offsets = num_dynamic_offsets as usize; + let offsets_range = + next_dynamic_offset..next_dynamic_offset + num_dynamic_offsets; + next_dynamic_offset = offsets_range.end; + let offsets = &base.dynamic_offsets[offsets_range.clone()]; let bind_group = state .trackers @@ -264,7 +267,7 @@ impl RenderBundleEncoder { buffer_memory_init_actions.extend_from_slice(&bind_group.used_buffer_ranges); texture_memory_init_actions.extend_from_slice(&bind_group.used_texture_ranges); - state.set_bind_group(index, bind_group_id, bind_group.layout_id, offsets); + state.set_bind_group(index, bind_group_id, bind_group.layout_id, offsets_range); state .trackers .merge_extend_stateful(&bind_group.used) @@ -414,7 +417,7 @@ impl RenderBundleEncoder { .map_pass_err(scope); } commands.extend(state.flush_vertices()); - commands.extend(state.flush_binds()); + commands.extend(state.flush_binds(base.dynamic_offsets)); commands.push(command); } RenderCommand::DrawIndexed { @@ -451,7 +454,7 @@ impl RenderBundleEncoder { } commands.extend(state.index.flush()); commands.extend(state.flush_vertices()); - commands.extend(state.flush_binds()); + commands.extend(state.flush_binds(base.dynamic_offsets)); commands.push(command); } RenderCommand::MultiDrawIndirect { @@ -484,7 +487,7 @@ impl RenderBundleEncoder { )); commands.extend(state.flush_vertices()); - commands.extend(state.flush_binds()); + commands.extend(state.flush_binds(base.dynamic_offsets)); commands.push(command); } RenderCommand::MultiDrawIndirect { @@ -519,7 +522,7 @@ impl RenderBundleEncoder { commands.extend(state.index.flush()); commands.extend(state.flush_vertices()); - commands.extend(state.flush_binds()); + commands.extend(state.flush_binds(base.dynamic_offsets)); commands.push(command); } RenderCommand::MultiDrawIndirect { .. } @@ -972,8 +975,8 @@ struct BindState { /// The layout of `group`. layout_id: id::Valid, - /// The range of dynamic offsets in `State::raw_dynamic_offsets` - /// for this bind group. + /// The range of dynamic offsets for this bind group, in the original + /// command stream's `BassPass::dynamic_offsets` array. dynamic_offsets: Range, /// True if this index's contents have been changed since the last time we @@ -1026,7 +1029,7 @@ struct VertexLimitState { /// update commands we actually need. #[derive(Debug)] struct State { - /// Resources used by this bundle. This will become `RenderBundle::used`. + /// Resources used by this bundle. This will become [`RenderBundle::used`]. trackers: TrackerSet, /// The current index buffer. We flush this state before indexed @@ -1041,16 +1044,12 @@ struct State { push_constant_ranges: PushConstantState, - /// The dynamic offsets for all `SetBindGroup` commands we've seen so far. - /// - /// Each occupied entry of `bind` has a `dynamic_offsets` range that says - /// which elements of this vector it owns. - raw_dynamic_offsets: Vec, - /// Dynamic offset values used by the cleaned-up command sequence. /// - /// These end up in the final `RenderBundle`. Each `SetBindGroup` command - /// consumes the next `num_dynamic_offsets` entries off the front. + /// This becomes the final [`RenderBundle`]'s [`BasePass`]'s + /// [`dynamic_offsets`] list. + /// + /// [`dynamic_offsets`]: BasePass::dynamic_offsets flat_dynamic_offsets: Vec, used_bind_groups: usize, @@ -1100,12 +1099,12 @@ impl State { slot: u8, bind_group_id: id::BindGroupId, layout_id: id::Valid, - offsets: &[wgt::DynamicOffset], + dynamic_offsets: Range, ) { // If this call wouldn't actually change this index's state, we can // return early. (If there are dynamic offsets, the range will always // be different.) - if offsets.is_empty() { + if dynamic_offsets.is_empty() { if let Some(ref contents) = self.bind[slot as usize] { if contents.bind_group_id == bind_group_id { return; @@ -1113,16 +1112,11 @@ impl State { } } - // Save `offsets` in the side array, and note where they landed. - let raw_start = self.raw_dynamic_offsets.len(); - self.raw_dynamic_offsets.extend(offsets); - let raw_end = self.raw_dynamic_offsets.len(); - // Record the index's new state. self.bind[slot as usize] = Some(BindState { bind_group_id, layout_id, - dynamic_offsets: raw_start..raw_end, + dynamic_offsets, is_dirty: true, }); @@ -1199,17 +1193,20 @@ impl State { } /// Generate `SetBindGroup` commands for any bind groups that need to be updated. - fn flush_binds(&mut self) -> impl Iterator + '_ { + fn flush_binds( + &mut self, + dynamic_offsets: &[wgt::DynamicOffset], + ) -> impl Iterator + '_ { // Append each dirty bind group's dynamic offsets to `flat_dynamic_offsets`. for contents in self.bind[..self.used_bind_groups].iter().flatten() { if contents.is_dirty { self.flat_dynamic_offsets - .extend_from_slice(&self.raw_dynamic_offsets[contents.dynamic_offsets.clone()]); + .extend_from_slice(&dynamic_offsets[contents.dynamic_offsets.clone()]); } } // Then, generate `SetBindGroup` commands to update the dirty bind - // groups. After this, all entries are clean. + // groups. After this, all bind groups are clean. self.bind[..self.used_bind_groups] .iter_mut() .enumerate()