From 287d8ee414e1926bab1b1e0591bf7c21939b45e3 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] 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 | 110 ++++++++++-------------- wgpu-core/src/command/render_command.rs | 2 - 5 files changed, 69 insertions(+), 98 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)?; + // 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, + }); + } - // The buffers are different formats - if pipeline_index_format != buffer_index_format { - return Err(DrawError::UnmatchedIndexFormats { - pipeline: pipeline_index_format, - buffer: buffer_index_format, - }); + 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(|_| {