From a2c60cea4f92bcd96f0e56cf3f9360f0a56e9527 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] 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(())