From 151d521ad9557f37a83b3584aa74fdf8bff4b0ab Mon Sep 17 00:00:00 2001 From: DasEtwas <18222134+DasEtwas@users.noreply.github.com> Date: Mon, 11 Jan 2021 20:13:14 +0100 Subject: [PATCH 1/2] Fix and simplify attachment dimension mismatch check This also improves error messages with detailed dimension and attachment type name info which should help developers can hopefully recognize. --- wgpu-core/src/command/render.rs | 83 ++++++++++++++++++++++++++++----- 1 file changed, 71 insertions(+), 12 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 26e7b62c10..fe4f358a7c 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -387,10 +387,22 @@ pub enum RenderPassErrorInner { Encoder(#[from] CommandEncoderError), #[error("attachment texture view {0:?} is invalid")] InvalidAttachment(id::TextureViewId), - #[error("there are attachments")] - NoAttachments, - #[error("attachments have different sizes")] - MismatchAttachments, + #[error("necessary attachments are missing")] + MissingAttachments, + #[error("color and/or depth attachments have differing sizes: a {mismatching_attachment_type_name} attachment of size {mismatching_dimensions_width}x{mismatching_dimensions_height} is not compatible with a {previous_attachment_type_name} attachment's size ({previous_dimensions_height}x{previous_dimensions_width})", + previous_dimensions_width = previous_dimensions.0, + previous_dimensions_height = previous_dimensions.1, + mismatching_dimensions_width = mismatching_dimensions.0, + mismatching_dimensions_height = mismatching_dimensions.1, + )] + AttachmentsDimensionMismatch { + /// depth or color + previous_attachment_type_name: &'static str, + /// depth or color + mismatching_attachment_type_name: &'static str, + previous_dimensions: (u32, u32), + mismatching_dimensions: (u32, u32), + }, #[error("attachment's sample count {0} is invalid")] InvalidSampleCount(u8), #[error("attachment with resolve target must be multi-sampled")] @@ -526,9 +538,12 @@ impl<'a, B: GfxBackend> RenderPassInfo<'a, B> { let mut render_attachments = AttachmentDataVec::::new(); + // need to be initialized to None!, check `AttachmentsDimensionMismatch` error let mut attachment_width = None; let mut attachment_height = None; - let mut valid_attachment = true; + let mut attachment_type_name = None; + + let mut mismatching_dimensions = None; let mut extent = None; let mut sample_count = 0; @@ -567,6 +582,25 @@ impl<'a, B: GfxBackend> RenderPassInfo<'a, B> { .use_extend(&*view_guard, at.attachment, (), ()) .map_err(|_| RenderPassErrorInner::InvalidAttachment(at.attachment))?; add_view(view)?; + + // get the width and height set by another attachment, then check if it matches + let (previous_width, previous_height) = ( + *attachment_width.get_or_insert(view.extent.width), + *attachment_height.get_or_insert(view.extent.height), + ); + + if previous_width != view.extent.width || previous_height != view.extent.height + { + mismatching_dimensions = Some(( + "depth", + (view.extent.width, view.extent.height), + (previous_width, previous_height), + )); + } else { + // this attachment's size was successfully inserted into `attachment_width` and `attachment_height` + attachment_type_name = Some("depth"); + } + depth_stencil_aspects = view.aspects; let source_id = match view.inner { @@ -625,9 +659,22 @@ impl<'a, B: GfxBackend> RenderPassInfo<'a, B> { .map_err(|_| RenderPassErrorInner::InvalidAttachment(at.attachment))?; add_view(view)?; - valid_attachment &= *attachment_width.get_or_insert(view.extent.width) - == view.extent.width - && *attachment_height.get_or_insert(view.extent.height) == view.extent.height; + // get the width and height set by another attachment, then check if it matches + let (previous_width, previous_height) = ( + *attachment_width.get_or_insert(view.extent.width), + *attachment_height.get_or_insert(view.extent.height), + ); + + if previous_width != view.extent.width || previous_height != view.extent.height { + mismatching_dimensions = Some(( + "color", + (view.extent.width, view.extent.height), + (previous_width, previous_height), + )); + } else { + // this attachment's size was successfully inserted into `attachment_width` and `attachment_height` + attachment_type_name = Some("color"); + } let layouts = match view.inner { TextureViewInner::Native { ref source_id, .. } => { @@ -685,8 +732,20 @@ impl<'a, B: GfxBackend> RenderPassInfo<'a, B> { colors.push((color_at, hal::image::Layout::ColorAttachmentOptimal)); } - if !valid_attachment { - return Err(RenderPassErrorInner::MismatchAttachments); + if let Some(( + mismatching_attachment_type_name, + previous_dimensions, + mismatching_dimensions, + )) = mismatching_dimensions + { + return Err(RenderPassErrorInner::AttachmentsDimensionMismatch { + // okay to unwrap: for `mismatching_dimensions` to be set, `attachment_{width/height}` must have been + // set, which includes `attachment_type_name` being set in a later `else` block + previous_attachment_type_name: attachment_type_name.unwrap(), + mismatching_attachment_type_name, + previous_dimensions, + mismatching_dimensions, + }); } for resolve_target in color_attachments.iter().flat_map(|at| at.resolve_target) { @@ -994,8 +1053,8 @@ impl<'a, B: GfxBackend> RenderPassInfo<'a, B> { used_swapchain_with_framebuffer, is_ds_read_only, extent: wgt::Extent3d { - width: attachment_width.ok_or(RenderPassErrorInner::NoAttachments)?, - height: attachment_height.ok_or(RenderPassErrorInner::NoAttachments)?, + width: attachment_width.ok_or(RenderPassErrorInner::MissingAttachments)?, + height: attachment_height.ok_or(RenderPassErrorInner::MissingAttachments)?, depth: 1, }, }) From 93f68add2751f805e732687083df89548935a277 Mon Sep 17 00:00:00 2001 From: DasEtwas <18222134+DasEtwas@users.noreply.github.com> Date: Mon, 11 Jan 2021 23:46:26 +0100 Subject: [PATCH 2/2] Decrease error variant code verbosity, apply reviews --- wgpu-core/src/command/render.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index fe4f358a7c..7c7503b149 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -389,11 +389,7 @@ pub enum RenderPassErrorInner { InvalidAttachment(id::TextureViewId), #[error("necessary attachments are missing")] MissingAttachments, - #[error("color and/or depth attachments have differing sizes: a {mismatching_attachment_type_name} attachment of size {mismatching_dimensions_width}x{mismatching_dimensions_height} is not compatible with a {previous_attachment_type_name} attachment's size ({previous_dimensions_height}x{previous_dimensions_width})", - previous_dimensions_width = previous_dimensions.0, - previous_dimensions_height = previous_dimensions.1, - mismatching_dimensions_width = mismatching_dimensions.0, - mismatching_dimensions_height = mismatching_dimensions.1, + #[error("color and/or depth attachments have differing sizes: a {mismatching_attachment_type_name} attachment of size {mismatching_dimensions:?} is not compatible with a {previous_attachment_type_name} attachment's size {previous_dimensions:?}", )] AttachmentsDimensionMismatch { /// depth or color @@ -538,10 +534,9 @@ impl<'a, B: GfxBackend> RenderPassInfo<'a, B> { let mut render_attachments = AttachmentDataVec::::new(); - // need to be initialized to None!, check `AttachmentsDimensionMismatch` error let mut attachment_width = None; let mut attachment_height = None; - let mut attachment_type_name = None; + let mut attachment_type_name = ""; let mut mismatching_dimensions = None; @@ -598,7 +593,7 @@ impl<'a, B: GfxBackend> RenderPassInfo<'a, B> { )); } else { // this attachment's size was successfully inserted into `attachment_width` and `attachment_height` - attachment_type_name = Some("depth"); + attachment_type_name = "depth"; } depth_stencil_aspects = view.aspects; @@ -673,7 +668,7 @@ impl<'a, B: GfxBackend> RenderPassInfo<'a, B> { )); } else { // this attachment's size was successfully inserted into `attachment_width` and `attachment_height` - attachment_type_name = Some("color"); + attachment_type_name = "color"; } let layouts = match view.inner { @@ -741,7 +736,7 @@ impl<'a, B: GfxBackend> RenderPassInfo<'a, B> { return Err(RenderPassErrorInner::AttachmentsDimensionMismatch { // okay to unwrap: for `mismatching_dimensions` to be set, `attachment_{width/height}` must have been // set, which includes `attachment_type_name` being set in a later `else` block - previous_attachment_type_name: attachment_type_name.unwrap(), + previous_attachment_type_name: attachment_type_name, mismatching_attachment_type_name, previous_dimensions, mismatching_dimensions,