From 2fb8fc34d2ae258fa00ba745a76bcef2537f61cc Mon Sep 17 00:00:00 2001 From: Mikko Lehtonen Date: Sat, 5 Sep 2020 02:54:35 +0300 Subject: [PATCH] Add LabeledContextError Purpose for this is to add more context to an error. create_render_pipeline error is improved, by showing the action (creating render pipeline) and the user provided label, to aid with the investigation. --- wgpu-core/src/device/mod.rs | 96 +++++++++++++++++++++++-------------- wgpu-core/src/lib.rs | 8 ++++ wgpu-core/src/pipeline.rs | 16 ++++++- 3 files changed, 84 insertions(+), 36 deletions(-) diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 78fde0958d..614a814cff 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -2595,7 +2595,7 @@ impl Global { implicit_pipeline_ids: Option>, ) -> Result< (id::RenderPipelineId, pipeline::ImplicitBindGroupCount), - pipeline::CreateRenderPipelineError, + crate::LabeledContextError, > { span!(_guard, INFO, "Device::create_render_pipeline"); @@ -2605,7 +2605,8 @@ impl Global { let samples = { let sc = desc.sample_count; if sc == 0 || sc > 32 || !conv::is_power_of_two(sc) { - return Err(pipeline::CreateRenderPipelineError::InvalidSampleCount(sc)); + return Err(pipeline::CreateRenderPipelineError::InvalidSampleCount(sc) + .with_label(&desc.label)); } sc as u8 }; @@ -2638,7 +2639,8 @@ impl Global { return Err(pipeline::CreateRenderPipelineError::UnalignedVertexStride { index: i as u32, stride: vb_state.stride, - }); + } + .with_label(&desc.label)); } vertex_buffers.alloc().init(hal::pso::VertexBufferDesc { binding: i as u32, @@ -2655,7 +2657,8 @@ impl Global { pipeline::CreateRenderPipelineError::InvalidVertexAttributeOffset { location: attribute.shader_location, offset: attribute.offset, - }, + } + .with_label(&desc.label), ); } attributes.alloc().init(hal::pso::AttributeDesc { @@ -2711,15 +2714,16 @@ impl Global { }; let (device_guard, mut token) = hub.devices.read(&mut token); - let device = device_guard - .get(device_id) - .map_err(|_| DeviceError::Invalid)?; + let device = device_guard.get(device_id).map_err(|_| { + pipeline::CreateRenderPipelineError::from(DeviceError::Invalid).with_label(&desc.label) + })?; if rasterization_state.clamp_depth && !device.features.contains(wgt::Features::DEPTH_CLAMPING) { return Err(pipeline::CreateRenderPipelineError::MissingFeature( wgt::Features::DEPTH_CLAMPING, - )); + ) + .with_label(&desc.label)); } let (raw_pipeline, layout_id, layout_ref_count, derived_bind_group_count) = { @@ -2781,17 +2785,21 @@ impl Global { let shader_module = shader_module_guard .get(desc.vertex_stage.module) - .map_err(|_| pipeline::CreateRenderPipelineError::Stage { - flag, - error: validation::StageError::InvalidModule, + .map_err(|_| { + pipeline::CreateRenderPipelineError::Stage { + flag, + error: validation::StageError::InvalidModule, + } + .with_label(&desc.label) })?; if let Some(ref module) = shader_module.module { let group_layouts = match desc.layout { Some(pipeline_layout_id) => Device::get_introspection_bind_group_layouts( - pipeline_layout_guard - .get(pipeline_layout_id) - .map_err(|_| pipeline::CreateRenderPipelineError::InvalidLayout)?, + pipeline_layout_guard.get(pipeline_layout_id).map_err(|_| { + pipeline::CreateRenderPipelineError::InvalidLayout + .with_label(&desc.label) + })?, &*bgl_guard, ), None => validation::IntrospectionBindGroupLayouts::Derived( @@ -2806,7 +2814,10 @@ impl Global { naga::ShaderStage::Vertex, interface, ) - .map_err(|error| pipeline::CreateRenderPipelineError::Stage { flag, error })?; + .map_err(|error| { + pipeline::CreateRenderPipelineError::Stage { flag, error } + .with_label(&desc.label) + })?; validated_stages |= flag; } @@ -2827,13 +2838,15 @@ impl Global { flag, error: validation::StageError::InvalidModule, } + .with_label(&desc.label) })?; let group_layouts = match desc.layout { Some(pipeline_layout_id) => Device::get_introspection_bind_group_layouts( - pipeline_layout_guard - .get(pipeline_layout_id) - .map_err(|_| pipeline::CreateRenderPipelineError::InvalidLayout)?, + pipeline_layout_guard.get(pipeline_layout_id).map_err(|_| { + pipeline::CreateRenderPipelineError::InvalidLayout + .with_label(&desc.label) + })?, &*bgl_guard, ), None => validation::IntrospectionBindGroupLayouts::Derived( @@ -2852,6 +2865,7 @@ impl Global { ) .map_err(|error| { pipeline::CreateRenderPipelineError::Stage { flag, error } + .with_label(&desc.label) })?; validated_stages |= flag; } @@ -2879,7 +2893,8 @@ impl Global { return Err( pipeline::CreateRenderPipelineError::IncompatibleOutputFormat { index: i as u8, - }, + } + .with_label(&desc.label), ); } } @@ -2889,7 +2904,10 @@ impl Global { None => wgt::ShaderStage::VERTEX, }; if desc.layout.is_none() && !validated_stages.contains(last_stage) { - Err(pipeline::ImplicitLayoutError::ReflectionError(last_stage))? + Err(pipeline::CreateRenderPipelineError::from( + pipeline::ImplicitLayoutError::ReflectionError(last_stage), + ) + .with_label(&desc.label))? } let primitive_assembler = hal::pso::PrimitiveAssemblerDesc::Vertex { @@ -2908,18 +2926,22 @@ impl Global { let (pipeline_layout_id, derived_bind_group_count) = match desc.layout { Some(id) => (id, 0), - None => self.derive_pipeline_layout( - device, - device_id, - implicit_pipeline_ids, - derived_group_layouts, - &mut *bgl_guard, - &mut *pipeline_layout_guard, - )?, + None => self + .derive_pipeline_layout( + device, + device_id, + implicit_pipeline_ids, + derived_group_layouts, + &mut *bgl_guard, + &mut *pipeline_layout_guard, + ) + .map_err(|e| { + pipeline::CreateRenderPipelineError::from(e).with_label(&desc.label) + })?, }; - let layout = pipeline_layout_guard - .get(pipeline_layout_id) - .map_err(|_| pipeline::CreateRenderPipelineError::InvalidLayout)?; + let layout = pipeline_layout_guard.get(pipeline_layout_id).map_err(|_| { + pipeline::CreateRenderPipelineError::InvalidLayout.with_label(&desc.label) + })?; let mut render_pass_cache = device.render_passes.lock(); let pipeline_desc = hal::pso::GraphicsPipelineDesc { @@ -2936,9 +2958,10 @@ impl Global { main_pass: match render_pass_cache.entry(rp_key) { Entry::Occupied(e) => e.into_mut(), Entry::Vacant(e) => { - let pass = device - .create_compatible_render_pass(e.key()) - .or(Err(DeviceError::OutOfMemory))?; + let pass = device.create_compatible_render_pass(e.key()).or(Err( + pipeline::CreateRenderPipelineError::from(DeviceError::OutOfMemory) + .with_label(&desc.label), + ))?; e.insert(pass) } }, @@ -2952,7 +2975,10 @@ impl Global { .raw .create_graphics_pipeline(&pipeline_desc, None) .map_err(|err| match err { - hal::pso::CreationError::OutOfMemory(_) => DeviceError::OutOfMemory, + hal::pso::CreationError::OutOfMemory(_) => { + pipeline::CreateRenderPipelineError::from(DeviceError::OutOfMemory) + .with_label(&desc.label) + } _ => panic!("failed to create graphics pipeline: {}", err), })? }; diff --git a/wgpu-core/src/lib.rs b/wgpu-core/src/lib.rs index f111d7ec9f..0bace30797 100644 --- a/wgpu-core/src/lib.rs +++ b/wgpu-core/src/lib.rs @@ -238,3 +238,11 @@ fn test_default_limits() { let limits = wgt::Limits::default(); assert!(limits.max_bind_groups <= MAX_BIND_GROUPS as u32); } + +#[derive(Clone, Debug, thiserror::Error)] +#[error("{description} (label: {label:?})")] +pub struct LabeledContextError { + source: E, + description: &'static str, + label: Option, +} diff --git a/wgpu-core/src/pipeline.rs b/wgpu-core/src/pipeline.rs index 0db85a69a6..8b527e38fc 100644 --- a/wgpu-core/src/pipeline.rs +++ b/wgpu-core/src/pipeline.rs @@ -183,13 +183,27 @@ pub enum CreateRenderPipelineError { }, #[error("missing required device features {0:?}")] MissingFeature(wgt::Features), - #[error("error in stage {flag:?}: {error}")] + #[error("error in stage {flag:?}")] Stage { flag: wgt::ShaderStage, + #[source] error: StageError, }, } +impl CreateRenderPipelineError { + pub(crate) fn with_label<'a>( + self, + label: &Option>, + ) -> crate::LabeledContextError { + crate::LabeledContextError { + source: self, + description: "Creating render pipeline", + label: label.as_ref().map(|inner| inner.to_string()), + } + } +} + bitflags::bitflags! { #[repr(transparent)] pub struct PipelineFlags: u32 {