From 967e9c02d81ea420bc74752a4ac81e8a3b686ad8 Mon Sep 17 00:00:00 2001 From: Connor Fitzgerald Date: Thu, 21 Apr 2022 15:23:58 -0400 Subject: [PATCH 1/3] Bind group deduplication --- wgpu-core/src/command/bundle.rs | 55 ++++++++++++++++++++++---------- wgpu-core/src/command/compute.rs | 46 +++++++++++++++++--------- wgpu-core/src/command/mod.rs | 47 ++++++++++++++++++++++++--- wgpu-core/src/command/render.rs | 54 ++++++++++++++++++++----------- 4 files changed, 147 insertions(+), 55 deletions(-) diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index c89c38d9db..a5d0b560fa 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/wgpu-core/src/command/bundle.rs @@ -36,8 +36,8 @@ invalidations or index format changes. use crate::{ binding_model::buffer_binding_type_alignment, command::{ - BasePass, DrawError, MapPassErr, PassErrorScope, RenderCommand, RenderCommandError, - StateChange, + BasePass, BindGroupStateChange, DrawError, MapPassErr, PassErrorScope, RenderCommand, + RenderCommandError, StateChange, }, conv, device::{ @@ -86,6 +86,10 @@ pub struct RenderBundleEncoder { parent_id: id::DeviceId, pub(crate) context: RenderPassContext, pub(crate) is_ds_read_only: bool, + + // Resource binding dedupe state. + current_bind_groups: BindGroupStateChange, + current_pipeline: StateChange, } impl RenderBundleEncoder { @@ -126,6 +130,9 @@ impl RenderBundleEncoder { } None => false, }, + + current_bind_groups: BindGroupStateChange::new(), + current_pipeline: StateChange::new(), }) } @@ -143,6 +150,9 @@ impl RenderBundleEncoder { multiview: None, }, is_ds_read_only: false, + + current_bind_groups: BindGroupStateChange::new(), + current_pipeline: StateChange::new(), } } @@ -180,7 +190,7 @@ impl RenderBundleEncoder { raw_dynamic_offsets: Vec::new(), flat_dynamic_offsets: Vec::new(), used_bind_groups: 0, - pipeline: StateChange::new(), + pipeline: None, }; let mut commands = Vec::new(); let mut base = self.base.as_ref(); @@ -252,9 +262,8 @@ impl RenderBundleEncoder { } RenderCommand::SetPipeline(pipeline_id) => { let scope = PassErrorScope::SetPipelineRender(pipeline_id); - if state.pipeline.set_and_check_redundant(pipeline_id) { - continue; - } + + state.pipeline = Some(pipeline_id); let pipeline = state .trackers @@ -370,7 +379,7 @@ impl RenderBundleEncoder { let scope = PassErrorScope::Draw { indexed: false, indirect: false, - pipeline: state.pipeline.last_state, + pipeline: state.pipeline, }; let vertex_limits = state.vertex_limits(); let last_vertex = first_vertex + vertex_count; @@ -405,7 +414,7 @@ impl RenderBundleEncoder { let scope = PassErrorScope::Draw { indexed: true, indirect: false, - pipeline: state.pipeline.last_state, + pipeline: state.pipeline, }; //TODO: validate that base_vertex + max_index() is within the provided range let vertex_limits = state.vertex_limits(); @@ -441,7 +450,7 @@ impl RenderBundleEncoder { let scope = PassErrorScope::Draw { indexed: false, indirect: true, - pipeline: state.pipeline.last_state, + pipeline: state.pipeline, }; device .require_downlevel_flags(wgt::DownlevelFlags::INDIRECT_EXECUTION) @@ -474,7 +483,7 @@ impl RenderBundleEncoder { let scope = PassErrorScope::Draw { indexed: true, indirect: true, - pipeline: state.pipeline.last_state, + pipeline: state.pipeline, }; device .require_downlevel_flags(wgt::DownlevelFlags::INDIRECT_EXECUTION) @@ -990,7 +999,7 @@ struct State { raw_dynamic_offsets: Vec, flat_dynamic_offsets: Vec, used_bind_groups: usize, - pipeline: StateChange, + pipeline: Option, } impl State { @@ -1222,17 +1231,25 @@ pub mod bundle_ffi { offsets: *const DynamicOffset, offset_length: usize, ) { + let redundant = bundle + .current_bind_groups + .set_and_check_redundant( + bind_group_id, + index, + &mut bundle.base.dynamic_offsets, + offsets, + offset_length, + ); + + if redundant { + return; + } + bundle.base.commands.push(RenderCommand::SetBindGroup { index: index.try_into().unwrap(), num_dynamic_offsets: offset_length.try_into().unwrap(), bind_group_id, }); - if offset_length != 0 { - bundle - .base - .dynamic_offsets - .extend_from_slice(slice::from_raw_parts(offsets, offset_length)); - } } #[no_mangle] @@ -1240,6 +1257,10 @@ pub mod bundle_ffi { bundle: &mut RenderBundleEncoder, pipeline_id: id::RenderPipelineId, ) { + if bundle.current_pipeline.set_and_check_redundant(pipeline_id) { + return; + } + bundle .base .commands diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index 38629543fe..ed3992e208 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -6,8 +6,8 @@ use crate::{ bind::Binder, end_pipeline_statistics_query, memory_init::{fixup_discarded_surfaces, SurfacesInDiscardState}, - BasePass, BasePassRef, CommandBuffer, CommandEncoderError, CommandEncoderStatus, - MapPassErr, PassErrorScope, QueryUseError, StateChange, + BasePass, BasePassRef, BindGroupStateChange, CommandBuffer, CommandEncoderError, + CommandEncoderStatus, MapPassErr, PassErrorScope, QueryUseError, StateChange, }, device::MissingDownlevelFlags, error::{ErrorFormatter, PrettyError}, @@ -76,6 +76,10 @@ pub enum ComputeCommand { pub struct ComputePass { base: BasePass, parent_id: id::CommandEncoderId, + + // Resource binding dedupe state. + current_bind_groups: BindGroupStateChange, + current_pipeline: StateChange, } impl ComputePass { @@ -83,6 +87,9 @@ impl ComputePass { Self { base: BasePass::new(&desc.label), parent_id, + + current_bind_groups: BindGroupStateChange::new(), + current_pipeline: StateChange::new(), } } @@ -222,7 +229,7 @@ where #[derive(Debug)] struct State { binder: Binder, - pipeline: StateChange, + pipeline: Option, trackers: StatefulTrackerSubset, debug_scope_depth: u32, } @@ -236,7 +243,7 @@ impl State { index: bind_mask.trailing_zeros(), }); } - if self.pipeline.is_unset() { + if self.pipeline.is_none() { return Err(DispatchError::MissingPipeline); } self.binder.check_late_buffer_bindings()?; @@ -325,7 +332,7 @@ impl Global { let mut state = State { binder: Binder::new(), - pipeline: StateChange::new(), + pipeline: None, trackers: StatefulTrackerSubset::new(A::VARIANT), debug_scope_depth: 0, }; @@ -420,9 +427,7 @@ impl Global { ComputeCommand::SetPipeline(pipeline_id) => { let scope = PassErrorScope::SetPipelineCompute(pipeline_id); - if state.pipeline.set_and_check_redundant(pipeline_id) { - continue; - } + state.pipeline = Some(pipeline_id); let pipeline = cmd_buf .trackers @@ -524,7 +529,7 @@ impl Global { ComputeCommand::Dispatch(groups) => { let scope = PassErrorScope::Dispatch { indirect: false, - pipeline: state.pipeline.last_state, + pipeline: state.pipeline, }; fixup_discarded_surfaces( @@ -568,7 +573,7 @@ impl Global { ComputeCommand::DispatchIndirect { buffer_id, offset } => { let scope = PassErrorScope::Dispatch { indirect: true, - pipeline: state.pipeline.last_state, + pipeline: state.pipeline, }; state.is_ready().map_pass_err(scope)?; @@ -750,16 +755,23 @@ pub mod compute_ffi { offsets: *const DynamicOffset, offset_length: usize, ) { + let redundant = pass.current_bind_groups.set_and_check_redundant( + bind_group_id, + index, + &mut pass.base.dynamic_offsets, + offsets, + offset_length, + ); + + if redundant { + return; + } + pass.base.commands.push(ComputeCommand::SetBindGroup { index: index.try_into().unwrap(), num_dynamic_offsets: offset_length.try_into().unwrap(), bind_group_id, }); - if offset_length != 0 { - pass.base - .dynamic_offsets - .extend_from_slice(slice::from_raw_parts(offsets, offset_length)); - } } #[no_mangle] @@ -767,6 +779,10 @@ pub mod compute_ffi { pass: &mut ComputePass, pipeline_id: id::ComputePipelineId, ) { + if pass.current_pipeline.set_and_check_redundant(pipeline_id) { + return; + } + pass.base .commands .push(ComputeCommand::SetPipeline(pipeline_id)); diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index e288acfd29..418cad6681 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -8,6 +8,8 @@ mod query; mod render; mod transfer; +use std::slice; + pub(crate) use self::clear::clear_texture_no_device; pub use self::{ bundle::*, clear::ClearError, compute::*, draw::*, query::*, render::*, transfer::*, @@ -405,7 +407,7 @@ where } } -#[derive(Debug)] +#[derive(Debug, Copy, Clone)] struct StateChange { last_state: Option, } @@ -419,14 +421,51 @@ impl StateChange { self.last_state = Some(new_state); already_set } - fn is_unset(&self) -> bool { - self.last_state.is_none() - } fn reset(&mut self) { self.last_state = None; } } +#[derive(Debug)] +struct BindGroupStateChange { + last_states: [StateChange; hal::MAX_BIND_GROUPS], +} + +impl BindGroupStateChange { + fn new() -> Self { + Self { last_states: [StateChange::new(); hal::MAX_BIND_GROUPS] } + } + + unsafe fn set_and_check_redundant( + &mut self, + bind_group_id: id::BindGroupId, + index: u32, + dynamic_offsets: &mut Vec, + offsets: *const wgt::DynamicOffset, + offset_length: usize, + ) -> bool { + // For now never deduplicate bind groups with dynamic offsets. + if offset_length == 0 { + // If this get returns None, that means we're well over the limit, so let the call through to get a proper error + if let Some(current_bind_group) = self.last_states.get_mut(index as usize) { + // Bail out if we're binding the same bind group. + if current_bind_group.set_and_check_redundant(bind_group_id) { + return true; + } + } + } else { + // We intentionally remove the memory of this bind group if we have dynamic offsets, + // such that if you try to bind this bind group later with _no_ dynamic offsets it + // tries to bind it again and gives a proper validation error. + if let Some(current_bind_group) = self.last_states.get_mut(index as usize) { + current_bind_group.reset(); + } + dynamic_offsets.extend_from_slice(slice::from_raw_parts(offsets, offset_length)); + } + false + } +} + trait MapPassErr { fn map_pass_err(self, scope: PassErrorScope) -> Result; } diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 3187d4bbc8..4a5ed71449 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -4,9 +4,9 @@ use crate::{ bind::Binder, end_pipeline_statistics_query, memory_init::{fixup_discarded_surfaces, SurfacesInDiscardState}, - BasePass, BasePassRef, CommandBuffer, CommandEncoderError, CommandEncoderStatus, DrawError, - ExecutionError, MapPassErr, PassErrorScope, QueryResetMap, QueryUseError, RenderCommand, - RenderCommandError, StateChange, + BasePass, BasePassRef, BindGroupStateChange, CommandBuffer, CommandEncoderError, + CommandEncoderStatus, DrawError, ExecutionError, MapPassErr, PassErrorScope, QueryResetMap, + QueryUseError, RenderCommand, RenderCommandError, StateChange, }, device::{ AttachmentData, Device, MissingDownlevelFlags, MissingFeatures, @@ -164,6 +164,10 @@ pub struct RenderPass { parent_id: id::CommandEncoderId, color_targets: ArrayVec, depth_stencil_target: Option, + + // Resource binding dedupe state. + current_bind_groups: BindGroupStateChange, + current_pipeline: StateChange, } impl RenderPass { @@ -173,6 +177,9 @@ impl RenderPass { parent_id, color_targets: desc.color_attachments.iter().cloned().collect(), depth_stencil_target: desc.depth_stencil_attachment.cloned(), + + current_bind_groups: BindGroupStateChange::new(), + current_pipeline: StateChange::new(), } } @@ -337,7 +344,7 @@ struct State { binder: Binder, blend_constant: OptionalState, stencil_reference: u32, - pipeline: StateChange, + pipeline: Option, index: IndexState, vertex: VertexState, debug_scope_depth: u32, @@ -361,7 +368,7 @@ impl State { index: bind_mask.trailing_zeros(), }); } - if self.pipeline.is_unset() { + if self.pipeline.is_none() { return Err(DrawError::MissingPipeline); } if self.blend_constant == OptionalState::Required { @@ -392,7 +399,7 @@ impl State { /// Reset the `RenderBundle`-related states. fn reset_bundle(&mut self) { self.binder.reset(); - self.pipeline.reset(); + self.pipeline = None; self.index.reset(); self.vertex.reset(); } @@ -1092,7 +1099,7 @@ impl Global { binder: Binder::new(), blend_constant: OptionalState::Unused, stencil_reference: 0, - pipeline: StateChange::new(), + pipeline: None, index: IndexState::default(), vertex: VertexState::default(), debug_scope_depth: 0, @@ -1187,9 +1194,7 @@ impl Global { } RenderCommand::SetPipeline(pipeline_id) => { let scope = PassErrorScope::SetPipelineRender(pipeline_id); - if state.pipeline.set_and_check_redundant(pipeline_id) { - continue; - } + state.pipeline = Some(pipeline_id); let pipeline = cmd_buf .trackers @@ -1505,7 +1510,7 @@ impl Global { let scope = PassErrorScope::Draw { indexed, indirect: false, - pipeline: state.pipeline.last_state, + pipeline: state.pipeline, }; state.is_ready(indexed).map_pass_err(scope)?; @@ -1545,7 +1550,7 @@ impl Global { let scope = PassErrorScope::Draw { indexed, indirect: false, - pipeline: state.pipeline.last_state, + pipeline: state.pipeline, }; state.is_ready(indexed).map_pass_err(scope)?; @@ -1589,7 +1594,7 @@ impl Global { let scope = PassErrorScope::Draw { indexed, indirect: true, - pipeline: state.pipeline.last_state, + pipeline: state.pipeline, }; state.is_ready(indexed).map_pass_err(scope)?; @@ -1662,7 +1667,7 @@ impl Global { let scope = PassErrorScope::Draw { indexed, indirect: true, - pipeline: state.pipeline.last_state, + pipeline: state.pipeline, }; state.is_ready(indexed).map_pass_err(scope)?; @@ -2012,16 +2017,23 @@ pub mod render_ffi { offsets: *const DynamicOffset, offset_length: usize, ) { + let redundant = pass.current_bind_groups.set_and_check_redundant( + bind_group_id, + index, + &mut pass.base.dynamic_offsets, + offsets, + offset_length, + ); + + if redundant { + return; + } + pass.base.commands.push(RenderCommand::SetBindGroup { index: index.try_into().unwrap(), num_dynamic_offsets: offset_length.try_into().unwrap(), bind_group_id, }); - if offset_length != 0 { - pass.base - .dynamic_offsets - .extend_from_slice(slice::from_raw_parts(offsets, offset_length)); - } } #[no_mangle] @@ -2029,6 +2041,10 @@ pub mod render_ffi { pass: &mut RenderPass, pipeline_id: id::RenderPipelineId, ) { + if pass.current_pipeline.set_and_check_redundant(pipeline_id) { + return; + } + pass.base .commands .push(RenderCommand::SetPipeline(pipeline_id)); From fd90b212722d6160ad346f9c4de3d821b0ca8bf1 Mon Sep 17 00:00:00 2001 From: Connor Fitzgerald Date: Thu, 21 Apr 2022 15:24:46 -0400 Subject: [PATCH 2/3] Clippy cleanups --- wgpu-core/src/validation.rs | 2 +- wgpu-hal/src/dx11/adapter.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/wgpu-core/src/validation.rs b/wgpu-core/src/validation.rs index 921ec49275..0b6fe82577 100644 --- a/wgpu-core/src/validation.rs +++ b/wgpu-core/src/validation.rs @@ -899,7 +899,7 @@ impl Interface { let mut entry_points = FastHashMap::default(); entry_points.reserve(module.entry_points.len()); - for (index, entry_point) in (&module.entry_points).iter().enumerate() { + for (index, entry_point) in module.entry_points.iter().enumerate() { let info = info.get_entry_point(index); let mut ep = EntryPoint::default(); for arg in entry_point.function.arguments.iter() { diff --git a/wgpu-hal/src/dx11/adapter.rs b/wgpu-hal/src/dx11/adapter.rs index 2e4d3b6214..576eb810f6 100644 --- a/wgpu-hal/src/dx11/adapter.rs +++ b/wgpu-hal/src/dx11/adapter.rs @@ -193,8 +193,8 @@ impl super::Adapter { let limits = wgt::Limits { max_texture_dimension_1d: max_texture_dimension_2d, - max_texture_dimension_2d: max_texture_dimension_2d, - max_texture_dimension_3d: max_texture_dimension_3d, + max_texture_dimension_2d, + max_texture_dimension_3d, max_texture_array_layers: max_texture_dimension_3d, max_bind_groups: u32::MAX, max_dynamic_uniform_buffers_per_pipeline_layout: max_constant_buffers, @@ -206,7 +206,7 @@ impl super::Adapter { max_uniform_buffers_per_shader_stage: max_constant_buffers, max_uniform_buffer_binding_size: 1 << 16, max_storage_buffer_binding_size: u32::MAX, - max_vertex_buffers: max_vertex_buffers, + max_vertex_buffers, max_vertex_attributes: max_vertex_buffers, max_vertex_buffer_array_stride: u32::MAX, max_push_constant_size: 1 << 16, From ef9dbeadfbf877420879fa396476497746835200 Mon Sep 17 00:00:00 2001 From: Connor Fitzgerald Date: Mon, 25 Apr 2022 00:03:46 -0400 Subject: [PATCH 3/3] Fix serde issues --- wgpu-core/src/command/bundle.rs | 18 +++++++++--------- wgpu-core/src/command/compute.rs | 2 ++ wgpu-core/src/command/mod.rs | 16 +++++++++++++++- wgpu-core/src/command/render.rs | 2 ++ 4 files changed, 28 insertions(+), 10 deletions(-) diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index a5d0b560fa..5bd7eafb1a 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/wgpu-core/src/command/bundle.rs @@ -88,7 +88,9 @@ pub struct RenderBundleEncoder { pub(crate) is_ds_read_only: bool, // Resource binding dedupe state. + #[cfg_attr(feature = "serial-pass", serde(skip))] current_bind_groups: BindGroupStateChange, + #[cfg_attr(feature = "serial-pass", serde(skip))] current_pipeline: StateChange, } @@ -1231,15 +1233,13 @@ pub mod bundle_ffi { offsets: *const DynamicOffset, offset_length: usize, ) { - let redundant = bundle - .current_bind_groups - .set_and_check_redundant( - bind_group_id, - index, - &mut bundle.base.dynamic_offsets, - offsets, - offset_length, - ); + let redundant = bundle.current_bind_groups.set_and_check_redundant( + bind_group_id, + index, + &mut bundle.base.dynamic_offsets, + offsets, + offset_length, + ); if redundant { return; diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index ed3992e208..9881e797d1 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -78,7 +78,9 @@ pub struct ComputePass { parent_id: id::CommandEncoderId, // Resource binding dedupe state. + #[cfg_attr(feature = "serial-pass", serde(skip))] current_bind_groups: BindGroupStateChange, + #[cfg_attr(feature = "serial-pass", serde(skip))] current_pipeline: StateChange, } diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index 418cad6681..a4e7967307 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -426,6 +426,12 @@ impl StateChange { } } +impl Default for StateChange { + fn default() -> Self { + Self::new() + } +} + #[derive(Debug)] struct BindGroupStateChange { last_states: [StateChange; hal::MAX_BIND_GROUPS], @@ -433,7 +439,9 @@ struct BindGroupStateChange { impl BindGroupStateChange { fn new() -> Self { - Self { last_states: [StateChange::new(); hal::MAX_BIND_GROUPS] } + Self { + last_states: [StateChange::new(); hal::MAX_BIND_GROUPS], + } } unsafe fn set_and_check_redundant( @@ -466,6 +474,12 @@ impl BindGroupStateChange { } } +impl Default for BindGroupStateChange { + fn default() -> Self { + Self::new() + } +} + trait MapPassErr { fn map_pass_err(self, scope: PassErrorScope) -> Result; } diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 4a5ed71449..2cd83a178f 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -166,7 +166,9 @@ pub struct RenderPass { depth_stencil_target: Option, // Resource binding dedupe state. + #[cfg_attr(feature = "serial-pass", serde(skip))] current_bind_groups: BindGroupStateChange, + #[cfg_attr(feature = "serial-pass", serde(skip))] current_pipeline: StateChange, }