WIP: refactor(core)!: factor out BufferOverrunError type

Author: ErichDonGubler

wgpu-core/src/command/transfer.rs

@@ -37,34 +37,68 @@ pub enum CopySide {
     Destination,
 }
 
-/// Error encountered while attempting a data transfer.
+/// Error encountered while checking offsets against a buffer.
 #[derive(Clone, Debug, Error)]
 #[non_exhaustive]
-pub enum TransferError {
-    #[error("Source and destination cannot be the same buffer")]
-    SameSourceDestinationBuffer,
-    #[error(transparent)]
-    MissingBufferUsage(#[from] MissingBufferUsageError),
-    #[error(transparent)]
-    MissingTextureUsage(#[from] MissingTextureUsageError),
-    #[error("Start offset ({offset}) is out-of-bounds for buffer of size {buffer_size}")]
-    BufferStartOffsetOverrun {
+pub enum BufferOverrunError {
+    #[error("start offset ({offset}) is out-of-bounds for buffer of size {buffer_size}")]
+    StartOffset {
         offset: BufferAddress,
         buffer_size: BufferAddress,
-        side: CopySide,
     },
     #[error(
-        "End offset (start at {} + size of {}) is out-of-bounds for buffer of size {}",
+        "end offset (start at {} + size of {}) is out-of-bounds for buffer of size {}",
         offset,
         size,
         buffer_size
     )]
-    BufferEndOffsetOverrun {
+    EndOffset {
         offset: BufferAddress,
         size: BufferAddress,
         buffer_size: BufferAddress,
-        side: CopySide,
     },
+}
+
+impl BufferOverrunError {
+    /// Returns the end offset if everything is validated.
+    ///
+    /// TODO: more
+    pub(crate) fn check(
+        offset: BufferAddress,
+        size: BufferAddress,
+        buffer_size: BufferAddress,
+    ) -> Result<BufferAddress, Self> {
+        if offset >= buffer_size {
+            return Err(Self::StartOffset {
+                offset,
+                buffer_size,
+            });
+        }
+        // NOTE: Should never underflow because of our earlier check.
+        if size > buffer_size - offset {
+            return Err(Self::EndOffset {
+                offset,
+                size,
+                buffer_size,
+            });
+        }
+        // NOTE: Should never overflow because of our earlier check.
+        Ok(offset + size)
+    }
+}
+
+/// Error encountered while attempting a data transfer.
+#[derive(Clone, Debug, Error)]
+#[non_exhaustive]
+pub enum TransferError {
+    #[error("Source and destination cannot be the same buffer")]
+    SameSourceDestinationBuffer,
+    #[error(transparent)]
+    MissingBufferUsage(#[from] MissingBufferUsageError),
+    #[error(transparent)]
+    MissingTextureUsage(#[from] MissingTextureUsageError),
+    #[error("Transfer {_0}")]
+    BufferOverrun(BufferOverrunError),
     #[error("Copy of {dimension:?} {start_offset}..{end_offset} would end up overrunning the bounds of the {side:?} texture of {dimension:?} size {texture_size}")]
     TextureOverrun {
         start_offset: u32,
@@ -244,6 +278,12 @@ impl From<BufferTextureCopyInfoError> for TransferError {
     }
 }
 
+impl BufferOverrunError {
+    fn to_transfer_error(&self, side: CopySide) -> TransferError {
+        todo!()
+    }
+}
+
 pub(crate) fn extract_texture_selector<T>(
     copy_texture: &wgt::TexelCopyTextureInfo<T>,
     copy_size: &Extent3d,
@@ -352,22 +392,9 @@ pub(crate) fn validate_linear_texture_data(
         return Err(TransferError::UnspecifiedRowsPerImage);
     };
 
-    if offset > buffer_size {
-        return Err(TransferError::BufferStartOffsetOverrun {
-            start_offset: offset,
-            buffer_size: bytes_in_copy,
-            side: buffer_side,
-        });
-    }
-    // NOTE: Should never underflow because of our earlier check.
-    if bytes_in_copy > buffer_size - offset {
-        return Err(TransferError::BufferEndOffsetOverrun {
-            offset,
-            size: bytes_in_copy,
-            buffer_size,
-            side: buffer_side,
-        });
-    }
+    // Avoid underflow in the subtraction by checking bytes_in_copy against buffer_size first.
+    let _ = BufferOverrunError::check(offset, bytes_in_copy, buffer_size)
+        .map_err(|e| e.to_transfer_error(buffer_side))?;
 
     let is_contiguous = (row_stride_bytes == row_bytes_dense || !requires_multiple_rows)
         && (image_stride_bytes == image_bytes_dense || !requires_multiple_images);

wgpu-core/src/resource.rs

@@ -301,22 +301,8 @@ pub enum BufferAccessError {
     MapAborted,
     #[error(transparent)]
     InvalidResource(#[from] InvalidResourceError),
-    #[error("Map start offset ({offset}) is out-of-bounds for buffer of size {buffer_size}")]
-    MapStartOffsetOverrun {
-        offset: wgt::BufferAddress,
-        buffer_size: wgt::BufferAddress,
-    },
-    #[error(
-        "Map end offset (start at {} + size of {}) is out-of-bounds for buffer of size {}",
-        offset,
-        size,
-        buffer_size
-    )]
-    MapEndOffsetOverrun {
-        offset: wgt::BufferAddress,
-        size: wgt::BufferAddress,
-        buffer_size: wgt::BufferAddress,
-    },
+    #[error("Map {_0}")]
+    MapOverrun(BufferOverrunError),
 }
 
 impl WebGpuError for BufferAccessError {
@@ -743,20 +729,20 @@ impl Buffer {
         let map_state = &*self.map_state.lock();
         match *map_state {
             BufferMapState::Init { ref staging_buffer } => {
-                if offset > self.size {
-                    return Err(BufferAccessError::MapStartOffsetOverrun {
-                        offset,
-                        buffer_size: self.size,
-                    });
-                }
-                // NOTE: Should never underflow because of our earlier check.
-                if range_size > self.size - offset {
-                    return Err(BufferAccessError::MapEndOffsetOverrun {
-                        offset,
-                        size: range_size,
-                        buffer_size: self.size,
-                    });
-                }
+(??)                if offset >= self.size {
+(??)                    return Err(BufferAccessError::MapStartOffsetOverrun {
+(??)                        offset,
+(??)                        buffer_size: self.size,
+(??)                    });
+(??)                }
+(??)                // NOTE: Should never underflow because of our earlier check.
+(??)                if range_size > self.size - offset {
+(??)                    return Err(BufferAccessError::MapEndOffsetOverrun {
+(??)                        offset,
+(??)                        size: range_size,
+(??)                        buffer_size: self.size,
+(??)                    });
+(??)                }
                 let ptr = unsafe { staging_buffer.ptr() };
                 let ptr = unsafe { NonNull::new_unchecked(ptr.as_ptr().offset(offset as isize)) };
                 Ok((ptr, range_size))