diff --git a/cts_runner/test.lst b/cts_runner/test.lst index 133fb46ff6..f01ffca0e2 100644 --- a/cts_runner/test.lst +++ b/cts_runner/test.lst @@ -81,14 +81,10 @@ webgpu:api,validation,encoding,encoder_open_state:render_pass_commands:* //FAIL: webgpu:api,validation,encoding,encoder_open_state:render_bundle_commands:* // https://github.com/gfx-rs/wgpu/issues/7857 webgpu:api,validation,encoding,encoder_open_state:compute_pass_commands:* -webgpu:api,validation,encoding,programmable,pipeline_bind_group_compat:bgl_binding_mismatch:encoderType="compute%20pass";* -webgpu:api,validation,encoding,programmable,pipeline_bind_group_compat:bgl_binding_mismatch:encoderType="render%20pass";* -webgpu:api,validation,encoding,programmable,pipeline_bind_group_compat:bgl_resource_type_mismatch:encoderType="compute%20pass";* -webgpu:api,validation,encoding,programmable,pipeline_bind_group_compat:bgl_resource_type_mismatch:encoderType="render%20pass";* -webgpu:api,validation,encoding,programmable,pipeline_bind_group_compat:bgl_visibility_mismatch:encoderType="compute%20pass";* -webgpu:api,validation,encoding,programmable,pipeline_bind_group_compat:bgl_visibility_mismatch:encoderType="render%20pass";* -webgpu:api,validation,encoding,programmable,pipeline_bind_group_compat:bind_groups_and_pipeline_layout_mismatch:encoderType="compute%20pass";* -webgpu:api,validation,encoding,programmable,pipeline_bind_group_compat:bind_groups_and_pipeline_layout_mismatch:encoderType="render%20pass";* +webgpu:api,validation,encoding,programmable,pipeline_bind_group_compat:bgl_binding_mismatch:* +webgpu:api,validation,encoding,programmable,pipeline_bind_group_compat:bgl_resource_type_mismatch:* +webgpu:api,validation,encoding,programmable,pipeline_bind_group_compat:bgl_visibility_mismatch:* +webgpu:api,validation,encoding,programmable,pipeline_bind_group_compat:bind_groups_and_pipeline_layout_mismatch:* webgpu:api,validation,encoding,programmable,pipeline_bind_group_compat:buffer_binding,render_pipeline:* webgpu:api,validation,encoding,programmable,pipeline_bind_group_compat:sampler_binding,render_pipeline:* webgpu:api,validation,encoding,queries,general:occlusion_query,query_index:* diff --git a/wgpu-core/src/command/bind.rs b/wgpu-core/src/command/bind.rs index d17144a270..55dc5269e9 100644 --- a/wgpu-core/src/command/bind.rs +++ b/wgpu-core/src/command/bind.rs @@ -7,7 +7,7 @@ use thiserror::Error; use crate::{ binding_model::{BindGroup, LateMinBufferBindingSizeMismatch, PipelineLayout}, pipeline::LateSizedBufferGroup, - resource::{Labeled, ResourceErrorIdent}, + resource::{Labeled, ParentDevice, ResourceErrorIdent}, }; mod compat { @@ -336,12 +336,20 @@ impl Binder { } } + /// Returns `true` if the pipeline layout has been changed, i.e. if the + /// new PL was not the same as the old PL. pub(super) fn change_pipeline_layout<'a>( &'a mut self, new: &Arc, late_sized_buffer_groups: &[LateSizedBufferGroup], - ) { - let old_id_opt = self.pipeline_layout.replace(new.clone()); + ) -> bool { + if let Some(old) = self.pipeline_layout.as_ref() { + if old.is_equal(new) { + return false; + } + } + + let old = self.pipeline_layout.replace(new.clone()); self.manager.update_expectations(&new.bind_group_layouts); @@ -372,12 +380,14 @@ impl Binder { } } - if let Some(old) = old_id_opt { + if let Some(old) = old { // root constants are the base compatibility property if old.immediate_size != new.immediate_size { self.manager.update_start_index(0); } } + + true } pub(super) fn assign_group<'a>( diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index 3e029199ac..43f0a62be1 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/wgpu-core/src/command/bundle.rs @@ -101,8 +101,8 @@ use crate::command::ArcReferences; use crate::{ binding_model::{BindError, BindGroup, PipelineLayout}, command::{ - BasePass, BindGroupStateChange, ColorAttachmentError, DrawError, IdReferences, MapPassErr, - PassErrorScope, RenderCommand, RenderCommandError, StateChange, + bind::Binder, BasePass, BindGroupStateChange, ColorAttachmentError, DrawError, + IdReferences, MapPassErr, PassErrorScope, RenderCommand, RenderCommandError, StateChange, }, device::{AttachmentData, Device, DeviceError, MissingDownlevelFlags, RenderPassContext}, hub::Hub, @@ -277,7 +277,6 @@ impl RenderBundleEncoder { let mut state = State { trackers: RenderBundleScope::new(), pipeline: None, - bind: (0..hal::MAX_BIND_GROUPS).map(|_| None).collect(), vertex: Default::default(), index: None, flat_dynamic_offsets: Vec::new(), @@ -286,6 +285,7 @@ impl RenderBundleEncoder { buffer_memory_init_actions: Vec::new(), texture_memory_init_actions: Vec::new(), next_dynamic_offset: 0, + binder: Binder::new(), }; let indices = &state.device.tracker_indices; @@ -372,7 +372,6 @@ impl RenderBundleEncoder { }; draw( &mut state, - &base.dynamic_offsets, vertex_count, instance_count, first_vertex, @@ -393,7 +392,6 @@ impl RenderBundleEncoder { }; draw_indexed( &mut state, - &base.dynamic_offsets, index_count, instance_count, first_index, @@ -411,14 +409,8 @@ impl RenderBundleEncoder { kind: DrawKind::Draw, family: DrawCommandFamily::DrawMeshTasks, }; - draw_mesh_tasks( - &mut state, - &base.dynamic_offsets, - group_count_x, - group_count_y, - group_count_z, - ) - .map_pass_err(scope)?; + draw_mesh_tasks(&mut state, group_count_x, group_count_y, group_count_z) + .map_pass_err(scope)?; } &RenderCommand::DrawIndirect { buffer, @@ -432,15 +424,8 @@ impl RenderBundleEncoder { kind: DrawKind::DrawIndirect, family, }; - multi_draw_indirect( - &mut state, - &base.dynamic_offsets, - &buffer_guard, - buffer, - offset, - family, - ) - .map_pass_err(scope)?; + multi_draw_indirect(&mut state, &buffer_guard, buffer, offset, family) + .map_pass_err(scope)?; } &RenderCommand::DrawIndirect { count, @@ -566,18 +551,13 @@ fn set_bind_group( bind_group.validate_dynamic_bindings(index, offsets)?; + unsafe { state.trackers.merge_bind_group(&bind_group.used)? }; + let bind_group = state.trackers.bind_groups.insert_single(bind_group); + state - .buffer_memory_init_actions - .extend_from_slice(&bind_group.used_buffer_ranges); - state - .texture_memory_init_actions - .extend_from_slice(&bind_group.used_texture_ranges); + .binder + .assign_group(index as usize, bind_group, offsets); - state.set_bind_group(index, &bind_group, offsets_range); - unsafe { state.trackers.merge_bind_group(&bind_group.used)? }; - state.trackers.bind_groups.insert_single(bind_group); - // Note: stateless trackers are not merged: the lifetime reference - // is held to the bind group itself. Ok(()) } @@ -615,9 +595,12 @@ fn set_pipeline( state.commands.push(cmd); } - state.invalidate_bind_groups(&pipeline_state, &pipeline.layout); state.pipeline = Some(pipeline_state); + state + .binder + .change_pipeline_layout(&pipeline.layout, &pipeline.late_sized_buffer_groups); + state.trackers.render_pipelines.insert_single(pipeline); Ok(()) } @@ -730,14 +713,13 @@ fn set_immediates( fn draw( state: &mut State, - dynamic_offsets: &[u32], vertex_count: u32, instance_count: u32, first_vertex: u32, first_instance: u32, ) -> Result<(), RenderBundleErrorInner> { + state.is_ready()?; let pipeline = state.pipeline()?; - let used_bind_groups = pipeline.used_bind_groups; let vertex_limits = super::VertexLimits::new(state.vertex_buffer_sizes(), &pipeline.steps); vertex_limits.validate_vertex_limit(first_vertex, vertex_count)?; @@ -745,7 +727,7 @@ fn draw( if instance_count > 0 && vertex_count > 0 { state.flush_vertices(); - state.flush_binds(used_bind_groups, dynamic_offsets); + state.flush_bindings(); state.commands.push(ArcRenderCommand::Draw { vertex_count, instance_count, @@ -758,15 +740,15 @@ fn draw( fn draw_indexed( state: &mut State, - dynamic_offsets: &[u32], index_count: u32, instance_count: u32, first_index: u32, base_vertex: i32, first_instance: u32, ) -> Result<(), RenderBundleErrorInner> { + state.is_ready()?; let pipeline = state.pipeline()?; - let used_bind_groups = pipeline.used_bind_groups; + let index = match state.index { Some(ref index) => index, None => return Err(DrawError::MissingIndexBuffer.into()), @@ -788,7 +770,7 @@ fn draw_indexed( if instance_count > 0 && index_count > 0 { state.flush_index(); state.flush_vertices(); - state.flush_binds(used_bind_groups, dynamic_offsets); + state.flush_bindings(); state.commands.push(ArcRenderCommand::DrawIndexed { index_count, instance_count, @@ -802,13 +784,11 @@ fn draw_indexed( fn draw_mesh_tasks( state: &mut State, - dynamic_offsets: &[u32], group_count_x: u32, group_count_y: u32, group_count_z: u32, ) -> Result<(), RenderBundleErrorInner> { - let pipeline = state.pipeline()?; - let used_bind_groups = pipeline.used_bind_groups; + state.is_ready()?; let groups_size_limit = state.device.limits.max_task_workgroups_per_dimension; let max_groups = state.device.limits.max_task_workgroup_total_count; @@ -825,7 +805,7 @@ fn draw_mesh_tasks( } if group_count_x > 0 && group_count_y > 0 && group_count_z > 0 { - state.flush_binds(used_bind_groups, dynamic_offsets); + state.flush_bindings(); state.commands.push(ArcRenderCommand::DrawMeshTasks { group_count_x, group_count_y, @@ -837,18 +817,17 @@ fn draw_mesh_tasks( fn multi_draw_indirect( state: &mut State, - dynamic_offsets: &[u32], buffer_guard: &crate::storage::Storage>, buffer_id: id::Id, offset: u64, family: DrawCommandFamily, ) -> Result<(), RenderBundleErrorInner> { + state.is_ready()?; state .device .require_downlevel_flags(wgt::DownlevelFlags::INDIRECT_EXECUTION)?; let pipeline = state.pipeline()?; - let used_bind_groups = pipeline.used_bind_groups; let buffer = buffer_guard.get(buffer_id).get()?; @@ -887,7 +866,7 @@ fn multi_draw_indirect( state.trackers.buffers.merge_single(&buffer, buffer_uses)?; state.flush_vertices(); - state.flush_binds(used_bind_groups, dynamic_offsets); + state.flush_bindings(); state.commands.push(ArcRenderCommand::DrawIndirect { buffer, offset, @@ -1299,21 +1278,6 @@ impl VertexState { } } -/// A bind group that has been set at a particular index during render bundle encoding. -#[derive(Debug)] -struct BindState { - /// The id of the bind group set at this index. - bind_group: Arc, - - /// 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 - /// generated a `SetBindGroup` command. - is_dirty: bool, -} - /// The bundle's current pipeline, and some cached information needed for validation. struct PipelineState { /// The pipeline @@ -1325,9 +1289,6 @@ struct PipelineState { /// Size of the immediate data ranges this pipeline uses. Copied from the pipeline layout. immediate_size: u32, - - /// The number of bind groups this pipeline uses. - used_bind_groups: usize, } impl PipelineState { @@ -1336,7 +1297,6 @@ impl PipelineState { pipeline: pipeline.clone(), steps: pipeline.vertex_steps.to_vec(), immediate_size: pipeline.layout.immediate_size, - used_bind_groups: pipeline.layout.bind_group_layouts.len(), } } @@ -1372,9 +1332,6 @@ struct State { /// The currently set pipeline, if any. pipeline: Option, - /// The bind group set at each index, if any. - bind: ArrayVec, { hal::MAX_BIND_GROUPS }>, - /// The state of each vertex buffer slot. vertex: [Option; hal::MAX_VERTEX_BUFFERS], @@ -1395,6 +1352,7 @@ struct State { buffer_memory_init_actions: Vec, texture_memory_init_actions: Vec, next_dynamic_offset: usize, + binder: Binder, } impl State { @@ -1405,86 +1363,6 @@ impl State { .ok_or(DrawError::MissingPipeline(pass::MissingPipeline).into()) } - /// Mark all non-empty bind group table entries from `index` onwards as dirty. - fn invalidate_bind_group_from(&mut self, index: usize) { - for contents in self.bind[index..].iter_mut().flatten() { - contents.is_dirty = true; - } - } - - fn set_bind_group( - &mut self, - slot: u32, - bind_group: &Arc, - 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 dynamic_offsets.is_empty() { - if let Some(ref contents) = self.bind[slot as usize] { - if contents.bind_group.is_equal(bind_group) { - return; - } - } - } - - // Record the index's new state. - self.bind[slot as usize] = Some(BindState { - bind_group: bind_group.clone(), - dynamic_offsets, - is_dirty: true, - }); - - // Once we've changed the bind group at a particular index, all - // subsequent indices need to be rewritten. - self.invalidate_bind_group_from(slot as usize + 1); - } - - /// Determine which bind group slots need to be re-set after a pipeline change. - /// - /// Given that we are switching from the current pipeline state to `new`, - /// whose layout is `layout`, mark all the bind group slots that we need to - /// emit new `SetBindGroup` commands for as dirty. - /// - /// According to `wgpu_hal`'s rules: - /// - /// - If the layout of any bind group slot changes, then that slot and - /// all following slots must have their bind groups re-established. - /// - /// - Changing the immediate data ranges at all requires re-establishing - /// all bind groups. - fn invalidate_bind_groups(&mut self, new: &PipelineState, layout: &PipelineLayout) { - match self.pipeline { - None => { - // Establishing entirely new pipeline state. - self.invalidate_bind_group_from(0); - } - Some(ref old) => { - if old.pipeline.is_equal(&new.pipeline) { - // Everything is derived from the pipeline, so if the id has - // not changed, there's no need to consider anything else. - return; - } - - // Any immediate data change invalidates all groups. - if old.immediate_size != new.immediate_size { - self.invalidate_bind_group_from(0); - } else { - let first_changed = self.bind.iter().zip(&layout.bind_group_layouts).position( - |(entry, layout)| match *entry { - Some(ref contents) => !contents.bind_group.layout.is_equal(layout), - None => false, - }, - ); - if let Some(slot) = first_changed { - self.invalidate_bind_group_from(slot); - } - } - } - } - } - /// Set the bundle's current index buffer and its associated parameters. fn set_index_buffer( &mut self, @@ -1527,37 +1405,44 @@ impl State { self.commands.extend(commands); } - /// Generate `SetBindGroup` commands for any bind groups that need to be updated. - fn flush_binds(&mut self, used_bind_groups: usize, dynamic_offsets: &[wgt::DynamicOffset]) { - // Append each dirty bind group's dynamic offsets to `flat_dynamic_offsets`. - for contents in self.bind[..used_bind_groups].iter().flatten() { - if contents.is_dirty { - self.flat_dynamic_offsets - .extend_from_slice(&dynamic_offsets[contents.dynamic_offsets.clone()]); - } + /// Validation for a draw command. + /// + /// This should be further deduplicated with similar validation on render/compute passes. + fn is_ready(&mut self) -> Result<(), DrawError> { + if let Some(pipeline) = self.pipeline.as_ref() { + self.binder + .check_compatibility(pipeline.pipeline.as_ref())?; + self.binder.check_late_buffer_bindings()?; + Ok(()) + } else { + Err(DrawError::MissingPipeline(pass::MissingPipeline)) } + } - // Then, generate `SetBindGroup` commands to update the dirty bind - // groups. After this, all bind groups are clean. - let commands = self.bind[..used_bind_groups] - .iter_mut() - .enumerate() - .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(ArcRenderCommand::SetBindGroup { - index: i.try_into().unwrap(), - bind_group: Some(contents.bind_group.clone()), - num_dynamic_offsets: offsets.end - offsets.start, - }); - } - } - None - }); - - self.commands.extend(commands); + /// Generate `SetBindGroup` commands for any bind groups that need to be updated. + /// + /// This should be further deduplicated with similar code on render/compute passes. + fn flush_bindings(&mut self) { + let range = self.binder.take_rebind_range(); + let entries = self.binder.entries(range); + + self.commands.extend(entries.map(|(i, entry)| { + let bind_group = entry.group.as_ref().unwrap(); + + self.buffer_memory_init_actions + .extend_from_slice(&bind_group.used_buffer_ranges); + self.texture_memory_init_actions + .extend_from_slice(&bind_group.used_texture_ranges); + + self.flat_dynamic_offsets + .extend_from_slice(&entry.dynamic_offsets); + + ArcRenderCommand::SetBindGroup { + index: i.try_into().unwrap(), + bind_group: Some(bind_group.clone()), + num_dynamic_offsets: entry.dynamic_offsets.len(), + } + })); } fn vertex_buffer_sizes(&self) -> impl Iterator> + '_ { diff --git a/wgpu-core/src/command/pass.rs b/wgpu-core/src/command/pass.rs index 0ded4bf1ef..fc3ce3494f 100644 --- a/wgpu-core/src/command/pass.rs +++ b/wgpu-core/src/command/pass.rs @@ -202,18 +202,10 @@ pub(super) fn change_pipeline_layout( where E: From, { - if state.binder.pipeline_layout.is_none() - || !state - .binder - .pipeline_layout - .as_ref() - .unwrap() - .is_equal(pipeline_layout) + if state + .binder + .change_pipeline_layout(pipeline_layout, late_sized_buffer_groups) { - state - .binder - .change_pipeline_layout(pipeline_layout, late_sized_buffer_groups); - f(); super::immediates_clear(