From 2f0d62c54c22c60cfa9cc09a2838daee4f3be5f6 Mon Sep 17 00:00:00 2001 From: Paul Kernfeld Date: Sat, 2 May 2020 13:16:09 -0400 Subject: [PATCH] Make assertions more verbose in src/command This contributes to #485 --- wgpu-core/src/command/mod.rs | 16 ++++++-- wgpu-core/src/command/render.rs | 63 ++++++++++++++++++++++++------- wgpu-core/src/command/transfer.rs | 52 +++++++++++++++++++++---- 3 files changed, 106 insertions(+), 25 deletions(-) diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index 8a57b2cf4c..8716eddda3 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -46,7 +46,12 @@ impl PhantomSlice { let aligned = pointer.add(align_offset); let size = count * mem::size_of::(); let end = aligned.add(size); - assert!(end <= bound); + assert!( + end <= bound, + "End of phantom slice ({:?}) exceeds bound ({:?})", + end, + bound + ); (end, slice::from_raw_parts(aligned as *const T, count)) } } @@ -90,7 +95,12 @@ impl RawPass { pub unsafe fn into_vec(self) -> (Vec, id::CommandEncoderId) { let size = self.size(); - assert!(size <= self.capacity); + assert!( + size <= self.capacity, + "Size of RawPass ({}) exceeds capacity ({})", + size, + self.capacity + ); let vec = Vec::from_raw_parts(self.base, size, self.capacity); (vec, self.parent) } @@ -236,7 +246,7 @@ impl Global { //TODO: actually close the last recorded command buffer let (mut comb_guard, _) = hub.command_buffers.write(&mut token); let comb = &mut comb_guard[encoder_id]; - assert!(comb.is_recording); + assert!(comb.is_recording, "Command buffer must be recording"); comb.is_recording = false; // stop tracking the swapchain image, if used if let Some((ref sc_id, _)) = comb.used_swap_chain { diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 05facd7741..b3aa0b91db 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -326,7 +326,12 @@ impl Global { let raw_data_end = unsafe { raw_data.as_ptr().add(raw_data.len()) }; let mut targets: RawRenderTargets = unsafe { mem::zeroed() }; - assert!(unsafe { peeker.add(RawRenderTargets::max_size()) <= raw_data_end }); + assert!( + unsafe { peeker.add(RawRenderTargets::max_size()) <= raw_data_end }, + "RawRenderTargets (size {}) is too big to fit within raw_data (size {})", + RawRenderTargets::max_size(), + raw_data.len() + ); peeker = unsafe { RawRenderTargets::peek_from(peeker, &mut targets) }; let color_attachments = targets @@ -401,7 +406,7 @@ impl Global { .use_extend(&*view_guard, at.attachment, (), ()) .unwrap(); if let Some(ex) = extent { - assert_eq!(ex, view.extent); + assert_eq!(ex, view.extent, "Extent state must match extent from view"); } else { extent = Some(view.extent); } @@ -455,7 +460,7 @@ impl Global { .use_extend(&*view_guard, at.attachment, (), ()) .unwrap(); if let Some(ex) = extent { - assert_eq!(ex, view.extent); + assert_eq!(ex, view.extent, "Extent state must match extent from view"); } else { extent = Some(view.extent); } @@ -481,7 +486,10 @@ impl Global { } TextureViewInner::SwapChain { ref source_id, .. } => { if let Some((ref sc_id, _)) = cmb.used_swap_chain { - assert_eq!(source_id.value, sc_id.value); + assert_eq!( + source_id.value, sc_id.value, + "Texture view's swap chain must match swap chain in use" + ); } else { assert!(used_swap_chain.is_none()); used_swap_chain = Some(source_id.clone()); @@ -513,7 +521,11 @@ impl Global { .views .use_extend(&*view_guard, resolve_target, (), ()) .unwrap(); - assert_eq!(extent, Some(view.extent)); + assert_eq!( + extent, + Some(view.extent), + "Extent state must match extent from view" + ); assert_eq!( view.samples, 1, "All resolve_targets must have a sample_count of 1" @@ -536,7 +548,10 @@ impl Global { } TextureViewInner::SwapChain { ref source_id, .. } => { if let Some((ref sc_id, _)) = cmb.used_swap_chain { - assert_eq!(source_id.value, sc_id.value); + assert_eq!( + source_id.value, sc_id.value, + "Texture view's swap chain must match swap chain in use" + ); } else { assert!(used_swap_chain.is_none()); used_swap_chain = Some(source_id.clone()); @@ -569,7 +584,11 @@ impl Global { for (source_id, view_range, consistent_usage) in output_attachments { let texture = &texture_guard[source_id.value]; - assert!(texture.usage.contains(TextureUsage::OUTPUT_ATTACHMENT)); + assert!( + texture.usage.contains(TextureUsage::OUTPUT_ATTACHMENT), + "Texture usage {:?} must contain the usage flag OUTPUT_ATTACHMENT", + texture.usage + ); let usage = consistent_usage.unwrap_or(TextureUsage::OUTPUT_ATTACHMENT); // this is important to record the `first` state. @@ -617,8 +636,11 @@ impl Global { } else { let sample_count_check = view_guard[color_attachments[i].attachment].samples; - assert!(sample_count_check > 1, - "RenderPassColorAttachmentDescriptor with a resolve_target must have an attachment with sample_count > 1"); + assert!( + sample_count_check > 1, + "RenderPassColorAttachmentDescriptor with a resolve_target must have an attachment with sample_count > 1, had a sample count of {}", + sample_count_check + ); resolve_ids.push(( attachment_index, hal::image::Layout::ColorAttachmentOptimal, @@ -819,7 +841,12 @@ impl Global { first_instance: 0, }; loop { - assert!(unsafe { peeker.add(RenderCommand::max_size()) } <= raw_data_end); + assert!( + unsafe { peeker.add(RenderCommand::max_size()) <= raw_data_end }, + "RenderCommand (size {}) is too big to fit within raw_data (size {})", + RenderCommand::max_size(), + raw_data.len() + ); peeker = unsafe { RenderCommand::peek_from(peeker, &mut command) }; match command { RenderCommand::SetBindGroup { @@ -1095,11 +1122,15 @@ impl Global { state.is_ready().unwrap(); assert!( first_vertex + vertex_count <= state.vertex.vertex_limit, - "Vertex out of range!" + "Vertex {} extends beyond limit {}", + first_vertex + vertex_count, + state.vertex.vertex_limit ); assert!( first_instance + instance_count <= state.vertex.instance_limit, - "Instance out of range!" + "Instance {} extends beyond limit {}", + first_instance + instance_count, + state.vertex.instance_limit ); unsafe { @@ -1121,11 +1152,15 @@ impl Global { //TODO: validate that base_vertex + max_index() is within the provided range assert!( first_index + index_count <= state.index.limit, - "Index out of range!" + "Index {} extends beyond limit {}", + first_index + index_count, + state.index.limit ); assert!( first_instance + instance_count <= state.vertex.instance_limit, - "Instance out of range!" + "Instance {} extends beyond limit {}", + first_instance + instance_count, + state.vertex.instance_limit ); unsafe { diff --git a/wgpu-core/src/command/transfer.rs b/wgpu-core/src/command/transfer.rs index 7fd861f544..d9c79d4284 100644 --- a/wgpu-core/src/command/transfer.rs +++ b/wgpu-core/src/command/transfer.rs @@ -93,7 +93,11 @@ impl Global { cmb.trackers .buffers .use_replace(&*buffer_guard, source, (), BufferUsage::COPY_SRC); - assert!(src_buffer.usage.contains(BufferUsage::COPY_SRC)); + assert!( + src_buffer.usage.contains(BufferUsage::COPY_SRC), + "Source buffer usage {:?} must contain usage flag COPY_SRC", + src_buffer.usage + ); barriers.extend(src_pending.map(|pending| pending.into_hal(src_buffer))); let (dst_buffer, dst_pending) = cmb.trackers.buffers.use_replace( @@ -102,7 +106,11 @@ impl Global { (), BufferUsage::COPY_DST, ); - assert!(dst_buffer.usage.contains(BufferUsage::COPY_DST)); + assert!( + dst_buffer.usage.contains(BufferUsage::COPY_DST), + "Destination buffer usage {:?} must contain usage flag COPY_DST", + dst_buffer.usage + ); barriers.extend(dst_pending.map(|pending| pending.into_hal(dst_buffer))); let region = hal::command::BufferCopy { @@ -159,7 +167,13 @@ impl Global { .bits as u32 / BITS_PER_BYTE; let buffer_width = source.bytes_per_row / bytes_per_texel; - assert_eq!(source.bytes_per_row % bytes_per_texel, 0); + assert_eq!( + source.bytes_per_row % bytes_per_texel, + 0, + "Source bytes per row ({}) must be a multiple of bytes per texel ({})", + source.bytes_per_row, + bytes_per_texel + ); let region = hal::command::BufferImageCopy { buffer_offset: source.offset, buffer_width, @@ -206,7 +220,11 @@ impl Global { source.to_selector(aspects), TextureUsage::COPY_SRC, ); - assert!(src_texture.usage.contains(TextureUsage::COPY_SRC)); + assert!( + src_texture.usage.contains(TextureUsage::COPY_SRC), + "Source texture usage ({:?}) must contain usage flag COPY_SRC", + src_texture.usage + ); let src_barriers = src_pending.map(|pending| pending.into_hal(src_texture)); let (dst_buffer, dst_barriers) = cmb.trackers.buffers.use_replace( @@ -215,7 +233,11 @@ impl Global { (), BufferUsage::COPY_DST, ); - assert!(dst_buffer.usage.contains(BufferUsage::COPY_DST)); + assert!( + dst_buffer.usage.contains(BufferUsage::COPY_DST), + "Destination buffer usage {:?} must contain usage flag COPY_DST", + dst_buffer.usage + ); let dst_barrier = dst_barriers.map(|pending| pending.into_hal(dst_buffer)); let bytes_per_texel = conv::map_texture_format(src_texture.format, cmb.private_features) @@ -223,7 +245,13 @@ impl Global { .bits as u32 / BITS_PER_BYTE; let buffer_width = destination.bytes_per_row / bytes_per_texel; - assert_eq!(destination.bytes_per_row % bytes_per_texel, 0); + assert_eq!( + destination.bytes_per_row % bytes_per_texel, + 0, + "Destination bytes per row ({}) must be a multiple of bytes per texel ({})", + destination.bytes_per_row, + bytes_per_texel + ); let region = hal::command::BufferImageCopy { buffer_offset: destination.offset, buffer_width, @@ -275,7 +303,11 @@ impl Global { source.to_selector(aspects), TextureUsage::COPY_SRC, ); - assert!(src_texture.usage.contains(TextureUsage::COPY_SRC)); + assert!( + src_texture.usage.contains(TextureUsage::COPY_SRC), + "Source texture usage {:?} must contain usage flag COPY_SRC", + src_texture.usage + ); barriers.extend(src_pending.map(|pending| pending.into_hal(src_texture))); let (dst_texture, dst_pending) = cmb.trackers.textures.use_replace( @@ -284,7 +316,11 @@ impl Global { destination.to_selector(aspects), TextureUsage::COPY_DST, ); - assert!(dst_texture.usage.contains(TextureUsage::COPY_DST)); + assert!( + dst_texture.usage.contains(TextureUsage::COPY_DST), + "Destination texture usage {:?} must contain usage flag COPY_DST", + dst_texture.usage + ); barriers.extend(dst_pending.map(|pending| pending.into_hal(dst_texture))); let region = hal::command::ImageCopy {