Merge #843

843: Reformat the alignment errors, add a stride check r=GabrielMajeri a=kvark

**Connections**
Addresses [one of the things](https://community-tc.services.mozilla.com/tasks/JQlmdXSiSK2ufM7mX8F1Tg/runs/0/logs/https%3A%2F%2Fcommunity-tc.services.mozilla.com%2Fapi%2Fqueue%2Fv1%2Ftask%2FJQlmdXSiSK2ufM7mX8F1Tg%2Fruns%2F0%2Fartifacts%2Fpublic%2Flogs%2Flive.log#L1999) Servo hit on the CTS

**Description**
This change affects the unaligned* error messages: instead of printing the exact value, they now refer to the constant in the code. On one hand, this involves an extra redirection for anyone who hits this. But on the other hand, it makes them aware of the actual constants they should be respecting. I think it's better in the long term, especially if we change any of these.

**Testing**
Not tested, but should work!

r? @GabrielMajeri 

Co-authored-by: Dzmitry Malyshau <dmalyshau@mozilla.com>

wgpu-core/src/binding_model.rs

@@ -66,10 +66,7 @@ pub enum CreateBindGroupError {
     SingleBindingExpected,
     #[error("unable to create a bind group with a swap chain image")]
     SwapChainImage,
-    #[error(
-        "buffer offset {0} must be a multiple of {}",
-        wgt::BIND_BUFFER_ALIGNMENT
-    )]
+    #[error("buffer offset {0} does not respect `BIND_BUFFER_ALIGNMENT`")]
     UnalignedBufferOffset(wgt::BufferAddress),
     #[error("uniform buffer binding range exceeds `max_uniform_buffer_binding_size` limit")]
     UniformBufferRangeTooLarge,
@@ -320,10 +317,7 @@ pub enum PushConstantUploadError {
         actual: wgt::ShaderStage,
         unmatched: wgt::ShaderStage,
     },
-    #[error(
-        "provided push constant offset {0} must be aligned to {}",
-        wgt::PUSH_CONSTANT_ALIGNMENT
-    )]
+    #[error("provided push constant offset {0} does not respect `PUSH_CONSTANT_ALIGNMENT`")]
     Unaligned(u32),
 }
 
@@ -448,8 +442,7 @@ pub enum BindError {
     #[error("number of dynamic offsets ({actual}) doesn't match the number of dynamic bindings in the bind group layout ({expected})")]
     MismatchedDynamicOffsetCount { actual: usize, expected: usize },
     #[error(
-        "dynamic binding at index {idx}: offset {offset} must be aligned to {}",
-        wgt::BIND_BUFFER_ALIGNMENT
+        "dynamic binding at index {idx}: offset {offset} does not respect `BIND_BUFFER_ALIGNMENT`"
     )]
     UnalignedDynamicBinding { idx: usize, offset: u32 },
     #[error("dynamic binding at index {idx} with offset {offset} would overrun the buffer (limit: {max})")]

wgpu-core/src/command/bundle.rs

@@ -600,10 +600,7 @@ impl State {
 pub enum RenderCommandError {
     #[error("bind group index {index} is greater than the device's requested `max_bind_group` limit {max}")]
     BindGroupIndexOutOfRange { index: u8, max: u32 },
-    #[error(
-        "dynamic buffer offset {0} is not a multiple of the required buffer alignment {}",
-        wgt::BIND_BUFFER_ALIGNMENT
-    )]
+    #[error("dynamic buffer offset {0} does not respect `BIND_BUFFER_ALIGNMENT`")]
     UnalignedBufferOffset(u64),
     #[error("number of buffer offsets ({actual}) does not match the number of dynamic bindings ({expected})")]
     InvalidDynamicOffsetCount { actual: usize, expected: usize },

wgpu-core/src/command/transfer.rs

@@ -38,15 +38,15 @@ pub enum TransferError {
     BufferOverrun,
     #[error("buffer offset is not aligned to block size")]
     UnalignedBufferOffset,
-    #[error("copy size is not a multiple of block size")]
+    #[error("copy size does not respect `COPY_BUFFER_ALIGNMENT`")]
     UnalignedCopySize,
-    #[error("copy width is not a multiple of block size")]
+    #[error("copy width is not a multiple of block width")]
     UnalignedCopyWidth,
-    #[error("copy height is not a multiple of block size")]
+    #[error("copy height is not a multiple of block height")]
     UnalignedCopyHeight,
-    #[error("bytes per row is not a multiple of the required alignment")]
+    #[error("bytes per row does not respect `COPY_BYTES_PER_ROW_ALIGNMENT`")]
     UnalignedBytesPerRow,
-    #[error("number of rows per image is not a multiple of the required alignment")]
+    #[error("number of rows per image is not a multiple of block height")]
     UnalignedRowsPerImage,
     #[error("number of bytes per row is less than the number of bytes in a complete row")]
     InvalidBytesPerRow,

wgpu-core/src/device/mod.rs

@@ -1430,13 +1430,19 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             }
         }
 
-        let mut count_validator = binding_model::BindingTypeMaxCountValidator::default();
-        let pipeline_layout = {
+        let layout = {
+            let mut count_validator = binding_model::BindingTypeMaxCountValidator::default();
             let (bind_group_layout_guard, _) = hub.bind_group_layouts.read(&mut token);
+
+            // validate total resource counts
             for &id in desc.bind_group_layouts.iter() {
                 let bind_group_layout = &bind_group_layout_guard[id];
                 count_validator.merge(&bind_group_layout.count_validator);
             }
+            count_validator
+                .validate(&device.limits)
+                .map_err(CreatePipelineLayoutError::TooManyBindings)?;
+
             let descriptor_set_layouts = desc
                 .bind_group_layouts
                 .iter()
@@ -1445,35 +1451,29 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                 .push_constant_ranges
                 .iter()
                 .map(|pc| (conv::map_shader_stage_flags(pc.stages), pc.range.clone()));
-            unsafe {
-                device
-                    .raw
-                    .create_pipeline_layout(descriptor_set_layouts, push_constants)
-                    .or(Err(binding_model::CreatePipelineLayoutError::OutOfMemory))?
-            }
-        };
-        count_validator
-            .validate(&device.limits)
-            .map_err(CreatePipelineLayoutError::TooManyBindings)?;
 
-        let layout = binding_model::PipelineLayout {
-            raw: pipeline_layout,
-            device_id: Stored {
-                value: device_id,
-                ref_count: device.life_guard.add_ref(),
-            },
-            life_guard: LifeGuard::new(),
-            bind_group_layout_ids: {
-                let (bind_group_layout_guard, _) = hub.bind_group_layouts.read(&mut token);
-                desc.bind_group_layouts
+            binding_model::PipelineLayout {
+                raw: unsafe {
+                    device
+                        .raw
+                        .create_pipeline_layout(descriptor_set_layouts, push_constants)
+                        .or(Err(binding_model::CreatePipelineLayoutError::OutOfMemory))?
+                },
+                device_id: Stored {
+                    value: device_id,
+                    ref_count: device.life_guard.add_ref(),
+                },
+                life_guard: LifeGuard::new(),
+                bind_group_layout_ids: desc
+                    .bind_group_layouts
                     .iter()
                     .map(|&id| {
                         bind_group_layout_guard[id].multi_ref_count.inc();
                         id
                     })
-                    .collect()
-            },
-            push_constant_ranges: desc.push_constant_ranges.iter().cloned().collect(),
+                    .collect(),
+                push_constant_ranges: desc.push_constant_ranges.iter().cloned().collect(),
+            }
         };
 
         let id = hub
@@ -2166,6 +2166,12 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             if vb_state.attributes.is_empty() {
                 continue;
             }
+            if vb_state.stride % wgt::VERTEX_STRIDE_ALIGNMENT != 0 {
+                return Err(pipeline::RenderPipelineError::UnalignedVertexStride {
+                    index: i as u32,
+                    stride: vb_state.stride,
+                });
+            }
             vertex_buffers.alloc().init(hal::pso::VertexBufferDesc {
                 binding: i as u32,
                 stride: vb_state.stride as u32,
@@ -3070,7 +3076,7 @@ pub enum BufferAccessError {
     NotMapped,
     #[error("not enough memory left")]
     OutOfMemory,
-    #[error("buffer map range is not aligned to {}", wgt::COPY_BUFFER_ALIGNMENT)]
+    #[error("buffer map range does not respect `COPY_BUFFER_ALIGNMENT`")]
     UnalignedRange,
 }
 

wgpu-core/src/device/queue.rs

@@ -579,21 +579,19 @@ pub enum PrepareStageError {
 #[derive(Clone, Debug, Error)]
 pub enum QueueWriteBufferError {
     #[error("write buffer with indices {start}..{end} would overrun buffer of size {size}")]
-    BufferOverrun { start: u64, end: u64, size: u64 },
+    BufferOverrun {
+        start: wgt::BufferAddress,
+        end: wgt::BufferAddress,
+        size: wgt::BufferAddress,
+    },
     #[error(transparent)]
     MissingBufferUsage(#[from] MissingBufferUsageError),
     #[error(transparent)]
     Stage(#[from] PrepareStageError),
-    #[error(
-        "buffer offset {0} must be a multiple of {}",
-        wgt::COPY_BUFFER_ALIGNMENT
-    )]
-    UnalignedBufferOffset(u64),
-    #[error(
-        "buffer write size {0} must be a multiple of {}",
-        wgt::COPY_BUFFER_ALIGNMENT
-    )]
-    UnalignedDataSize(u64),
+    #[error("buffer offset {0} does not respect `COPY_BUFFER_ALIGNMENT`")]
+    UnalignedBufferOffset(wgt::BufferAddress),
+    #[error("buffer write size {0} does not respect `COPY_BUFFER_ALIGNMENT`")]
+    UnalignedDataSize(wgt::BufferAddress),
 }
 
 #[derive(Clone, Debug, Error)]

wgpu-core/src/pipeline.rs

@@ -63,6 +63,8 @@ pub enum RenderPipelineError {
     IncompatibleOutputFormat { index: u8 },
     #[error("invalid sample count {0}")]
     InvalidSampleCount(u32),
+    #[error("vertex buffer {index} stride {stride} does not respect `VERTEX_STRIDE_ALIGNMENT`")]
+    UnalignedVertexStride { index: u32, stride: BufferAddress },
     #[error("vertex attribute at location {location} has invalid offset {offset}")]
     InvalidVertexAttributeOffset {
         location: wgt::ShaderLocation,

wgpu-types/src/lib.rs

@@ -27,6 +27,8 @@ pub const COPY_BYTES_PER_ROW_ALIGNMENT: u32 = 256;
 pub const BIND_BUFFER_ALIGNMENT: BufferAddress = 256;
 /// Buffer to buffer copy offsets and sizes must be aligned to this number.
 pub const COPY_BUFFER_ALIGNMENT: BufferAddress = 4;
+/// Vertex buffer strides have to be aligned to this number.
+pub const VERTEX_STRIDE_ALIGNMENT: BufferAddress = 4;
 /// Alignment all push constants need
 pub const PUSH_CONSTANT_ALIGNMENT: u32 = 4;