From bc6020f269f9597901c6219f20e73345617f04e3 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 11:44:34 +0200 Subject: [PATCH 01/47] use `.error_ident()` for `api_log!`s in `render_pass_end_impl` --- wgpu-core/src/command/render.rs | 59 +++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index ab44dc8798..b095820255 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1498,10 +1498,12 @@ impl Global { num_dynamic_offsets, bind_group, } => { - let bind_group_id = bind_group.as_info().id(); - api_log!("RenderPass::set_bind_group {index} {bind_group_id:?}"); + api_log!( + "RenderPass::set_bind_group {index} {}", + bind_group.error_ident() + ); - let scope = PassErrorScope::SetBindGroup(bind_group_id); + let scope = PassErrorScope::SetBindGroup(bind_group.as_info().id()); let max_bind_groups = device.limits.max_bind_groups; if index >= max_bind_groups { return Err(RenderCommandError::BindGroupIndexOutOfRange { @@ -1575,11 +1577,10 @@ impl Global { } } ArcRenderCommand::SetPipeline(pipeline) => { - let pipeline_id = pipeline.as_info().id(); - api_log!("RenderPass::set_pipeline {pipeline_id:?}"); + api_log!("RenderPass::set_pipeline {}", pipeline.error_ident()); - let scope = PassErrorScope::SetPipelineRender(pipeline_id); - state.pipeline = Some(pipeline_id); + let scope = PassErrorScope::SetPipelineRender(pipeline.as_info().id()); + state.pipeline = Some(pipeline.as_info().id()); let pipeline = tracker.render_pipelines.insert_single(pipeline); @@ -1701,10 +1702,9 @@ impl Global { offset, size, } => { - let buffer_id = buffer.as_info().id(); - api_log!("RenderPass::set_index_buffer {buffer_id:?}"); + api_log!("RenderPass::set_index_buffer {}", buffer.error_ident()); - let scope = PassErrorScope::SetIndexBuffer(buffer_id); + let scope = PassErrorScope::SetIndexBuffer(buffer.as_info().id()); info.usage_scope .buffers @@ -1724,7 +1724,7 @@ impl Global { Some(s) => offset + s.get(), None => buffer.size, }; - state.index.bound_buffer_view = Some((buffer_id, offset..end)); + state.index.bound_buffer_view = Some((buffer.as_info().id(), offset..end)); state.index.format = Some(index_format); state.index.update_limit(); @@ -1752,10 +1752,12 @@ impl Global { offset, size, } => { - let buffer_id = buffer.as_info().id(); - api_log!("RenderPass::set_vertex_buffer {slot} {buffer_id:?}"); + api_log!( + "RenderPass::set_vertex_buffer {slot} {}", + buffer.error_ident() + ); - let scope = PassErrorScope::SetVertexBuffer(buffer_id); + let scope = PassErrorScope::SetVertexBuffer(buffer.as_info().id()); info.usage_scope .buffers @@ -2041,8 +2043,10 @@ impl Global { count, indexed, } => { - let indirect_buffer_id = indirect_buffer.as_info().id(); - api_log!("RenderPass::draw_indirect (indexed:{indexed}) {indirect_buffer_id:?} {offset} {count:?}"); + api_log!( + "RenderPass::draw_indirect (indexed:{indexed}) {} {offset} {count:?}", + indirect_buffer.error_ident() + ); let scope = PassErrorScope::Draw { kind: if count.is_some() { @@ -2118,9 +2122,11 @@ impl Global { max_count, indexed, } => { - let indirect_buffer_id = indirect_buffer.as_info().id(); - let count_buffer_id = count_buffer.as_info().id(); - api_log!("RenderPass::multi_draw_indirect_count (indexed:{indexed}) {indirect_buffer_id:?} {offset} {count_buffer_id:?} {count_buffer_offset:?} {max_count:?}"); + api_log!( + "RenderPass::multi_draw_indirect_count (indexed:{indexed}) {} {offset} {} {count_buffer_offset:?} {max_count:?}", + indirect_buffer.error_ident(), + count_buffer.error_ident() + ); let scope = PassErrorScope::Draw { kind: DrawKind::MultiDrawIndirectCount, @@ -2266,8 +2272,10 @@ impl Global { query_set, query_index, } => { - let query_set_id = query_set.as_info().id(); - api_log!("RenderPass::write_timestamps {query_set_id:?} {query_index}"); + api_log!( + "RenderPass::write_timestamps {query_index} {}", + query_set.error_ident() + ); let scope = PassErrorScope::WriteTimestamp; device @@ -2318,8 +2326,10 @@ impl Global { query_set, query_index, } => { - let query_set_id = query_set.as_info().id(); - api_log!("RenderPass::begin_pipeline_statistics_query {query_set_id:?} {query_index}"); + api_log!( + "RenderPass::begin_pipeline_statistics_query {query_index} {}", + query_set.error_ident() + ); let scope = PassErrorScope::BeginPipelineStatisticsQuery; let query_set = tracker.query_sets.insert_single(query_set); @@ -2341,8 +2351,7 @@ impl Global { .map_pass_err(scope)?; } ArcRenderCommand::ExecuteBundle(bundle) => { - let bundle_id = bundle.as_info().id(); - api_log!("RenderPass::execute_bundle {bundle_id:?}"); + api_log!("RenderPass::execute_bundle {}", bundle.error_ident()); let scope = PassErrorScope::ExecuteBundle; // Have to clone the bundle arc, otherwise we keep a mutable reference to the bundle From e0d2befda882b0caf59e2f691e013863b5dc3d0f Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 11:45:02 +0200 Subject: [PATCH 02/47] inline `pipeline.as_info().id()` --- wgpu-core/src/command/compute.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index e4831bdb0e..19dd1b0da1 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -630,12 +630,11 @@ impl Global { } } ArcComputeCommand::SetPipeline(pipeline) => { - let pipeline_id = pipeline.as_info().id(); - let scope = PassErrorScope::SetPipelineCompute(pipeline_id); + let scope = PassErrorScope::SetPipelineCompute(pipeline.as_info().id()); pipeline.same_device_as(cmd_buf).map_pass_err(scope)?; - state.pipeline = Some(pipeline_id); + state.pipeline = Some(pipeline.as_info().id()); let pipeline = tracker.compute_pipelines.insert_single(pipeline); From 99317905891d1325580df4f55dded63f19f40b88 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 12:06:47 +0200 Subject: [PATCH 03/47] simplify `IndexState` --- wgpu-core/src/command/render.rs | 38 +++++++++++++-------------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index b095820255..2508ae26f7 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -323,32 +323,24 @@ impl OptionalState { #[derive(Debug, Default)] struct IndexState { - bound_buffer_view: Option<(id::BufferId, Range)>, - format: Option, + buffer_format: Option, pipeline_format: Option, limit: u64, } impl IndexState { - fn update_limit(&mut self) { - self.limit = match self.bound_buffer_view { - Some((_, ref range)) => { - let format = self - .format - .expect("IndexState::update_limit must be called after a index buffer is set"); - let shift = match format { - IndexFormat::Uint16 => 1, - IndexFormat::Uint32 => 2, - }; - - (range.end - range.start) >> shift - } - None => 0, - } + fn update_buffer(&mut self, range: Range, format: IndexFormat) { + self.buffer_format = Some(format); + let shift = match format { + IndexFormat::Uint16 => 1, + IndexFormat::Uint32 => 2, + }; + self.limit = (range.end - range.start) >> shift; } fn reset(&mut self) { - self.bound_buffer_view = None; + self.buffer_format = None; + self.pipeline_format = None; self.limit = 0; } } @@ -482,7 +474,10 @@ impl State { // Pipeline expects an index buffer if let Some(pipeline_index_format) = self.index.pipeline_format { // We have a buffer bound - let buffer_index_format = self.index.format.ok_or(DrawError::MissingIndexBuffer)?; + let buffer_index_format = self + .index + .buffer_format + .ok_or(DrawError::MissingIndexBuffer)?; // The buffers are different formats if pipeline_index_format != buffer_index_format { @@ -1724,10 +1719,7 @@ impl Global { Some(s) => offset + s.get(), None => buffer.size, }; - state.index.bound_buffer_view = Some((buffer.as_info().id(), offset..end)); - - state.index.format = Some(index_format); - state.index.update_limit(); + state.index.update_buffer(offset..end, index_format); buffer_memory_init_actions.extend( buffer.initialization_status.read().create_action( From 6cffe1d845b1b455589cd13cb7f40cf11ef4467d Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Fri, 21 Jun 2024 21:57:46 +0200 Subject: [PATCH 04/47] extract render bundle command processing into their own functions --- wgpu-core/src/command/bundle.rs | 693 +++++++++++++++++++------------- 1 file changed, 406 insertions(+), 287 deletions(-) diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index 5639847ceb..e716e96516 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/wgpu-core/src/command/bundle.rs @@ -411,113 +411,11 @@ impl RenderBundleEncoder { bind_group_id, } => { let scope = PassErrorScope::SetBindGroup(bind_group_id); - - let bind_group = bind_group_guard - .get(bind_group_id) - .map_err(|_| RenderCommandError::InvalidBindGroupId(bind_group_id)) - .map_pass_err(scope)?; - - state - .trackers - .bind_groups - .write() - .add_single(bind_group); - - bind_group.same_device(device).map_pass_err(scope)?; - - let max_bind_groups = device.limits.max_bind_groups; - if index >= max_bind_groups { - return Err(RenderCommandError::BindGroupIndexOutOfRange { - index, - max: max_bind_groups, - }) - .map_pass_err(scope); - } - - // Identify the next `num_dynamic_offsets` entries from `base.dynamic_offsets`. - 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()]; - - if bind_group.dynamic_binding_info.len() != offsets.len() { - return Err(RenderCommandError::InvalidDynamicOffsetCount { - actual: offsets.len(), - expected: bind_group.dynamic_binding_info.len(), - }) - .map_pass_err(scope); - } - - // Check for misaligned offsets. - for (offset, info) in offsets - .iter() - .map(|offset| *offset as wgt::BufferAddress) - .zip(bind_group.dynamic_binding_info.iter()) - { - let (alignment, limit_name) = - buffer_binding_type_alignment(&device.limits, info.binding_type); - if offset % alignment as u64 != 0 { - return Err(RenderCommandError::UnalignedBufferOffset( - offset, limit_name, alignment, - )) - .map_pass_err(scope); - } - } - - 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_guard.get(bind_group_id).as_ref().unwrap(), &bind_group.layout, offsets_range); - unsafe { - state - .trackers - .merge_bind_group(&bind_group.used) - .map_pass_err(scope)? - }; - //Note: stateless trackers are not merged: the lifetime reference - // is held to the bind group itself. + set_bind_group(bind_group_id, &bind_group_guard, &mut state, device, index, &mut next_dynamic_offset, num_dynamic_offsets, base, &mut buffer_memory_init_actions, &mut texture_memory_init_actions).map_pass_err(scope)?; } RenderCommand::SetPipeline(pipeline_id) => { let scope = PassErrorScope::SetPipelineRender(pipeline_id); - - let pipeline = pipeline_guard - .get(pipeline_id) - .map_err(|_| RenderCommandError::InvalidPipeline(pipeline_id)) - .map_pass_err(scope)?; - - state - .trackers - .render_pipelines - .write() - .add_single(pipeline); - - pipeline.same_device(device).map_pass_err(scope)?; - - self.context - .check_compatible(&pipeline.pass_context, RenderPassCompatibilityCheckType::RenderPipeline) - .map_err(RenderCommandError::IncompatiblePipelineTargets) - .map_pass_err(scope)?; - - if (pipeline.flags.contains(PipelineFlags::WRITES_DEPTH) - && self.is_depth_read_only) - || (pipeline.flags.contains(PipelineFlags::WRITES_STENCIL) - && self.is_stencil_read_only) - { - return Err(RenderCommandError::IncompatiblePipelineRods) - .map_pass_err(scope); - } - - let pipeline_state = PipelineState::new(pipeline, pipeline_id); - - commands.push(ArcRenderCommand::SetPipeline(pipeline.clone())); - - // If this pipeline uses push constants, zero out their values. - if let Some(iter) = pipeline_state.zero_push_constants() { - commands.extend(iter) - } - - state.invalidate_bind_groups(&pipeline_state, &pipeline.layout); - state.pipeline = Some(pipeline_state); + set_pipeline(&self, pipeline_id, &pipeline_guard, &mut state, device, &mut commands).map_pass_err(scope)?; } RenderCommand::SetIndexBuffer { buffer_id, @@ -526,32 +424,7 @@ impl RenderBundleEncoder { size, } => { let scope = PassErrorScope::SetIndexBuffer(buffer_id); - - let buffer = buffer_guard - .get(buffer_id) - .map_err(|_| RenderCommandError::InvalidBufferId(buffer_id)) - .map_pass_err(scope)?; - - state - .trackers - .buffers - .write() - .merge_single(buffer, hal::BufferUses::INDEX) - .map_pass_err(scope)?; - - buffer.same_device(device).map_pass_err(scope)?; - buffer.check_usage(wgt::BufferUsages::INDEX).map_pass_err(scope)?; - - let end = match size { - Some(s) => offset + s.get(), - None => buffer.size, - }; - buffer_memory_init_actions.extend(buffer.initialization_status.read().create_action( - buffer, - offset..end, - MemoryInitKind::NeedsInitializedMemory, - )); - state.set_index_buffer(buffer.clone(), index_format, offset..end); + set_index_buffer(buffer_id, &buffer_guard, &mut state, device, size, offset, &mut buffer_memory_init_actions, index_format).map_pass_err(scope)?; } RenderCommand::SetVertexBuffer { slot, @@ -560,40 +433,7 @@ impl RenderBundleEncoder { size, } => { let scope = PassErrorScope::SetVertexBuffer(buffer_id); - - let max_vertex_buffers = device.limits.max_vertex_buffers; - if slot >= max_vertex_buffers { - return Err(RenderCommandError::VertexBufferIndexOutOfRange { - index: slot, - max: max_vertex_buffers, - }) - .map_pass_err(scope); - } - - let buffer = buffer_guard - .get(buffer_id) - .map_err(|_| RenderCommandError::InvalidBufferId(buffer_id)) - .map_pass_err(scope)?; - - state - .trackers - .buffers.write() - .merge_single(buffer, hal::BufferUses::VERTEX) - .map_pass_err(scope)?; - - buffer.same_device(device).map_pass_err(scope)?; - buffer.check_usage(wgt::BufferUsages::VERTEX).map_pass_err(scope)?; - - let end = match size { - Some(s) => offset + s.get(), - None => buffer.size, - }; - buffer_memory_init_actions.extend(buffer.initialization_status.read().create_action( - buffer, - offset..end, - MemoryInitKind::NeedsInitializedMemory, - )); - state.vertex[slot as usize] = Some(VertexState::new(buffer.clone(), offset..end)); + set_vertex_buffer(buffer_id, device, slot, &buffer_guard, &mut state, size, offset, &mut buffer_memory_init_actions).map_pass_err(scope)?; } RenderCommand::SetPushConstant { stages, @@ -602,15 +442,7 @@ impl RenderBundleEncoder { values_offset, } => { let scope = PassErrorScope::SetPushConstant; - let end_offset = offset + size_bytes; - - let pipeline_state = state.pipeline(scope)?; - - pipeline_state.pipeline.layout - .validate_push_constant_ranges(stages, offset, end_offset) - .map_pass_err(scope)?; - - commands.push(ArcRenderCommand::SetPushConstant { stages, offset, size_bytes, values_offset }); + set_push_constant(offset, size_bytes, &state, stages, &mut commands, values_offset).map_pass_err(scope)?; } RenderCommand::Draw { vertex_count, @@ -623,28 +455,7 @@ impl RenderBundleEncoder { indexed: false, pipeline: state.pipeline_id(), }; - let pipeline = state.pipeline(scope)?; - let used_bind_groups = pipeline.used_bind_groups; - - validate_draw( - &state.vertex[..], - &pipeline.steps, - first_vertex, - vertex_count, - first_instance, - instance_count, - ).map_pass_err(scope)?; - - if instance_count > 0 && vertex_count > 0 { - commands.extend(state.flush_vertices()); - commands.extend(state.flush_binds(used_bind_groups, &base.dynamic_offsets)); - commands.push(ArcRenderCommand::Draw { - vertex_count, - instance_count, - first_vertex, - first_instance, - }); - } + draw(&mut state, first_vertex, vertex_count, first_instance, instance_count, &mut commands, base).map_pass_err(scope)?; } RenderCommand::DrawIndexed { index_count, @@ -658,29 +469,7 @@ impl RenderBundleEncoder { indexed: true, pipeline: state.pipeline_id(), }; - let pipeline = state.pipeline(scope)?; - let used_bind_groups = pipeline.used_bind_groups; - let index = match state.index { - Some(ref index) => index, - None => return Err(DrawError::MissingIndexBuffer).map_pass_err(scope), - }; - - validate_indexed_draw( - &state.vertex[..], - &pipeline.steps, - index, - first_index, - index_count, - first_instance, - instance_count, - ).map_pass_err(scope)?; - - if instance_count > 0 && index_count > 0 { - commands.extend(state.flush_index()); - commands.extend(state.flush_vertices()); - commands.extend(state.flush_binds(used_bind_groups, &base.dynamic_offsets)); - commands.push(ArcRenderCommand::DrawIndexed { index_count, instance_count, first_index, base_vertex, first_instance }); - } + draw_indexed(&mut state, first_index, index_count, first_instance, instance_count, &mut commands, base, base_vertex).map_pass_err(scope)?; } RenderCommand::MultiDrawIndirect { buffer_id, @@ -693,36 +482,7 @@ impl RenderBundleEncoder { indexed: false, pipeline: state.pipeline_id(), }; - device - .require_downlevel_flags(wgt::DownlevelFlags::INDIRECT_EXECUTION) - .map_pass_err(scope)?; - - let pipeline = state.pipeline(scope)?; - let used_bind_groups = pipeline.used_bind_groups; - - let buffer = buffer_guard - .get(buffer_id) - .map_err(|_| RenderCommandError::InvalidBufferId(buffer_id)) - .map_pass_err(scope)?; - - state - .trackers - .buffers.write() - .merge_single(buffer, hal::BufferUses::INDIRECT) - .map_pass_err(scope)?; - - buffer.same_device(device).map_pass_err(scope)?; - buffer.check_usage(wgt::BufferUsages::INDIRECT).map_pass_err(scope)?; - - buffer_memory_init_actions.extend(buffer.initialization_status.read().create_action( - buffer, - offset..(offset + mem::size_of::() as u64), - MemoryInitKind::NeedsInitializedMemory, - )); - - commands.extend(state.flush_vertices()); - commands.extend(state.flush_binds(used_bind_groups, &base.dynamic_offsets)); - commands.push(ArcRenderCommand::MultiDrawIndirect { buffer: buffer.clone(), offset, count: None, indexed: false }); + multi_draw_indirect(&mut state, device, &buffer_guard, buffer_id, &mut buffer_memory_init_actions, offset, &mut commands, base).map_pass_err(scope)?; } RenderCommand::MultiDrawIndirect { buffer_id, @@ -735,42 +495,7 @@ impl RenderBundleEncoder { indexed: true, pipeline: state.pipeline_id(), }; - device - .require_downlevel_flags(wgt::DownlevelFlags::INDIRECT_EXECUTION) - .map_pass_err(scope)?; - - let pipeline = state.pipeline(scope)?; - let used_bind_groups = pipeline.used_bind_groups; - - let buffer = buffer_guard - .get(buffer_id) - .map_err(|_| RenderCommandError::InvalidBufferId(buffer_id)) - .map_pass_err(scope)?; - - state - .trackers - .buffers.write() - .merge_single(buffer, hal::BufferUses::INDIRECT) - .map_pass_err(scope)?; - - buffer.same_device(device).map_pass_err(scope)?; - buffer.check_usage(wgt::BufferUsages::INDIRECT).map_pass_err(scope)?; - - buffer_memory_init_actions.extend(buffer.initialization_status.read().create_action( - buffer, - offset..(offset + mem::size_of::() as u64), - MemoryInitKind::NeedsInitializedMemory, - )); - - 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(used_bind_groups, &base.dynamic_offsets)); - commands.push(ArcRenderCommand::MultiDrawIndirect { buffer: buffer.clone(), offset, count: None, indexed: true }); + multi_draw_indirect2(&mut state, device, &buffer_guard, buffer_id, &mut buffer_memory_init_actions, offset, &mut commands, base).map_pass_err(scope)?; } RenderCommand::MultiDrawIndirect { .. } | RenderCommand::MultiDrawIndirectCount { .. } => unimplemented!(), @@ -828,6 +553,401 @@ impl RenderBundleEncoder { } } +fn set_bind_group( + bind_group_id: id::Id, + bind_group_guard: &crate::lock::RwLockReadGuard>>, + state: &mut State, + device: &Arc>, + index: u32, + next_dynamic_offset: &mut usize, + num_dynamic_offsets: usize, + base: &BasePass, + buffer_memory_init_actions: &mut Vec>, + texture_memory_init_actions: &mut Vec>, +) -> Result<(), RenderBundleErrorInner> { + let bind_group = bind_group_guard + .get(bind_group_id) + .map_err(|_| RenderCommandError::InvalidBindGroupId(bind_group_id))?; + + state.trackers.bind_groups.write().add_single(bind_group); + + bind_group.same_device(device)?; + + let max_bind_groups = device.limits.max_bind_groups; + if index >= max_bind_groups { + return Err(RenderCommandError::BindGroupIndexOutOfRange { + index, + max: max_bind_groups, + } + .into()); + } + + // Identify the next `num_dynamic_offsets` entries from `base.dynamic_offsets`. + 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()]; + + if bind_group.dynamic_binding_info.len() != offsets.len() { + return Err(RenderCommandError::InvalidDynamicOffsetCount { + actual: offsets.len(), + expected: bind_group.dynamic_binding_info.len(), + } + .into()); + } + + // Check for misaligned offsets. + for (offset, info) in offsets + .iter() + .map(|offset| *offset as wgt::BufferAddress) + .zip(bind_group.dynamic_binding_info.iter()) + { + let (alignment, limit_name) = + buffer_binding_type_alignment(&device.limits, info.binding_type); + if offset % alignment as u64 != 0 { + return Err( + RenderCommandError::UnalignedBufferOffset(offset, limit_name, alignment).into(), + ); + } + } + + 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_guard.get(bind_group_id).as_ref().unwrap(), + &bind_group.layout, + offsets_range, + ); + unsafe { state.trackers.merge_bind_group(&bind_group.used)? }; + // Note: stateless trackers are not merged: the lifetime reference + // is held to the bind group itself. + Ok(()) +} + +fn set_pipeline( + encoder: &RenderBundleEncoder, + pipeline_id: id::Id, + pipeline_guard: &crate::lock::RwLockReadGuard>>, + state: &mut State, + device: &Arc>, + commands: &mut Vec>, +) -> Result<(), RenderBundleErrorInner> { + let pipeline = pipeline_guard + .get(pipeline_id) + .map_err(|_| RenderCommandError::InvalidPipeline(pipeline_id))?; + + state.trackers.render_pipelines.write().add_single(pipeline); + + pipeline.same_device(device)?; + + encoder + .context + .check_compatible( + &pipeline.pass_context, + RenderPassCompatibilityCheckType::RenderPipeline, + ) + .map_err(RenderCommandError::IncompatiblePipelineTargets)?; + + if (pipeline.flags.contains(PipelineFlags::WRITES_DEPTH) && encoder.is_depth_read_only) + || (pipeline.flags.contains(PipelineFlags::WRITES_STENCIL) && encoder.is_stencil_read_only) + { + return Err(RenderCommandError::IncompatiblePipelineRods.into()); + } + + let pipeline_state = PipelineState::new(pipeline, pipeline_id); + + commands.push(ArcRenderCommand::SetPipeline(pipeline.clone())); + + // If this pipeline uses push constants, zero out their values. + if let Some(iter) = pipeline_state.zero_push_constants() { + commands.extend(iter) + } + + state.invalidate_bind_groups(&pipeline_state, &pipeline.layout); + state.pipeline = Some(pipeline_state); + Ok(()) +} + +fn set_index_buffer( + buffer_id: id::Id, + buffer_guard: &crate::lock::RwLockReadGuard>>, + state: &mut State, + device: &Arc>, + size: Option, + offset: u64, + buffer_memory_init_actions: &mut Vec>, + index_format: wgt::IndexFormat, +) -> Result<(), RenderBundleErrorInner> { + let buffer = buffer_guard + .get(buffer_id) + .map_err(|_| RenderCommandError::InvalidBufferId(buffer_id))?; + + state + .trackers + .buffers + .write() + .merge_single(buffer, hal::BufferUses::INDEX)?; + + buffer.same_device(device)?; + buffer.check_usage(wgt::BufferUsages::INDEX)?; + + let end = match size { + Some(s) => offset + s.get(), + None => buffer.size, + }; + buffer_memory_init_actions.extend(buffer.initialization_status.read().create_action( + buffer, + offset..end, + MemoryInitKind::NeedsInitializedMemory, + )); + state.set_index_buffer(buffer.clone(), index_format, offset..end); + Ok(()) +} + +fn set_vertex_buffer( + buffer_id: id::Id, + device: &Arc>, + slot: u32, + buffer_guard: &crate::lock::RwLockReadGuard>>, + state: &mut State, + size: Option, + offset: u64, + buffer_memory_init_actions: &mut Vec>, +) -> Result<(), RenderBundleErrorInner> { + let max_vertex_buffers = device.limits.max_vertex_buffers; + if slot >= max_vertex_buffers { + return Err(RenderCommandError::VertexBufferIndexOutOfRange { + index: slot, + max: max_vertex_buffers, + } + .into()); + } + + let buffer = buffer_guard + .get(buffer_id) + .map_err(|_| RenderCommandError::InvalidBufferId(buffer_id))?; + + state + .trackers + .buffers + .write() + .merge_single(buffer, hal::BufferUses::VERTEX)?; + + buffer.same_device(device)?; + buffer.check_usage(wgt::BufferUsages::VERTEX)?; + + let end = match size { + Some(s) => offset + s.get(), + None => buffer.size, + }; + buffer_memory_init_actions.extend(buffer.initialization_status.read().create_action( + buffer, + offset..end, + MemoryInitKind::NeedsInitializedMemory, + )); + state.vertex[slot as usize] = Some(VertexState::new(buffer.clone(), offset..end)); + Ok(()) +} + +fn set_push_constant( + offset: u32, + size_bytes: u32, + state: &State, + stages: wgt::ShaderStages, + commands: &mut Vec>, + values_offset: Option, +) -> Result<(), RenderBundleErrorInner> { + let end_offset = offset + size_bytes; + + let pipeline_state = state.pipeline()?; + + pipeline_state + .pipeline + .layout + .validate_push_constant_ranges(stages, offset, end_offset)?; + + commands.push(ArcRenderCommand::SetPushConstant { + stages, + offset, + size_bytes, + values_offset, + }); + Ok(()) +} + +fn draw( + state: &mut State, + first_vertex: u32, + vertex_count: u32, + first_instance: u32, + instance_count: u32, + commands: &mut Vec>, + base: &BasePass, +) -> Result<(), RenderBundleErrorInner> { + let pipeline = state.pipeline()?; + let used_bind_groups = pipeline.used_bind_groups; + + validate_draw( + &state.vertex[..], + &pipeline.steps, + first_vertex, + vertex_count, + first_instance, + instance_count, + )?; + + if instance_count > 0 && vertex_count > 0 { + commands.extend(state.flush_vertices()); + commands.extend(state.flush_binds(used_bind_groups, &base.dynamic_offsets)); + commands.push(ArcRenderCommand::Draw { + vertex_count, + instance_count, + first_vertex, + first_instance, + }); + } + Ok(()) +} + +fn draw_indexed( + state: &mut State, + first_index: u32, + index_count: u32, + first_instance: u32, + instance_count: u32, + commands: &mut Vec>, + base: &BasePass, + base_vertex: i32, +) -> Result<(), RenderBundleErrorInner> { + 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()), + }; + + validate_indexed_draw( + &state.vertex[..], + &pipeline.steps, + index, + first_index, + index_count, + first_instance, + instance_count, + )?; + + if instance_count > 0 && index_count > 0 { + commands.extend(state.flush_index()); + commands.extend(state.flush_vertices()); + commands.extend(state.flush_binds(used_bind_groups, &base.dynamic_offsets)); + commands.push(ArcRenderCommand::DrawIndexed { + index_count, + instance_count, + first_index, + base_vertex, + first_instance, + }); + } + Ok(()) +} + +fn multi_draw_indirect( + state: &mut State, + device: &Arc>, + buffer_guard: &crate::lock::RwLockReadGuard>>, + buffer_id: id::Id, + buffer_memory_init_actions: &mut Vec>, + offset: u64, + commands: &mut Vec>, + base: &BasePass, +) -> Result<(), RenderBundleErrorInner> { + 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) + .map_err(|_| RenderCommandError::InvalidBufferId(buffer_id))?; + + state + .trackers + .buffers + .write() + .merge_single(buffer, hal::BufferUses::INDIRECT)?; + + buffer.same_device(device)?; + buffer.check_usage(wgt::BufferUsages::INDIRECT)?; + + buffer_memory_init_actions.extend(buffer.initialization_status.read().create_action( + buffer, + offset..(offset + mem::size_of::() as u64), + MemoryInitKind::NeedsInitializedMemory, + )); + + commands.extend(state.flush_vertices()); + commands.extend(state.flush_binds(used_bind_groups, &base.dynamic_offsets)); + commands.push(ArcRenderCommand::MultiDrawIndirect { + buffer: buffer.clone(), + offset, + count: None, + indexed: false, + }); + Ok(()) +} + +fn multi_draw_indirect2( + state: &mut State, + device: &Arc>, + buffer_guard: &crate::lock::RwLockReadGuard>>, + buffer_id: id::Id, + buffer_memory_init_actions: &mut Vec>, + offset: u64, + commands: &mut Vec>, + base: &BasePass, +) -> Result<(), RenderBundleErrorInner> { + 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) + .map_err(|_| RenderCommandError::InvalidBufferId(buffer_id))?; + + state + .trackers + .buffers + .write() + .merge_single(buffer, hal::BufferUses::INDIRECT)?; + + buffer.same_device(device)?; + buffer.check_usage(wgt::BufferUsages::INDIRECT)?; + + buffer_memory_init_actions.extend(buffer.initialization_status.read().create_action( + buffer, + offset..(offset + mem::size_of::() as u64), + MemoryInitKind::NeedsInitializedMemory, + )); + + let index = match state.index { + Some(ref mut index) => index, + None => return Err(DrawError::MissingIndexBuffer.into()), + }; + + commands.extend(index.flush()); + commands.extend(state.flush_vertices()); + commands.extend(state.flush_binds(used_bind_groups, &base.dynamic_offsets)); + commands.push(ArcRenderCommand::MultiDrawIndirect { + buffer: buffer.clone(), + offset, + count: None, + indexed: true, + }); + Ok(()) +} + /// Error type returned from `RenderBundleEncoder::new` if the sample count is invalid. #[derive(Clone, Debug, Error)] #[non_exhaustive] @@ -1305,11 +1425,10 @@ impl State { } /// Return the current pipeline state. Return an error if none is set. - fn pipeline(&self, scope: PassErrorScope) -> Result<&PipelineState, RenderBundleError> { + fn pipeline(&self) -> Result<&PipelineState, RenderBundleErrorInner> { self.pipeline .as_ref() - .ok_or(DrawError::MissingPipeline) - .map_pass_err(scope) + .ok_or(DrawError::MissingPipeline.into()) } /// Mark all non-empty bind group table entries from `index` onwards as dirty. From ac90e267c5281a3475bdc4cee7b04a84aee4f016 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Fri, 21 Jun 2024 21:59:05 +0200 Subject: [PATCH 05/47] add more fields to `State` and cleanup fn params --- wgpu-core/src/command/bundle.rs | 291 +++++++++++++++++--------------- 1 file changed, 155 insertions(+), 136 deletions(-) diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index e716e96516..4774242cbb 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/wgpu-core/src/command/bundle.rs @@ -366,9 +366,14 @@ impl RenderBundleEncoder { vertex: (0..hal::MAX_VERTEX_BUFFERS).map(|_| None).collect(), index: None, flat_dynamic_offsets: Vec::new(), + device: device.clone(), + commands: Vec::new(), + buffer_memory_init_actions: Vec::new(), + texture_memory_init_actions: Vec::new(), + next_dynamic_offset: 0, }; - let indices = &device.tracker_indices; + let indices = &state.device.tracker_indices; state .trackers .buffers @@ -395,12 +400,6 @@ impl RenderBundleEncoder { .write() .set_size(indices.query_sets.size()); - let mut commands = Vec::new(); - let mut buffer_memory_init_actions = Vec::new(); - let mut texture_memory_init_actions = Vec::new(); - - let mut next_dynamic_offset = 0; - let base = &self.base; for &command in &base.commands { @@ -411,11 +410,11 @@ impl RenderBundleEncoder { bind_group_id, } => { let scope = PassErrorScope::SetBindGroup(bind_group_id); - set_bind_group(bind_group_id, &bind_group_guard, &mut state, device, index, &mut next_dynamic_offset, num_dynamic_offsets, base, &mut buffer_memory_init_actions, &mut texture_memory_init_actions).map_pass_err(scope)?; + set_bind_group(&mut state, &bind_group_guard, &base.dynamic_offsets, index, num_dynamic_offsets, bind_group_id).map_pass_err(scope)?; } RenderCommand::SetPipeline(pipeline_id) => { let scope = PassErrorScope::SetPipelineRender(pipeline_id); - set_pipeline(&self, pipeline_id, &pipeline_guard, &mut state, device, &mut commands).map_pass_err(scope)?; + set_pipeline(&mut state, &pipeline_guard, &self.context, self.is_depth_read_only, self.is_stencil_read_only, pipeline_id).map_pass_err(scope)?; } RenderCommand::SetIndexBuffer { buffer_id, @@ -424,7 +423,7 @@ impl RenderBundleEncoder { size, } => { let scope = PassErrorScope::SetIndexBuffer(buffer_id); - set_index_buffer(buffer_id, &buffer_guard, &mut state, device, size, offset, &mut buffer_memory_init_actions, index_format).map_pass_err(scope)?; + set_index_buffer(&mut state, &buffer_guard, buffer_id, index_format, offset, size).map_pass_err(scope)?; } RenderCommand::SetVertexBuffer { slot, @@ -433,7 +432,7 @@ impl RenderBundleEncoder { size, } => { let scope = PassErrorScope::SetVertexBuffer(buffer_id); - set_vertex_buffer(buffer_id, device, slot, &buffer_guard, &mut state, size, offset, &mut buffer_memory_init_actions).map_pass_err(scope)?; + set_vertex_buffer(&mut state, &buffer_guard, slot, buffer_id, offset, size).map_pass_err(scope)?; } RenderCommand::SetPushConstant { stages, @@ -442,7 +441,7 @@ impl RenderBundleEncoder { values_offset, } => { let scope = PassErrorScope::SetPushConstant; - set_push_constant(offset, size_bytes, &state, stages, &mut commands, values_offset).map_pass_err(scope)?; + set_push_constant(&mut state, stages, offset, size_bytes, values_offset).map_pass_err(scope)?; } RenderCommand::Draw { vertex_count, @@ -455,7 +454,7 @@ impl RenderBundleEncoder { indexed: false, pipeline: state.pipeline_id(), }; - draw(&mut state, first_vertex, vertex_count, first_instance, instance_count, &mut commands, base).map_pass_err(scope)?; + draw(&mut state, &base.dynamic_offsets, vertex_count, instance_count, first_vertex, first_instance).map_pass_err(scope)?; } RenderCommand::DrawIndexed { index_count, @@ -469,7 +468,7 @@ impl RenderBundleEncoder { indexed: true, pipeline: state.pipeline_id(), }; - draw_indexed(&mut state, first_index, index_count, first_instance, instance_count, &mut commands, base, base_vertex).map_pass_err(scope)?; + draw_indexed(&mut state, &base.dynamic_offsets, index_count, instance_count, first_index, base_vertex, first_instance).map_pass_err(scope)?; } RenderCommand::MultiDrawIndirect { buffer_id, @@ -482,7 +481,7 @@ impl RenderBundleEncoder { indexed: false, pipeline: state.pipeline_id(), }; - multi_draw_indirect(&mut state, device, &buffer_guard, buffer_id, &mut buffer_memory_init_actions, offset, &mut commands, base).map_pass_err(scope)?; + multi_draw_indirect(&mut state, &base.dynamic_offsets, &buffer_guard, buffer_id, offset).map_pass_err(scope)?; } RenderCommand::MultiDrawIndirect { buffer_id, @@ -495,7 +494,7 @@ impl RenderBundleEncoder { indexed: true, pipeline: state.pipeline_id(), }; - multi_draw_indirect2(&mut state, device, &buffer_guard, buffer_id, &mut buffer_memory_init_actions, offset, &mut commands, base).map_pass_err(scope)?; + multi_draw_indirect2(&mut state, &base.dynamic_offsets, &buffer_guard, buffer_id, offset).map_pass_err(scope)?; } RenderCommand::MultiDrawIndirect { .. } | RenderCommand::MultiDrawIndirectCount { .. } => unimplemented!(), @@ -515,25 +514,38 @@ impl RenderBundleEncoder { } } + let State { + trackers, + flat_dynamic_offsets, + device, + commands, + buffer_memory_init_actions, + texture_memory_init_actions, + .. + } = state; + + let tracker_indices = device.tracker_indices.bundles.clone(); + let discard_hal_labels = device + .instance_flags + .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS); + Ok(RenderBundle { base: BasePass { label: desc.label.as_ref().map(|cow| cow.to_string()), commands, - dynamic_offsets: state.flat_dynamic_offsets, + dynamic_offsets: flat_dynamic_offsets, string_data: Vec::new(), push_constant_data: Vec::new(), }, is_depth_read_only: self.is_depth_read_only, is_stencil_read_only: self.is_stencil_read_only, - device: device.clone(), - used: state.trackers, + device, + used: trackers, buffer_memory_init_actions, texture_memory_init_actions, context: self.context, - info: ResourceInfo::new(&desc.label, Some(device.tracker_indices.bundles.clone())), - discard_hal_labels: device - .instance_flags - .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS), + info: ResourceInfo::new(&desc.label, Some(tracker_indices)), + discard_hal_labels, }) } @@ -554,16 +566,12 @@ impl RenderBundleEncoder { } fn set_bind_group( - bind_group_id: id::Id, - bind_group_guard: &crate::lock::RwLockReadGuard>>, state: &mut State, - device: &Arc>, + bind_group_guard: &crate::lock::RwLockReadGuard>>, + dynamic_offsets: &[u32], index: u32, - next_dynamic_offset: &mut usize, num_dynamic_offsets: usize, - base: &BasePass, - buffer_memory_init_actions: &mut Vec>, - texture_memory_init_actions: &mut Vec>, + bind_group_id: id::Id, ) -> Result<(), RenderBundleErrorInner> { let bind_group = bind_group_guard .get(bind_group_id) @@ -571,9 +579,9 @@ fn set_bind_group( state.trackers.bind_groups.write().add_single(bind_group); - bind_group.same_device(device)?; + bind_group.same_device(&state.device)?; - let max_bind_groups = device.limits.max_bind_groups; + let max_bind_groups = state.device.limits.max_bind_groups; if index >= max_bind_groups { return Err(RenderCommandError::BindGroupIndexOutOfRange { index, @@ -582,10 +590,10 @@ fn set_bind_group( .into()); } - // Identify the next `num_dynamic_offsets` entries from `base.dynamic_offsets`. - 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()]; + // Identify the next `num_dynamic_offsets` entries from `dynamic_offsets`. + let offsets_range = state.next_dynamic_offset..state.next_dynamic_offset + num_dynamic_offsets; + state.next_dynamic_offset = offsets_range.end; + let offsets = &dynamic_offsets[offsets_range.clone()]; if bind_group.dynamic_binding_info.len() != offsets.len() { return Err(RenderCommandError::InvalidDynamicOffsetCount { @@ -602,7 +610,7 @@ fn set_bind_group( .zip(bind_group.dynamic_binding_info.iter()) { let (alignment, limit_name) = - buffer_binding_type_alignment(&device.limits, info.binding_type); + buffer_binding_type_alignment(&state.device.limits, info.binding_type); if offset % alignment as u64 != 0 { return Err( RenderCommandError::UnalignedBufferOffset(offset, limit_name, alignment).into(), @@ -610,8 +618,12 @@ fn set_bind_group( } } - 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 + .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); state.set_bind_group( index, @@ -626,12 +638,12 @@ fn set_bind_group( } fn set_pipeline( - encoder: &RenderBundleEncoder, - pipeline_id: id::Id, - pipeline_guard: &crate::lock::RwLockReadGuard>>, state: &mut State, - device: &Arc>, - commands: &mut Vec>, + pipeline_guard: &crate::lock::RwLockReadGuard>>, + context: &RenderPassContext, + is_depth_read_only: bool, + is_stencil_read_only: bool, + pipeline_id: id::Id, ) -> Result<(), RenderBundleErrorInner> { let pipeline = pipeline_guard .get(pipeline_id) @@ -639,29 +651,30 @@ fn set_pipeline( state.trackers.render_pipelines.write().add_single(pipeline); - pipeline.same_device(device)?; + pipeline.same_device(&state.device)?; - encoder - .context + context .check_compatible( &pipeline.pass_context, RenderPassCompatibilityCheckType::RenderPipeline, ) .map_err(RenderCommandError::IncompatiblePipelineTargets)?; - if (pipeline.flags.contains(PipelineFlags::WRITES_DEPTH) && encoder.is_depth_read_only) - || (pipeline.flags.contains(PipelineFlags::WRITES_STENCIL) && encoder.is_stencil_read_only) + if (pipeline.flags.contains(PipelineFlags::WRITES_DEPTH) && is_depth_read_only) + || (pipeline.flags.contains(PipelineFlags::WRITES_STENCIL) && is_stencil_read_only) { return Err(RenderCommandError::IncompatiblePipelineRods.into()); } let pipeline_state = PipelineState::new(pipeline, pipeline_id); - commands.push(ArcRenderCommand::SetPipeline(pipeline.clone())); + state + .commands + .push(ArcRenderCommand::SetPipeline(pipeline.clone())); // If this pipeline uses push constants, zero out their values. if let Some(iter) = pipeline_state.zero_push_constants() { - commands.extend(iter) + state.commands.extend(iter) } state.invalidate_bind_groups(&pipeline_state, &pipeline.layout); @@ -670,14 +683,12 @@ fn set_pipeline( } fn set_index_buffer( - buffer_id: id::Id, - buffer_guard: &crate::lock::RwLockReadGuard>>, state: &mut State, - device: &Arc>, - size: Option, - offset: u64, - buffer_memory_init_actions: &mut Vec>, + buffer_guard: &crate::lock::RwLockReadGuard>>, + buffer_id: id::Id, index_format: wgt::IndexFormat, + offset: u64, + size: Option, ) -> Result<(), RenderBundleErrorInner> { let buffer = buffer_guard .get(buffer_id) @@ -689,33 +700,33 @@ fn set_index_buffer( .write() .merge_single(buffer, hal::BufferUses::INDEX)?; - buffer.same_device(device)?; + buffer.same_device(&state.device)?; buffer.check_usage(wgt::BufferUsages::INDEX)?; let end = match size { Some(s) => offset + s.get(), None => buffer.size, }; - buffer_memory_init_actions.extend(buffer.initialization_status.read().create_action( - buffer, - offset..end, - MemoryInitKind::NeedsInitializedMemory, - )); + state + .buffer_memory_init_actions + .extend(buffer.initialization_status.read().create_action( + buffer, + offset..end, + MemoryInitKind::NeedsInitializedMemory, + )); state.set_index_buffer(buffer.clone(), index_format, offset..end); Ok(()) } fn set_vertex_buffer( - buffer_id: id::Id, - device: &Arc>, - slot: u32, - buffer_guard: &crate::lock::RwLockReadGuard>>, state: &mut State, - size: Option, + buffer_guard: &crate::lock::RwLockReadGuard>>, + slot: u32, + buffer_id: id::Id, offset: u64, - buffer_memory_init_actions: &mut Vec>, + size: Option, ) -> Result<(), RenderBundleErrorInner> { - let max_vertex_buffers = device.limits.max_vertex_buffers; + let max_vertex_buffers = state.device.limits.max_vertex_buffers; if slot >= max_vertex_buffers { return Err(RenderCommandError::VertexBufferIndexOutOfRange { index: slot, @@ -734,28 +745,29 @@ fn set_vertex_buffer( .write() .merge_single(buffer, hal::BufferUses::VERTEX)?; - buffer.same_device(device)?; + buffer.same_device(&state.device)?; buffer.check_usage(wgt::BufferUsages::VERTEX)?; let end = match size { Some(s) => offset + s.get(), None => buffer.size, }; - buffer_memory_init_actions.extend(buffer.initialization_status.read().create_action( - buffer, - offset..end, - MemoryInitKind::NeedsInitializedMemory, - )); + state + .buffer_memory_init_actions + .extend(buffer.initialization_status.read().create_action( + buffer, + offset..end, + MemoryInitKind::NeedsInitializedMemory, + )); state.vertex[slot as usize] = Some(VertexState::new(buffer.clone(), offset..end)); Ok(()) } fn set_push_constant( + state: &mut State, + stages: wgt::ShaderStages, offset: u32, size_bytes: u32, - state: &State, - stages: wgt::ShaderStages, - commands: &mut Vec>, values_offset: Option, ) -> Result<(), RenderBundleErrorInner> { let end_offset = offset + size_bytes; @@ -767,7 +779,7 @@ fn set_push_constant( .layout .validate_push_constant_ranges(stages, offset, end_offset)?; - commands.push(ArcRenderCommand::SetPushConstant { + state.commands.push(ArcRenderCommand::SetPushConstant { stages, offset, size_bytes, @@ -778,12 +790,11 @@ fn set_push_constant( fn draw( state: &mut State, - first_vertex: u32, + dynamic_offsets: &[u32], vertex_count: u32, - first_instance: u32, instance_count: u32, - commands: &mut Vec>, - base: &BasePass, + first_vertex: u32, + first_instance: u32, ) -> Result<(), RenderBundleErrorInner> { let pipeline = state.pipeline()?; let used_bind_groups = pipeline.used_bind_groups; @@ -798,9 +809,9 @@ fn draw( )?; if instance_count > 0 && vertex_count > 0 { - commands.extend(state.flush_vertices()); - commands.extend(state.flush_binds(used_bind_groups, &base.dynamic_offsets)); - commands.push(ArcRenderCommand::Draw { + state.flush_vertices(); + state.flush_binds(used_bind_groups, dynamic_offsets); + state.commands.push(ArcRenderCommand::Draw { vertex_count, instance_count, first_vertex, @@ -812,13 +823,12 @@ fn draw( fn draw_indexed( state: &mut State, - first_index: u32, + dynamic_offsets: &[u32], index_count: u32, - first_instance: u32, instance_count: u32, - commands: &mut Vec>, - base: &BasePass, + first_index: u32, base_vertex: i32, + first_instance: u32, ) -> Result<(), RenderBundleErrorInner> { let pipeline = state.pipeline()?; let used_bind_groups = pipeline.used_bind_groups; @@ -838,10 +848,10 @@ fn draw_indexed( )?; if instance_count > 0 && index_count > 0 { - commands.extend(state.flush_index()); - commands.extend(state.flush_vertices()); - commands.extend(state.flush_binds(used_bind_groups, &base.dynamic_offsets)); - commands.push(ArcRenderCommand::DrawIndexed { + state.flush_index(); + state.flush_vertices(); + state.flush_binds(used_bind_groups, dynamic_offsets); + state.commands.push(ArcRenderCommand::DrawIndexed { index_count, instance_count, first_index, @@ -854,15 +864,14 @@ fn draw_indexed( fn multi_draw_indirect( state: &mut State, - device: &Arc>, + dynamic_offsets: &[u32], buffer_guard: &crate::lock::RwLockReadGuard>>, buffer_id: id::Id, - buffer_memory_init_actions: &mut Vec>, offset: u64, - commands: &mut Vec>, - base: &BasePass, ) -> Result<(), RenderBundleErrorInner> { - device.require_downlevel_flags(wgt::DownlevelFlags::INDIRECT_EXECUTION)?; + state + .device + .require_downlevel_flags(wgt::DownlevelFlags::INDIRECT_EXECUTION)?; let pipeline = state.pipeline()?; let used_bind_groups = pipeline.used_bind_groups; @@ -877,18 +886,20 @@ fn multi_draw_indirect( .write() .merge_single(buffer, hal::BufferUses::INDIRECT)?; - buffer.same_device(device)?; + buffer.same_device(&state.device)?; buffer.check_usage(wgt::BufferUsages::INDIRECT)?; - buffer_memory_init_actions.extend(buffer.initialization_status.read().create_action( - buffer, - offset..(offset + mem::size_of::() as u64), - MemoryInitKind::NeedsInitializedMemory, - )); + state + .buffer_memory_init_actions + .extend(buffer.initialization_status.read().create_action( + buffer, + offset..(offset + mem::size_of::() as u64), + MemoryInitKind::NeedsInitializedMemory, + )); - commands.extend(state.flush_vertices()); - commands.extend(state.flush_binds(used_bind_groups, &base.dynamic_offsets)); - commands.push(ArcRenderCommand::MultiDrawIndirect { + state.flush_vertices(); + state.flush_binds(used_bind_groups, dynamic_offsets); + state.commands.push(ArcRenderCommand::MultiDrawIndirect { buffer: buffer.clone(), offset, count: None, @@ -899,15 +910,14 @@ fn multi_draw_indirect( fn multi_draw_indirect2( state: &mut State, - device: &Arc>, + dynamic_offsets: &[u32], buffer_guard: &crate::lock::RwLockReadGuard>>, buffer_id: id::Id, - buffer_memory_init_actions: &mut Vec>, offset: u64, - commands: &mut Vec>, - base: &BasePass, ) -> Result<(), RenderBundleErrorInner> { - device.require_downlevel_flags(wgt::DownlevelFlags::INDIRECT_EXECUTION)?; + state + .device + .require_downlevel_flags(wgt::DownlevelFlags::INDIRECT_EXECUTION)?; let pipeline = state.pipeline()?; let used_bind_groups = pipeline.used_bind_groups; @@ -922,24 +932,26 @@ fn multi_draw_indirect2( .write() .merge_single(buffer, hal::BufferUses::INDIRECT)?; - buffer.same_device(device)?; + buffer.same_device(&state.device)?; buffer.check_usage(wgt::BufferUsages::INDIRECT)?; - buffer_memory_init_actions.extend(buffer.initialization_status.read().create_action( - buffer, - offset..(offset + mem::size_of::() as u64), - MemoryInitKind::NeedsInitializedMemory, - )); + state + .buffer_memory_init_actions + .extend(buffer.initialization_status.read().create_action( + buffer, + offset..(offset + mem::size_of::() as u64), + MemoryInitKind::NeedsInitializedMemory, + )); let index = match state.index { Some(ref mut index) => index, None => return Err(DrawError::MissingIndexBuffer.into()), }; - commands.extend(index.flush()); - commands.extend(state.flush_vertices()); - commands.extend(state.flush_binds(used_bind_groups, &base.dynamic_offsets)); - commands.push(ArcRenderCommand::MultiDrawIndirect { + state.commands.extend(index.flush()); + state.flush_vertices(); + state.flush_binds(used_bind_groups, dynamic_offsets); + state.commands.push(ArcRenderCommand::MultiDrawIndirect { buffer: buffer.clone(), offset, count: None, @@ -1416,6 +1428,12 @@ struct State { /// /// [`dynamic_offsets`]: BasePass::dynamic_offsets flat_dynamic_offsets: Vec, + + device: Arc>, + commands: Vec>, + buffer_memory_init_actions: Vec>, + texture_memory_init_actions: Vec>, + next_dynamic_offset: usize, } impl State { @@ -1541,23 +1559,22 @@ impl State { /// Generate a `SetIndexBuffer` command to prepare for an indexed draw /// command, if needed. - fn flush_index(&mut self) -> Option> { - self.index.as_mut().and_then(|index| index.flush()) + fn flush_index(&mut self) { + let commands = self.index.as_mut().and_then(|index| index.flush()); + self.commands.extend(commands); } - fn flush_vertices(&mut self) -> impl Iterator> + '_ { - self.vertex + fn flush_vertices(&mut self) { + let commands = self + .vertex .iter_mut() .enumerate() - .flat_map(|(i, vs)| vs.as_mut().and_then(|vs| vs.flush(i as u32))) + .flat_map(|(i, vs)| vs.as_mut().and_then(|vs| vs.flush(i as u32))); + 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], - ) -> impl Iterator> + '_ { + 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 { @@ -1568,7 +1585,7 @@ impl State { // Then, generate `SetBindGroup` commands to update the dirty bind // groups. After this, all bind groups are clean. - self.bind[..used_bind_groups] + let commands = self.bind[..used_bind_groups] .iter_mut() .enumerate() .flat_map(|(i, entry)| { @@ -1584,7 +1601,9 @@ impl State { } } None - }) + }); + + self.commands.extend(commands); } } From 2da8218bbf4c417e8bdfa4ef872b70722ff98c26 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Fri, 21 Jun 2024 22:00:10 +0200 Subject: [PATCH 06/47] move comment that was preventing fmt from running --- wgpu-core/src/command/bundle.rs | 78 ++++++++++++++++++++++++++++----- 1 file changed, 68 insertions(+), 10 deletions(-) diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index 4774242cbb..4c67b991f1 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/wgpu-core/src/command/bundle.rs @@ -410,11 +410,27 @@ impl RenderBundleEncoder { bind_group_id, } => { let scope = PassErrorScope::SetBindGroup(bind_group_id); - set_bind_group(&mut state, &bind_group_guard, &base.dynamic_offsets, index, num_dynamic_offsets, bind_group_id).map_pass_err(scope)?; + set_bind_group( + &mut state, + &bind_group_guard, + &base.dynamic_offsets, + index, + num_dynamic_offsets, + bind_group_id, + ) + .map_pass_err(scope)?; } RenderCommand::SetPipeline(pipeline_id) => { let scope = PassErrorScope::SetPipelineRender(pipeline_id); - set_pipeline(&mut state, &pipeline_guard, &self.context, self.is_depth_read_only, self.is_stencil_read_only, pipeline_id).map_pass_err(scope)?; + set_pipeline( + &mut state, + &pipeline_guard, + &self.context, + self.is_depth_read_only, + self.is_stencil_read_only, + pipeline_id, + ) + .map_pass_err(scope)?; } RenderCommand::SetIndexBuffer { buffer_id, @@ -423,7 +439,15 @@ impl RenderBundleEncoder { size, } => { let scope = PassErrorScope::SetIndexBuffer(buffer_id); - set_index_buffer(&mut state, &buffer_guard, buffer_id, index_format, offset, size).map_pass_err(scope)?; + set_index_buffer( + &mut state, + &buffer_guard, + buffer_id, + index_format, + offset, + size, + ) + .map_pass_err(scope)?; } RenderCommand::SetVertexBuffer { slot, @@ -432,7 +456,8 @@ impl RenderBundleEncoder { size, } => { let scope = PassErrorScope::SetVertexBuffer(buffer_id); - set_vertex_buffer(&mut state, &buffer_guard, slot, buffer_id, offset, size).map_pass_err(scope)?; + set_vertex_buffer(&mut state, &buffer_guard, slot, buffer_id, offset, size) + .map_pass_err(scope)?; } RenderCommand::SetPushConstant { stages, @@ -441,7 +466,8 @@ impl RenderBundleEncoder { values_offset, } => { let scope = PassErrorScope::SetPushConstant; - set_push_constant(&mut state, stages, offset, size_bytes, values_offset).map_pass_err(scope)?; + set_push_constant(&mut state, stages, offset, size_bytes, values_offset) + .map_pass_err(scope)?; } RenderCommand::Draw { vertex_count, @@ -454,7 +480,15 @@ impl RenderBundleEncoder { indexed: false, pipeline: state.pipeline_id(), }; - draw(&mut state, &base.dynamic_offsets, vertex_count, instance_count, first_vertex, first_instance).map_pass_err(scope)?; + draw( + &mut state, + &base.dynamic_offsets, + vertex_count, + instance_count, + first_vertex, + first_instance, + ) + .map_pass_err(scope)?; } RenderCommand::DrawIndexed { index_count, @@ -468,7 +502,16 @@ impl RenderBundleEncoder { indexed: true, pipeline: state.pipeline_id(), }; - draw_indexed(&mut state, &base.dynamic_offsets, index_count, instance_count, first_index, base_vertex, first_instance).map_pass_err(scope)?; + draw_indexed( + &mut state, + &base.dynamic_offsets, + index_count, + instance_count, + first_index, + base_vertex, + first_instance, + ) + .map_pass_err(scope)?; } RenderCommand::MultiDrawIndirect { buffer_id, @@ -481,7 +524,14 @@ impl RenderBundleEncoder { indexed: false, pipeline: state.pipeline_id(), }; - multi_draw_indirect(&mut state, &base.dynamic_offsets, &buffer_guard, buffer_id, offset).map_pass_err(scope)?; + multi_draw_indirect( + &mut state, + &base.dynamic_offsets, + &buffer_guard, + buffer_id, + offset, + ) + .map_pass_err(scope)?; } RenderCommand::MultiDrawIndirect { buffer_id, @@ -494,14 +544,22 @@ impl RenderBundleEncoder { indexed: true, pipeline: state.pipeline_id(), }; - multi_draw_indirect2(&mut state, &base.dynamic_offsets, &buffer_guard, buffer_id, offset).map_pass_err(scope)?; + multi_draw_indirect2( + &mut state, + &base.dynamic_offsets, + &buffer_guard, + buffer_id, + offset, + ) + .map_pass_err(scope)?; } RenderCommand::MultiDrawIndirect { .. } | RenderCommand::MultiDrawIndirectCount { .. } => unimplemented!(), RenderCommand::PushDebugGroup { color: _, len: _ } => unimplemented!(), RenderCommand::InsertDebugMarker { color: _, len: _ } => unimplemented!(), RenderCommand::PopDebugGroup => unimplemented!(), - RenderCommand::WriteTimestamp { .. } // Must check the TIMESTAMP_QUERY_INSIDE_PASSES feature + // Must check the TIMESTAMP_QUERY_INSIDE_PASSES feature + RenderCommand::WriteTimestamp { .. } | RenderCommand::BeginOcclusionQuery { .. } | RenderCommand::EndOcclusionQuery | RenderCommand::BeginPipelineStatisticsQuery { .. } From 67d77df0cc7bb191ed56367640af2ff4919410d0 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Fri, 21 Jun 2024 22:03:30 +0200 Subject: [PATCH 07/47] remove `multi_draw_indirect2` --- wgpu-core/src/command/bundle.rs | 86 +++++---------------------------- 1 file changed, 12 insertions(+), 74 deletions(-) diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index 4c67b991f1..246be74d04 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/wgpu-core/src/command/bundle.rs @@ -517,11 +517,11 @@ impl RenderBundleEncoder { buffer_id, offset, count: None, - indexed: false, + indexed, } => { let scope = PassErrorScope::Draw { kind: DrawKind::DrawIndirect, - indexed: false, + indexed, pipeline: state.pipeline_id(), }; multi_draw_indirect( @@ -530,26 +530,7 @@ impl RenderBundleEncoder { &buffer_guard, buffer_id, offset, - ) - .map_pass_err(scope)?; - } - RenderCommand::MultiDrawIndirect { - buffer_id, - offset, - count: None, - indexed: true, - } => { - let scope = PassErrorScope::Draw { - kind: DrawKind::DrawIndirect, - indexed: true, - pipeline: state.pipeline_id(), - }; - multi_draw_indirect2( - &mut state, - &base.dynamic_offsets, - &buffer_guard, - buffer_id, - offset, + indexed, ) .map_pass_err(scope)?; } @@ -926,6 +907,7 @@ fn multi_draw_indirect( buffer_guard: &crate::lock::RwLockReadGuard>>, buffer_id: id::Id, offset: u64, + indexed: bool, ) -> Result<(), RenderBundleErrorInner> { state .device @@ -955,65 +937,21 @@ fn multi_draw_indirect( MemoryInitKind::NeedsInitializedMemory, )); - state.flush_vertices(); - state.flush_binds(used_bind_groups, dynamic_offsets); - state.commands.push(ArcRenderCommand::MultiDrawIndirect { - buffer: buffer.clone(), - offset, - count: None, - indexed: false, - }); - Ok(()) -} - -fn multi_draw_indirect2( - state: &mut State, - dynamic_offsets: &[u32], - buffer_guard: &crate::lock::RwLockReadGuard>>, - buffer_id: id::Id, - offset: u64, -) -> Result<(), RenderBundleErrorInner> { - 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) - .map_err(|_| RenderCommandError::InvalidBufferId(buffer_id))?; - - state - .trackers - .buffers - .write() - .merge_single(buffer, hal::BufferUses::INDIRECT)?; - - buffer.same_device(&state.device)?; - buffer.check_usage(wgt::BufferUsages::INDIRECT)?; - - state - .buffer_memory_init_actions - .extend(buffer.initialization_status.read().create_action( - buffer, - offset..(offset + mem::size_of::() as u64), - MemoryInitKind::NeedsInitializedMemory, - )); - - let index = match state.index { - Some(ref mut index) => index, - None => return Err(DrawError::MissingIndexBuffer.into()), - }; + if indexed { + let index = match state.index { + Some(ref mut index) => index, + None => return Err(DrawError::MissingIndexBuffer.into()), + }; + state.commands.extend(index.flush()); + } - state.commands.extend(index.flush()); state.flush_vertices(); state.flush_binds(used_bind_groups, dynamic_offsets); state.commands.push(ArcRenderCommand::MultiDrawIndirect { buffer: buffer.clone(), offset, count: None, - indexed: true, + indexed, }); Ok(()) } From 5bbf63fa62df644793442fc2fbb10f28b2c9a9b0 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 13:59:09 +0200 Subject: [PATCH 08/47] put all state in `State` --- wgpu-core/src/command/compute.rs | 209 ++++++++++++++++++------------- 1 file changed, 121 insertions(+), 88 deletions(-) diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index 19dd1b0da1..4c6e005d46 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -9,12 +9,12 @@ use crate::{ CommandBuffer, CommandEncoderError, CommandEncoderStatus, MapPassErr, PassErrorScope, QueryUseError, StateChange, }, - device::{DeviceError, MissingDownlevelFlags, MissingFeatures}, + device::{Device, DeviceError, MissingDownlevelFlags, MissingFeatures}, error::{ErrorFormatter, PrettyError}, global::Global, hal_api::HalApi, hal_label, id, - init_tracker::MemoryInitKind, + init_tracker::{BufferInitTrackerAction, MemoryInitKind}, resource::{self, DestroyedResourceError, MissingBufferUsageError, ParentDevice, Resource}, snatch::SnatchGuard, track::{ResourceUsageCompatibilityError, Tracker, TrackerIndex, UsageScope}, @@ -33,7 +33,7 @@ use wgt::{BufferAddress, DynamicOffset}; use std::sync::Arc; use std::{fmt, mem, str}; -use super::DynComputePass; +use super::{memory_init::CommandBufferTextureMemoryActions, DynComputePass}; pub struct ComputePass { /// All pass data & records is stored here. @@ -248,14 +248,32 @@ where } } -struct State<'a, A: HalApi> { +struct State<'scope, 'snatch_guard, 'cmd_buf, A: HalApi> { binder: Binder, pipeline: Option, - scope: UsageScope<'a, A>, + scope: UsageScope<'scope, A>, debug_scope_depth: u32, + + snatch_guard: SnatchGuard<'snatch_guard>, + + device: &'cmd_buf Arc>, + tracker: &'cmd_buf mut Tracker, + buffer_memory_init_actions: &'cmd_buf mut Vec>, + texture_memory_actions: &'cmd_buf mut CommandBufferTextureMemoryActions, + + temp_offsets: Vec, + dynamic_offset_count: usize, + string_offset: usize, + active_query: Option<(Arc>, u32)>, + + intermediate_trackers: Tracker, + + /// Immediate texture inits required because of prior discards. Need to + /// be inserted before texture reads. + pending_discard_init_fixups: SurfacesInDiscardState, } -impl<'a, A: HalApi> State<'a, A> { +impl<'scope, 'snatch_guard, 'cmd_buf, A: HalApi> State<'scope, 'snatch_guard, 'cmd_buf, A> { fn is_ready(&self) -> Result<(), DispatchError> { let bind_mask = self.binder.invalid_mask(); if bind_mask != 0 { @@ -280,9 +298,7 @@ impl<'a, A: HalApi> State<'a, A> { fn flush_states( &mut self, raw_encoder: &mut A::CommandEncoder, - base_trackers: &mut Tracker, indirect_buffer: Option, - snatch_guard: &SnatchGuard, ) -> Result<(), ResourceUsageCompatibilityError> { for bind_group in self.binder.list_active() { unsafe { self.scope.merge_bind_group(&bind_group.used)? }; @@ -292,21 +308,25 @@ impl<'a, A: HalApi> State<'a, A> { for bind_group in self.binder.list_active() { unsafe { - base_trackers + self.intermediate_trackers .set_and_remove_from_usage_scope_sparse(&mut self.scope, &bind_group.used) } } // Add the state of the indirect buffer if it hasn't been hit before. unsafe { - base_trackers + self.intermediate_trackers .buffers .set_and_remove_from_usage_scope_sparse(&mut self.scope.buffers, indirect_buffer); } log::trace!("Encoding dispatch barriers"); - CommandBuffer::drain_barriers(raw_encoder, base_trackers, snatch_guard); + CommandBuffer::drain_barriers( + raw_encoder, + &mut self.intermediate_trackers, + &self.snatch_guard, + ); Ok(()) } } @@ -474,9 +494,6 @@ impl Global { let encoder = &mut cmd_buf_data.encoder; let status = &mut cmd_buf_data.status; - let tracker = &mut cmd_buf_data.trackers; - let buffer_memory_init_actions = &mut cmd_buf_data.buffer_memory_init_actions; - let texture_memory_actions = &mut cmd_buf_data.texture_memory_actions; // We automatically keep extending command buffers over time, and because // we want to insert a command buffer _before_ what we're about to record, @@ -491,25 +508,39 @@ impl Global { pipeline: None, scope: device.new_usage_scope(), debug_scope_depth: 0, + + snatch_guard: device.snatchable_lock.read(), + + device, + tracker: &mut cmd_buf_data.trackers, + buffer_memory_init_actions: &mut cmd_buf_data.buffer_memory_init_actions, + texture_memory_actions: &mut cmd_buf_data.texture_memory_actions, + + temp_offsets: Vec::new(), + dynamic_offset_count: 0, + string_offset: 0, + active_query: None, + + intermediate_trackers: Tracker::new(), + + pending_discard_init_fixups: SurfacesInDiscardState::new(), }; - let mut temp_offsets = Vec::new(); - let mut dynamic_offset_count = 0; - let mut string_offset = 0; - let mut active_query = None; - - let snatch_guard = device.snatchable_lock.read(); - - let indices = &device.tracker_indices; - tracker.buffers.set_size(indices.buffers.size()); - tracker.textures.set_size(indices.textures.size()); - tracker.bind_groups.set_size(indices.bind_groups.size()); - tracker + + let indices = &state.device.tracker_indices; + state.tracker.buffers.set_size(indices.buffers.size()); + state.tracker.textures.set_size(indices.textures.size()); + state + .tracker + .bind_groups + .set_size(indices.bind_groups.size()); + state + .tracker .compute_pipelines .set_size(indices.compute_pipelines.size()); - tracker.query_sets.set_size(indices.query_sets.size()); + state.tracker.query_sets.set_size(indices.query_sets.size()); let timestamp_writes = if let Some(tw) = timestamp_writes.take() { - let query_set = tracker.query_sets.insert_single(tw.query_set); + let query_set = state.tracker.query_sets.insert_single(tw.query_set); // Unlike in render passes we can't delay resetting the query sets since // there is no auxiliary pass. @@ -539,10 +570,6 @@ impl Global { None }; - let discard_hal_labels = self - .instance - .flags - .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS); let hal_desc = hal::ComputePassDescriptor { label: hal_label(base.label.as_deref(), self.instance.flags), timestamp_writes, @@ -552,12 +579,6 @@ impl Global { raw.begin_compute_pass(&hal_desc); } - let mut intermediate_trackers = Tracker::::new(); - - // Immediate texture inits required because of prior discards. Need to - // be inserted before texture reads. - let mut pending_discard_init_fixups = SurfacesInDiscardState::new(); - // TODO: We should be draining the commands here, avoiding extra copies in the process. // (A command encoder can't be executed twice!) for command in base.commands { @@ -580,19 +601,19 @@ impl Global { .map_pass_err(scope); } - temp_offsets.clear(); - temp_offsets.extend_from_slice( - &base.dynamic_offsets - [dynamic_offset_count..dynamic_offset_count + num_dynamic_offsets], + state.temp_offsets.clear(); + state.temp_offsets.extend_from_slice( + &base.dynamic_offsets[state.dynamic_offset_count + ..state.dynamic_offset_count + num_dynamic_offsets], ); - dynamic_offset_count += num_dynamic_offsets; + state.dynamic_offset_count += num_dynamic_offsets; - let bind_group = tracker.bind_groups.insert_single(bind_group); + let bind_group = state.tracker.bind_groups.insert_single(bind_group); bind_group - .validate_dynamic_bindings(index, &temp_offsets, &cmd_buf.limits) + .validate_dynamic_bindings(index, &state.temp_offsets, &cmd_buf.limits) .map_pass_err(scope)?; - buffer_memory_init_actions.extend( + state.buffer_memory_init_actions.extend( bind_group.used_buffer_ranges.iter().filter_map(|action| { action .buffer @@ -603,20 +624,22 @@ impl Global { ); for action in bind_group.used_texture_ranges.iter() { - pending_discard_init_fixups - .extend(texture_memory_actions.register_init_action(action)); + state + .pending_discard_init_fixups + .extend(state.texture_memory_actions.register_init_action(action)); } let pipeline_layout = state.binder.pipeline_layout.clone(); let entries = state .binder - .assign_group(index as usize, bind_group, &temp_offsets); + .assign_group(index as usize, bind_group, &state.temp_offsets); if !entries.is_empty() && pipeline_layout.is_some() { let pipeline_layout = pipeline_layout.as_ref().unwrap().raw(); for (i, e) in entries.iter().enumerate() { if let Some(group) = e.group.as_ref() { - let raw_bg = group.try_raw(&snatch_guard).map_pass_err(scope)?; + let raw_bg = + group.try_raw(&state.snatch_guard).map_pass_err(scope)?; unsafe { raw.set_bind_group( pipeline_layout, @@ -636,7 +659,7 @@ impl Global { state.pipeline = Some(pipeline.as_info().id()); - let pipeline = tracker.compute_pipelines.insert_single(pipeline); + let pipeline = state.tracker.compute_pipelines.insert_single(pipeline); unsafe { raw.set_compute_pipeline(pipeline.raw()); @@ -659,7 +682,7 @@ impl Global { for (i, e) in entries.iter().enumerate() { if let Some(group) = e.group.as_ref() { let raw_bg = - group.try_raw(&snatch_guard).map_pass_err(scope)?; + group.try_raw(&state.snatch_guard).map_pass_err(scope)?; unsafe { raw.set_bind_group( pipeline.layout.raw(), @@ -741,9 +764,7 @@ impl Global { }; state.is_ready().map_pass_err(scope)?; - state - .flush_states(raw, &mut intermediate_trackers, None, &snatch_guard) - .map_pass_err(scope)?; + state.flush_states(raw, None).map_pass_err(scope)?; let groups_size_limit = cmd_buf.limits.max_compute_workgroups_per_dimension; @@ -774,7 +795,8 @@ impl Global { state.is_ready().map_pass_err(scope)?; - device + state + .device .require_downlevel_flags(wgt::DownlevelFlags::INDIRECT_EXECUTION) .map_pass_err(scope)?; @@ -797,11 +819,9 @@ impl Global { .map_pass_err(scope); } - let buf_raw = buffer.try_raw(&snatch_guard).map_pass_err(scope)?; - let stride = 3 * 4; // 3 integers, x/y/z group size - buffer_memory_init_actions.extend( + state.buffer_memory_init_actions.extend( buffer.initialization_status.read().create_action( &buffer, offset..(offset + stride), @@ -810,28 +830,30 @@ impl Global { ); state - .flush_states( - raw, - &mut intermediate_trackers, - Some(buffer.as_info().tracker_index()), - &snatch_guard, - ) + .flush_states(raw, Some(buffer.as_info().tracker_index())) .map_pass_err(scope)?; + + let buf_raw = buffer.try_raw(&state.snatch_guard).map_pass_err(scope)?; unsafe { raw.dispatch_indirect(buf_raw, offset); } } ArcComputeCommand::PushDebugGroup { color: _, len } => { state.debug_scope_depth += 1; - if !discard_hal_labels { - let label = - str::from_utf8(&base.string_data[string_offset..string_offset + len]) - .unwrap(); + if !state + .device + .instance_flags + .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS) + { + let label = str::from_utf8( + &base.string_data[state.string_offset..state.string_offset + len], + ) + .unwrap(); unsafe { raw.begin_debug_marker(label); } } - string_offset += len; + state.string_offset += len; } ArcComputeCommand::PopDebugGroup => { let scope = PassErrorScope::PopDebugGroup; @@ -841,20 +863,29 @@ impl Global { .map_pass_err(scope); } state.debug_scope_depth -= 1; - if !discard_hal_labels { + if !state + .device + .instance_flags + .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS) + { unsafe { raw.end_debug_marker(); } } } ArcComputeCommand::InsertDebugMarker { color: _, len } => { - if !discard_hal_labels { - let label = - str::from_utf8(&base.string_data[string_offset..string_offset + len]) - .unwrap(); + if !state + .device + .instance_flags + .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS) + { + let label = str::from_utf8( + &base.string_data[state.string_offset..state.string_offset + len], + ) + .unwrap(); unsafe { raw.insert_debug_marker(label) } } - string_offset += len; + state.string_offset += len; } ArcComputeCommand::WriteTimestamp { query_set, @@ -864,11 +895,12 @@ impl Global { query_set.same_device_as(cmd_buf).map_pass_err(scope)?; - device + state + .device .require_features(wgt::Features::TIMESTAMP_QUERY_INSIDE_PASSES) .map_pass_err(scope)?; - let query_set = tracker.query_sets.insert_single(query_set); + let query_set = state.tracker.query_sets.insert_single(query_set); query_set .validate_and_write_timestamp(raw, query_index, None) @@ -882,20 +914,21 @@ impl Global { query_set.same_device_as(cmd_buf).map_pass_err(scope)?; - let query_set = tracker.query_sets.insert_single(query_set); + let query_set = state.tracker.query_sets.insert_single(query_set); validate_and_begin_pipeline_statistics_query( query_set.clone(), raw, query_index, None, - &mut active_query, + &mut state.active_query, ) .map_pass_err(scope)?; } ArcComputeCommand::EndPipelineStatisticsQuery => { let scope = PassErrorScope::EndPipelineStatisticsQuery; - end_pipeline_statistics_query(raw, &mut active_query).map_pass_err(scope)?; + end_pipeline_statistics_query(raw, &mut state.active_query) + .map_pass_err(scope)?; } } } @@ -916,17 +949,17 @@ impl Global { // Use that buffer to insert barriers and clear discarded images. let transit = encoder.open().map_pass_err(pass_scope)?; fixup_discarded_surfaces( - pending_discard_init_fixups.into_iter(), + state.pending_discard_init_fixups.into_iter(), transit, - &mut tracker.textures, - device, - &snatch_guard, + &mut state.tracker.textures, + state.device, + &state.snatch_guard, ); CommandBuffer::insert_barriers_from_tracker( transit, - tracker, - &intermediate_trackers, - &snatch_guard, + state.tracker, + &state.intermediate_trackers, + &state.snatch_guard, ); // Close the command buffer, and swap it with the previous. encoder.close_and_swap().map_pass_err(pass_scope)?; From 3309a072e3c34d85cfa43ed07931eab7dcd25235 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 14:05:14 +0200 Subject: [PATCH 09/47] remove `CommandBuffer.limits` --- wgpu-core/src/binding_model.rs | 4 ++-- wgpu-core/src/command/compute.rs | 7 ++++--- wgpu-core/src/command/mod.rs | 2 -- wgpu-core/src/command/render.rs | 2 +- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/wgpu-core/src/binding_model.rs b/wgpu-core/src/binding_model.rs index a64fe78465..3443bcf782 100644 --- a/wgpu-core/src/binding_model.rs +++ b/wgpu-core/src/binding_model.rs @@ -892,7 +892,6 @@ impl BindGroup { &self, bind_group_index: u32, offsets: &[wgt::DynamicOffset], - limits: &wgt::Limits, ) -> Result<(), BindError> { if self.dynamic_binding_info.len() != offsets.len() { return Err(BindError::MismatchedDynamicOffsetCount { @@ -908,7 +907,8 @@ impl BindGroup { .zip(offsets.iter()) .enumerate() { - let (alignment, limit_name) = buffer_binding_type_alignment(limits, info.binding_type); + let (alignment, limit_name) = + buffer_binding_type_alignment(&self.device.limits, info.binding_type); if offset as wgt::BufferAddress % alignment as u64 != 0 { return Err(BindError::UnalignedDynamicBinding { group: bind_group_index, diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index 4c6e005d46..533383c633 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -592,7 +592,7 @@ impl Global { bind_group.same_device_as(cmd_buf).map_pass_err(scope)?; - let max_bind_groups = cmd_buf.limits.max_bind_groups; + let max_bind_groups = state.device.limits.max_bind_groups; if index >= max_bind_groups { return Err(ComputePassErrorInner::BindGroupIndexOutOfRange { index, @@ -610,7 +610,7 @@ impl Global { let bind_group = state.tracker.bind_groups.insert_single(bind_group); bind_group - .validate_dynamic_bindings(index, &state.temp_offsets, &cmd_buf.limits) + .validate_dynamic_bindings(index, &state.temp_offsets) .map_pass_err(scope)?; state.buffer_memory_init_actions.extend( @@ -766,7 +766,8 @@ impl Global { state.flush_states(raw, None).map_pass_err(scope)?; - let groups_size_limit = cmd_buf.limits.max_compute_workgroups_per_dimension; + let groups_size_limit = + state.device.limits.max_compute_workgroups_per_dimension; if groups[0] > groups_size_limit || groups[1] > groups_size_limit diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index b237a375a6..b4669f8e7a 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -305,7 +305,6 @@ impl CommandBufferMutable { /// whose contents eventually become the property of the submission queue. pub struct CommandBuffer { pub(crate) device: Arc>, - limits: wgt::Limits, support_clear_texture: bool, pub(crate) info: ResourceInfo>, @@ -344,7 +343,6 @@ impl CommandBuffer { ) -> Self { CommandBuffer { device: device.clone(), - limits: device.limits.clone(), support_clear_texture: device.features.contains(wgt::Features::CLEAR_TEXTURE), info: ResourceInfo::new(label, None), data: Mutex::new( diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 2508ae26f7..e2721e71d1 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1522,7 +1522,7 @@ impl Global { .map_pass_err(scope)?; bind_group - .validate_dynamic_bindings(index, &temp_offsets, &cmd_buf.limits) + .validate_dynamic_bindings(index, &temp_offsets) .map_pass_err(scope)?; // merge the resource tracker in From f8656d3082df71cdb798d25e2e7beb442bf817b2 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 14:16:30 +0200 Subject: [PATCH 10/47] extract `set_bind_group` from `compute_pass_end_impl` --- wgpu-core/src/command/compute.rs | 144 +++++++++++++++++-------------- 1 file changed, 81 insertions(+), 63 deletions(-) diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index 533383c633..96a1d4488b 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -1,5 +1,7 @@ use crate::{ - binding_model::{BindError, LateMinBufferBindingSizeMismatch, PushConstantUploadError}, + binding_model::{ + BindError, BindGroup, LateMinBufferBindingSizeMismatch, PushConstantUploadError, + }, command::{ bind::Binder, compute_command::{ArcComputeCommand, ComputeCommand}, @@ -589,68 +591,16 @@ impl Global { bind_group, } => { let scope = PassErrorScope::SetBindGroup(bind_group.as_info().id()); - - bind_group.same_device_as(cmd_buf).map_pass_err(scope)?; - - let max_bind_groups = state.device.limits.max_bind_groups; - if index >= max_bind_groups { - return Err(ComputePassErrorInner::BindGroupIndexOutOfRange { - index, - max: max_bind_groups, - }) - .map_pass_err(scope); - } - - state.temp_offsets.clear(); - state.temp_offsets.extend_from_slice( - &base.dynamic_offsets[state.dynamic_offset_count - ..state.dynamic_offset_count + num_dynamic_offsets], - ); - state.dynamic_offset_count += num_dynamic_offsets; - - let bind_group = state.tracker.bind_groups.insert_single(bind_group); - bind_group - .validate_dynamic_bindings(index, &state.temp_offsets) - .map_pass_err(scope)?; - - state.buffer_memory_init_actions.extend( - bind_group.used_buffer_ranges.iter().filter_map(|action| { - action - .buffer - .initialization_status - .read() - .check_action(action) - }), - ); - - for action in bind_group.used_texture_ranges.iter() { - state - .pending_discard_init_fixups - .extend(state.texture_memory_actions.register_init_action(action)); - } - - let pipeline_layout = state.binder.pipeline_layout.clone(); - let entries = - state - .binder - .assign_group(index as usize, bind_group, &state.temp_offsets); - if !entries.is_empty() && pipeline_layout.is_some() { - let pipeline_layout = pipeline_layout.as_ref().unwrap().raw(); - for (i, e) in entries.iter().enumerate() { - if let Some(group) = e.group.as_ref() { - let raw_bg = - group.try_raw(&state.snatch_guard).map_pass_err(scope)?; - unsafe { - raw.set_bind_group( - pipeline_layout, - index + i as u32, - raw_bg, - &e.dynamic_offsets, - ); - } - } - } - } + set_bind_group( + &mut state, + raw, + cmd_buf, + &base.dynamic_offsets, + index, + num_dynamic_offsets, + bind_group, + ) + .map_pass_err(scope)?; } ArcComputeCommand::SetPipeline(pipeline) => { let scope = PassErrorScope::SetPipelineCompute(pipeline.as_info().id()); @@ -969,6 +919,74 @@ impl Global { } } +fn set_bind_group( + state: &mut State, + raw: &mut A::CommandEncoder, + cmd_buf: &CommandBuffer, + dynamic_offsets: &[DynamicOffset], + index: u32, + num_dynamic_offsets: usize, + bind_group: Arc>, +) -> Result<(), ComputePassErrorInner> { + bind_group.same_device_as(cmd_buf)?; + + let max_bind_groups = state.device.limits.max_bind_groups; + if index >= max_bind_groups { + return Err(ComputePassErrorInner::BindGroupIndexOutOfRange { + index, + max: max_bind_groups, + }); + } + + state.temp_offsets.clear(); + state.temp_offsets.extend_from_slice( + &dynamic_offsets + [state.dynamic_offset_count..state.dynamic_offset_count + num_dynamic_offsets], + ); + state.dynamic_offset_count += num_dynamic_offsets; + + let bind_group = state.tracker.bind_groups.insert_single(bind_group); + bind_group.validate_dynamic_bindings(index, &state.temp_offsets)?; + + state + .buffer_memory_init_actions + .extend(bind_group.used_buffer_ranges.iter().filter_map(|action| { + action + .buffer + .initialization_status + .read() + .check_action(action) + })); + + for action in bind_group.used_texture_ranges.iter() { + state + .pending_discard_init_fixups + .extend(state.texture_memory_actions.register_init_action(action)); + } + + let pipeline_layout = state.binder.pipeline_layout.clone(); + let entries = state + .binder + .assign_group(index as usize, bind_group, &state.temp_offsets); + if !entries.is_empty() && pipeline_layout.is_some() { + let pipeline_layout = pipeline_layout.as_ref().unwrap().raw(); + for (i, e) in entries.iter().enumerate() { + if let Some(group) = e.group.as_ref() { + let raw_bg = group.try_raw(&state.snatch_guard)?; + unsafe { + raw.set_bind_group( + pipeline_layout, + index + i as u32, + raw_bg, + &e.dynamic_offsets, + ); + } + } + } + } + Ok(()) +} + // Recording a compute pass. impl Global { pub fn compute_pass_set_bind_group( From 45fce5b5c6bdbf9d814c3082ac4ffa24457b3046 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 14:18:41 +0200 Subject: [PATCH 11/47] extract `set_pipeline` from `compute_pass_end_impl` --- wgpu-core/src/command/compute.rs | 126 ++++++++++++++++--------------- 1 file changed, 64 insertions(+), 62 deletions(-) diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index 96a1d4488b..c80daf3236 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -604,68 +604,7 @@ impl Global { } ArcComputeCommand::SetPipeline(pipeline) => { let scope = PassErrorScope::SetPipelineCompute(pipeline.as_info().id()); - - pipeline.same_device_as(cmd_buf).map_pass_err(scope)?; - - state.pipeline = Some(pipeline.as_info().id()); - - let pipeline = state.tracker.compute_pipelines.insert_single(pipeline); - - unsafe { - raw.set_compute_pipeline(pipeline.raw()); - } - - // Rebind resources - if state.binder.pipeline_layout.is_none() - || !state - .binder - .pipeline_layout - .as_ref() - .unwrap() - .is_equal(&pipeline.layout) - { - let (start_index, entries) = state.binder.change_pipeline_layout( - &pipeline.layout, - &pipeline.late_sized_buffer_groups, - ); - if !entries.is_empty() { - for (i, e) in entries.iter().enumerate() { - if let Some(group) = e.group.as_ref() { - let raw_bg = - group.try_raw(&state.snatch_guard).map_pass_err(scope)?; - unsafe { - raw.set_bind_group( - pipeline.layout.raw(), - start_index as u32 + i as u32, - raw_bg, - &e.dynamic_offsets, - ); - } - } - } - } - - // Clear push constant ranges - let non_overlapping = super::bind::compute_nonoverlapping_ranges( - &pipeline.layout.push_constant_ranges, - ); - for range in non_overlapping { - let offset = range.range.start; - let size_bytes = range.range.end - offset; - super::push_constant_clear( - offset, - size_bytes, - |clear_offset, clear_data| unsafe { - raw.set_push_constants( - pipeline.layout.raw(), - wgt::ShaderStages::COMPUTE, - clear_offset, - clear_data, - ); - }, - ); - } - } + set_pipeline(&mut state, raw, cmd_buf, pipeline).map_pass_err(scope)?; } ArcComputeCommand::SetPushConstant { offset, @@ -987,6 +926,69 @@ fn set_bind_group( Ok(()) } +fn set_pipeline( + state: &mut State, + raw: &mut A::CommandEncoder, + cmd_buf: &CommandBuffer, + pipeline: Arc>, +) -> Result<(), ComputePassErrorInner> { + pipeline.same_device_as(cmd_buf)?; + + state.pipeline = Some(pipeline.as_info().id()); + + let pipeline = state.tracker.compute_pipelines.insert_single(pipeline); + + unsafe { + raw.set_compute_pipeline(pipeline.raw()); + } + + // Rebind resources + if state.binder.pipeline_layout.is_none() + || !state + .binder + .pipeline_layout + .as_ref() + .unwrap() + .is_equal(&pipeline.layout) + { + let (start_index, entries) = state + .binder + .change_pipeline_layout(&pipeline.layout, &pipeline.late_sized_buffer_groups); + if !entries.is_empty() { + for (i, e) in entries.iter().enumerate() { + if let Some(group) = e.group.as_ref() { + let raw_bg = group.try_raw(&state.snatch_guard)?; + unsafe { + raw.set_bind_group( + pipeline.layout.raw(), + start_index as u32 + i as u32, + raw_bg, + &e.dynamic_offsets, + ); + } + } + } + } + + // Clear push constant ranges + let non_overlapping = + super::bind::compute_nonoverlapping_ranges(&pipeline.layout.push_constant_ranges); + for range in non_overlapping { + let offset = range.range.start; + let size_bytes = range.range.end - offset; + super::push_constant_clear(offset, size_bytes, |clear_offset, clear_data| unsafe { + raw.set_push_constants( + pipeline.layout.raw(), + wgt::ShaderStages::COMPUTE, + clear_offset, + clear_data, + ); + }); + } + } + Ok(()) +} + // Recording a compute pass. impl Global { pub fn compute_pass_set_bind_group( From 90ef6b9a18a51517a6e5a6e0cf504e2e4f962ca1 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 14:21:57 +0200 Subject: [PATCH 12/47] extract `set_push_constant` from `compute_pass_end_impl` --- wgpu-core/src/command/compute.rs | 80 +++++++++++++++++++------------- 1 file changed, 47 insertions(+), 33 deletions(-) diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index c80daf3236..c84b1d78d6 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -612,39 +612,15 @@ impl Global { values_offset, } => { let scope = PassErrorScope::SetPushConstant; - - let end_offset_bytes = offset + size_bytes; - let values_end_offset = - (values_offset + size_bytes / wgt::PUSH_CONSTANT_ALIGNMENT) as usize; - let data_slice = - &base.push_constant_data[(values_offset as usize)..values_end_offset]; - - let pipeline_layout = state - .binder - .pipeline_layout - .as_ref() - //TODO: don't error here, lazily update the push constants - .ok_or(ComputePassErrorInner::Dispatch( - DispatchError::MissingPipeline, - )) - .map_pass_err(scope)?; - - pipeline_layout - .validate_push_constant_ranges( - wgt::ShaderStages::COMPUTE, - offset, - end_offset_bytes, - ) - .map_pass_err(scope)?; - - unsafe { - raw.set_push_constants( - pipeline_layout.raw(), - wgt::ShaderStages::COMPUTE, - offset, - data_slice, - ); - } + set_push_constant( + &state, + raw, + &base.push_constant_data, + offset, + size_bytes, + values_offset, + ) + .map_pass_err(scope)?; } ArcComputeCommand::Dispatch(groups) => { let scope = PassErrorScope::Dispatch { @@ -989,6 +965,44 @@ fn set_pipeline( Ok(()) } +fn set_push_constant( + state: &State, + raw: &mut A::CommandEncoder, + push_constant_data: &[u32], + offset: u32, + size_bytes: u32, + values_offset: u32, +) -> Result<(), ComputePassErrorInner> { + let end_offset_bytes = offset + size_bytes; + let values_end_offset = (values_offset + size_bytes / wgt::PUSH_CONSTANT_ALIGNMENT) as usize; + let data_slice = &push_constant_data[(values_offset as usize)..values_end_offset]; + + let pipeline_layout = state + .binder + .pipeline_layout + .as_ref() + //TODO: don't error here, lazily update the push constants + .ok_or(ComputePassErrorInner::Dispatch( + DispatchError::MissingPipeline, + ))?; + + pipeline_layout.validate_push_constant_ranges( + wgt::ShaderStages::COMPUTE, + offset, + end_offset_bytes, + )?; + + unsafe { + raw.set_push_constants( + pipeline_layout.raw(), + wgt::ShaderStages::COMPUTE, + offset, + data_slice, + ); + } + Ok(()) +} + // Recording a compute pass. impl Global { pub fn compute_pass_set_bind_group( From a8993cfb213ebdcb1cfeb945401b51db74456803 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 14:24:33 +0200 Subject: [PATCH 13/47] extract `dispatch` from `compute_pass_end_impl` --- wgpu-core/src/command/compute.rs | 53 ++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index c84b1d78d6..68b2d47c52 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -627,29 +627,7 @@ impl Global { indirect: false, pipeline: state.pipeline, }; - state.is_ready().map_pass_err(scope)?; - - state.flush_states(raw, None).map_pass_err(scope)?; - - let groups_size_limit = - state.device.limits.max_compute_workgroups_per_dimension; - - if groups[0] > groups_size_limit - || groups[1] > groups_size_limit - || groups[2] > groups_size_limit - { - return Err(ComputePassErrorInner::Dispatch( - DispatchError::InvalidGroupSize { - current: groups, - limit: groups_size_limit, - }, - )) - .map_pass_err(scope); - } - - unsafe { - raw.dispatch(groups); - } + dispatch(&mut state, raw, groups).map_pass_err(scope)?; } ArcComputeCommand::DispatchIndirect { buffer, offset } => { let scope = PassErrorScope::Dispatch { @@ -1003,6 +981,35 @@ fn set_push_constant( Ok(()) } +fn dispatch( + state: &mut State, + raw: &mut A::CommandEncoder, + groups: [u32; 3], +) -> Result<(), ComputePassErrorInner> { + state.is_ready()?; + + state.flush_states(raw, None)?; + + let groups_size_limit = state.device.limits.max_compute_workgroups_per_dimension; + + if groups[0] > groups_size_limit + || groups[1] > groups_size_limit + || groups[2] > groups_size_limit + { + return Err(ComputePassErrorInner::Dispatch( + DispatchError::InvalidGroupSize { + current: groups, + limit: groups_size_limit, + }, + )); + } + + unsafe { + raw.dispatch(groups); + } + Ok(()) +} + // Recording a compute pass. impl Global { pub fn compute_pass_set_bind_group( From a349fc8db76003ab62e3218d558d32a31cd0d71b Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 14:26:37 +0200 Subject: [PATCH 14/47] extract `dispatch_indirect` from `compute_pass_end_impl` --- wgpu-core/src/command/compute.rs | 100 ++++++++++++++++--------------- 1 file changed, 53 insertions(+), 47 deletions(-) diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index 68b2d47c52..7c3789ec8f 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -17,7 +17,9 @@ use crate::{ hal_api::HalApi, hal_label, id, init_tracker::{BufferInitTrackerAction, MemoryInitKind}, - resource::{self, DestroyedResourceError, MissingBufferUsageError, ParentDevice, Resource}, + resource::{ + self, Buffer, DestroyedResourceError, MissingBufferUsageError, ParentDevice, Resource, + }, snatch::SnatchGuard, track::{ResourceUsageCompatibilityError, Tracker, TrackerIndex, UsageScope}, Label, @@ -634,53 +636,8 @@ impl Global { indirect: true, pipeline: state.pipeline, }; - - buffer.same_device_as(cmd_buf).map_pass_err(scope)?; - - state.is_ready().map_pass_err(scope)?; - - state - .device - .require_downlevel_flags(wgt::DownlevelFlags::INDIRECT_EXECUTION) + dispatch_indirect(&mut state, raw, cmd_buf, buffer, offset) .map_pass_err(scope)?; - - state - .scope - .buffers - .merge_single(&buffer, hal::BufferUses::INDIRECT) - .map_pass_err(scope)?; - buffer - .check_usage(wgt::BufferUsages::INDIRECT) - .map_pass_err(scope)?; - - let end_offset = offset + mem::size_of::() as u64; - if end_offset > buffer.size { - return Err(ComputePassErrorInner::IndirectBufferOverrun { - offset, - end_offset, - buffer_size: buffer.size, - }) - .map_pass_err(scope); - } - - let stride = 3 * 4; // 3 integers, x/y/z group size - - state.buffer_memory_init_actions.extend( - buffer.initialization_status.read().create_action( - &buffer, - offset..(offset + stride), - MemoryInitKind::NeedsInitializedMemory, - ), - ); - - state - .flush_states(raw, Some(buffer.as_info().tracker_index())) - .map_pass_err(scope)?; - - let buf_raw = buffer.try_raw(&state.snatch_guard).map_pass_err(scope)?; - unsafe { - raw.dispatch_indirect(buf_raw, offset); - } } ArcComputeCommand::PushDebugGroup { color: _, len } => { state.debug_scope_depth += 1; @@ -1010,6 +967,55 @@ fn dispatch( Ok(()) } +fn dispatch_indirect( + state: &mut State, + raw: &mut A::CommandEncoder, + cmd_buf: &CommandBuffer, + buffer: Arc>, + offset: u64, +) -> Result<(), ComputePassErrorInner> { + buffer.same_device_as(cmd_buf)?; + + state.is_ready()?; + + state + .device + .require_downlevel_flags(wgt::DownlevelFlags::INDIRECT_EXECUTION)?; + + state + .scope + .buffers + .merge_single(&buffer, hal::BufferUses::INDIRECT)?; + buffer.check_usage(wgt::BufferUsages::INDIRECT)?; + + let end_offset = offset + mem::size_of::() as u64; + if end_offset > buffer.size { + return Err(ComputePassErrorInner::IndirectBufferOverrun { + offset, + end_offset, + buffer_size: buffer.size, + }); + } + + let stride = 3 * 4; // 3 integers, x/y/z group size + + state + .buffer_memory_init_actions + .extend(buffer.initialization_status.read().create_action( + &buffer, + offset..(offset + stride), + MemoryInitKind::NeedsInitializedMemory, + )); + + state.flush_states(raw, Some(buffer.as_info().tracker_index()))?; + + let buf_raw = buffer.try_raw(&state.snatch_guard)?; + unsafe { + raw.dispatch_indirect(buf_raw, offset); + } + Ok(()) +} + // Recording a compute pass. impl Global { pub fn compute_pass_set_bind_group( From 06b3d44bc3c8a82f4bf11a02e8bc564c9b894217 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 14:28:46 +0200 Subject: [PATCH 15/47] extract `push_debug_group` from `compute_pass_end_impl` --- wgpu-core/src/command/compute.rs | 37 +++++++++++++++++++------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index 7c3789ec8f..c78f639394 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -640,21 +640,7 @@ impl Global { .map_pass_err(scope)?; } ArcComputeCommand::PushDebugGroup { color: _, len } => { - state.debug_scope_depth += 1; - if !state - .device - .instance_flags - .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS) - { - let label = str::from_utf8( - &base.string_data[state.string_offset..state.string_offset + len], - ) - .unwrap(); - unsafe { - raw.begin_debug_marker(label); - } - } - state.string_offset += len; + push_debug_group(&mut state, raw, &base.string_data, len); } ArcComputeCommand::PopDebugGroup => { let scope = PassErrorScope::PopDebugGroup; @@ -1016,6 +1002,27 @@ fn dispatch_indirect( Ok(()) } +fn push_debug_group( + state: &mut State, + raw: &mut A::CommandEncoder, + string_data: &[u8], + len: usize, +) { + state.debug_scope_depth += 1; + if !state + .device + .instance_flags + .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS) + { + let label = + str::from_utf8(&string_data[state.string_offset..state.string_offset + len]).unwrap(); + unsafe { + raw.begin_debug_marker(label); + } + } + state.string_offset += len; +} + // Recording a compute pass. impl Global { pub fn compute_pass_set_bind_group( From 9ad79f8537d6cd4f54c05add295a8d56504b352f Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 14:30:43 +0200 Subject: [PATCH 16/47] extract `pop_debug_group` from `compute_pass_end_impl` --- wgpu-core/src/command/compute.rs | 36 +++++++++++++++++++------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index c78f639394..de088f2432 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -644,21 +644,7 @@ impl Global { } ArcComputeCommand::PopDebugGroup => { let scope = PassErrorScope::PopDebugGroup; - - if state.debug_scope_depth == 0 { - return Err(ComputePassErrorInner::InvalidPopDebugGroup) - .map_pass_err(scope); - } - state.debug_scope_depth -= 1; - if !state - .device - .instance_flags - .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS) - { - unsafe { - raw.end_debug_marker(); - } - } + pop_debug_group(&mut state, raw).map_pass_err(scope)?; } ArcComputeCommand::InsertDebugMarker { color: _, len } => { if !state @@ -1023,6 +1009,26 @@ fn push_debug_group( state.string_offset += len; } +fn pop_debug_group( + state: &mut State, + raw: &mut A::CommandEncoder, +) -> Result<(), ComputePassErrorInner> { + if state.debug_scope_depth == 0 { + return Err(ComputePassErrorInner::InvalidPopDebugGroup); + } + state.debug_scope_depth -= 1; + if !state + .device + .instance_flags + .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS) + { + unsafe { + raw.end_debug_marker(); + } + } + Ok(()) +} + // Recording a compute pass. impl Global { pub fn compute_pass_set_bind_group( From 4a932fc06d4eaacc404312fb7dde7d8fdc61e6ad Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 14:31:48 +0200 Subject: [PATCH 17/47] extract `insert_debug_marker` from `compute_pass_end_impl` --- wgpu-core/src/command/compute.rs | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index de088f2432..7fd521715a 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -647,18 +647,7 @@ impl Global { pop_debug_group(&mut state, raw).map_pass_err(scope)?; } ArcComputeCommand::InsertDebugMarker { color: _, len } => { - if !state - .device - .instance_flags - .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS) - { - let label = str::from_utf8( - &base.string_data[state.string_offset..state.string_offset + len], - ) - .unwrap(); - unsafe { raw.insert_debug_marker(label) } - } - state.string_offset += len; + insert_debug_marker(&mut state, raw, &base.string_data, len); } ArcComputeCommand::WriteTimestamp { query_set, @@ -1029,6 +1018,24 @@ fn pop_debug_group( Ok(()) } +fn insert_debug_marker( + state: &mut State, + raw: &mut A::CommandEncoder, + string_data: &[u8], + len: usize, +) { + if !state + .device + .instance_flags + .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS) + { + let label = + str::from_utf8(&string_data[state.string_offset..state.string_offset + len]).unwrap(); + unsafe { raw.insert_debug_marker(label) } + } + state.string_offset += len; +} + // Recording a compute pass. impl Global { pub fn compute_pass_set_bind_group( From 2564330b1d8a2b886a9a4888fc10cbebc5f9fb9b Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 14:33:58 +0200 Subject: [PATCH 18/47] extract `write_timestamp` from `compute_pass_end_impl` --- wgpu-core/src/command/compute.rs | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index 7fd521715a..d4da468c0f 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -654,18 +654,7 @@ impl Global { query_index, } => { let scope = PassErrorScope::WriteTimestamp; - - query_set.same_device_as(cmd_buf).map_pass_err(scope)?; - - state - .device - .require_features(wgt::Features::TIMESTAMP_QUERY_INSIDE_PASSES) - .map_pass_err(scope)?; - - let query_set = state.tracker.query_sets.insert_single(query_set); - - query_set - .validate_and_write_timestamp(raw, query_index, None) + write_timestamp(&mut state, raw, cmd_buf, query_set, query_index) .map_pass_err(scope)?; } ArcComputeCommand::BeginPipelineStatisticsQuery { @@ -1036,6 +1025,25 @@ fn insert_debug_marker( state.string_offset += len; } +fn write_timestamp( + state: &mut State, + raw: &mut A::CommandEncoder, + cmd_buf: &CommandBuffer, + query_set: Arc>, + query_index: u32, +) -> Result<(), ComputePassErrorInner> { + query_set.same_device_as(cmd_buf)?; + + state + .device + .require_features(wgt::Features::TIMESTAMP_QUERY_INSIDE_PASSES)?; + + let query_set = state.tracker.query_sets.insert_single(query_set); + + query_set.validate_and_write_timestamp(raw, query_index, None)?; + Ok(()) +} + // Recording a compute pass. impl Global { pub fn compute_pass_set_bind_group( From 51c4ad33ea5db36f9365b6f0ffc2e800fdeb99b4 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 14:45:31 +0200 Subject: [PATCH 19/47] move same device check and tracker insertion inside `validate_and_begin_pipeline_statistics_query` --- wgpu-core/src/command/compute.rs | 9 +++------ wgpu-core/src/command/query.rs | 10 +++++++++- wgpu-core/src/command/render.rs | 6 +++--- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index d4da468c0f..428783105a 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -662,14 +662,11 @@ impl Global { query_index, } => { let scope = PassErrorScope::BeginPipelineStatisticsQuery; - - query_set.same_device_as(cmd_buf).map_pass_err(scope)?; - - let query_set = state.tracker.query_sets.insert_single(query_set); - validate_and_begin_pipeline_statistics_query( - query_set.clone(), + query_set, raw, + &mut state.tracker.query_sets, + cmd_buf, query_index, None, &mut state.active_query, diff --git a/wgpu-core/src/command/query.rs b/wgpu-core/src/command/query.rs index 16557a4b4d..1da5badb71 100644 --- a/wgpu-core/src/command/query.rs +++ b/wgpu-core/src/command/query.rs @@ -10,7 +10,7 @@ use crate::{ id, init_tracker::MemoryInitKind, resource::{DestroyedResourceError, ParentDevice, QuerySet}, - track::TrackerIndex, + track::{StatelessTracker, TrackerIndex}, FastHashMap, }; use std::{iter, marker::PhantomData, sync::Arc}; @@ -124,6 +124,8 @@ impl crate::error::PrettyError for QueryError { #[derive(Clone, Debug, Error)] #[non_exhaustive] pub enum QueryUseError { + #[error(transparent)] + Device(#[from] DeviceError), #[error("Query {query_index} is out of bounds for a query set of size {query_set_size}")] OutOfBounds { query_index: u32, @@ -269,10 +271,14 @@ pub(super) fn end_occlusion_query( pub(super) fn validate_and_begin_pipeline_statistics_query( query_set: Arc>, raw_encoder: &mut A::CommandEncoder, + tracker: &mut StatelessTracker>, + cmd_buf: &CommandBuffer, query_index: u32, reset_state: Option<&mut QueryResetMap>, active_query: &mut Option<(Arc>, u32)>, ) -> Result<(), QueryUseError> { + query_set.same_device_as(cmd_buf)?; + let needs_reset = reset_state.is_none(); query_set.validate_query( SimplifiedQueryType::PipelineStatistics, @@ -280,6 +286,8 @@ pub(super) fn validate_and_begin_pipeline_statistics_query( reset_state, )?; + tracker.add_single(&query_set); + if let Some((_old, old_idx)) = active_query.take() { return Err(QueryUseError::AlreadyStarted { active_query_index: old_idx, diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index e2721e71d1..85f99faf6c 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -2324,11 +2324,11 @@ impl Global { ); let scope = PassErrorScope::BeginPipelineStatisticsQuery; - let query_set = tracker.query_sets.insert_single(query_set); - validate_and_begin_pipeline_statistics_query( - query_set.clone(), + query_set, raw, + &mut tracker.query_sets, + cmd_buf.as_ref(), query_index, Some(&mut cmd_buf_data.pending_query_resets), &mut active_query, From 3b52c1d80dde323d4fd45b0d68af23cf7f2c5836 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 15:22:13 +0200 Subject: [PATCH 20/47] move the raw encoder in `State` --- wgpu-core/src/command/compute.rs | 121 +++++++++++++++---------------- 1 file changed, 57 insertions(+), 64 deletions(-) diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index 428783105a..217cde1151 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -252,7 +252,7 @@ where } } -struct State<'scope, 'snatch_guard, 'cmd_buf, A: HalApi> { +struct State<'scope, 'snatch_guard, 'cmd_buf, 'raw_encoder, A: HalApi> { binder: Binder, pipeline: Option, scope: UsageScope<'scope, A>, @@ -261,6 +261,9 @@ struct State<'scope, 'snatch_guard, 'cmd_buf, A: HalApi> { snatch_guard: SnatchGuard<'snatch_guard>, device: &'cmd_buf Arc>, + + raw_encoder: &'raw_encoder mut A::CommandEncoder, + tracker: &'cmd_buf mut Tracker, buffer_memory_init_actions: &'cmd_buf mut Vec>, texture_memory_actions: &'cmd_buf mut CommandBufferTextureMemoryActions, @@ -277,7 +280,9 @@ struct State<'scope, 'snatch_guard, 'cmd_buf, A: HalApi> { pending_discard_init_fixups: SurfacesInDiscardState, } -impl<'scope, 'snatch_guard, 'cmd_buf, A: HalApi> State<'scope, 'snatch_guard, 'cmd_buf, A> { +impl<'scope, 'snatch_guard, 'cmd_buf, 'raw_encoder, A: HalApi> + State<'scope, 'snatch_guard, 'cmd_buf, 'raw_encoder, A> +{ fn is_ready(&self) -> Result<(), DispatchError> { let bind_mask = self.binder.invalid_mask(); if bind_mask != 0 { @@ -301,7 +306,6 @@ impl<'scope, 'snatch_guard, 'cmd_buf, A: HalApi> State<'scope, 'snatch_guard, 'c // part of the usage scope. fn flush_states( &mut self, - raw_encoder: &mut A::CommandEncoder, indirect_buffer: Option, ) -> Result<(), ResourceUsageCompatibilityError> { for bind_group in self.binder.list_active() { @@ -327,7 +331,7 @@ impl<'scope, 'snatch_guard, 'cmd_buf, A: HalApi> State<'scope, 'snatch_guard, 'c log::trace!("Encoding dispatch barriers"); CommandBuffer::drain_barriers( - raw_encoder, + self.raw_encoder, &mut self.intermediate_trackers, &self.snatch_guard, ); @@ -505,7 +509,7 @@ impl Global { encoder.close().map_pass_err(pass_scope)?; // will be reset to true if recording is done without errors *status = CommandEncoderStatus::Error; - let raw = encoder.open().map_pass_err(pass_scope)?; + let raw_encoder = encoder.open().map_pass_err(pass_scope)?; let mut state = State { binder: Binder::new(), @@ -516,6 +520,7 @@ impl Global { snatch_guard: device.snatchable_lock.read(), device, + raw_encoder, tracker: &mut cmd_buf_data.trackers, buffer_memory_init_actions: &mut cmd_buf_data.buffer_memory_init_actions, texture_memory_actions: &mut cmd_buf_data.texture_memory_actions, @@ -561,7 +566,9 @@ impl Global { // But no point in erroring over that nuance here! if let Some(range) = range { unsafe { - raw.reset_queries(query_set.raw.as_ref().unwrap(), range); + state + .raw_encoder + .reset_queries(query_set.raw.as_ref().unwrap(), range); } } @@ -580,7 +587,7 @@ impl Global { }; unsafe { - raw.begin_compute_pass(&hal_desc); + state.raw_encoder.begin_compute_pass(&hal_desc); } // TODO: We should be draining the commands here, avoiding extra copies in the process. @@ -595,7 +602,6 @@ impl Global { let scope = PassErrorScope::SetBindGroup(bind_group.as_info().id()); set_bind_group( &mut state, - raw, cmd_buf, &base.dynamic_offsets, index, @@ -606,7 +612,7 @@ impl Global { } ArcComputeCommand::SetPipeline(pipeline) => { let scope = PassErrorScope::SetPipelineCompute(pipeline.as_info().id()); - set_pipeline(&mut state, raw, cmd_buf, pipeline).map_pass_err(scope)?; + set_pipeline(&mut state, cmd_buf, pipeline).map_pass_err(scope)?; } ArcComputeCommand::SetPushConstant { offset, @@ -615,8 +621,7 @@ impl Global { } => { let scope = PassErrorScope::SetPushConstant; set_push_constant( - &state, - raw, + &mut state, &base.push_constant_data, offset, size_bytes, @@ -629,32 +634,31 @@ impl Global { indirect: false, pipeline: state.pipeline, }; - dispatch(&mut state, raw, groups).map_pass_err(scope)?; + dispatch(&mut state, groups).map_pass_err(scope)?; } ArcComputeCommand::DispatchIndirect { buffer, offset } => { let scope = PassErrorScope::Dispatch { indirect: true, pipeline: state.pipeline, }; - dispatch_indirect(&mut state, raw, cmd_buf, buffer, offset) - .map_pass_err(scope)?; + dispatch_indirect(&mut state, cmd_buf, buffer, offset).map_pass_err(scope)?; } ArcComputeCommand::PushDebugGroup { color: _, len } => { - push_debug_group(&mut state, raw, &base.string_data, len); + push_debug_group(&mut state, &base.string_data, len); } ArcComputeCommand::PopDebugGroup => { let scope = PassErrorScope::PopDebugGroup; - pop_debug_group(&mut state, raw).map_pass_err(scope)?; + pop_debug_group(&mut state).map_pass_err(scope)?; } ArcComputeCommand::InsertDebugMarker { color: _, len } => { - insert_debug_marker(&mut state, raw, &base.string_data, len); + insert_debug_marker(&mut state, &base.string_data, len); } ArcComputeCommand::WriteTimestamp { query_set, query_index, } => { let scope = PassErrorScope::WriteTimestamp; - write_timestamp(&mut state, raw, cmd_buf, query_set, query_index) + write_timestamp(&mut state, cmd_buf, query_set, query_index) .map_pass_err(scope)?; } ArcComputeCommand::BeginPipelineStatisticsQuery { @@ -664,7 +668,7 @@ impl Global { let scope = PassErrorScope::BeginPipelineStatisticsQuery; validate_and_begin_pipeline_statistics_query( query_set, - raw, + state.raw_encoder, &mut state.tracker.query_sets, cmd_buf, query_index, @@ -675,20 +679,28 @@ impl Global { } ArcComputeCommand::EndPipelineStatisticsQuery => { let scope = PassErrorScope::EndPipelineStatisticsQuery; - end_pipeline_statistics_query(raw, &mut state.active_query) + end_pipeline_statistics_query(state.raw_encoder, &mut state.active_query) .map_pass_err(scope)?; } } } unsafe { - raw.end_compute_pass(); + state.raw_encoder.end_compute_pass(); } // We've successfully recorded the compute pass, bring the // command buffer out of the error state. *status = CommandEncoderStatus::Recording; + let State { + snatch_guard, + tracker, + intermediate_trackers, + pending_discard_init_fixups, + .. + } = state; + // Stop the current command buffer. encoder.close().map_pass_err(pass_scope)?; @@ -697,17 +709,17 @@ impl Global { // Use that buffer to insert barriers and clear discarded images. let transit = encoder.open().map_pass_err(pass_scope)?; fixup_discarded_surfaces( - state.pending_discard_init_fixups.into_iter(), + pending_discard_init_fixups.into_iter(), transit, - &mut state.tracker.textures, - state.device, - &state.snatch_guard, + &mut tracker.textures, + device, + &snatch_guard, ); CommandBuffer::insert_barriers_from_tracker( transit, - state.tracker, - &state.intermediate_trackers, - &state.snatch_guard, + tracker, + &intermediate_trackers, + &snatch_guard, ); // Close the command buffer, and swap it with the previous. encoder.close_and_swap().map_pass_err(pass_scope)?; @@ -718,7 +730,6 @@ impl Global { fn set_bind_group( state: &mut State, - raw: &mut A::CommandEncoder, cmd_buf: &CommandBuffer, dynamic_offsets: &[DynamicOffset], index: u32, @@ -771,7 +782,7 @@ fn set_bind_group( if let Some(group) = e.group.as_ref() { let raw_bg = group.try_raw(&state.snatch_guard)?; unsafe { - raw.set_bind_group( + state.raw_encoder.set_bind_group( pipeline_layout, index + i as u32, raw_bg, @@ -786,7 +797,6 @@ fn set_bind_group( fn set_pipeline( state: &mut State, - raw: &mut A::CommandEncoder, cmd_buf: &CommandBuffer, pipeline: Arc>, ) -> Result<(), ComputePassErrorInner> { @@ -797,7 +807,7 @@ fn set_pipeline( let pipeline = state.tracker.compute_pipelines.insert_single(pipeline); unsafe { - raw.set_compute_pipeline(pipeline.raw()); + state.raw_encoder.set_compute_pipeline(pipeline.raw()); } // Rebind resources @@ -817,7 +827,7 @@ fn set_pipeline( if let Some(group) = e.group.as_ref() { let raw_bg = group.try_raw(&state.snatch_guard)?; unsafe { - raw.set_bind_group( + state.raw_encoder.set_bind_group( pipeline.layout.raw(), start_index as u32 + i as u32, raw_bg, @@ -835,7 +845,7 @@ fn set_pipeline( let offset = range.range.start; let size_bytes = range.range.end - offset; super::push_constant_clear(offset, size_bytes, |clear_offset, clear_data| unsafe { - raw.set_push_constants( + state.raw_encoder.set_push_constants( pipeline.layout.raw(), wgt::ShaderStages::COMPUTE, clear_offset, @@ -848,8 +858,7 @@ fn set_pipeline( } fn set_push_constant( - state: &State, - raw: &mut A::CommandEncoder, + state: &mut State, push_constant_data: &[u32], offset: u32, size_bytes: u32, @@ -875,7 +884,7 @@ fn set_push_constant( )?; unsafe { - raw.set_push_constants( + state.raw_encoder.set_push_constants( pipeline_layout.raw(), wgt::ShaderStages::COMPUTE, offset, @@ -887,12 +896,11 @@ fn set_push_constant( fn dispatch( state: &mut State, - raw: &mut A::CommandEncoder, groups: [u32; 3], ) -> Result<(), ComputePassErrorInner> { state.is_ready()?; - state.flush_states(raw, None)?; + state.flush_states(None)?; let groups_size_limit = state.device.limits.max_compute_workgroups_per_dimension; @@ -909,14 +917,13 @@ fn dispatch( } unsafe { - raw.dispatch(groups); + state.raw_encoder.dispatch(groups); } Ok(()) } fn dispatch_indirect( state: &mut State, - raw: &mut A::CommandEncoder, cmd_buf: &CommandBuffer, buffer: Arc>, offset: u64, @@ -954,21 +961,16 @@ fn dispatch_indirect( MemoryInitKind::NeedsInitializedMemory, )); - state.flush_states(raw, Some(buffer.as_info().tracker_index()))?; + state.flush_states(Some(buffer.as_info().tracker_index()))?; let buf_raw = buffer.try_raw(&state.snatch_guard)?; unsafe { - raw.dispatch_indirect(buf_raw, offset); + state.raw_encoder.dispatch_indirect(buf_raw, offset); } Ok(()) } -fn push_debug_group( - state: &mut State, - raw: &mut A::CommandEncoder, - string_data: &[u8], - len: usize, -) { +fn push_debug_group(state: &mut State, string_data: &[u8], len: usize) { state.debug_scope_depth += 1; if !state .device @@ -978,16 +980,13 @@ fn push_debug_group( let label = str::from_utf8(&string_data[state.string_offset..state.string_offset + len]).unwrap(); unsafe { - raw.begin_debug_marker(label); + state.raw_encoder.begin_debug_marker(label); } } state.string_offset += len; } -fn pop_debug_group( - state: &mut State, - raw: &mut A::CommandEncoder, -) -> Result<(), ComputePassErrorInner> { +fn pop_debug_group(state: &mut State) -> Result<(), ComputePassErrorInner> { if state.debug_scope_depth == 0 { return Err(ComputePassErrorInner::InvalidPopDebugGroup); } @@ -998,18 +997,13 @@ fn pop_debug_group( .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS) { unsafe { - raw.end_debug_marker(); + state.raw_encoder.end_debug_marker(); } } Ok(()) } -fn insert_debug_marker( - state: &mut State, - raw: &mut A::CommandEncoder, - string_data: &[u8], - len: usize, -) { +fn insert_debug_marker(state: &mut State, string_data: &[u8], len: usize) { if !state .device .instance_flags @@ -1017,14 +1011,13 @@ fn insert_debug_marker( { let label = str::from_utf8(&string_data[state.string_offset..state.string_offset + len]).unwrap(); - unsafe { raw.insert_debug_marker(label) } + unsafe { state.raw_encoder.insert_debug_marker(label) } } state.string_offset += len; } fn write_timestamp( state: &mut State, - raw: &mut A::CommandEncoder, cmd_buf: &CommandBuffer, query_set: Arc>, query_index: u32, @@ -1037,7 +1030,7 @@ fn write_timestamp( let query_set = state.tracker.query_sets.insert_single(query_set); - query_set.validate_and_write_timestamp(raw, query_index, None)?; + query_set.validate_and_write_timestamp(state.raw_encoder, query_index, None)?; Ok(()) } From f6b362f3a8ef513bf421f46a95f0ea17424e3dd3 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 15:45:30 +0200 Subject: [PATCH 21/47] put all state in `State` --- wgpu-core/src/command/render.rs | 328 ++++++++++++++++++++------------ 1 file changed, 207 insertions(+), 121 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 85f99faf6c..5272237b72 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1,6 +1,7 @@ use crate::command::{ validate_and_begin_occlusion_query, validate_and_begin_pipeline_statistics_query, }; +use crate::init_tracker::BufferInitTrackerAction; use crate::resource::Resource; use crate::snatch::SnatchGuard; use crate::{ @@ -432,8 +433,7 @@ impl VertexState { } } -#[derive(Debug)] -struct State { +struct State<'attachment, 'scope, 'snatch_guard, 'cmd_buf, 'raw_encoder, A: HalApi> { pipeline_flags: PipelineFlags, binder: Binder, blend_constant: OptionalState, @@ -442,9 +442,28 @@ struct State { index: IndexState, vertex: VertexState, debug_scope_depth: u32, + + info: RenderPassInfo<'attachment, 'scope, A>, + + snatch_guard: &'snatch_guard SnatchGuard<'snatch_guard>, + + device: &'cmd_buf Arc>, + + raw_encoder: &'raw_encoder mut A::CommandEncoder, + + tracker: &'cmd_buf mut Tracker, + buffer_memory_init_actions: &'cmd_buf mut Vec>, + texture_memory_actions: &'cmd_buf mut CommandBufferTextureMemoryActions, + + temp_offsets: Vec, + dynamic_offset_count: usize, + string_offset: usize, + active_query: Option<(Arc>, u32)>, } -impl State { +impl<'attachment, 'scope, 'snatch_guard, 'cmd_buf, 'raw_encoder, A: HalApi> + State<'attachment, 'scope, 'snatch_guard, 'cmd_buf, 'raw_encoder, A> +{ fn is_ready(&self, indexed: bool) -> Result<(), DrawError> { // Determine how many vertex buffers have already been bound let vertex_buffer_count = self.vertex.inputs.iter().take_while(|v| v.bound).count() as u32; @@ -1380,10 +1399,6 @@ impl Global { base.label.unwrap_or("") ); - let discard_hal_labels = self - .instance - .flags - .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS); let hal_label = hal_label(base.label.as_deref(), self.instance.flags); let pass_scope = PassErrorScope::PassEncoder(encoder_id); @@ -1393,7 +1408,7 @@ impl Global { let cmd_buf: Arc> = CommandBuffer::get_encoder(hub, encoder_id).map_pass_err(pass_scope)?; let device = &cmd_buf.device; - let snatch_guard = device.snatchable_lock.read(); + let snatch_guard = &device.snatchable_lock.read(); let (scope, pending_discard_init_fixups) = { let mut cmd_buf_data = cmd_buf.data.lock(); @@ -1441,7 +1456,7 @@ impl Global { encoder_id ); - let mut info = RenderPassInfo::start( + let info = RenderPassInfo::start( device, hal_label, color_attachments, @@ -1454,7 +1469,7 @@ impl Global { pending_query_resets, &*view_guard, &*query_set_guard, - &snatch_guard, + snatch_guard, ) .map_pass_err(pass_scope)?; @@ -1480,11 +1495,22 @@ impl Global { index: IndexState::default(), vertex: VertexState::default(), debug_scope_depth: 0, + + info, + + snatch_guard, + + device, + raw_encoder: raw, + tracker, + buffer_memory_init_actions, + texture_memory_actions, + + temp_offsets: Vec::new(), + dynamic_offset_count: 0, + string_offset: 0, + active_query: None, }; - let mut temp_offsets = Vec::new(); - let mut dynamic_offset_count = 0; - let mut string_offset = 0; - let mut active_query = None; for command in base.commands { match command { @@ -1499,7 +1525,7 @@ impl Global { ); let scope = PassErrorScope::SetBindGroup(bind_group.as_info().id()); - let max_bind_groups = device.limits.max_bind_groups; + let max_bind_groups = state.device.limits.max_bind_groups; if index >= max_bind_groups { return Err(RenderCommandError::BindGroupIndexOutOfRange { index, @@ -1508,33 +1534,35 @@ impl Global { .map_pass_err(scope); } - temp_offsets.clear(); - temp_offsets.extend_from_slice( - &base.dynamic_offsets - [dynamic_offset_count..dynamic_offset_count + num_dynamic_offsets], + state.temp_offsets.clear(); + state.temp_offsets.extend_from_slice( + &base.dynamic_offsets[state.dynamic_offset_count + ..state.dynamic_offset_count + num_dynamic_offsets], ); - dynamic_offset_count += num_dynamic_offsets; + state.dynamic_offset_count += num_dynamic_offsets; - let bind_group = tracker.bind_groups.insert_single(bind_group); + let bind_group = state.tracker.bind_groups.insert_single(bind_group); bind_group .same_device_as(cmd_buf.as_ref()) .map_pass_err(scope)?; bind_group - .validate_dynamic_bindings(index, &temp_offsets) + .validate_dynamic_bindings(index, &state.temp_offsets) .map_pass_err(scope)?; // merge the resource tracker in unsafe { - info.usage_scope + state + .info + .usage_scope .merge_bind_group(&bind_group.used) .map_pass_err(scope)?; } //Note: stateless trackers are not merged: the lifetime reference // is held to the bind group itself. - buffer_memory_init_actions.extend( + state.buffer_memory_init_actions.extend( bind_group.used_buffer_ranges.iter().filter_map(|action| { action .buffer @@ -1544,23 +1572,26 @@ impl Global { }), ); for action in bind_group.used_texture_ranges.iter() { - info.pending_discard_init_fixups - .extend(texture_memory_actions.register_init_action(action)); + state + .info + .pending_discard_init_fixups + .extend(state.texture_memory_actions.register_init_action(action)); } let pipeline_layout = state.binder.pipeline_layout.clone(); - let entries = - state - .binder - .assign_group(index as usize, bind_group, &temp_offsets); + let entries = state.binder.assign_group( + index as usize, + bind_group, + &state.temp_offsets, + ); if !entries.is_empty() && pipeline_layout.is_some() { let pipeline_layout = pipeline_layout.as_ref().unwrap().raw(); for (i, e) in entries.iter().enumerate() { if let Some(group) = e.group.as_ref() { let raw_bg = - group.try_raw(&snatch_guard).map_pass_err(scope)?; + group.try_raw(state.snatch_guard).map_pass_err(scope)?; unsafe { - raw.set_bind_group( + state.raw_encoder.set_bind_group( pipeline_layout, index + i as u32, raw_bg, @@ -1577,13 +1608,15 @@ impl Global { let scope = PassErrorScope::SetPipelineRender(pipeline.as_info().id()); state.pipeline = Some(pipeline.as_info().id()); - let pipeline = tracker.render_pipelines.insert_single(pipeline); + let pipeline = state.tracker.render_pipelines.insert_single(pipeline); pipeline .same_device_as(cmd_buf.as_ref()) .map_pass_err(scope)?; - info.context + state + .info + .context .check_compatible( &pipeline.pass_context, RenderPassCompatibilityCheckType::RenderPipeline, @@ -1594,9 +1627,9 @@ impl Global { state.pipeline_flags = pipeline.flags; if (pipeline.flags.contains(PipelineFlags::WRITES_DEPTH) - && info.is_depth_read_only) + && state.info.is_depth_read_only) || (pipeline.flags.contains(PipelineFlags::WRITES_STENCIL) - && info.is_stencil_read_only) + && state.info.is_stencil_read_only) { return Err(RenderCommandError::IncompatiblePipelineRods) .map_pass_err(scope); @@ -1607,12 +1640,14 @@ impl Global { .require(pipeline.flags.contains(PipelineFlags::BLEND_CONSTANT)); unsafe { - raw.set_render_pipeline(pipeline.raw()); + state.raw_encoder.set_render_pipeline(pipeline.raw()); } if pipeline.flags.contains(PipelineFlags::STENCIL_REFERENCE) { unsafe { - raw.set_stencil_reference(state.stencil_reference); + state + .raw_encoder + .set_stencil_reference(state.stencil_reference); } } @@ -1632,10 +1667,11 @@ impl Global { if !entries.is_empty() { for (i, e) in entries.iter().enumerate() { if let Some(group) = e.group.as_ref() { - let raw_bg = - group.try_raw(&snatch_guard).map_pass_err(scope)?; + let raw_bg = group + .try_raw(state.snatch_guard) + .map_pass_err(scope)?; unsafe { - raw.set_bind_group( + state.raw_encoder.set_bind_group( pipeline.layout.raw(), start_index as u32 + i as u32, raw_bg, @@ -1657,7 +1693,7 @@ impl Global { offset, size_bytes, |clear_offset, clear_data| unsafe { - raw.set_push_constants( + state.raw_encoder.set_push_constants( pipeline.layout.raw(), range.stages, clear_offset, @@ -1701,7 +1737,9 @@ impl Global { let scope = PassErrorScope::SetIndexBuffer(buffer.as_info().id()); - info.usage_scope + state + .info + .usage_scope .buffers .merge_single(&buffer, hal::BufferUses::INDEX) .map_pass_err(scope)?; @@ -1713,7 +1751,7 @@ impl Global { buffer .check_usage(BufferUsages::INDEX) .map_pass_err(scope)?; - let buf_raw = buffer.try_raw(&snatch_guard).map_pass_err(scope)?; + let buf_raw = buffer.try_raw(state.snatch_guard).map_pass_err(scope)?; let end = match size { Some(s) => offset + s.get(), @@ -1721,7 +1759,7 @@ impl Global { }; state.index.update_buffer(offset..end, index_format); - buffer_memory_init_actions.extend( + state.buffer_memory_init_actions.extend( buffer.initialization_status.read().create_action( &buffer, offset..end, @@ -1735,7 +1773,7 @@ impl Global { size, }; unsafe { - raw.set_index_buffer(bb, index_format); + state.raw_encoder.set_index_buffer(bb, index_format); } } ArcRenderCommand::SetVertexBuffer { @@ -1751,7 +1789,9 @@ impl Global { let scope = PassErrorScope::SetVertexBuffer(buffer.as_info().id()); - info.usage_scope + state + .info + .usage_scope .buffers .merge_single(&buffer, hal::BufferUses::VERTEX) .map_pass_err(scope)?; @@ -1760,7 +1800,7 @@ impl Global { .same_device_as(cmd_buf.as_ref()) .map_pass_err(scope)?; - let max_vertex_buffers = device.limits.max_vertex_buffers; + let max_vertex_buffers = state.device.limits.max_vertex_buffers; if slot >= max_vertex_buffers { return Err(RenderCommandError::VertexBufferIndexOutOfRange { index: slot, @@ -1772,7 +1812,7 @@ impl Global { buffer .check_usage(BufferUsages::VERTEX) .map_pass_err(scope)?; - let buf_raw = buffer.try_raw(&snatch_guard).map_pass_err(scope)?; + let buf_raw = buffer.try_raw(state.snatch_guard).map_pass_err(scope)?; let empty_slots = (1 + slot as usize).saturating_sub(state.vertex.inputs.len()); @@ -1788,7 +1828,7 @@ impl Global { }; vertex_state.bound = true; - buffer_memory_init_actions.extend( + state.buffer_memory_init_actions.extend( buffer.initialization_status.read().create_action( &buffer, offset..(offset + vertex_state.total_size), @@ -1802,7 +1842,7 @@ impl Global { size, }; unsafe { - raw.set_vertex_buffer(slot, bb); + state.raw_encoder.set_vertex_buffer(slot, bb); } state.vertex.update_limits(); } @@ -1817,7 +1857,7 @@ impl Global { color.a as f32, ]; unsafe { - raw.set_blend_constants(&array); + state.raw_encoder.set_blend_constants(&array); } } ArcRenderCommand::SetStencilReference(value) => { @@ -1829,7 +1869,7 @@ impl Global { .contains(PipelineFlags::STENCIL_REFERENCE) { unsafe { - raw.set_stencil_reference(value); + state.raw_encoder.set_stencil_reference(value); } } } @@ -1845,12 +1885,12 @@ impl Global { || rect.y < 0.0 || rect.w <= 0.0 || rect.h <= 0.0 - || rect.x + rect.w > info.extent.width as f32 - || rect.y + rect.h > info.extent.height as f32 + || rect.x + rect.w > state.info.extent.width as f32 + || rect.y + rect.h > state.info.extent.height as f32 { return Err(RenderCommandError::InvalidViewportRect( *rect, - info.extent, + state.info.extent, )) .map_pass_err(scope); } @@ -1867,7 +1907,7 @@ impl Global { h: rect.h, }; unsafe { - raw.set_viewport(&r, depth_min..depth_max); + state.raw_encoder.set_viewport(&r, depth_min..depth_max); } } ArcRenderCommand::SetPushConstant { @@ -1902,7 +1942,7 @@ impl Global { .map_pass_err(scope)?; unsafe { - raw.set_push_constants( + state.raw_encoder.set_push_constants( pipeline_layout.raw(), stages, offset, @@ -1914,11 +1954,14 @@ impl Global { api_log!("RenderPass::set_scissor_rect {rect:?}"); let scope = PassErrorScope::SetScissorRect; - if rect.x + rect.w > info.extent.width - || rect.y + rect.h > info.extent.height + if rect.x + rect.w > state.info.extent.width + || rect.y + rect.h > state.info.extent.height { - return Err(RenderCommandError::InvalidScissorRect(*rect, info.extent)) - .map_pass_err(scope); + return Err(RenderCommandError::InvalidScissorRect( + *rect, + state.info.extent, + )) + .map_pass_err(scope); } let r = hal::Rect { x: rect.x, @@ -1927,7 +1970,7 @@ impl Global { h: rect.h, }; unsafe { - raw.set_scissor_rect(&r); + state.raw_encoder.set_scissor_rect(&r); } } ArcRenderCommand::Draw { @@ -1971,7 +2014,7 @@ impl Global { unsafe { if instance_count > 0 && vertex_count > 0 { - raw.draw( + state.raw_encoder.draw( first_vertex, vertex_count, first_instance, @@ -2019,7 +2062,7 @@ impl Global { unsafe { if instance_count > 0 && index_count > 0 { - raw.draw_indexed( + state.raw_encoder.draw_indexed( first_index, index_count, base_vertex, @@ -2057,15 +2100,19 @@ impl Global { }; if count.is_some() { - device + state + .device .require_features(wgt::Features::MULTI_DRAW_INDIRECT) .map_pass_err(scope)?; } - device + state + .device .require_downlevel_flags(wgt::DownlevelFlags::INDIRECT_EXECUTION) .map_pass_err(scope)?; - info.usage_scope + state + .info + .usage_scope .buffers .merge_single(&indirect_buffer, hal::BufferUses::INDIRECT) .map_pass_err(scope)?; @@ -2073,8 +2120,9 @@ impl Global { indirect_buffer .check_usage(BufferUsages::INDIRECT) .map_pass_err(scope)?; - let indirect_raw = - indirect_buffer.try_raw(&snatch_guard).map_pass_err(scope)?; + let indirect_raw = indirect_buffer + .try_raw(state.snatch_guard) + .map_pass_err(scope)?; let actual_count = count.map_or(1, |c| c.get()); @@ -2089,7 +2137,7 @@ impl Global { .map_pass_err(scope); } - buffer_memory_init_actions.extend( + state.buffer_memory_init_actions.extend( indirect_buffer.initialization_status.read().create_action( &indirect_buffer, offset..end_offset, @@ -2099,10 +2147,16 @@ impl Global { match indexed { false => unsafe { - raw.draw_indirect(indirect_raw, offset, actual_count); + state + .raw_encoder + .draw_indirect(indirect_raw, offset, actual_count); }, true => unsafe { - raw.draw_indexed_indirect(indirect_raw, offset, actual_count); + state.raw_encoder.draw_indexed_indirect( + indirect_raw, + offset, + actual_count, + ); }, } } @@ -2132,14 +2186,18 @@ impl Global { true => mem::size_of::(), } as u64; - device + state + .device .require_features(wgt::Features::MULTI_DRAW_INDIRECT_COUNT) .map_pass_err(scope)?; - device + state + .device .require_downlevel_flags(wgt::DownlevelFlags::INDIRECT_EXECUTION) .map_pass_err(scope)?; - info.usage_scope + state + .info + .usage_scope .buffers .merge_single(&indirect_buffer, hal::BufferUses::INDIRECT) .map_pass_err(scope)?; @@ -2147,10 +2205,13 @@ impl Global { indirect_buffer .check_usage(BufferUsages::INDIRECT) .map_pass_err(scope)?; - let indirect_raw = - indirect_buffer.try_raw(&snatch_guard).map_pass_err(scope)?; + let indirect_raw = indirect_buffer + .try_raw(state.snatch_guard) + .map_pass_err(scope)?; - info.usage_scope + state + .info + .usage_scope .buffers .merge_single(&count_buffer, hal::BufferUses::INDIRECT) .map_pass_err(scope)?; @@ -2158,7 +2219,9 @@ impl Global { count_buffer .check_usage(BufferUsages::INDIRECT) .map_pass_err(scope)?; - let count_raw = count_buffer.try_raw(&snatch_guard).map_pass_err(scope)?; + let count_raw = count_buffer + .try_raw(state.snatch_guard) + .map_pass_err(scope)?; let end_offset = offset + stride * max_count as u64; if end_offset > indirect_buffer.size { @@ -2170,7 +2233,7 @@ impl Global { }) .map_pass_err(scope); } - buffer_memory_init_actions.extend( + state.buffer_memory_init_actions.extend( indirect_buffer.initialization_status.read().create_action( &indirect_buffer, offset..end_offset, @@ -2188,7 +2251,7 @@ impl Global { }) .map_pass_err(scope); } - buffer_memory_init_actions.extend( + state.buffer_memory_init_actions.extend( count_buffer.initialization_status.read().create_action( &count_buffer, count_buffer_offset..end_count_offset, @@ -2198,7 +2261,7 @@ impl Global { match indexed { false => unsafe { - raw.draw_indirect_count( + state.raw_encoder.draw_indirect_count( indirect_raw, offset, count_raw, @@ -2207,7 +2270,7 @@ impl Global { ); }, true => unsafe { - raw.draw_indexed_indirect_count( + state.raw_encoder.draw_indexed_indirect_count( indirect_raw, offset, count_raw, @@ -2219,18 +2282,22 @@ impl Global { } ArcRenderCommand::PushDebugGroup { color: _, len } => { state.debug_scope_depth += 1; - if !discard_hal_labels { + if !state + .device + .instance_flags + .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS) + { let label = str::from_utf8( - &base.string_data[string_offset..string_offset + len], + &base.string_data[state.string_offset..state.string_offset + len], ) .unwrap(); api_log!("RenderPass::push_debug_group {label:?}"); unsafe { - raw.begin_debug_marker(label); + state.raw_encoder.begin_debug_marker(label); } } - string_offset += len; + state.string_offset += len; } ArcRenderCommand::PopDebugGroup => { api_log!("RenderPass::pop_debug_group"); @@ -2241,24 +2308,32 @@ impl Global { .map_pass_err(scope); } state.debug_scope_depth -= 1; - if !discard_hal_labels { + if !state + .device + .instance_flags + .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS) + { unsafe { - raw.end_debug_marker(); + state.raw_encoder.end_debug_marker(); } } } ArcRenderCommand::InsertDebugMarker { color: _, len } => { - if !discard_hal_labels { + if !state + .device + .instance_flags + .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS) + { let label = str::from_utf8( - &base.string_data[string_offset..string_offset + len], + &base.string_data[state.string_offset..state.string_offset + len], ) .unwrap(); api_log!("RenderPass::insert_debug_marker {label:?}"); unsafe { - raw.insert_debug_marker(label); + state.raw_encoder.insert_debug_marker(label); } } - string_offset += len; + state.string_offset += len; } ArcRenderCommand::WriteTimestamp { query_set, @@ -2270,15 +2345,16 @@ impl Global { ); let scope = PassErrorScope::WriteTimestamp; - device + state + .device .require_features(wgt::Features::TIMESTAMP_QUERY_INSIDE_PASSES) .map_pass_err(scope)?; - let query_set = tracker.query_sets.insert_single(query_set); + let query_set = state.tracker.query_sets.insert_single(query_set); query_set .validate_and_write_timestamp( - raw, + state.raw_encoder, query_index, Some(&mut cmd_buf_data.pending_query_resets), ) @@ -2297,14 +2373,14 @@ impl Global { .map_err(|_| RenderPassErrorInner::InvalidQuerySet(query_set_id)) .map_pass_err(scope)?; - tracker.query_sets.add_single(query_set); + state.tracker.query_sets.add_single(query_set); validate_and_begin_occlusion_query( query_set.clone(), - raw, + state.raw_encoder, query_index, Some(&mut cmd_buf_data.pending_query_resets), - &mut active_query, + &mut state.active_query, ) .map_pass_err(scope)?; } @@ -2312,7 +2388,8 @@ impl Global { api_log!("RenderPass::end_occlusion_query"); let scope = PassErrorScope::EndOcclusionQuery; - end_occlusion_query(raw, &mut active_query).map_pass_err(scope)?; + end_occlusion_query(state.raw_encoder, &mut state.active_query) + .map_pass_err(scope)?; } ArcRenderCommand::BeginPipelineStatisticsQuery { query_set, @@ -2326,12 +2403,12 @@ impl Global { validate_and_begin_pipeline_statistics_query( query_set, - raw, - &mut tracker.query_sets, + state.raw_encoder, + &mut state.tracker.query_sets, cmd_buf.as_ref(), query_index, Some(&mut cmd_buf_data.pending_query_resets), - &mut active_query, + &mut state.active_query, ) .map_pass_err(scope)?; } @@ -2339,7 +2416,7 @@ impl Global { api_log!("RenderPass::end_pipeline_statistics_query"); let scope = PassErrorScope::EndPipelineStatisticsQuery; - end_pipeline_statistics_query(raw, &mut active_query) + end_pipeline_statistics_query(state.raw_encoder, &mut state.active_query) .map_pass_err(scope)?; } ArcRenderCommand::ExecuteBundle(bundle) => { @@ -2348,13 +2425,15 @@ impl Global { // Have to clone the bundle arc, otherwise we keep a mutable reference to the bundle // while later trying to add the bundle's resources to the tracker. - let bundle = tracker.bundles.insert_single(bundle).clone(); + let bundle = state.tracker.bundles.insert_single(bundle).clone(); bundle .same_device_as(cmd_buf.as_ref()) .map_pass_err(scope)?; - info.context + state + .info + .context .check_compatible( &bundle.context, RenderPassCompatibilityCheckType::RenderBundle, @@ -2362,13 +2441,13 @@ impl Global { .map_err(RenderPassErrorInner::IncompatibleBundleTargets) .map_pass_err(scope)?; - if (info.is_depth_read_only && !bundle.is_depth_read_only) - || (info.is_stencil_read_only && !bundle.is_stencil_read_only) + if (state.info.is_depth_read_only && !bundle.is_depth_read_only) + || (state.info.is_stencil_read_only && !bundle.is_stencil_read_only) { return Err( RenderPassErrorInner::IncompatibleBundleReadOnlyDepthStencil { - pass_depth: info.is_depth_read_only, - pass_stencil: info.is_stencil_read_only, + pass_depth: state.info.is_depth_read_only, + pass_stencil: state.info.is_stencil_read_only, bundle_depth: bundle.is_depth_read_only, bundle_stencil: bundle.is_stencil_read_only, }, @@ -2376,7 +2455,7 @@ impl Global { .map_pass_err(scope); } - buffer_memory_init_actions.extend( + state.buffer_memory_init_actions.extend( bundle .buffer_memory_init_actions .iter() @@ -2389,11 +2468,13 @@ impl Global { }), ); for action in bundle.texture_memory_init_actions.iter() { - info.pending_discard_init_fixups - .extend(texture_memory_actions.register_init_action(action)); + state + .info + .pending_discard_init_fixups + .extend(state.texture_memory_actions.register_init_action(action)); } - unsafe { bundle.execute(raw, &snatch_guard) } + unsafe { bundle.execute(state.raw_encoder, state.snatch_guard) } .map_err(|e| match e { ExecutionError::DestroyedResource(e) => { RenderCommandError::DestroyedResource(e) @@ -2405,10 +2486,13 @@ impl Global { .map_pass_err(scope)?; unsafe { - info.usage_scope + state + .info + .usage_scope .merge_render_bundle(&bundle.used) .map_pass_err(scope)?; - tracker + state + .tracker .add_from_render_bundle(&bundle.used) .map_pass_err(scope)?; }; @@ -2418,8 +2502,10 @@ impl Global { } log::trace!("Merging renderpass into cmd_buf {:?}", encoder_id); - let (trackers, pending_discard_init_fixups) = - info.finish(raw, &snatch_guard).map_pass_err(pass_scope)?; + let (trackers, pending_discard_init_fixups) = state + .info + .finish(state.raw_encoder, state.snatch_guard) + .map_pass_err(pass_scope)?; encoder.close().map_pass_err(pass_scope)?; (trackers, pending_discard_init_fixups) @@ -2444,12 +2530,12 @@ impl Global { transit, &mut tracker.textures, &cmd_buf.device, - &snatch_guard, + snatch_guard, ); cmd_buf_data.pending_query_resets.reset_queries(transit); - CommandBuffer::insert_barriers_from_scope(transit, tracker, &scope, &snatch_guard); + CommandBuffer::insert_barriers_from_scope(transit, tracker, &scope, snatch_guard); } *status = CommandEncoderStatus::Recording; From 7e140e1bc8380ecd0c45629dfb8325e9390d4281 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 15:51:07 +0200 Subject: [PATCH 22/47] extract `set_bind_group` from `render_pass_end_impl` --- wgpu-core/src/command/render.rs | 174 +++++++++++++++++--------------- 1 file changed, 92 insertions(+), 82 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 5272237b72..69cf13f043 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1,3 +1,4 @@ +use crate::binding_model::BindGroup; use crate::command::{ validate_and_begin_occlusion_query, validate_and_begin_pipeline_statistics_query, }; @@ -38,7 +39,7 @@ use arrayvec::ArrayVec; use hal::CommandEncoder as _; use thiserror::Error; use wgt::{ - BufferAddress, BufferSize, BufferUsages, Color, IndexFormat, TextureUsages, + BufferAddress, BufferSize, BufferUsages, Color, DynamicOffset, IndexFormat, TextureUsages, TextureViewDimension, VertexStepMode, }; @@ -1519,88 +1520,16 @@ impl Global { num_dynamic_offsets, bind_group, } => { - api_log!( - "RenderPass::set_bind_group {index} {}", - bind_group.error_ident() - ); - let scope = PassErrorScope::SetBindGroup(bind_group.as_info().id()); - let max_bind_groups = state.device.limits.max_bind_groups; - if index >= max_bind_groups { - return Err(RenderCommandError::BindGroupIndexOutOfRange { - index, - max: max_bind_groups, - }) - .map_pass_err(scope); - } - - state.temp_offsets.clear(); - state.temp_offsets.extend_from_slice( - &base.dynamic_offsets[state.dynamic_offset_count - ..state.dynamic_offset_count + num_dynamic_offsets], - ); - state.dynamic_offset_count += num_dynamic_offsets; - - let bind_group = state.tracker.bind_groups.insert_single(bind_group); - - bind_group - .same_device_as(cmd_buf.as_ref()) - .map_pass_err(scope)?; - - bind_group - .validate_dynamic_bindings(index, &state.temp_offsets) - .map_pass_err(scope)?; - - // merge the resource tracker in - unsafe { - state - .info - .usage_scope - .merge_bind_group(&bind_group.used) - .map_pass_err(scope)?; - } - //Note: stateless trackers are not merged: the lifetime reference - // is held to the bind group itself. - - state.buffer_memory_init_actions.extend( - bind_group.used_buffer_ranges.iter().filter_map(|action| { - action - .buffer - .initialization_status - .read() - .check_action(action) - }), - ); - for action in bind_group.used_texture_ranges.iter() { - state - .info - .pending_discard_init_fixups - .extend(state.texture_memory_actions.register_init_action(action)); - } - - let pipeline_layout = state.binder.pipeline_layout.clone(); - let entries = state.binder.assign_group( - index as usize, + set_bind_group( + &mut state, + &cmd_buf, + &base.dynamic_offsets, + index, + num_dynamic_offsets, bind_group, - &state.temp_offsets, - ); - if !entries.is_empty() && pipeline_layout.is_some() { - let pipeline_layout = pipeline_layout.as_ref().unwrap().raw(); - for (i, e) in entries.iter().enumerate() { - if let Some(group) = e.group.as_ref() { - let raw_bg = - group.try_raw(state.snatch_guard).map_pass_err(scope)?; - unsafe { - state.raw_encoder.set_bind_group( - pipeline_layout, - index + i as u32, - raw_bg, - &e.dynamic_offsets, - ); - } - } - } - } + ) + .map_pass_err(scope)?; } ArcRenderCommand::SetPipeline(pipeline) => { api_log!("RenderPass::set_pipeline {}", pipeline.error_ident()); @@ -2545,13 +2474,94 @@ impl Global { } } +fn set_bind_group( + state: &mut State, + cmd_buf: &Arc>, + dynamic_offsets: &[DynamicOffset], + index: u32, + num_dynamic_offsets: usize, + bind_group: Arc>, +) -> Result<(), RenderPassErrorInner> { + api_log!( + "RenderPass::set_bind_group {index} {}", + bind_group.error_ident() + ); + + let max_bind_groups = state.device.limits.max_bind_groups; + if index >= max_bind_groups { + return Err(RenderCommandError::BindGroupIndexOutOfRange { + index, + max: max_bind_groups, + } + .into()); + } + + state.temp_offsets.clear(); + state.temp_offsets.extend_from_slice( + &dynamic_offsets + [state.dynamic_offset_count..state.dynamic_offset_count + num_dynamic_offsets], + ); + state.dynamic_offset_count += num_dynamic_offsets; + + let bind_group = state.tracker.bind_groups.insert_single(bind_group); + + bind_group.same_device_as(cmd_buf.as_ref())?; + + bind_group.validate_dynamic_bindings(index, &state.temp_offsets)?; + + // merge the resource tracker in + unsafe { + state.info.usage_scope.merge_bind_group(&bind_group.used)?; + } + //Note: stateless trackers are not merged: the lifetime reference + // is held to the bind group itself. + + state + .buffer_memory_init_actions + .extend(bind_group.used_buffer_ranges.iter().filter_map(|action| { + action + .buffer + .initialization_status + .read() + .check_action(action) + })); + for action in bind_group.used_texture_ranges.iter() { + state + .info + .pending_discard_init_fixups + .extend(state.texture_memory_actions.register_init_action(action)); + } + + let pipeline_layout = state.binder.pipeline_layout.clone(); + let entries = state + .binder + .assign_group(index as usize, bind_group, &state.temp_offsets); + if !entries.is_empty() && pipeline_layout.is_some() { + let pipeline_layout = pipeline_layout.as_ref().unwrap().raw(); + for (i, e) in entries.iter().enumerate() { + if let Some(group) = e.group.as_ref() { + let raw_bg = group.try_raw(state.snatch_guard)?; + unsafe { + state.raw_encoder.set_bind_group( + pipeline_layout, + index + i as u32, + raw_bg, + &e.dynamic_offsets, + ); + } + } + } + } + Ok(()) +} + impl Global { pub fn render_pass_set_bind_group( &self, pass: &mut RenderPass, index: u32, bind_group_id: id::BindGroupId, - offsets: &[wgt::DynamicOffset], + offsets: &[DynamicOffset], ) -> Result<(), RenderPassError> { let scope = PassErrorScope::SetBindGroup(bind_group_id); let base = pass From 84a44ae0d74ead35f55c0d634a3b51648922fc6e Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 15:54:57 +0200 Subject: [PATCH 23/47] extract `set_pipeline` from `render_pass_end_impl` --- wgpu-core/src/command/render.rs | 241 ++++++++++++++++---------------- 1 file changed, 119 insertions(+), 122 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 69cf13f043..d788e2b7d5 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -3,6 +3,7 @@ use crate::command::{ validate_and_begin_occlusion_query, validate_and_begin_pipeline_statistics_query, }; use crate::init_tracker::BufferInitTrackerAction; +use crate::pipeline::RenderPipeline; use crate::resource::Resource; use crate::snatch::SnatchGuard; use crate::{ @@ -1532,129 +1533,8 @@ impl Global { .map_pass_err(scope)?; } ArcRenderCommand::SetPipeline(pipeline) => { - api_log!("RenderPass::set_pipeline {}", pipeline.error_ident()); - let scope = PassErrorScope::SetPipelineRender(pipeline.as_info().id()); - state.pipeline = Some(pipeline.as_info().id()); - - let pipeline = state.tracker.render_pipelines.insert_single(pipeline); - - pipeline - .same_device_as(cmd_buf.as_ref()) - .map_pass_err(scope)?; - - state - .info - .context - .check_compatible( - &pipeline.pass_context, - RenderPassCompatibilityCheckType::RenderPipeline, - ) - .map_err(RenderCommandError::IncompatiblePipelineTargets) - .map_pass_err(scope)?; - - state.pipeline_flags = pipeline.flags; - - if (pipeline.flags.contains(PipelineFlags::WRITES_DEPTH) - && state.info.is_depth_read_only) - || (pipeline.flags.contains(PipelineFlags::WRITES_STENCIL) - && state.info.is_stencil_read_only) - { - return Err(RenderCommandError::IncompatiblePipelineRods) - .map_pass_err(scope); - } - - state - .blend_constant - .require(pipeline.flags.contains(PipelineFlags::BLEND_CONSTANT)); - - unsafe { - state.raw_encoder.set_render_pipeline(pipeline.raw()); - } - - if pipeline.flags.contains(PipelineFlags::STENCIL_REFERENCE) { - unsafe { - state - .raw_encoder - .set_stencil_reference(state.stencil_reference); - } - } - - // Rebind resource - if state.binder.pipeline_layout.is_none() - || !state - .binder - .pipeline_layout - .as_ref() - .unwrap() - .is_equal(&pipeline.layout) - { - let (start_index, entries) = state.binder.change_pipeline_layout( - &pipeline.layout, - &pipeline.late_sized_buffer_groups, - ); - if !entries.is_empty() { - for (i, e) in entries.iter().enumerate() { - if let Some(group) = e.group.as_ref() { - let raw_bg = group - .try_raw(state.snatch_guard) - .map_pass_err(scope)?; - unsafe { - state.raw_encoder.set_bind_group( - pipeline.layout.raw(), - start_index as u32 + i as u32, - raw_bg, - &e.dynamic_offsets, - ); - } - } - } - } - - // Clear push constant ranges - let non_overlapping = super::bind::compute_nonoverlapping_ranges( - &pipeline.layout.push_constant_ranges, - ); - for range in non_overlapping { - let offset = range.range.start; - let size_bytes = range.range.end - offset; - super::push_constant_clear( - offset, - size_bytes, - |clear_offset, clear_data| unsafe { - state.raw_encoder.set_push_constants( - pipeline.layout.raw(), - range.stages, - clear_offset, - clear_data, - ); - }, - ); - } - } - - state.index.pipeline_format = pipeline.strip_index_format; - - let vertex_steps_len = pipeline.vertex_steps.len(); - state.vertex.buffers_required = vertex_steps_len as u32; - - // Initialize each `vertex.inputs[i].step` from - // `pipeline.vertex_steps[i]`. Enlarge `vertex.inputs` - // as necessary to accommodate all slots in the - // pipeline. If `vertex.inputs` is longer, fill the - // extra entries with default `VertexStep`s. - while state.vertex.inputs.len() < vertex_steps_len { - state.vertex.inputs.push(VertexBufferState::EMPTY); - } - - // This is worse as a `zip`, but it's close. - let mut steps = pipeline.vertex_steps.iter(); - for input in state.vertex.inputs.iter_mut() { - input.step = steps.next().cloned().unwrap_or_default(); - } - - // Update vertex buffer limits. - state.vertex.update_limits(); + set_pipeline(&mut state, &cmd_buf, pipeline).map_pass_err(scope)?; } ArcRenderCommand::SetIndexBuffer { buffer, @@ -2555,6 +2435,123 @@ fn set_bind_group( Ok(()) } +fn set_pipeline( + state: &mut State, + cmd_buf: &Arc>, + pipeline: Arc>, +) -> Result<(), RenderPassErrorInner> { + api_log!("RenderPass::set_pipeline {}", pipeline.error_ident()); + + state.pipeline = Some(pipeline.as_info().id()); + + let pipeline = state.tracker.render_pipelines.insert_single(pipeline); + + pipeline.same_device_as(cmd_buf.as_ref())?; + + state + .info + .context + .check_compatible( + &pipeline.pass_context, + RenderPassCompatibilityCheckType::RenderPipeline, + ) + .map_err(RenderCommandError::IncompatiblePipelineTargets)?; + + state.pipeline_flags = pipeline.flags; + + if (pipeline.flags.contains(PipelineFlags::WRITES_DEPTH) && state.info.is_depth_read_only) + || (pipeline.flags.contains(PipelineFlags::WRITES_STENCIL) + && state.info.is_stencil_read_only) + { + return Err(RenderCommandError::IncompatiblePipelineRods.into()); + } + + state + .blend_constant + .require(pipeline.flags.contains(PipelineFlags::BLEND_CONSTANT)); + + unsafe { + state.raw_encoder.set_render_pipeline(pipeline.raw()); + } + + if pipeline.flags.contains(PipelineFlags::STENCIL_REFERENCE) { + unsafe { + state + .raw_encoder + .set_stencil_reference(state.stencil_reference); + } + } + + // Rebind resource + if state.binder.pipeline_layout.is_none() + || !state + .binder + .pipeline_layout + .as_ref() + .unwrap() + .is_equal(&pipeline.layout) + { + let (start_index, entries) = state + .binder + .change_pipeline_layout(&pipeline.layout, &pipeline.late_sized_buffer_groups); + if !entries.is_empty() { + for (i, e) in entries.iter().enumerate() { + if let Some(group) = e.group.as_ref() { + let raw_bg = group.try_raw(state.snatch_guard)?; + unsafe { + state.raw_encoder.set_bind_group( + pipeline.layout.raw(), + start_index as u32 + i as u32, + raw_bg, + &e.dynamic_offsets, + ); + } + } + } + } + + // Clear push constant ranges + let non_overlapping = + super::bind::compute_nonoverlapping_ranges(&pipeline.layout.push_constant_ranges); + for range in non_overlapping { + let offset = range.range.start; + let size_bytes = range.range.end - offset; + super::push_constant_clear(offset, size_bytes, |clear_offset, clear_data| unsafe { + state.raw_encoder.set_push_constants( + pipeline.layout.raw(), + range.stages, + clear_offset, + clear_data, + ); + }); + } + } + + state.index.pipeline_format = pipeline.strip_index_format; + + let vertex_steps_len = pipeline.vertex_steps.len(); + state.vertex.buffers_required = vertex_steps_len as u32; + + // Initialize each `vertex.inputs[i].step` from + // `pipeline.vertex_steps[i]`. Enlarge `vertex.inputs` + // as necessary to accommodate all slots in the + // pipeline. If `vertex.inputs` is longer, fill the + // extra entries with default `VertexStep`s. + while state.vertex.inputs.len() < vertex_steps_len { + state.vertex.inputs.push(VertexBufferState::EMPTY); + } + + // This is worse as a `zip`, but it's close. + let mut steps = pipeline.vertex_steps.iter(); + for input in state.vertex.inputs.iter_mut() { + input.step = steps.next().cloned().unwrap_or_default(); + } + + // Update vertex buffer limits. + state.vertex.update_limits(); + Ok(()) +} + impl Global { pub fn render_pass_set_bind_group( &self, From 3fb6bd40dbcc43a0a638fd4c67504ce988e80a32 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 15:58:36 +0200 Subject: [PATCH 24/47] extract `set_index_buffer` from `render_pass_end_impl` --- wgpu-core/src/command/render.rs | 87 ++++++++++++++++++--------------- 1 file changed, 47 insertions(+), 40 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index d788e2b7d5..2bff56d5f3 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1542,48 +1542,9 @@ impl Global { offset, size, } => { - api_log!("RenderPass::set_index_buffer {}", buffer.error_ident()); - let scope = PassErrorScope::SetIndexBuffer(buffer.as_info().id()); - - state - .info - .usage_scope - .buffers - .merge_single(&buffer, hal::BufferUses::INDEX) - .map_pass_err(scope)?; - - buffer - .same_device_as(cmd_buf.as_ref()) - .map_pass_err(scope)?; - - buffer - .check_usage(BufferUsages::INDEX) + set_index_buffer(&mut state, &cmd_buf, buffer, index_format, offset, size) .map_pass_err(scope)?; - let buf_raw = buffer.try_raw(state.snatch_guard).map_pass_err(scope)?; - - let end = match size { - Some(s) => offset + s.get(), - None => buffer.size, - }; - state.index.update_buffer(offset..end, index_format); - - state.buffer_memory_init_actions.extend( - buffer.initialization_status.read().create_action( - &buffer, - offset..end, - MemoryInitKind::NeedsInitializedMemory, - ), - ); - - let bb = hal::BufferBinding { - buffer: buf_raw, - offset, - size, - }; - unsafe { - state.raw_encoder.set_index_buffer(bb, index_format); - } } ArcRenderCommand::SetVertexBuffer { slot, @@ -2552,6 +2513,52 @@ fn set_pipeline( Ok(()) } +fn set_index_buffer( + state: &mut State, + cmd_buf: &Arc>, + buffer: Arc>, + index_format: IndexFormat, + offset: u64, + size: Option, +) -> Result<(), RenderPassErrorInner> { + api_log!("RenderPass::set_index_buffer {}", buffer.error_ident()); + + state + .info + .usage_scope + .buffers + .merge_single(&buffer, hal::BufferUses::INDEX)?; + + buffer.same_device_as(cmd_buf.as_ref())?; + + buffer.check_usage(BufferUsages::INDEX)?; + let buf_raw = buffer.try_raw(state.snatch_guard)?; + + let end = match size { + Some(s) => offset + s.get(), + None => buffer.size, + }; + state.index.update_buffer(offset..end, index_format); + + state + .buffer_memory_init_actions + .extend(buffer.initialization_status.read().create_action( + &buffer, + offset..end, + MemoryInitKind::NeedsInitializedMemory, + )); + + let bb = hal::BufferBinding { + buffer: buf_raw, + offset, + size, + }; + unsafe { + state.raw_encoder.set_index_buffer(bb, index_format); + } + Ok(()) +} + impl Global { pub fn render_pass_set_bind_group( &self, From c120a0f3f4f8b01373a523d3a63d79c67da3f798 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 16:01:18 +0200 Subject: [PATCH 25/47] extract `set_vertex_buffer` from `render_pass_end_impl` --- wgpu-core/src/command/render.rs | 128 +++++++++++++++++--------------- 1 file changed, 67 insertions(+), 61 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 2bff56d5f3..67627f3e69 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1552,69 +1552,9 @@ impl Global { offset, size, } => { - api_log!( - "RenderPass::set_vertex_buffer {slot} {}", - buffer.error_ident() - ); - let scope = PassErrorScope::SetVertexBuffer(buffer.as_info().id()); - - state - .info - .usage_scope - .buffers - .merge_single(&buffer, hal::BufferUses::VERTEX) - .map_pass_err(scope)?; - - buffer - .same_device_as(cmd_buf.as_ref()) + set_vertex_buffer(&mut state, &cmd_buf, slot, buffer, offset, size) .map_pass_err(scope)?; - - let max_vertex_buffers = state.device.limits.max_vertex_buffers; - if slot >= max_vertex_buffers { - return Err(RenderCommandError::VertexBufferIndexOutOfRange { - index: slot, - max: max_vertex_buffers, - }) - .map_pass_err(scope); - } - - buffer - .check_usage(BufferUsages::VERTEX) - .map_pass_err(scope)?; - let buf_raw = buffer.try_raw(state.snatch_guard).map_pass_err(scope)?; - - let empty_slots = - (1 + slot as usize).saturating_sub(state.vertex.inputs.len()); - state - .vertex - .inputs - .extend(iter::repeat(VertexBufferState::EMPTY).take(empty_slots)); - let vertex_state = &mut state.vertex.inputs[slot as usize]; - //TODO: where are we checking that the offset is in bound? - vertex_state.total_size = match size { - Some(s) => s.get(), - None => buffer.size - offset, - }; - vertex_state.bound = true; - - state.buffer_memory_init_actions.extend( - buffer.initialization_status.read().create_action( - &buffer, - offset..(offset + vertex_state.total_size), - MemoryInitKind::NeedsInitializedMemory, - ), - ); - - let bb = hal::BufferBinding { - buffer: buf_raw, - offset, - size, - }; - unsafe { - state.raw_encoder.set_vertex_buffer(slot, bb); - } - state.vertex.update_limits(); } ArcRenderCommand::SetBlendConstant(ref color) => { api_log!("RenderPass::set_blend_constant"); @@ -2559,6 +2499,72 @@ fn set_index_buffer( Ok(()) } +fn set_vertex_buffer( + state: &mut State, + cmd_buf: &Arc>, + slot: u32, + buffer: Arc>, + offset: u64, + size: Option, +) -> Result<(), RenderPassErrorInner> { + api_log!( + "RenderPass::set_vertex_buffer {slot} {}", + buffer.error_ident() + ); + + state + .info + .usage_scope + .buffers + .merge_single(&buffer, hal::BufferUses::VERTEX)?; + + buffer.same_device_as(cmd_buf.as_ref())?; + + let max_vertex_buffers = state.device.limits.max_vertex_buffers; + if slot >= max_vertex_buffers { + return Err(RenderCommandError::VertexBufferIndexOutOfRange { + index: slot, + max: max_vertex_buffers, + } + .into()); + } + + buffer.check_usage(BufferUsages::VERTEX)?; + let buf_raw = buffer.try_raw(state.snatch_guard)?; + + let empty_slots = (1 + slot as usize).saturating_sub(state.vertex.inputs.len()); + state + .vertex + .inputs + .extend(iter::repeat(VertexBufferState::EMPTY).take(empty_slots)); + let vertex_state = &mut state.vertex.inputs[slot as usize]; + //TODO: where are we checking that the offset is in bound? + vertex_state.total_size = match size { + Some(s) => s.get(), + None => buffer.size - offset, + }; + vertex_state.bound = true; + + state + .buffer_memory_init_actions + .extend(buffer.initialization_status.read().create_action( + &buffer, + offset..(offset + vertex_state.total_size), + MemoryInitKind::NeedsInitializedMemory, + )); + + let bb = hal::BufferBinding { + buffer: buf_raw, + offset, + size, + }; + unsafe { + state.raw_encoder.set_vertex_buffer(slot, bb); + } + state.vertex.update_limits(); + Ok(()) +} + impl Global { pub fn render_pass_set_bind_group( &self, From 99276f5e40b1fccf065d518715fbc1a39c193d74 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 16:04:02 +0200 Subject: [PATCH 26/47] extract `set_blend_constant` from `render_pass_end_impl` --- wgpu-core/src/command/render.rs | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 67627f3e69..96c701e250 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1557,18 +1557,7 @@ impl Global { .map_pass_err(scope)?; } ArcRenderCommand::SetBlendConstant(ref color) => { - api_log!("RenderPass::set_blend_constant"); - - state.blend_constant = OptionalState::Set; - let array = [ - color.r as f32, - color.g as f32, - color.b as f32, - color.a as f32, - ]; - unsafe { - state.raw_encoder.set_blend_constants(&array); - } + set_blend_constant(&mut state, color); } ArcRenderCommand::SetStencilReference(value) => { api_log!("RenderPass::set_stencil_reference {value}"); @@ -2565,6 +2554,21 @@ fn set_vertex_buffer( Ok(()) } +fn set_blend_constant(state: &mut State, color: &Color) { + api_log!("RenderPass::set_blend_constant"); + + state.blend_constant = OptionalState::Set; + let array = [ + color.r as f32, + color.g as f32, + color.b as f32, + color.a as f32, + ]; + unsafe { + state.raw_encoder.set_blend_constants(&array); + } +} + impl Global { pub fn render_pass_set_bind_group( &self, From 51512ae1e4256af490153d43c0ca050059676df3 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 16:04:40 +0200 Subject: [PATCH 27/47] extract `set_stencil_reference` from `render_pass_end_impl` --- wgpu-core/src/command/render.rs | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 96c701e250..22adf15258 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1560,17 +1560,7 @@ impl Global { set_blend_constant(&mut state, color); } ArcRenderCommand::SetStencilReference(value) => { - api_log!("RenderPass::set_stencil_reference {value}"); - - state.stencil_reference = value; - if state - .pipeline_flags - .contains(PipelineFlags::STENCIL_REFERENCE) - { - unsafe { - state.raw_encoder.set_stencil_reference(value); - } - } + set_stencil_reference(&mut state, value); } ArcRenderCommand::SetViewport { ref rect, @@ -2569,6 +2559,20 @@ fn set_blend_constant(state: &mut State, color: &Color) { } } +fn set_stencil_reference(state: &mut State, value: u32) { + api_log!("RenderPass::set_stencil_reference {value}"); + + state.stencil_reference = value; + if state + .pipeline_flags + .contains(PipelineFlags::STENCIL_REFERENCE) + { + unsafe { + state.raw_encoder.set_stencil_reference(value); + } + } +} + impl Global { pub fn render_pass_set_bind_group( &self, From c0e93fdf610a5d3fed69e4fe44fbd37f62c9e93c Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 16:09:11 +0200 Subject: [PATCH 28/47] extract `set_viewport` from `render_pass_end_impl` --- wgpu-core/src/command/render.rs | 64 +++++++++++++++++---------------- 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 22adf15258..d96c1b4917 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1563,41 +1563,12 @@ impl Global { set_stencil_reference(&mut state, value); } ArcRenderCommand::SetViewport { - ref rect, + rect, depth_min, depth_max, } => { - api_log!("RenderPass::set_viewport {rect:?}"); - let scope = PassErrorScope::SetViewport; - if rect.x < 0.0 - || rect.y < 0.0 - || rect.w <= 0.0 - || rect.h <= 0.0 - || rect.x + rect.w > state.info.extent.width as f32 - || rect.y + rect.h > state.info.extent.height as f32 - { - return Err(RenderCommandError::InvalidViewportRect( - *rect, - state.info.extent, - )) - .map_pass_err(scope); - } - if !(0.0..=1.0).contains(&depth_min) || !(0.0..=1.0).contains(&depth_max) { - return Err(RenderCommandError::InvalidViewportDepth( - depth_min, depth_max, - )) - .map_pass_err(scope); - } - let r = hal::Rect { - x: rect.x, - y: rect.y, - w: rect.w, - h: rect.h, - }; - unsafe { - state.raw_encoder.set_viewport(&r, depth_min..depth_max); - } + set_viewport(&mut state, rect, depth_min, depth_max).map_pass_err(scope)?; } ArcRenderCommand::SetPushConstant { stages, @@ -2573,6 +2544,37 @@ fn set_stencil_reference(state: &mut State, value: u32) { } } +fn set_viewport( + state: &mut State, + rect: Rect, + depth_min: f32, + depth_max: f32, +) -> Result<(), RenderPassErrorInner> { + api_log!("RenderPass::set_viewport {rect:?}"); + if rect.x < 0.0 + || rect.y < 0.0 + || rect.w <= 0.0 + || rect.h <= 0.0 + || rect.x + rect.w > state.info.extent.width as f32 + || rect.y + rect.h > state.info.extent.height as f32 + { + return Err(RenderCommandError::InvalidViewportRect(rect, state.info.extent).into()); + } + if !(0.0..=1.0).contains(&depth_min) || !(0.0..=1.0).contains(&depth_max) { + return Err(RenderCommandError::InvalidViewportDepth(depth_min, depth_max).into()); + } + let r = hal::Rect { + x: rect.x, + y: rect.y, + w: rect.w, + h: rect.h, + }; + unsafe { + state.raw_encoder.set_viewport(&r, depth_min..depth_max); + } + Ok(()) +} + impl Global { pub fn render_pass_set_bind_group( &self, From 3e05cec217651b1e91a284a49cb1de69c273185f Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 16:13:22 +0200 Subject: [PATCH 29/47] extract `set_push_constant` from `render_pass_end_impl` --- wgpu-core/src/command/render.rs | 81 +++++++++++++++++++-------------- 1 file changed, 46 insertions(+), 35 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index d96c1b4917..9bccceeeee 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -40,8 +40,8 @@ use arrayvec::ArrayVec; use hal::CommandEncoder as _; use thiserror::Error; use wgt::{ - BufferAddress, BufferSize, BufferUsages, Color, DynamicOffset, IndexFormat, TextureUsages, - TextureViewDimension, VertexStepMode, + BufferAddress, BufferSize, BufferUsages, Color, DynamicOffset, IndexFormat, ShaderStages, + TextureUsages, TextureViewDimension, VertexStepMode, }; #[cfg(feature = "serde")] @@ -1576,39 +1576,16 @@ impl Global { size_bytes, values_offset, } => { - api_log!("RenderPass::set_push_constants"); - let scope = PassErrorScope::SetPushConstant; - let values_offset = values_offset - .ok_or(RenderPassErrorInner::InvalidValuesOffset) - .map_pass_err(scope)?; - - let end_offset_bytes = offset + size_bytes; - let values_end_offset = - (values_offset + size_bytes / wgt::PUSH_CONSTANT_ALIGNMENT) as usize; - let data_slice = - &base.push_constant_data[(values_offset as usize)..values_end_offset]; - - let pipeline_layout = state - .binder - .pipeline_layout - .as_ref() - .ok_or(DrawError::MissingPipeline) - .map_pass_err(scope)?; - - pipeline_layout - .validate_push_constant_ranges(stages, offset, end_offset_bytes) - .map_err(RenderCommandError::from) - .map_pass_err(scope)?; - - unsafe { - state.raw_encoder.set_push_constants( - pipeline_layout.raw(), - stages, - offset, - data_slice, - ) - } + set_push_constant( + &mut state, + &base.push_constant_data, + stages, + offset, + size_bytes, + values_offset, + ) + .map_pass_err(scope)?; } ArcRenderCommand::SetScissor(ref rect) => { api_log!("RenderPass::set_scissor_rect {rect:?}"); @@ -2575,6 +2552,40 @@ fn set_viewport( Ok(()) } +fn set_push_constant( + state: &mut State, + push_constant_data: &[u32], + stages: ShaderStages, + offset: u32, + size_bytes: u32, + values_offset: Option, +) -> Result<(), RenderPassErrorInner> { + api_log!("RenderPass::set_push_constants"); + + let values_offset = values_offset.ok_or(RenderPassErrorInner::InvalidValuesOffset)?; + + let end_offset_bytes = offset + size_bytes; + let values_end_offset = (values_offset + size_bytes / wgt::PUSH_CONSTANT_ALIGNMENT) as usize; + let data_slice = &push_constant_data[(values_offset as usize)..values_end_offset]; + + let pipeline_layout = state + .binder + .pipeline_layout + .as_ref() + .ok_or(DrawError::MissingPipeline)?; + + pipeline_layout + .validate_push_constant_ranges(stages, offset, end_offset_bytes) + .map_err(RenderCommandError::from)?; + + unsafe { + state + .raw_encoder + .set_push_constants(pipeline_layout.raw(), stages, offset, data_slice) + } + Ok(()) +} + impl Global { pub fn render_pass_set_bind_group( &self, @@ -2740,7 +2751,7 @@ impl Global { pub fn render_pass_set_push_constants( &self, pass: &mut RenderPass, - stages: wgt::ShaderStages, + stages: ShaderStages, offset: u32, data: &[u8], ) -> Result<(), RenderPassError> { From 3420c4517d24229825518a7207b5bd273355942e Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 16:16:22 +0200 Subject: [PATCH 30/47] extract `set_scissor` from `render_pass_end_impl` --- wgpu-core/src/command/render.rs | 44 +++++++++++++++++---------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 9bccceeeee..fe5328df51 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1587,28 +1587,9 @@ impl Global { ) .map_pass_err(scope)?; } - ArcRenderCommand::SetScissor(ref rect) => { - api_log!("RenderPass::set_scissor_rect {rect:?}"); - + ArcRenderCommand::SetScissor(rect) => { let scope = PassErrorScope::SetScissorRect; - if rect.x + rect.w > state.info.extent.width - || rect.y + rect.h > state.info.extent.height - { - return Err(RenderCommandError::InvalidScissorRect( - *rect, - state.info.extent, - )) - .map_pass_err(scope); - } - let r = hal::Rect { - x: rect.x, - y: rect.y, - w: rect.w, - h: rect.h, - }; - unsafe { - state.raw_encoder.set_scissor_rect(&r); - } + set_scissor(&mut state, rect).map_pass_err(scope)?; } ArcRenderCommand::Draw { vertex_count, @@ -2586,6 +2567,27 @@ fn set_push_constant( Ok(()) } +fn set_scissor( + state: &mut State, + rect: Rect, +) -> Result<(), RenderPassErrorInner> { + api_log!("RenderPass::set_scissor_rect {rect:?}"); + + if rect.x + rect.w > state.info.extent.width || rect.y + rect.h > state.info.extent.height { + return Err(RenderCommandError::InvalidScissorRect(rect, state.info.extent).into()); + } + let r = hal::Rect { + x: rect.x, + y: rect.y, + w: rect.w, + h: rect.h, + }; + unsafe { + state.raw_encoder.set_scissor_rect(&r); + } + Ok(()) +} + impl Global { pub fn render_pass_set_bind_group( &self, From 8081125eeea965b63a3ef3df49f5aca27e5b0a57 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 16:19:06 +0200 Subject: [PATCH 31/47] extract `draw` from `render_pass_end_impl` --- wgpu-core/src/command/render.rs | 88 ++++++++++++++++++--------------- 1 file changed, 49 insertions(+), 39 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index fe5328df51..6bf08d5bdd 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1597,49 +1597,19 @@ impl Global { first_vertex, first_instance, } => { - api_log!( - "RenderPass::draw {vertex_count} {instance_count} {first_vertex} {first_instance}" - ); - - let indexed = false; let scope = PassErrorScope::Draw { kind: DrawKind::Draw, - indexed, + indexed: false, pipeline: state.pipeline, }; - state.is_ready(indexed).map_pass_err(scope)?; - - let last_vertex = first_vertex as u64 + vertex_count as u64; - let vertex_limit = state.vertex.vertex_limit; - if last_vertex > vertex_limit { - return Err(DrawError::VertexBeyondLimit { - last_vertex, - vertex_limit, - slot: state.vertex.vertex_limit_slot, - }) - .map_pass_err(scope); - } - let last_instance = first_instance as u64 + instance_count as u64; - let instance_limit = state.vertex.instance_limit; - if last_instance > instance_limit { - return Err(DrawError::InstanceBeyondLimit { - last_instance, - instance_limit, - slot: state.vertex.instance_limit_slot, - }) - .map_pass_err(scope); - } - - unsafe { - if instance_count > 0 && vertex_count > 0 { - state.raw_encoder.draw( - first_vertex, - vertex_count, - first_instance, - instance_count, - ); - } - } + draw( + &mut state, + vertex_count, + instance_count, + first_vertex, + first_instance, + ) + .map_pass_err(scope)?; } ArcRenderCommand::DrawIndexed { index_count, @@ -2588,6 +2558,46 @@ fn set_scissor( Ok(()) } +fn draw( + state: &mut State, + vertex_count: u32, + instance_count: u32, + first_vertex: u32, + first_instance: u32, +) -> Result<(), DrawError> { + api_log!("RenderPass::draw {vertex_count} {instance_count} {first_vertex} {first_instance}"); + + state.is_ready(false)?; + + let last_vertex = first_vertex as u64 + vertex_count as u64; + let vertex_limit = state.vertex.vertex_limit; + if last_vertex > vertex_limit { + return Err(DrawError::VertexBeyondLimit { + last_vertex, + vertex_limit, + slot: state.vertex.vertex_limit_slot, + }); + } + let last_instance = first_instance as u64 + instance_count as u64; + let instance_limit = state.vertex.instance_limit; + if last_instance > instance_limit { + return Err(DrawError::InstanceBeyondLimit { + last_instance, + instance_limit, + slot: state.vertex.instance_limit_slot, + }); + } + + unsafe { + if instance_count > 0 && vertex_count > 0 { + state + .raw_encoder + .draw(first_vertex, vertex_count, first_instance, instance_count); + } + } + Ok(()) +} + impl Global { pub fn render_pass_set_bind_group( &self, From 66790fedd67872c1fb2842fedbfcb61bc59c49b2 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 16:21:47 +0200 Subject: [PATCH 32/47] extract `draw_indexed` from `render_pass_end_impl` --- wgpu-core/src/command/render.rs | 91 +++++++++++++++++++-------------- 1 file changed, 54 insertions(+), 37 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 6bf08d5bdd..bfd5598cae 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1618,47 +1618,20 @@ impl Global { base_vertex, first_instance, } => { - api_log!("RenderPass::draw_indexed {index_count} {instance_count} {first_index} {base_vertex} {first_instance}"); - - let indexed = true; let scope = PassErrorScope::Draw { kind: DrawKind::Draw, - indexed, + indexed: true, pipeline: state.pipeline, }; - state.is_ready(indexed).map_pass_err(scope)?; - - let last_index = first_index as u64 + index_count as u64; - let index_limit = state.index.limit; - if last_index > index_limit { - return Err(DrawError::IndexBeyondLimit { - last_index, - index_limit, - }) - .map_pass_err(scope); - } - let last_instance = first_instance as u64 + instance_count as u64; - let instance_limit = state.vertex.instance_limit; - if last_instance > instance_limit { - return Err(DrawError::InstanceBeyondLimit { - last_instance, - instance_limit, - slot: state.vertex.instance_limit_slot, - }) - .map_pass_err(scope); - } - - unsafe { - if instance_count > 0 && index_count > 0 { - state.raw_encoder.draw_indexed( - first_index, - index_count, - base_vertex, - first_instance, - instance_count, - ); - } - } + draw_indexed( + &mut state, + index_count, + instance_count, + first_index, + base_vertex, + first_instance, + ) + .map_pass_err(scope)?; } ArcRenderCommand::MultiDrawIndirect { buffer: indirect_buffer, @@ -2598,6 +2571,50 @@ fn draw( Ok(()) } +fn draw_indexed( + state: &mut State, + index_count: u32, + instance_count: u32, + first_index: u32, + base_vertex: i32, + first_instance: u32, +) -> Result<(), DrawError> { + api_log!("RenderPass::draw_indexed {index_count} {instance_count} {first_index} {base_vertex} {first_instance}"); + + state.is_ready(true)?; + + let last_index = first_index as u64 + index_count as u64; + let index_limit = state.index.limit; + if last_index > index_limit { + return Err(DrawError::IndexBeyondLimit { + last_index, + index_limit, + }); + } + let last_instance = first_instance as u64 + instance_count as u64; + let instance_limit = state.vertex.instance_limit; + if last_instance > instance_limit { + return Err(DrawError::InstanceBeyondLimit { + last_instance, + instance_limit, + slot: state.vertex.instance_limit_slot, + }); + } + + unsafe { + if instance_count > 0 && index_count > 0 { + state.raw_encoder.draw_indexed( + first_index, + index_count, + base_vertex, + first_instance, + instance_count, + ); + } + } + Ok(()) +} + impl Global { pub fn render_pass_set_bind_group( &self, From 97918b59adf70c9e9b48352c474a8fd7ddbaec33 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 16:26:08 +0200 Subject: [PATCH 33/47] extract `multi_draw_indirect` from `render_pass_end_impl` --- wgpu-core/src/command/render.rs | 146 ++++++++++++++++---------------- 1 file changed, 74 insertions(+), 72 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index bfd5598cae..ced425b7a0 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1634,16 +1634,11 @@ impl Global { .map_pass_err(scope)?; } ArcRenderCommand::MultiDrawIndirect { - buffer: indirect_buffer, + buffer, offset, count, indexed, } => { - api_log!( - "RenderPass::draw_indirect (indexed:{indexed}) {} {offset} {count:?}", - indirect_buffer.error_ident() - ); - let scope = PassErrorScope::Draw { kind: if count.is_some() { DrawKind::MultiDrawIndirect @@ -1653,73 +1648,8 @@ impl Global { indexed, pipeline: state.pipeline, }; - state.is_ready(indexed).map_pass_err(scope)?; - - let stride = match indexed { - false => mem::size_of::(), - true => mem::size_of::(), - }; - - if count.is_some() { - state - .device - .require_features(wgt::Features::MULTI_DRAW_INDIRECT) - .map_pass_err(scope)?; - } - state - .device - .require_downlevel_flags(wgt::DownlevelFlags::INDIRECT_EXECUTION) - .map_pass_err(scope)?; - - state - .info - .usage_scope - .buffers - .merge_single(&indirect_buffer, hal::BufferUses::INDIRECT) + multi_draw_indirect(&mut state, buffer, offset, count, indexed) .map_pass_err(scope)?; - - indirect_buffer - .check_usage(BufferUsages::INDIRECT) - .map_pass_err(scope)?; - let indirect_raw = indirect_buffer - .try_raw(state.snatch_guard) - .map_pass_err(scope)?; - - let actual_count = count.map_or(1, |c| c.get()); - - let end_offset = offset + stride as u64 * actual_count as u64; - if end_offset > indirect_buffer.size { - return Err(RenderPassErrorInner::IndirectBufferOverrun { - count, - offset, - end_offset, - buffer_size: indirect_buffer.size, - }) - .map_pass_err(scope); - } - - state.buffer_memory_init_actions.extend( - indirect_buffer.initialization_status.read().create_action( - &indirect_buffer, - offset..end_offset, - MemoryInitKind::NeedsInitializedMemory, - ), - ); - - match indexed { - false => unsafe { - state - .raw_encoder - .draw_indirect(indirect_raw, offset, actual_count); - }, - true => unsafe { - state.raw_encoder.draw_indexed_indirect( - indirect_raw, - offset, - actual_count, - ); - }, - } } ArcRenderCommand::MultiDrawIndirectCount { buffer: indirect_buffer, @@ -2615,6 +2545,78 @@ fn draw_indexed( Ok(()) } +fn multi_draw_indirect( + state: &mut State, + indirect_buffer: Arc>, + offset: u64, + count: Option, + indexed: bool, +) -> Result<(), RenderPassErrorInner> { + api_log!( + "RenderPass::draw_indirect (indexed:{indexed}) {} {offset} {count:?}", + indirect_buffer.error_ident() + ); + + state.is_ready(indexed)?; + + let stride = match indexed { + false => mem::size_of::(), + true => mem::size_of::(), + }; + + if count.is_some() { + state + .device + .require_features(wgt::Features::MULTI_DRAW_INDIRECT)?; + } + state + .device + .require_downlevel_flags(wgt::DownlevelFlags::INDIRECT_EXECUTION)?; + + state + .info + .usage_scope + .buffers + .merge_single(&indirect_buffer, hal::BufferUses::INDIRECT)?; + + indirect_buffer.check_usage(BufferUsages::INDIRECT)?; + let indirect_raw = indirect_buffer.try_raw(state.snatch_guard)?; + + let actual_count = count.map_or(1, |c| c.get()); + + let end_offset = offset + stride as u64 * actual_count as u64; + if end_offset > indirect_buffer.size { + return Err(RenderPassErrorInner::IndirectBufferOverrun { + count, + offset, + end_offset, + buffer_size: indirect_buffer.size, + }); + } + + state.buffer_memory_init_actions.extend( + indirect_buffer.initialization_status.read().create_action( + &indirect_buffer, + offset..end_offset, + MemoryInitKind::NeedsInitializedMemory, + ), + ); + + match indexed { + false => unsafe { + state + .raw_encoder + .draw_indirect(indirect_raw, offset, actual_count); + }, + true => unsafe { + state + .raw_encoder + .draw_indexed_indirect(indirect_raw, offset, actual_count); + }, + } + Ok(()) +} + impl Global { pub fn render_pass_set_bind_group( &self, From 7de1447fed0c89f5057b888732c1446e05c74b91 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 16:29:49 +0200 Subject: [PATCH 34/47] extract `multi_draw_indirect_count` from `render_pass_end_impl` --- wgpu-core/src/command/render.rs | 222 +++++++++++++++++--------------- 1 file changed, 115 insertions(+), 107 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index ced425b7a0..f82b5275ac 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1652,124 +1652,28 @@ impl Global { .map_pass_err(scope)?; } ArcRenderCommand::MultiDrawIndirectCount { - buffer: indirect_buffer, + buffer, offset, count_buffer, count_buffer_offset, max_count, indexed, } => { - api_log!( - "RenderPass::multi_draw_indirect_count (indexed:{indexed}) {} {offset} {} {count_buffer_offset:?} {max_count:?}", - indirect_buffer.error_ident(), - count_buffer.error_ident() - ); - let scope = PassErrorScope::Draw { kind: DrawKind::MultiDrawIndirectCount, indexed, pipeline: state.pipeline, }; - state.is_ready(indexed).map_pass_err(scope)?; - - let stride = match indexed { - false => mem::size_of::(), - true => mem::size_of::(), - } as u64; - - state - .device - .require_features(wgt::Features::MULTI_DRAW_INDIRECT_COUNT) - .map_pass_err(scope)?; - state - .device - .require_downlevel_flags(wgt::DownlevelFlags::INDIRECT_EXECUTION) - .map_pass_err(scope)?; - - state - .info - .usage_scope - .buffers - .merge_single(&indirect_buffer, hal::BufferUses::INDIRECT) - .map_pass_err(scope)?; - - indirect_buffer - .check_usage(BufferUsages::INDIRECT) - .map_pass_err(scope)?; - let indirect_raw = indirect_buffer - .try_raw(state.snatch_guard) - .map_pass_err(scope)?; - - state - .info - .usage_scope - .buffers - .merge_single(&count_buffer, hal::BufferUses::INDIRECT) - .map_pass_err(scope)?; - - count_buffer - .check_usage(BufferUsages::INDIRECT) - .map_pass_err(scope)?; - let count_raw = count_buffer - .try_raw(state.snatch_guard) - .map_pass_err(scope)?; - - let end_offset = offset + stride * max_count as u64; - if end_offset > indirect_buffer.size { - return Err(RenderPassErrorInner::IndirectBufferOverrun { - count: None, - offset, - end_offset, - buffer_size: indirect_buffer.size, - }) - .map_pass_err(scope); - } - state.buffer_memory_init_actions.extend( - indirect_buffer.initialization_status.read().create_action( - &indirect_buffer, - offset..end_offset, - MemoryInitKind::NeedsInitializedMemory, - ), - ); - - let begin_count_offset = count_buffer_offset; - let end_count_offset = count_buffer_offset + 4; - if end_count_offset > count_buffer.size { - return Err(RenderPassErrorInner::IndirectCountBufferOverrun { - begin_count_offset, - end_count_offset, - count_buffer_size: count_buffer.size, - }) - .map_pass_err(scope); - } - state.buffer_memory_init_actions.extend( - count_buffer.initialization_status.read().create_action( - &count_buffer, - count_buffer_offset..end_count_offset, - MemoryInitKind::NeedsInitializedMemory, - ), - ); - - match indexed { - false => unsafe { - state.raw_encoder.draw_indirect_count( - indirect_raw, - offset, - count_raw, - count_buffer_offset, - max_count, - ); - }, - true => unsafe { - state.raw_encoder.draw_indexed_indirect_count( - indirect_raw, - offset, - count_raw, - count_buffer_offset, - max_count, - ); - }, - } + multi_draw_indirect_count( + &mut state, + buffer, + offset, + count_buffer, + count_buffer_offset, + max_count, + indexed, + ) + .map_pass_err(scope)?; } ArcRenderCommand::PushDebugGroup { color: _, len } => { state.debug_scope_depth += 1; @@ -2617,6 +2521,110 @@ fn multi_draw_indirect( Ok(()) } +fn multi_draw_indirect_count( + state: &mut State, + indirect_buffer: Arc>, + offset: u64, + count_buffer: Arc>, + count_buffer_offset: u64, + max_count: u32, + indexed: bool, +) -> Result<(), RenderPassErrorInner> { + api_log!( + "RenderPass::multi_draw_indirect_count (indexed:{indexed}) {} {offset} {} {count_buffer_offset:?} {max_count:?}", + indirect_buffer.error_ident(), + count_buffer.error_ident() + ); + + state.is_ready(indexed)?; + + let stride = match indexed { + false => mem::size_of::(), + true => mem::size_of::(), + } as u64; + + state + .device + .require_features(wgt::Features::MULTI_DRAW_INDIRECT_COUNT)?; + state + .device + .require_downlevel_flags(wgt::DownlevelFlags::INDIRECT_EXECUTION)?; + + state + .info + .usage_scope + .buffers + .merge_single(&indirect_buffer, hal::BufferUses::INDIRECT)?; + + indirect_buffer.check_usage(BufferUsages::INDIRECT)?; + let indirect_raw = indirect_buffer.try_raw(state.snatch_guard)?; + + state + .info + .usage_scope + .buffers + .merge_single(&count_buffer, hal::BufferUses::INDIRECT)?; + + count_buffer.check_usage(BufferUsages::INDIRECT)?; + let count_raw = count_buffer.try_raw(state.snatch_guard)?; + + let end_offset = offset + stride * max_count as u64; + if end_offset > indirect_buffer.size { + return Err(RenderPassErrorInner::IndirectBufferOverrun { + count: None, + offset, + end_offset, + buffer_size: indirect_buffer.size, + }); + } + state.buffer_memory_init_actions.extend( + indirect_buffer.initialization_status.read().create_action( + &indirect_buffer, + offset..end_offset, + MemoryInitKind::NeedsInitializedMemory, + ), + ); + + let begin_count_offset = count_buffer_offset; + let end_count_offset = count_buffer_offset + 4; + if end_count_offset > count_buffer.size { + return Err(RenderPassErrorInner::IndirectCountBufferOverrun { + begin_count_offset, + end_count_offset, + count_buffer_size: count_buffer.size, + }); + } + state.buffer_memory_init_actions.extend( + count_buffer.initialization_status.read().create_action( + &count_buffer, + count_buffer_offset..end_count_offset, + MemoryInitKind::NeedsInitializedMemory, + ), + ); + + match indexed { + false => unsafe { + state.raw_encoder.draw_indirect_count( + indirect_raw, + offset, + count_raw, + count_buffer_offset, + max_count, + ); + }, + true => unsafe { + state.raw_encoder.draw_indexed_indirect_count( + indirect_raw, + offset, + count_raw, + count_buffer_offset, + max_count, + ); + }, + } + Ok(()) +} + impl Global { pub fn render_pass_set_bind_group( &self, From 6bd3fc1ccf1b317dcba210477dfb2ad62fd9ed65 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 17:19:07 +0200 Subject: [PATCH 35/47] extract `push_debug_group` from `render_pass_end_impl` --- wgpu-core/src/command/render.rs | 36 +++++++++++++++++---------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index f82b5275ac..391fdba490 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1676,23 +1676,7 @@ impl Global { .map_pass_err(scope)?; } ArcRenderCommand::PushDebugGroup { color: _, len } => { - state.debug_scope_depth += 1; - if !state - .device - .instance_flags - .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS) - { - let label = str::from_utf8( - &base.string_data[state.string_offset..state.string_offset + len], - ) - .unwrap(); - - api_log!("RenderPass::push_debug_group {label:?}"); - unsafe { - state.raw_encoder.begin_debug_marker(label); - } - } - state.string_offset += len; + push_debug_group(&mut state, &base.string_data, len); } ArcRenderCommand::PopDebugGroup => { api_log!("RenderPass::pop_debug_group"); @@ -2625,6 +2609,24 @@ fn multi_draw_indirect_count( Ok(()) } +fn push_debug_group(state: &mut State, string_data: &[u8], len: usize) { + state.debug_scope_depth += 1; + if !state + .device + .instance_flags + .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS) + { + let label = + str::from_utf8(&string_data[state.string_offset..state.string_offset + len]).unwrap(); + + api_log!("RenderPass::push_debug_group {label:?}"); + unsafe { + state.raw_encoder.begin_debug_marker(label); + } + } + state.string_offset += len; +} + impl Global { pub fn render_pass_set_bind_group( &self, From 8ebb72f0721dca86d12ec3556119c0d6d5d978f2 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 17:20:51 +0200 Subject: [PATCH 36/47] extract `pop_debug_group` from `render_pass_end_impl` --- wgpu-core/src/command/render.rs | 36 ++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 391fdba490..232738e91c 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1679,23 +1679,8 @@ impl Global { push_debug_group(&mut state, &base.string_data, len); } ArcRenderCommand::PopDebugGroup => { - api_log!("RenderPass::pop_debug_group"); - let scope = PassErrorScope::PopDebugGroup; - if state.debug_scope_depth == 0 { - return Err(RenderPassErrorInner::InvalidPopDebugGroup) - .map_pass_err(scope); - } - state.debug_scope_depth -= 1; - if !state - .device - .instance_flags - .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS) - { - unsafe { - state.raw_encoder.end_debug_marker(); - } - } + pop_debug_group(&mut state).map_pass_err(scope)?; } ArcRenderCommand::InsertDebugMarker { color: _, len } => { if !state @@ -2627,6 +2612,25 @@ fn push_debug_group(state: &mut State, string_data: &[u8], len: us state.string_offset += len; } +fn pop_debug_group(state: &mut State) -> Result<(), RenderPassErrorInner> { + api_log!("RenderPass::pop_debug_group"); + + if state.debug_scope_depth == 0 { + return Err(RenderPassErrorInner::InvalidPopDebugGroup); + } + state.debug_scope_depth -= 1; + if !state + .device + .instance_flags + .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS) + { + unsafe { + state.raw_encoder.end_debug_marker(); + } + } + Ok(()) +} + impl Global { pub fn render_pass_set_bind_group( &self, From df45624435d6756791963011a04f160044bde7ca Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 17:24:23 +0200 Subject: [PATCH 37/47] extract `insert_debug_marker` from `render_pass_end_impl` --- wgpu-core/src/command/render.rs | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 232738e91c..aea5896261 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1683,21 +1683,7 @@ impl Global { pop_debug_group(&mut state).map_pass_err(scope)?; } ArcRenderCommand::InsertDebugMarker { color: _, len } => { - if !state - .device - .instance_flags - .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS) - { - let label = str::from_utf8( - &base.string_data[state.string_offset..state.string_offset + len], - ) - .unwrap(); - api_log!("RenderPass::insert_debug_marker {label:?}"); - unsafe { - state.raw_encoder.insert_debug_marker(label); - } - } - state.string_offset += len; + insert_debug_marker(&mut state, &base.string_data, len); } ArcRenderCommand::WriteTimestamp { query_set, @@ -2631,6 +2617,22 @@ fn pop_debug_group(state: &mut State) -> Result<(), RenderPassErro Ok(()) } +fn insert_debug_marker(state: &mut State, string_data: &[u8], len: usize) { + if !state + .device + .instance_flags + .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS) + { + let label = + str::from_utf8(&string_data[state.string_offset..state.string_offset + len]).unwrap(); + api_log!("RenderPass::insert_debug_marker {label:?}"); + unsafe { + state.raw_encoder.insert_debug_marker(label); + } + } + state.string_offset += len; +} + impl Global { pub fn render_pass_set_bind_group( &self, From e28d934b7c0f774692a60b5e2730602d1dd67f84 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 25 Jun 2024 17:38:07 +0200 Subject: [PATCH 38/47] extract `write_timestamp` from `render_pass_end_impl` --- wgpu-core/src/command/render.rs | 51 +++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index aea5896261..90bf57d5b2 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1689,26 +1689,14 @@ impl Global { query_set, query_index, } => { - api_log!( - "RenderPass::write_timestamps {query_index} {}", - query_set.error_ident() - ); let scope = PassErrorScope::WriteTimestamp; - - state - .device - .require_features(wgt::Features::TIMESTAMP_QUERY_INSIDE_PASSES) - .map_pass_err(scope)?; - - let query_set = state.tracker.query_sets.insert_single(query_set); - - query_set - .validate_and_write_timestamp( - state.raw_encoder, - query_index, - Some(&mut cmd_buf_data.pending_query_resets), - ) - .map_pass_err(scope)?; + write_timestamp( + &mut state, + &mut cmd_buf_data.pending_query_resets, + query_set, + query_index, + ) + .map_pass_err(scope)?; } ArcRenderCommand::BeginOcclusionQuery { query_index } => { api_log!("RenderPass::begin_occlusion_query {query_index}"); @@ -2633,6 +2621,31 @@ fn insert_debug_marker(state: &mut State, string_data: &[u8], len: state.string_offset += len; } +fn write_timestamp( + state: &mut State, + pending_query_resets: &mut QueryResetMap, + query_set: Arc>, + query_index: u32, +) -> Result<(), RenderPassErrorInner> { + api_log!( + "RenderPass::write_timestamps {query_index} {}", + query_set.error_ident() + ); + + state + .device + .require_features(wgt::Features::TIMESTAMP_QUERY_INSIDE_PASSES)?; + + let query_set = state.tracker.query_sets.insert_single(query_set); + + query_set.validate_and_write_timestamp( + state.raw_encoder, + query_index, + Some(pending_query_resets), + )?; + Ok(()) +} + impl Global { pub fn render_pass_set_bind_group( &self, From bcfb22be6d4f87755c9c478e5f8063b2df4a70f3 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Wed, 26 Jun 2024 10:31:39 +0200 Subject: [PATCH 39/47] resolve occlusion query set prior to `render_pass_end_impl` --- wgpu-core/src/command/query.rs | 3 ++ wgpu-core/src/command/render.rs | 49 ++++++++++++++++++--------------- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/wgpu-core/src/command/query.rs b/wgpu-core/src/command/query.rs index 1da5badb71..4595b70d32 100644 --- a/wgpu-core/src/command/query.rs +++ b/wgpu-core/src/command/query.rs @@ -230,6 +230,7 @@ impl QuerySet { pub(super) fn validate_and_begin_occlusion_query( query_set: Arc>, raw_encoder: &mut A::CommandEncoder, + tracker: &mut StatelessTracker>, query_index: u32, reset_state: Option<&mut QueryResetMap>, active_query: &mut Option<(Arc>, u32)>, @@ -237,6 +238,8 @@ pub(super) fn validate_and_begin_occlusion_query( let needs_reset = reset_state.is_none(); query_set.validate_query(SimplifiedQueryType::Occlusion, query_index, reset_state)?; + tracker.add_single(&query_set); + if let Some((_old, old_idx)) = active_query.take() { return Err(QueryUseError::AlreadyStarted { active_query_index: old_idx, diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 90bf57d5b2..4901083cbf 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -831,7 +831,7 @@ impl<'a, 'd, A: HalApi> RenderPassInfo<'a, 'd, A> { color_attachments: &[Option], depth_stencil_attachment: Option<&RenderPassDepthStencilAttachment>, timestamp_writes: Option<&RenderPassTimestampWrites>, - occlusion_query_set: Option, + occlusion_query_set: Option>>, encoder: &mut CommandEncoder, trackers: &mut Tracker, texture_memory_actions: &mut CommandBufferTextureMemoryActions, @@ -1226,13 +1226,8 @@ impl<'a, 'd, A: HalApi> RenderPassInfo<'a, 'd, A> { None }; - let occlusion_query_set = if let Some(occlusion_query_set) = occlusion_query_set { - let query_set = query_set_guard - .get(occlusion_query_set) - .map_err(|_| RenderPassErrorInner::InvalidQuerySet(occlusion_query_set))?; - - trackers.query_sets.add_single(query_set); - + let occlusion_query_set = if let Some(query_set) = occlusion_query_set { + let query_set = trackers.query_sets.insert_single(query_set); Some(query_set.raw.as_ref().unwrap()) } else { None @@ -1368,7 +1363,20 @@ impl Global { timestamp_writes: Option<&RenderPassTimestampWrites>, occlusion_query_set_id: Option, ) -> Result<(), RenderPassError> { - let commands = RenderCommand::resolve_render_command_ids(A::hub(self), &base.commands)?; + let pass_scope = PassErrorScope::PassEncoder(encoder_id); + + let hub = A::hub(self); + + let commands = RenderCommand::resolve_render_command_ids(hub, &base.commands)?; + + let occlusion_query_set = occlusion_query_set_id + .map(|id| { + hub.query_sets + .get(id) + .map_err(|_| RenderPassErrorInner::InvalidQuerySet(id)) + }) + .transpose() + .map_pass_err(pass_scope)?; self.render_pass_end_impl::( encoder_id, @@ -1382,7 +1390,7 @@ impl Global { color_attachments, depth_stencil_attachment, timestamp_writes, - occlusion_query_set_id, + occlusion_query_set, ) } @@ -1394,7 +1402,7 @@ impl Global { color_attachments: &[Option], depth_stencil_attachment: Option<&RenderPassDepthStencilAttachment>, timestamp_writes: Option<&RenderPassTimestampWrites>, - occlusion_query_set_id: Option, + occlusion_query_set: Option>>, ) -> Result<(), RenderPassError> { profiling::scope!( "CommandEncoder::run_render_pass {}", @@ -1429,7 +1437,9 @@ impl Global { target_colors: color_attachments.to_vec(), target_depth_stencil: depth_stencil_attachment.cloned(), timestamp_writes: timestamp_writes.cloned(), - occlusion_query_set_id, + occlusion_query_set_id: occlusion_query_set + .as_ref() + .map(|query_set| query_set.as_info().id()), }); } @@ -1464,7 +1474,7 @@ impl Global { color_attachments, depth_stencil_attachment, timestamp_writes, - occlusion_query_set_id, + occlusion_query_set.clone(), encoder, tracker, texture_memory_actions, @@ -1702,20 +1712,15 @@ impl Global { api_log!("RenderPass::begin_occlusion_query {query_index}"); let scope = PassErrorScope::BeginOcclusionQuery; - let query_set_id = occlusion_query_set_id + let query_set = occlusion_query_set + .clone() .ok_or(RenderPassErrorInner::MissingOcclusionQuerySet) .map_pass_err(scope)?; - let query_set = query_set_guard - .get(query_set_id) - .map_err(|_| RenderPassErrorInner::InvalidQuerySet(query_set_id)) - .map_pass_err(scope)?; - - state.tracker.query_sets.add_single(query_set); - validate_and_begin_occlusion_query( - query_set.clone(), + query_set, state.raw_encoder, + &mut state.tracker.query_sets, query_index, Some(&mut cmd_buf_data.pending_query_resets), &mut state.active_query, From 2f7a248fdcde1aed2974d84f40dc3352876e2cb6 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Wed, 26 Jun 2024 10:34:54 +0200 Subject: [PATCH 40/47] extract `execute_bundle` from `render_pass_end_impl` --- wgpu-core/src/command/render.rs | 146 +++++++++++++++----------------- 1 file changed, 70 insertions(+), 76 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 4901083cbf..bfda64bd96 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1763,83 +1763,8 @@ impl Global { .map_pass_err(scope)?; } ArcRenderCommand::ExecuteBundle(bundle) => { - api_log!("RenderPass::execute_bundle {}", bundle.error_ident()); let scope = PassErrorScope::ExecuteBundle; - - // Have to clone the bundle arc, otherwise we keep a mutable reference to the bundle - // while later trying to add the bundle's resources to the tracker. - let bundle = state.tracker.bundles.insert_single(bundle).clone(); - - bundle - .same_device_as(cmd_buf.as_ref()) - .map_pass_err(scope)?; - - state - .info - .context - .check_compatible( - &bundle.context, - RenderPassCompatibilityCheckType::RenderBundle, - ) - .map_err(RenderPassErrorInner::IncompatibleBundleTargets) - .map_pass_err(scope)?; - - if (state.info.is_depth_read_only && !bundle.is_depth_read_only) - || (state.info.is_stencil_read_only && !bundle.is_stencil_read_only) - { - return Err( - RenderPassErrorInner::IncompatibleBundleReadOnlyDepthStencil { - pass_depth: state.info.is_depth_read_only, - pass_stencil: state.info.is_stencil_read_only, - bundle_depth: bundle.is_depth_read_only, - bundle_stencil: bundle.is_stencil_read_only, - }, - ) - .map_pass_err(scope); - } - - state.buffer_memory_init_actions.extend( - bundle - .buffer_memory_init_actions - .iter() - .filter_map(|action| { - action - .buffer - .initialization_status - .read() - .check_action(action) - }), - ); - for action in bundle.texture_memory_init_actions.iter() { - state - .info - .pending_discard_init_fixups - .extend(state.texture_memory_actions.register_init_action(action)); - } - - unsafe { bundle.execute(state.raw_encoder, state.snatch_guard) } - .map_err(|e| match e { - ExecutionError::DestroyedResource(e) => { - RenderCommandError::DestroyedResource(e) - } - ExecutionError::Unimplemented(what) => { - RenderCommandError::Unimplemented(what) - } - }) - .map_pass_err(scope)?; - - unsafe { - state - .info - .usage_scope - .merge_render_bundle(&bundle.used) - .map_pass_err(scope)?; - state - .tracker - .add_from_render_bundle(&bundle.used) - .map_pass_err(scope)?; - }; - state.reset_bundle(); + execute_bundle(&mut state, &cmd_buf, bundle).map_pass_err(scope)?; } } } @@ -2651,6 +2576,75 @@ fn write_timestamp( Ok(()) } +fn execute_bundle( + state: &mut State, + cmd_buf: &Arc>, + bundle: Arc>, +) -> Result<(), RenderPassErrorInner> { + api_log!("RenderPass::execute_bundle {}", bundle.error_ident()); + + // Have to clone the bundle arc, otherwise we keep a mutable reference to the bundle + // while later trying to add the bundle's resources to the tracker. + let bundle = state.tracker.bundles.insert_single(bundle).clone(); + + bundle.same_device_as(cmd_buf.as_ref())?; + + state + .info + .context + .check_compatible( + &bundle.context, + RenderPassCompatibilityCheckType::RenderBundle, + ) + .map_err(RenderPassErrorInner::IncompatibleBundleTargets)?; + + if (state.info.is_depth_read_only && !bundle.is_depth_read_only) + || (state.info.is_stencil_read_only && !bundle.is_stencil_read_only) + { + return Err( + RenderPassErrorInner::IncompatibleBundleReadOnlyDepthStencil { + pass_depth: state.info.is_depth_read_only, + pass_stencil: state.info.is_stencil_read_only, + bundle_depth: bundle.is_depth_read_only, + bundle_stencil: bundle.is_stencil_read_only, + }, + ); + } + + state + .buffer_memory_init_actions + .extend( + bundle + .buffer_memory_init_actions + .iter() + .filter_map(|action| { + action + .buffer + .initialization_status + .read() + .check_action(action) + }), + ); + for action in bundle.texture_memory_init_actions.iter() { + state + .info + .pending_discard_init_fixups + .extend(state.texture_memory_actions.register_init_action(action)); + } + + unsafe { bundle.execute(state.raw_encoder, state.snatch_guard) }.map_err(|e| match e { + ExecutionError::DestroyedResource(e) => RenderCommandError::DestroyedResource(e), + ExecutionError::Unimplemented(what) => RenderCommandError::Unimplemented(what), + })?; + + unsafe { + state.info.usage_scope.merge_render_bundle(&bundle.used)?; + state.tracker.add_from_render_bundle(&bundle.used)?; + }; + state.reset_bundle(); + Ok(()) +} + impl Global { pub fn render_pass_set_bind_group( &self, From 875a16fb142426ca67deed0a766514c364af0f41 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Wed, 26 Jun 2024 11:11:49 +0200 Subject: [PATCH 41/47] move `pipeline` ident to `DispatchError::IncompatibleBindGroup` --- wgpu-core/src/command/compute.rs | 66 +++++++++++------------- wgpu-core/src/command/compute_command.rs | 5 +- wgpu-core/src/command/mod.rs | 10 +--- 3 files changed, 31 insertions(+), 50 deletions(-) diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index 217cde1151..3cbea05b12 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -17,8 +17,10 @@ use crate::{ hal_api::HalApi, hal_label, id, init_tracker::{BufferInitTrackerAction, MemoryInitKind}, + pipeline::ComputePipeline, resource::{ self, Buffer, DestroyedResourceError, MissingBufferUsageError, ParentDevice, Resource, + ResourceErrorIdent, }, snatch::SnatchGuard, track::{ResourceUsageCompatibilityError, Tracker, TrackerIndex, UsageScope}, @@ -136,13 +138,17 @@ struct ArcComputePassDescriptor<'a, A: HalApi> { pub timestamp_writes: Option>, } -#[derive(Clone, Debug, Error, Eq, PartialEq)] +#[derive(Clone, Debug, Error)] #[non_exhaustive] pub enum DispatchError { #[error("Compute pipeline must be set")] MissingPipeline, - #[error("Incompatible bind group at index {index} in the current compute pipeline")] - IncompatibleBindGroup { index: u32, diff: Vec }, + #[error("Bind group at index {index} is incompatible with the current set {pipeline}")] + IncompatibleBindGroup { + index: u32, + pipeline: ResourceErrorIdent, + diff: Vec, + }, #[error( "Each current dispatch group size dimension ({current:?}) must be less or equal to {limit}" )] @@ -254,7 +260,7 @@ where struct State<'scope, 'snatch_guard, 'cmd_buf, 'raw_encoder, A: HalApi> { binder: Binder, - pipeline: Option, + pipeline: Option>>, scope: UsageScope<'scope, A>, debug_scope_depth: u32, @@ -284,22 +290,20 @@ impl<'scope, 'snatch_guard, 'cmd_buf, 'raw_encoder, A: HalApi> State<'scope, 'snatch_guard, 'cmd_buf, 'raw_encoder, A> { fn is_ready(&self) -> Result<(), DispatchError> { - let bind_mask = self.binder.invalid_mask(); - if bind_mask != 0 { - //let (expected, provided) = self.binder.entries[index as usize].info(); - let index = bind_mask.trailing_zeros(); - - return Err(DispatchError::IncompatibleBindGroup { - index, - diff: self.binder.bgl_diff(), - }); - } - if self.pipeline.is_none() { - return Err(DispatchError::MissingPipeline); + if let Some(pipeline) = self.pipeline.as_ref() { + let bind_mask = self.binder.invalid_mask(); + if bind_mask != 0 { + return Err(DispatchError::IncompatibleBindGroup { + index: bind_mask.trailing_zeros(), + pipeline: pipeline.error_ident(), + diff: self.binder.bgl_diff(), + }); + } + self.binder.check_late_buffer_bindings()?; + Ok(()) + } else { + Err(DispatchError::MissingPipeline) } - self.binder.check_late_buffer_bindings()?; - - Ok(()) } // `extra_buffer` is there to represent the indirect buffer that is also @@ -630,17 +634,11 @@ impl Global { .map_pass_err(scope)?; } ArcComputeCommand::Dispatch(groups) => { - let scope = PassErrorScope::Dispatch { - indirect: false, - pipeline: state.pipeline, - }; + let scope = PassErrorScope::Dispatch { indirect: false }; dispatch(&mut state, groups).map_pass_err(scope)?; } ArcComputeCommand::DispatchIndirect { buffer, offset } => { - let scope = PassErrorScope::Dispatch { - indirect: true, - pipeline: state.pipeline, - }; + let scope = PassErrorScope::Dispatch { indirect: true }; dispatch_indirect(&mut state, cmd_buf, buffer, offset).map_pass_err(scope)?; } ArcComputeCommand::PushDebugGroup { color: _, len } => { @@ -798,11 +796,11 @@ fn set_bind_group( fn set_pipeline( state: &mut State, cmd_buf: &CommandBuffer, - pipeline: Arc>, + pipeline: Arc>, ) -> Result<(), ComputePassErrorInner> { pipeline.same_device_as(cmd_buf)?; - state.pipeline = Some(pipeline.as_info().id()); + state.pipeline = Some(pipeline.clone()); let pipeline = state.tracker.compute_pipelines.insert_single(pipeline); @@ -1150,10 +1148,7 @@ impl Global { groups_y: u32, groups_z: u32, ) -> Result<(), ComputePassError> { - let scope = PassErrorScope::Dispatch { - indirect: false, - pipeline: pass.current_pipeline.last_state, - }; + let scope = PassErrorScope::Dispatch { indirect: false }; let base = pass.base_mut(scope)?; base.commands.push(ArcComputeCommand::::Dispatch([ @@ -1170,10 +1165,7 @@ impl Global { offset: BufferAddress, ) -> Result<(), ComputePassError> { let hub = A::hub(self); - let scope = PassErrorScope::Dispatch { - indirect: true, - pipeline: pass.current_pipeline.last_state, - }; + let scope = PassErrorScope::Dispatch { indirect: true }; let base = pass.base_mut(scope)?; let buffer = hub diff --git a/wgpu-core/src/command/compute_command.rs b/wgpu-core/src/command/compute_command.rs index bd98bda157..e4662910f8 100644 --- a/wgpu-core/src/command/compute_command.rs +++ b/wgpu-core/src/command/compute_command.rs @@ -128,10 +128,7 @@ impl ComputeCommand { ArcComputeCommand::DispatchIndirect { buffer: buffers_guard.get_owned(buffer_id).map_err(|_| { ComputePassError { - scope: PassErrorScope::Dispatch { - indirect: true, - pipeline: None, // TODO: not used right now, but once we do the resolve during recording we can use this again. - }, + scope: PassErrorScope::Dispatch { indirect: true }, inner: ComputePassErrorInner::InvalidBufferId(buffer_id), } })?, diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index b4669f8e7a..a0d3a0850f 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -907,10 +907,7 @@ pub enum PassErrorScope { #[error("In a execute_bundle command")] ExecuteBundle, #[error("In a dispatch command, indirect:{indirect}")] - Dispatch { - indirect: bool, - pipeline: Option, - }, + Dispatch { indirect: bool }, #[error("In a push_debug_group command")] PushDebugGroup, #[error("In a pop_debug_group command")] @@ -949,11 +946,6 @@ impl PrettyError for PassErrorScope { } => { fmt.render_pipeline_label(&id); } - Self::Dispatch { - pipeline: Some(id), .. - } => { - fmt.compute_pipeline_label(&id); - } _ => {} } } From 9f1cba1eeea23f97fb2265a7a915e1b3e887db8b Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Wed, 26 Jun 2024 11:57:19 +0200 Subject: [PATCH 42/47] move `pipeline` ident to appropriate errors --- wgpu-core/src/command/bundle.rs | 15 +--- wgpu-core/src/command/draw.rs | 29 ++++-- wgpu-core/src/command/mod.rs | 11 +-- wgpu-core/src/command/render.rs | 112 ++++++++++-------------- wgpu-core/src/command/render_command.rs | 2 - 5 files changed, 70 insertions(+), 99 deletions(-) diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index 246be74d04..3623ec27e2 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/wgpu-core/src/command/bundle.rs @@ -478,7 +478,6 @@ impl RenderBundleEncoder { let scope = PassErrorScope::Draw { kind: DrawKind::Draw, indexed: false, - pipeline: state.pipeline_id(), }; draw( &mut state, @@ -500,7 +499,6 @@ impl RenderBundleEncoder { let scope = PassErrorScope::Draw { kind: DrawKind::Draw, indexed: true, - pipeline: state.pipeline_id(), }; draw_indexed( &mut state, @@ -522,7 +520,6 @@ impl RenderBundleEncoder { let scope = PassErrorScope::Draw { kind: DrawKind::DrawIndirect, indexed, - pipeline: state.pipeline_id(), }; multi_draw_indirect( &mut state, @@ -705,7 +702,7 @@ fn set_pipeline( return Err(RenderCommandError::IncompatiblePipelineRods.into()); } - let pipeline_state = PipelineState::new(pipeline, pipeline_id); + let pipeline_state = PipelineState::new(pipeline); state .commands @@ -1337,8 +1334,6 @@ struct PipelineState { /// The pipeline pipeline: Arc>, - pipeline_id: id::RenderPipelineId, - /// How this pipeline's vertex shader traverses each vertex buffer, indexed /// by vertex buffer slot number. steps: Vec, @@ -1352,10 +1347,9 @@ struct PipelineState { } impl PipelineState { - fn new(pipeline: &Arc>, pipeline_id: id::RenderPipelineId) -> Self { + fn new(pipeline: &Arc>) -> Self { Self { pipeline: pipeline.clone(), - pipeline_id, steps: pipeline.vertex_steps.to_vec(), push_constant_ranges: pipeline .layout @@ -1433,11 +1427,6 @@ struct State { } impl State { - /// Return the id of the current pipeline, if any. - fn pipeline_id(&self) -> Option { - self.pipeline.as_ref().map(|p| p.pipeline_id) - } - /// Return the current pipeline state. Return an error if none is set. fn pipeline(&self) -> Result<&PipelineState, RenderBundleErrorInner> { self.pipeline diff --git a/wgpu-core/src/command/draw.rs b/wgpu-core/src/command/draw.rs index d9745acc0b..752459ace3 100644 --- a/wgpu-core/src/command/draw.rs +++ b/wgpu-core/src/command/draw.rs @@ -2,7 +2,10 @@ use crate::{ binding_model::{LateMinBufferBindingSizeMismatch, PushConstantUploadError}, error::ErrorFormatter, id, - resource::{DestroyedResourceError, MissingBufferUsageError, MissingTextureUsageError}, + resource::{ + DestroyedResourceError, MissingBufferUsageError, MissingTextureUsageError, + ResourceErrorIdent, + }, track::ResourceUsageCompatibilityError, }; use wgt::VertexStepMode; @@ -10,19 +13,26 @@ use wgt::VertexStepMode; use thiserror::Error; /// Error validating a draw call. -#[derive(Clone, Debug, Error, Eq, PartialEq)] +#[derive(Clone, Debug, Error)] #[non_exhaustive] pub enum DrawError { #[error("Blend constant needs to be set")] MissingBlendConstant, #[error("Render pipeline must be set")] MissingPipeline, - #[error("Vertex buffer {index} must be set")] - MissingVertexBuffer { index: u32 }, + #[error("Currently set {pipeline} requires vertex buffer {index} to be set")] + MissingVertexBuffer { + pipeline: ResourceErrorIdent, + index: u32, + }, #[error("Index buffer must be set")] MissingIndexBuffer, - #[error("Incompatible bind group at index {index} in the current render pipeline")] - IncompatibleBindGroup { index: u32, diff: Vec }, + #[error("Bind group at index {index} is incompatible with the current set {pipeline}")] + IncompatibleBindGroup { + index: u32, + pipeline: ResourceErrorIdent, + diff: Vec, + }, #[error("Vertex {last_vertex} extends beyond limit {vertex_limit} imposed by the buffer in slot {slot}. Did you bind the correct `Vertex` step-rate vertex buffer?")] VertexBeyondLimit { last_vertex: u64, @@ -45,11 +55,12 @@ pub enum DrawError { #[error("Index {last_index} extends beyond limit {index_limit}. Did you bind the correct index buffer?")] IndexBeyondLimit { last_index: u64, index_limit: u64 }, #[error( - "Pipeline index format ({pipeline:?}) and buffer index format ({buffer:?}) do not match" + "Index buffer format {buffer_format:?} doesn't match {pipeline}'s index format {pipeline_format:?}" )] UnmatchedIndexFormats { - pipeline: wgt::IndexFormat, - buffer: wgt::IndexFormat, + pipeline: ResourceErrorIdent, + pipeline_format: wgt::IndexFormat, + buffer_format: wgt::IndexFormat, }, #[error(transparent)] BindingSizeTooSmall(#[from] LateMinBufferBindingSizeMismatch), diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index a0d3a0850f..f27982905d 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -887,11 +887,7 @@ pub enum PassErrorScope { #[error("In a set_scissor_rect command")] SetScissorRect, #[error("In a draw command, kind: {kind:?}")] - Draw { - kind: DrawKind, - indexed: bool, - pipeline: Option, - }, + Draw { kind: DrawKind, indexed: bool }, #[error("While resetting queries after the renderpass was ran")] QueryReset, #[error("In a write_timestamp command")] @@ -941,11 +937,6 @@ impl PrettyError for PassErrorScope { Self::SetIndexBuffer(id) => { fmt.buffer_label(&id); } - Self::Draw { - pipeline: Some(id), .. - } => { - fmt.render_pipeline_label(&id); - } _ => {} } } diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index bfda64bd96..56be96b28c 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -327,7 +327,6 @@ impl OptionalState { #[derive(Debug, Default)] struct IndexState { buffer_format: Option, - pipeline_format: Option, limit: u64, } @@ -343,7 +342,6 @@ impl IndexState { fn reset(&mut self) { self.buffer_format = None; - self.pipeline_format = None; self.limit = 0; } } @@ -378,8 +376,6 @@ struct VertexState { instance_limit: u64, /// Buffer slot which the shortest instance rate vertex buffer is bound to instance_limit_slot: u32, - /// Total amount of buffers required by the pipeline. - buffers_required: u32, } impl VertexState { @@ -440,7 +436,7 @@ struct State<'attachment, 'scope, 'snatch_guard, 'cmd_buf, 'raw_encoder, A: HalA binder: Binder, blend_constant: OptionalState, stencil_reference: u32, - pipeline: Option, + pipeline: Option>>, index: IndexState, vertex: VertexState, debug_scope_depth: u32, @@ -467,52 +463,55 @@ impl<'attachment, 'scope, 'snatch_guard, 'cmd_buf, 'raw_encoder, A: HalApi> State<'attachment, 'scope, 'snatch_guard, 'cmd_buf, 'raw_encoder, A> { fn is_ready(&self, indexed: bool) -> Result<(), DrawError> { - // Determine how many vertex buffers have already been bound - let vertex_buffer_count = self.vertex.inputs.iter().take_while(|v| v.bound).count() as u32; - // Compare with the needed quantity - if vertex_buffer_count < self.vertex.buffers_required { - return Err(DrawError::MissingVertexBuffer { - index: vertex_buffer_count, - }); - } + if let Some(pipeline) = self.pipeline.as_ref() { + let bind_mask = self.binder.invalid_mask(); + if bind_mask != 0 { + return Err(DrawError::IncompatibleBindGroup { + index: bind_mask.trailing_zeros(), + pipeline: pipeline.error_ident(), + diff: self.binder.bgl_diff(), + }); + } + self.binder.check_late_buffer_bindings()?; - let bind_mask = self.binder.invalid_mask(); - if bind_mask != 0 { - //let (expected, provided) = self.binder.entries[index as usize].info(); - return Err(DrawError::IncompatibleBindGroup { - index: bind_mask.trailing_zeros(), - diff: self.binder.bgl_diff(), - }); - } - if self.pipeline.is_none() { - return Err(DrawError::MissingPipeline); - } - if self.blend_constant == OptionalState::Required { - return Err(DrawError::MissingBlendConstant); - } + if self.blend_constant == OptionalState::Required { + return Err(DrawError::MissingBlendConstant); + } - if indexed { - // Pipeline expects an index buffer - if let Some(pipeline_index_format) = self.index.pipeline_format { - // We have a buffer bound - let buffer_index_format = self - .index - .buffer_format - .ok_or(DrawError::MissingIndexBuffer)?; - - // The buffers are different formats - if pipeline_index_format != buffer_index_format { - return Err(DrawError::UnmatchedIndexFormats { - pipeline: pipeline_index_format, - buffer: buffer_index_format, - }); + // Determine how many vertex buffers have already been bound + let vertex_buffer_count = + self.vertex.inputs.iter().take_while(|v| v.bound).count() as u32; + // Compare with the needed quantity + if vertex_buffer_count < pipeline.vertex_steps.len() as u32 { + return Err(DrawError::MissingVertexBuffer { + pipeline: pipeline.error_ident(), + index: vertex_buffer_count, + }); + } + + if indexed { + // Pipeline expects an index buffer + if let Some(pipeline_index_format) = pipeline.strip_index_format { + // We have a buffer bound + let buffer_index_format = self + .index + .buffer_format + .ok_or(DrawError::MissingIndexBuffer)?; + + // The buffers are different formats + if pipeline_index_format != buffer_index_format { + return Err(DrawError::UnmatchedIndexFormats { + pipeline: pipeline.error_ident(), + pipeline_format: pipeline_index_format, + buffer_format: buffer_index_format, + }); + } } } + Ok(()) + } else { + Err(DrawError::MissingPipeline) } - - self.binder.check_late_buffer_bindings()?; - - Ok(()) } /// Reset the `RenderBundle`-related states. @@ -1610,7 +1609,6 @@ impl Global { let scope = PassErrorScope::Draw { kind: DrawKind::Draw, indexed: false, - pipeline: state.pipeline, }; draw( &mut state, @@ -1631,7 +1629,6 @@ impl Global { let scope = PassErrorScope::Draw { kind: DrawKind::Draw, indexed: true, - pipeline: state.pipeline, }; draw_indexed( &mut state, @@ -1656,7 +1653,6 @@ impl Global { DrawKind::DrawIndirect }, indexed, - pipeline: state.pipeline, }; multi_draw_indirect(&mut state, buffer, offset, count, indexed) .map_pass_err(scope)?; @@ -1672,7 +1668,6 @@ impl Global { let scope = PassErrorScope::Draw { kind: DrawKind::MultiDrawIndirectCount, indexed, - pipeline: state.pipeline, }; multi_draw_indirect_count( &mut state, @@ -1901,7 +1896,7 @@ fn set_pipeline( ) -> Result<(), RenderPassErrorInner> { api_log!("RenderPass::set_pipeline {}", pipeline.error_ident()); - state.pipeline = Some(pipeline.as_info().id()); + state.pipeline = Some(pipeline.clone()); let pipeline = state.tracker.render_pipelines.insert_single(pipeline); @@ -1986,17 +1981,12 @@ fn set_pipeline( } } - state.index.pipeline_format = pipeline.strip_index_format; - - let vertex_steps_len = pipeline.vertex_steps.len(); - state.vertex.buffers_required = vertex_steps_len as u32; - // Initialize each `vertex.inputs[i].step` from // `pipeline.vertex_steps[i]`. Enlarge `vertex.inputs` // as necessary to accommodate all slots in the // pipeline. If `vertex.inputs` is longer, fill the // extra entries with default `VertexStep`s. - while state.vertex.inputs.len() < vertex_steps_len { + while state.vertex.inputs.len() < pipeline.vertex_steps.len() { state.vertex.inputs.push(VertexBufferState::EMPTY); } @@ -2857,7 +2847,6 @@ impl Global { let scope = PassErrorScope::Draw { kind: DrawKind::Draw, indexed: false, - pipeline: pass.current_pipeline.last_state, }; let base = pass.base_mut(scope)?; @@ -2883,7 +2872,6 @@ impl Global { let scope = PassErrorScope::Draw { kind: DrawKind::Draw, indexed: true, - pipeline: pass.current_pipeline.last_state, }; let base = pass.base_mut(scope)?; @@ -2907,7 +2895,6 @@ impl Global { let scope = PassErrorScope::Draw { kind: DrawKind::DrawIndirect, indexed: false, - pipeline: pass.current_pipeline.last_state, }; let base = pass.base_mut(scope)?; @@ -2930,7 +2917,6 @@ impl Global { let scope = PassErrorScope::Draw { kind: DrawKind::DrawIndirect, indexed: true, - pipeline: pass.current_pipeline.last_state, }; let base = pass.base_mut(scope)?; @@ -2954,7 +2940,6 @@ impl Global { let scope = PassErrorScope::Draw { kind: DrawKind::MultiDrawIndirect, indexed: false, - pipeline: pass.current_pipeline.last_state, }; let base = pass.base_mut(scope)?; @@ -2978,7 +2963,6 @@ impl Global { let scope = PassErrorScope::Draw { kind: DrawKind::MultiDrawIndirect, indexed: true, - pipeline: pass.current_pipeline.last_state, }; let base = pass.base_mut(scope)?; @@ -3004,7 +2988,6 @@ impl Global { let scope = PassErrorScope::Draw { kind: DrawKind::MultiDrawIndirectCount, indexed: false, - pipeline: pass.current_pipeline.last_state, }; let base = pass.base_mut(scope)?; @@ -3032,7 +3015,6 @@ impl Global { let scope = PassErrorScope::Draw { kind: DrawKind::MultiDrawIndirectCount, indexed: true, - pipeline: pass.current_pipeline.last_state, }; let base = pass.base_mut(scope)?; diff --git a/wgpu-core/src/command/render_command.rs b/wgpu-core/src/command/render_command.rs index 22072bfc6e..9dd2f00437 100644 --- a/wgpu-core/src/command/render_command.rs +++ b/wgpu-core/src/command/render_command.rs @@ -315,7 +315,6 @@ impl RenderCommand { DrawKind::DrawIndirect }, indexed, - pipeline: None, }, inner: RenderCommandError::InvalidBufferId(buffer_id).into(), } @@ -336,7 +335,6 @@ impl RenderCommand { let scope = PassErrorScope::Draw { kind: DrawKind::MultiDrawIndirectCount, indexed, - pipeline: None, }; ArcRenderCommand::MultiDrawIndirectCount { buffer: buffers_guard.get_owned(buffer_id).map_err(|_| { From 65eb5529f8aad89d1a695ded83995f3142e87f9f Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Wed, 26 Jun 2024 12:02:25 +0200 Subject: [PATCH 43/47] remove buffer id from set index/vertex buffer scopes all relevant inner errors already print the label of the buffers --- wgpu-core/src/command/bundle.rs | 4 ++-- wgpu-core/src/command/mod.rs | 10 ++-------- wgpu-core/src/command/render.rs | 8 ++++---- wgpu-core/src/command/render_command.rs | 4 ++-- 4 files changed, 10 insertions(+), 16 deletions(-) diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index 3623ec27e2..aa98df797d 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/wgpu-core/src/command/bundle.rs @@ -438,7 +438,7 @@ impl RenderBundleEncoder { offset, size, } => { - let scope = PassErrorScope::SetIndexBuffer(buffer_id); + let scope = PassErrorScope::SetIndexBuffer; set_index_buffer( &mut state, &buffer_guard, @@ -455,7 +455,7 @@ impl RenderBundleEncoder { offset, size, } => { - let scope = PassErrorScope::SetVertexBuffer(buffer_id); + let scope = PassErrorScope::SetVertexBuffer; set_vertex_buffer(&mut state, &buffer_guard, slot, buffer_id, offset, size) .map_pass_err(scope)?; } diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index f27982905d..58ce0a162d 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -875,9 +875,9 @@ pub enum PassErrorScope { #[error("In a set_push_constant command")] SetPushConstant, #[error("In a set_vertex_buffer command")] - SetVertexBuffer(id::BufferId), + SetVertexBuffer, #[error("In a set_index_buffer command")] - SetIndexBuffer(id::BufferId), + SetIndexBuffer, #[error("In a set_blend_constant command")] SetBlendConstant, #[error("In a set_stencil_reference command")] @@ -931,12 +931,6 @@ impl PrettyError for PassErrorScope { Self::SetPipelineCompute(id) => { fmt.compute_pipeline_label(&id); } - Self::SetVertexBuffer(id) => { - fmt.buffer_label(&id); - } - Self::SetIndexBuffer(id) => { - fmt.buffer_label(&id); - } _ => {} } } diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 56be96b28c..9860b16a89 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1551,7 +1551,7 @@ impl Global { offset, size, } => { - let scope = PassErrorScope::SetIndexBuffer(buffer.as_info().id()); + let scope = PassErrorScope::SetIndexBuffer; set_index_buffer(&mut state, &cmd_buf, buffer, index_format, offset, size) .map_pass_err(scope)?; } @@ -1561,7 +1561,7 @@ impl Global { offset, size, } => { - let scope = PassErrorScope::SetVertexBuffer(buffer.as_info().id()); + let scope = PassErrorScope::SetVertexBuffer; set_vertex_buffer(&mut state, &cmd_buf, slot, buffer, offset, size) .map_pass_err(scope)?; } @@ -2697,7 +2697,7 @@ impl Global { offset: BufferAddress, size: Option, ) -> Result<(), RenderPassError> { - let scope = PassErrorScope::SetIndexBuffer(buffer_id); + let scope = PassErrorScope::SetIndexBuffer; let base = pass.base_mut(scope)?; base.commands.push(RenderCommand::SetIndexBuffer { @@ -2718,7 +2718,7 @@ impl Global { offset: BufferAddress, size: Option, ) -> Result<(), RenderPassError> { - let scope = PassErrorScope::SetVertexBuffer(buffer_id); + let scope = PassErrorScope::SetVertexBuffer; let base = pass.base_mut(scope)?; base.commands.push(RenderCommand::SetVertexBuffer { diff --git a/wgpu-core/src/command/render_command.rs b/wgpu-core/src/command/render_command.rs index 9dd2f00437..125e642eec 100644 --- a/wgpu-core/src/command/render_command.rs +++ b/wgpu-core/src/command/render_command.rs @@ -228,7 +228,7 @@ impl RenderCommand { } => ArcRenderCommand::SetIndexBuffer { buffer: buffers_guard.get_owned(buffer_id).map_err(|_| { RenderPassError { - scope: PassErrorScope::SetIndexBuffer(buffer_id), + scope: PassErrorScope::SetIndexBuffer, inner: RenderCommandError::InvalidBufferId(buffer_id).into(), } })?, @@ -246,7 +246,7 @@ impl RenderCommand { slot, buffer: buffers_guard.get_owned(buffer_id).map_err(|_| { RenderPassError { - scope: PassErrorScope::SetVertexBuffer(buffer_id), + scope: PassErrorScope::SetVertexBuffer, inner: RenderCommandError::InvalidBufferId(buffer_id).into(), } })?, From 8232b8b16d29c0b0713c7eda09a0e50b53c20bf7 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Wed, 26 Jun 2024 12:07:52 +0200 Subject: [PATCH 44/47] remove compute pipeline id from set compute pipeline scope and make sure that we use `ResourceErrorIdent` in all relevant inner errors --- wgpu-core/src/command/compute.rs | 4 ++-- wgpu-core/src/command/compute_command.rs | 2 +- wgpu-core/src/command/mod.rs | 5 +---- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index 3cbea05b12..91aed2421d 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -615,7 +615,7 @@ impl Global { .map_pass_err(scope)?; } ArcComputeCommand::SetPipeline(pipeline) => { - let scope = PassErrorScope::SetPipelineCompute(pipeline.as_info().id()); + let scope = PassErrorScope::SetPipelineCompute; set_pipeline(&mut state, cmd_buf, pipeline).map_pass_err(scope)?; } ArcComputeCommand::SetPushConstant { @@ -1083,7 +1083,7 @@ impl Global { ) -> Result<(), ComputePassError> { let redundant = pass.current_pipeline.set_and_check_redundant(pipeline_id); - let scope = PassErrorScope::SetPipelineCompute(pipeline_id); + let scope = PassErrorScope::SetPipelineCompute; let base = pass.base_mut(scope)?; if redundant { diff --git a/wgpu-core/src/command/compute_command.rs b/wgpu-core/src/command/compute_command.rs index e4662910f8..0a37f3d781 100644 --- a/wgpu-core/src/command/compute_command.rs +++ b/wgpu-core/src/command/compute_command.rs @@ -107,7 +107,7 @@ impl ComputeCommand { pipelines_guard .get_owned(pipeline_id) .map_err(|_| ComputePassError { - scope: PassErrorScope::SetPipelineCompute(pipeline_id), + scope: PassErrorScope::SetPipelineCompute, inner: ComputePassErrorInner::InvalidPipeline(pipeline_id), })?, ), diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index 58ce0a162d..952de9a55d 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -871,7 +871,7 @@ pub enum PassErrorScope { #[error("In a set_pipeline command")] SetPipelineRender(id::RenderPipelineId), #[error("In a set_pipeline command")] - SetPipelineCompute(id::ComputePipelineId), + SetPipelineCompute, #[error("In a set_push_constant command")] SetPushConstant, #[error("In a set_vertex_buffer command")] @@ -928,9 +928,6 @@ impl PrettyError for PassErrorScope { Self::SetPipelineRender(id) => { fmt.render_pipeline_label(&id); } - Self::SetPipelineCompute(id) => { - fmt.compute_pipeline_label(&id); - } _ => {} } } From 9dba43eb2c4be484da6ceea2edaafe2bbfafa0d6 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Wed, 26 Jun 2024 12:36:05 +0200 Subject: [PATCH 45/47] remove render pipeline id from set render pipeline scope and make sure that we use `ResourceErrorIdent` in all relevant inner errors --- wgpu-core/src/command/bundle.rs | 20 ++++++------- wgpu-core/src/command/draw.rs | 6 ++-- wgpu-core/src/command/mod.rs | 5 +--- wgpu-core/src/command/render.rs | 26 +++++++---------- wgpu-core/src/command/render_command.rs | 2 +- wgpu-core/src/device/mod.rs | 37 +++++++++++-------------- 6 files changed, 41 insertions(+), 55 deletions(-) diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index aa98df797d..081f16a447 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/wgpu-core/src/command/bundle.rs @@ -86,8 +86,8 @@ use crate::{ }, conv, device::{ - AttachmentData, Device, DeviceError, MissingDownlevelFlags, - RenderPassCompatibilityCheckType, RenderPassContext, SHADER_STAGE_COUNT, + AttachmentData, Device, DeviceError, MissingDownlevelFlags, RenderPassContext, + SHADER_STAGE_COUNT, }, error::{ErrorFormatter, PrettyError}, hal_api::HalApi, @@ -421,7 +421,7 @@ impl RenderBundleEncoder { .map_pass_err(scope)?; } RenderCommand::SetPipeline(pipeline_id) => { - let scope = PassErrorScope::SetPipelineRender(pipeline_id); + let scope = PassErrorScope::SetPipelineRender; set_pipeline( &mut state, &pipeline_guard, @@ -690,16 +690,14 @@ fn set_pipeline( pipeline.same_device(&state.device)?; context - .check_compatible( - &pipeline.pass_context, - RenderPassCompatibilityCheckType::RenderPipeline, - ) + .check_compatible(&pipeline.pass_context, pipeline.as_ref()) .map_err(RenderCommandError::IncompatiblePipelineTargets)?; - if (pipeline.flags.contains(PipelineFlags::WRITES_DEPTH) && is_depth_read_only) - || (pipeline.flags.contains(PipelineFlags::WRITES_STENCIL) && is_stencil_read_only) - { - return Err(RenderCommandError::IncompatiblePipelineRods.into()); + if pipeline.flags.contains(PipelineFlags::WRITES_DEPTH) && is_depth_read_only { + return Err(RenderCommandError::IncompatibleDepthAccess(pipeline.error_ident()).into()); + } + if pipeline.flags.contains(PipelineFlags::WRITES_STENCIL) && is_stencil_read_only { + return Err(RenderCommandError::IncompatibleStencilAccess(pipeline.error_ident()).into()); } let pipeline_state = PipelineState::new(pipeline); diff --git a/wgpu-core/src/command/draw.rs b/wgpu-core/src/command/draw.rs index 752459ace3..667efb6537 100644 --- a/wgpu-core/src/command/draw.rs +++ b/wgpu-core/src/command/draw.rs @@ -91,8 +91,10 @@ pub enum RenderCommandError { InvalidQuerySet(id::QuerySetId), #[error("Render pipeline targets are incompatible with render pass")] IncompatiblePipelineTargets(#[from] crate::device::RenderPassCompatibilityError), - #[error("Pipeline writes to depth/stencil, while the pass has read-only depth/stencil")] - IncompatiblePipelineRods, + #[error("{0} writes to depth, while the pass has read-only depth access")] + IncompatibleDepthAccess(ResourceErrorIdent), + #[error("{0} writes to stencil, while the pass has read-only stencil access")] + IncompatibleStencilAccess(ResourceErrorIdent), #[error(transparent)] ResourceUsageCompatibility(#[from] ResourceUsageCompatibilityError), #[error(transparent)] diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index 952de9a55d..7737ec2bea 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -869,7 +869,7 @@ pub enum PassErrorScope { #[error("In a set_bind_group command")] SetBindGroup(id::BindGroupId), #[error("In a set_pipeline command")] - SetPipelineRender(id::RenderPipelineId), + SetPipelineRender, #[error("In a set_pipeline command")] SetPipelineCompute, #[error("In a set_push_constant command")] @@ -925,9 +925,6 @@ impl PrettyError for PassErrorScope { Self::SetBindGroup(id) => { fmt.bind_group_label(&id); } - Self::SetPipelineRender(id) => { - fmt.render_pipeline_label(&id); - } _ => {} } } diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 9860b16a89..11312c39d1 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -19,7 +19,7 @@ use crate::{ }, device::{ AttachmentData, Device, DeviceError, MissingDownlevelFlags, MissingFeatures, - RenderPassCompatibilityCheckType, RenderPassCompatibilityError, RenderPassContext, + RenderPassCompatibilityError, RenderPassContext, }, error::{ErrorFormatter, PrettyError}, global::Global, @@ -1542,7 +1542,7 @@ impl Global { .map_pass_err(scope)?; } ArcRenderCommand::SetPipeline(pipeline) => { - let scope = PassErrorScope::SetPipelineRender(pipeline.as_info().id()); + let scope = PassErrorScope::SetPipelineRender; set_pipeline(&mut state, &cmd_buf, pipeline).map_pass_err(scope)?; } ArcRenderCommand::SetIndexBuffer { @@ -1905,19 +1905,16 @@ fn set_pipeline( state .info .context - .check_compatible( - &pipeline.pass_context, - RenderPassCompatibilityCheckType::RenderPipeline, - ) + .check_compatible(&pipeline.pass_context, pipeline.as_ref()) .map_err(RenderCommandError::IncompatiblePipelineTargets)?; state.pipeline_flags = pipeline.flags; - if (pipeline.flags.contains(PipelineFlags::WRITES_DEPTH) && state.info.is_depth_read_only) - || (pipeline.flags.contains(PipelineFlags::WRITES_STENCIL) - && state.info.is_stencil_read_only) - { - return Err(RenderCommandError::IncompatiblePipelineRods.into()); + if pipeline.flags.contains(PipelineFlags::WRITES_DEPTH) && state.info.is_depth_read_only { + return Err(RenderCommandError::IncompatibleDepthAccess(pipeline.error_ident()).into()); + } + if pipeline.flags.contains(PipelineFlags::WRITES_STENCIL) && state.info.is_stencil_read_only { + return Err(RenderCommandError::IncompatibleStencilAccess(pipeline.error_ident()).into()); } state @@ -2582,10 +2579,7 @@ fn execute_bundle( state .info .context - .check_compatible( - &bundle.context, - RenderPassCompatibilityCheckType::RenderBundle, - ) + .check_compatible(&bundle.context, bundle.as_ref()) .map_err(RenderPassErrorInner::IncompatibleBundleTargets)?; if (state.info.is_depth_read_only && !bundle.is_depth_read_only) @@ -2674,7 +2668,7 @@ impl Global { pass: &mut RenderPass, pipeline_id: id::RenderPipelineId, ) -> Result<(), RenderPassError> { - let scope = PassErrorScope::SetPipelineRender(pipeline_id); + let scope = PassErrorScope::SetPipelineRender; let redundant = pass.current_pipeline.set_and_check_redundant(pipeline_id); let base = pass.base_mut(scope)?; diff --git a/wgpu-core/src/command/render_command.rs b/wgpu-core/src/command/render_command.rs index 125e642eec..9ef929c118 100644 --- a/wgpu-core/src/command/render_command.rs +++ b/wgpu-core/src/command/render_command.rs @@ -163,7 +163,7 @@ impl RenderCommand { pipelines_guard .get_owned(pipeline_id) .map_err(|_| RenderPassError { - scope: PassErrorScope::SetPipelineRender(pipeline_id), + scope: PassErrorScope::SetPipelineRender, inner: RenderCommandError::InvalidPipeline(pipeline_id).into(), })?, ), diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 855cea1b42..ab774d87c0 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -4,7 +4,8 @@ use crate::{ hub::Hub, id::{BindGroupLayoutId, PipelineLayoutId}, resource::{ - Buffer, BufferAccessError, BufferAccessResult, BufferMapOperation, ResourceErrorIdent, + Buffer, BufferAccessError, BufferAccessResult, BufferMapOperation, Resource, + ResourceErrorIdent, }, snatch::SnatchGuard, Label, DOWNLEVEL_ERROR_MESSAGE, @@ -69,12 +70,6 @@ impl AttachmentData { } } -#[derive(Debug, Copy, Clone)] -pub enum RenderPassCompatibilityCheckType { - RenderPipeline, - RenderBundle, -} - #[derive(Clone, Debug, Hash, PartialEq)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] pub(crate) struct RenderPassContext { @@ -86,44 +81,44 @@ pub(crate) struct RenderPassContext { #[non_exhaustive] pub enum RenderPassCompatibilityError { #[error( - "Incompatible color attachments at indices {indices:?}: the RenderPass uses textures with formats {expected:?} but the {ty:?} uses attachments with formats {actual:?}", + "Incompatible color attachments at indices {indices:?}: the RenderPass uses textures with formats {expected:?} but the {res} uses attachments with formats {actual:?}", )] IncompatibleColorAttachment { indices: Vec, expected: Vec>, actual: Vec>, - ty: RenderPassCompatibilityCheckType, + res: ResourceErrorIdent, }, #[error( - "Incompatible depth-stencil attachment format: the RenderPass uses a texture with format {expected:?} but the {ty:?} uses an attachment with format {actual:?}", + "Incompatible depth-stencil attachment format: the RenderPass uses a texture with format {expected:?} but the {res} uses an attachment with format {actual:?}", )] IncompatibleDepthStencilAttachment { expected: Option, actual: Option, - ty: RenderPassCompatibilityCheckType, + res: ResourceErrorIdent, }, #[error( - "Incompatible sample count: the RenderPass uses textures with sample count {expected:?} but the {ty:?} uses attachments with format {actual:?}", + "Incompatible sample count: the RenderPass uses textures with sample count {expected:?} but the {res} uses attachments with format {actual:?}", )] IncompatibleSampleCount { expected: u32, actual: u32, - ty: RenderPassCompatibilityCheckType, + res: ResourceErrorIdent, }, - #[error("Incompatible multiview setting: the RenderPass uses setting {expected:?} but the {ty:?} uses setting {actual:?}")] + #[error("Incompatible multiview setting: the RenderPass uses setting {expected:?} but the {res} uses setting {actual:?}")] IncompatibleMultiview { expected: Option, actual: Option, - ty: RenderPassCompatibilityCheckType, + res: ResourceErrorIdent, }, } impl RenderPassContext { // Assumes the renderpass only contains one subpass - pub(crate) fn check_compatible( + pub(crate) fn check_compatible( &self, other: &Self, - ty: RenderPassCompatibilityCheckType, + res: &T, ) -> Result<(), RenderPassCompatibilityError> { if self.attachments.colors != other.attachments.colors { let indices = self @@ -138,7 +133,7 @@ impl RenderPassContext { indices, expected: self.attachments.colors.iter().cloned().collect(), actual: other.attachments.colors.iter().cloned().collect(), - ty, + res: res.error_ident(), }); } if self.attachments.depth_stencil != other.attachments.depth_stencil { @@ -146,7 +141,7 @@ impl RenderPassContext { RenderPassCompatibilityError::IncompatibleDepthStencilAttachment { expected: self.attachments.depth_stencil, actual: other.attachments.depth_stencil, - ty, + res: res.error_ident(), }, ); } @@ -154,14 +149,14 @@ impl RenderPassContext { return Err(RenderPassCompatibilityError::IncompatibleSampleCount { expected: self.sample_count, actual: other.sample_count, - ty, + res: res.error_ident(), }); } if self.multiview != other.multiview { return Err(RenderPassCompatibilityError::IncompatibleMultiview { expected: self.multiview, actual: other.multiview, - ty, + res: res.error_ident(), }); } Ok(()) From 2c3b82b9b4e4525b18af7818760229fc49292463 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Wed, 26 Jun 2024 12:52:11 +0200 Subject: [PATCH 46/47] use `.validate_dynamic_bindings()` in render bundle's `set_bind_group` --- wgpu-core/src/command/bundle.rs | 27 ++++----------------------- wgpu-core/src/command/draw.rs | 2 -- 2 files changed, 4 insertions(+), 25 deletions(-) diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index 081f16a447..1e08bac6f9 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/wgpu-core/src/command/bundle.rs @@ -79,7 +79,7 @@ index format changes. #![allow(clippy::reversed_empty_ranges)] use crate::{ - binding_model::{buffer_binding_type_alignment, BindGroup, BindGroupLayout, PipelineLayout}, + binding_model::{BindError, BindGroup, BindGroupLayout, PipelineLayout}, command::{ BasePass, BindGroupStateChange, ColorAttachmentError, DrawError, MapPassErr, PassErrorScope, RenderCommandError, StateChange, @@ -631,28 +631,7 @@ fn set_bind_group( state.next_dynamic_offset = offsets_range.end; let offsets = &dynamic_offsets[offsets_range.clone()]; - if bind_group.dynamic_binding_info.len() != offsets.len() { - return Err(RenderCommandError::InvalidDynamicOffsetCount { - actual: offsets.len(), - expected: bind_group.dynamic_binding_info.len(), - } - .into()); - } - - // Check for misaligned offsets. - for (offset, info) in offsets - .iter() - .map(|offset| *offset as wgt::BufferAddress) - .zip(bind_group.dynamic_binding_info.iter()) - { - let (alignment, limit_name) = - buffer_binding_type_alignment(&state.device.limits, info.binding_type); - if offset % alignment as u64 != 0 { - return Err( - RenderCommandError::UnalignedBufferOffset(offset, limit_name, alignment).into(), - ); - } - } + bind_group.validate_dynamic_bindings(index, offsets)?; state .buffer_memory_init_actions @@ -1601,6 +1580,8 @@ pub(super) enum RenderBundleErrorInner { Draw(#[from] DrawError), #[error(transparent)] MissingDownlevelFlags(#[from] MissingDownlevelFlags), + #[error(transparent)] + Bind(#[from] BindError), } impl From for RenderBundleErrorInner diff --git a/wgpu-core/src/command/draw.rs b/wgpu-core/src/command/draw.rs index 667efb6537..d43039a896 100644 --- a/wgpu-core/src/command/draw.rs +++ b/wgpu-core/src/command/draw.rs @@ -83,8 +83,6 @@ pub enum RenderCommandError { VertexBufferIndexOutOfRange { index: u32, max: u32 }, #[error("Dynamic buffer offset {0} does not respect device's requested `{1}` limit {2}")] UnalignedBufferOffset(u64, &'static str, u32), - #[error("Number of buffer offsets ({actual}) does not match the number of dynamic bindings ({expected})")] - InvalidDynamicOffsetCount { actual: usize, expected: usize }, #[error("Render pipeline {0:?} is invalid")] InvalidPipeline(id::RenderPipelineId), #[error("QuerySet {0:?} is invalid")] From 5c80956a228a9c9e948828c26722c4a04efb2f2d Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Wed, 26 Jun 2024 13:02:25 +0200 Subject: [PATCH 47/47] remove bind group id from set bind group scope and make sure that we use `ResourceErrorIdent` in all relevant inner errors --- wgpu-core/src/binding_model.rs | 14 ++++++++++---- wgpu-core/src/command/bundle.rs | 2 +- wgpu-core/src/command/compute.rs | 4 ++-- wgpu-core/src/command/compute_command.rs | 2 +- wgpu-core/src/command/mod.rs | 5 +---- wgpu-core/src/command/render.rs | 4 ++-- wgpu-core/src/command/render_command.rs | 2 +- 7 files changed, 18 insertions(+), 15 deletions(-) diff --git a/wgpu-core/src/binding_model.rs b/wgpu-core/src/binding_model.rs index 3443bcf782..68300706a1 100644 --- a/wgpu-core/src/binding_model.rs +++ b/wgpu-core/src/binding_model.rs @@ -8,7 +8,7 @@ use crate::{ init_tracker::{BufferInitTrackerAction, TextureInitTrackerAction}, resource::{ DestroyedResourceError, MissingBufferUsageError, MissingTextureUsageError, ParentDevice, - Resource, ResourceInfo, ResourceType, + Resource, ResourceErrorIdent, ResourceInfo, ResourceType, }, resource_log, snatch::{SnatchGuard, Snatchable}, @@ -771,19 +771,21 @@ pub enum BindingResource<'a> { #[non_exhaustive] pub enum BindError { #[error( - "Bind group {group} expects {expected} dynamic offset{s0}. However {actual} dynamic offset{s1} were provided.", + "{bind_group} {group} expects {expected} dynamic offset{s0}. However {actual} dynamic offset{s1} were provided.", s0 = if *.expected >= 2 { "s" } else { "" }, s1 = if *.actual >= 2 { "s" } else { "" }, )] MismatchedDynamicOffsetCount { + bind_group: ResourceErrorIdent, group: u32, actual: usize, expected: usize, }, #[error( - "Dynamic binding index {idx} (targeting bind group {group}, binding {binding}) with value {offset}, does not respect device's requested `{limit_name}` limit: {alignment}" + "Dynamic binding index {idx} (targeting {bind_group} {group}, binding {binding}) with value {offset}, does not respect device's requested `{limit_name}` limit: {alignment}" )] UnalignedDynamicBinding { + bind_group: ResourceErrorIdent, idx: usize, group: u32, binding: u32, @@ -792,10 +794,11 @@ pub enum BindError { limit_name: &'static str, }, #[error( - "Dynamic binding offset index {idx} with offset {offset} would overrun the buffer bound to bind group {group} -> binding {binding}. \ + "Dynamic binding offset index {idx} with offset {offset} would overrun the buffer bound to {bind_group} {group} -> binding {binding}. \ Buffer size is {buffer_size} bytes, the binding binds bytes {binding_range:?}, meaning the maximum the binding can be offset is {maximum_dynamic_offset} bytes", )] DynamicBindingOutOfBounds { + bind_group: ResourceErrorIdent, idx: usize, group: u32, binding: u32, @@ -895,6 +898,7 @@ impl BindGroup { ) -> Result<(), BindError> { if self.dynamic_binding_info.len() != offsets.len() { return Err(BindError::MismatchedDynamicOffsetCount { + bind_group: self.error_ident(), group: bind_group_index, expected: self.dynamic_binding_info.len(), actual: offsets.len(), @@ -911,6 +915,7 @@ impl BindGroup { buffer_binding_type_alignment(&self.device.limits, info.binding_type); if offset as wgt::BufferAddress % alignment as u64 != 0 { return Err(BindError::UnalignedDynamicBinding { + bind_group: self.error_ident(), group: bind_group_index, binding: info.binding_idx, idx, @@ -922,6 +927,7 @@ impl BindGroup { if offset as wgt::BufferAddress > info.maximum_dynamic_offset { return Err(BindError::DynamicBindingOutOfBounds { + bind_group: self.error_ident(), group: bind_group_index, binding: info.binding_idx, idx, diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index 1e08bac6f9..5f913fd791 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/wgpu-core/src/command/bundle.rs @@ -409,7 +409,7 @@ impl RenderBundleEncoder { num_dynamic_offsets, bind_group_id, } => { - let scope = PassErrorScope::SetBindGroup(bind_group_id); + let scope = PassErrorScope::SetBindGroup; set_bind_group( &mut state, &bind_group_guard, diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index 91aed2421d..73b8838073 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -603,7 +603,7 @@ impl Global { num_dynamic_offsets, bind_group, } => { - let scope = PassErrorScope::SetBindGroup(bind_group.as_info().id()); + let scope = PassErrorScope::SetBindGroup; set_bind_group( &mut state, cmd_buf, @@ -1041,7 +1041,7 @@ impl Global { bind_group_id: id::BindGroupId, offsets: &[DynamicOffset], ) -> Result<(), ComputePassError> { - let scope = PassErrorScope::SetBindGroup(bind_group_id); + let scope = PassErrorScope::SetBindGroup; let base = pass .base .as_mut() diff --git a/wgpu-core/src/command/compute_command.rs b/wgpu-core/src/command/compute_command.rs index 0a37f3d781..eb8ce9fa34 100644 --- a/wgpu-core/src/command/compute_command.rs +++ b/wgpu-core/src/command/compute_command.rs @@ -97,7 +97,7 @@ impl ComputeCommand { num_dynamic_offsets, bind_group: bind_group_guard.get_owned(bind_group_id).map_err(|_| { ComputePassError { - scope: PassErrorScope::SetBindGroup(bind_group_id), + scope: PassErrorScope::SetBindGroup, inner: ComputePassErrorInner::InvalidBindGroupId(bind_group_id), } })?, diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index 7737ec2bea..844691c7c2 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -867,7 +867,7 @@ pub enum PassErrorScope { #[error("In a pass parameter")] Pass(Option), #[error("In a set_bind_group command")] - SetBindGroup(id::BindGroupId), + SetBindGroup, #[error("In a set_pipeline command")] SetPipelineRender, #[error("In a set_pipeline command")] @@ -922,9 +922,6 @@ impl PrettyError for PassErrorScope { Self::Pass(Some(id)) => { fmt.command_buffer_label(&id); } - Self::SetBindGroup(id) => { - fmt.bind_group_label(&id); - } _ => {} } } diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 11312c39d1..133bdad26e 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1530,7 +1530,7 @@ impl Global { num_dynamic_offsets, bind_group, } => { - let scope = PassErrorScope::SetBindGroup(bind_group.as_info().id()); + let scope = PassErrorScope::SetBindGroup; set_bind_group( &mut state, &cmd_buf, @@ -2637,7 +2637,7 @@ impl Global { bind_group_id: id::BindGroupId, offsets: &[DynamicOffset], ) -> Result<(), RenderPassError> { - let scope = PassErrorScope::SetBindGroup(bind_group_id); + let scope = PassErrorScope::SetBindGroup; let base = pass .base .as_mut() diff --git a/wgpu-core/src/command/render_command.rs b/wgpu-core/src/command/render_command.rs index 9ef929c118..3140b78e68 100644 --- a/wgpu-core/src/command/render_command.rs +++ b/wgpu-core/src/command/render_command.rs @@ -153,7 +153,7 @@ impl RenderCommand { num_dynamic_offsets, bind_group: bind_group_guard.get_owned(bind_group_id).map_err(|_| { RenderPassError { - scope: PassErrorScope::SetBindGroup(bind_group_id), + scope: PassErrorScope::SetBindGroup, inner: RenderPassErrorInner::InvalidBindGroup(index), } })?,