Merge #2154

2154: hal: Improve buffer documentation and cleanup error handling r=kvark a=msiglreith

Fixes #issue
PR checklist:
- [ ] `make` succeeds (on *nix)
- [ ] `make reftests` succeeds
- [ ] tested examples with the following backends:


Co-authored-by: msiglreith <[email protected]>

src/backend/dx11/src/device.rs

@@ -667,7 +667,7 @@ impl hal::Device<Backend> for Device {
         buffer: &Buffer,
         format: Option<format::Format>,
         range: R,
-    ) -> Result<BufferView, buffer::ViewError> {
+    ) -> Result<BufferView, buffer::ViewCreationError> {
         unimplemented!()
     }
 

src/backend/dx12/src/device.rs

@@ -1910,14 +1910,14 @@ impl d::Device<B> for Device {
         buffer: &n::Buffer,
         format: Option<format::Format>,
         range: R,
-    ) -> Result<n::BufferView, buffer::ViewError> {
+    ) -> Result<n::BufferView, buffer::ViewCreationError> {
         let buffer_features = {
             let idx = format.map(|fmt| fmt as usize).unwrap_or(0);
             self.format_properties[idx].buffer_features
         };
         let (format, format_desc) = match format.and_then(conv::map_format) {
             Some(fmt) => (fmt, format.unwrap().surface_desc()),
-            None => return Err(buffer::ViewError::Unsupported),
+            None => return Err(buffer::ViewCreationError::UnsupportedFormat { format }),
         };
 
         let start = *range.start().unwrap_or(&0);

src/backend/empty/src/lib.rs

@@ -177,7 +177,7 @@ impl hal::Device<Backend> for Device {
         unimplemented!()
     }
 
-    fn create_buffer_view<R: RangeArg<u64>>(&self, _: &(), _: Option<format::Format>, _: R) -> Result<(), buffer::ViewError> {
+    fn create_buffer_view<R: RangeArg<u64>>(&self, _: &(), _: Option<format::Format>, _: R) -> Result<(), buffer::ViewCreationError> {
         unimplemented!()
     }
 

src/backend/gl/src/device.rs

@@ -672,16 +672,15 @@ impl d::Device<B> for Device {
     ) -> Result<UnboundBuffer, buffer::CreationError> {
         if !self.share.legacy_features.contains(LegacyFeatures::CONSTANT_BUFFER) &&
             usage.contains(buffer::Usage::UNIFORM) {
-            error!("Constant buffers are not supported by this GL version");
-            return Err(buffer::CreationError::Other);
+            return Err(buffer::CreationError::UnsupportedUsage { usage });
         }
 
         let target = if self.share.private_caps.buffer_role_change {
             gl::ARRAY_BUFFER
         } else {
             match conv::buffer_usage_to_gl_target(usage) {
                 Some(target) => target,
-                None => return Err(buffer::CreationError::Usage(usage)),
+                None => return Err(buffer::CreationError::UnsupportedUsage { usage }),
             }
         };
 
@@ -838,7 +837,7 @@ impl d::Device<B> for Device {
 
     fn create_buffer_view<R: RangeArg<u64>>(
         &self, _: &n::Buffer, _: Option<Format>, _: R
-    ) -> Result<n::BufferView, buffer::ViewError> {
+    ) -> Result<n::BufferView, buffer::ViewCreationError> {
         unimplemented!()
     }
 

src/backend/metal/src/device.rs

@@ -167,7 +167,7 @@ fn create_depth_stencil_state(
             raw.set_front_face_stencil(Some(&front_desc));
 
             let back_desc = metal::StencilDescriptor::new();
-            back_desc.set_stencil_compare_function(conv::map_compare_function(back.fun));    
+            back_desc.set_stencil_compare_function(conv::map_compare_function(back.fun));
 
             dss.stencil.back_read_mask = back.mask_read;
             match back.mask_read {
@@ -1665,22 +1665,22 @@ impl hal::Device<Backend> for Device {
 
     fn create_buffer_view<R: RangeArg<u64>>(
         &self, buffer: &n::Buffer, format_maybe: Option<format::Format>, range: R
-    ) -> Result<n::BufferView, buffer::ViewError> {
+    ) -> Result<n::BufferView, buffer::ViewCreationError> {
         let start = buffer.range.start + *range.start().unwrap_or(&0);
         let end_rough = *range.end().unwrap_or(&buffer.raw.length());
         let format = match format_maybe {
             Some(fmt) => fmt,
-            None => return Err(buffer::ViewError::Unsupported),
+            None => return Err(buffer::ViewCreationError::UnsupportedFormat { format: format_maybe }),
         };
         let format_desc = format.surface_desc();
         if format_desc.aspects != format::Aspects::COLOR {
             // no depth/stencil support for buffer views here
-            return Err(buffer::ViewError::Unsupported)
+            return Err(buffer::ViewCreationError::UnsupportedFormat { format: format_maybe })
         }
         let block_count = (end_rough - start) * 8 / format_desc.bits as u64;
         let mtl_format = self.private_caps
             .map_format(format)
-            .ok_or(buffer::ViewError::Unsupported)?;
+            .ok_or(buffer::ViewCreationError::UnsupportedFormat { format: format_maybe })?;
 
         let descriptor = metal::TextureDescriptor::new();
         descriptor.set_texture_type(MTLTextureType::D2);

src/backend/vulkan/src/device.rs

@@ -929,7 +929,7 @@ impl d::Device<B> for Device {
 
     fn create_buffer_view<R: RangeArg<u64>>(
         &self, buffer: &n::Buffer, format: Option<format::Format>, range: R
-    ) -> Result<n::BufferView, buffer::ViewError> {
+    ) -> Result<n::BufferView, buffer::ViewCreationError> {
         let (offset, size) = conv::map_range_arg(&range);
         let info = vk::BufferViewCreateInfo {
             s_type: vk::StructureType::BufferViewCreateInfo,

src/hal/src/buffer.rs

@@ -1,72 +1,58 @@
-//! Memory buffers
+//! Memory buffers.
+//!
+//! # Buffer
+//!
+//! Buffers interpret memory slices as linear continguous data array.
+//! They can be used as shader resources, vertex buffers, index buffers or for
+//! specifying the action commands for indirect exection.
 
-use std::error::Error;
-use std::fmt;
-
-use {IndexType, Backend};
+use {format, IndexType, Backend};
 
 
 /// An offset inside a buffer, in bytes.
 pub type Offset = u64;
 
+/// Buffer state.
+pub type State = Access;
+
 /// Error creating a buffer.
-#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
+#[derive(Fail, Debug, Clone, PartialEq, Eq)]
 pub enum CreationError {
-    /// Required `Usage` is not supported.
-    Usage(Usage),
-    /// Some other problem.
-    Other,
-}
-
-impl fmt::Display for CreationError {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        let description = self.description();
-        match *self {
-            CreationError::Usage(usage) => write!(f, "{}: {:?}", description, usage),
-            _ => write!(f, "{}", description)
-        }
-    }
+    /// Memory allocation on the host side failed.
+    /// This could be caused by a lack of memory.
+    #[fail(display = "Host memory allocation failed.")]
+    OutOfHostMemory,
+    /// Memory allocation on the device side failed.
+    /// This could be caused by a lack of memory.
+    #[fail(display = "Device memory allocation failed.")]
+    OutOfDeviceMemory,
+    /// Requested buffer usage is not supported.
+    ///
+    /// Older GL version don't support constant buffers or multiple usage flags.
+    #[fail(display = "Buffer usage unsupported ({:?}).", usage)]
+    UnsupportedUsage {
+        /// Unsupported usage passed on buffer creation.
+        usage: Usage,
+    },
 }
 
-impl Error for CreationError {
-    fn description(&self) -> &str {
-        match *self {
-            CreationError::Usage(_) =>
-                "Required `Usage` is not supported",
-            CreationError::Other =>
-                "Some other problem",
-        }
-    }
-}
-
-/// Error creating a `BufferView`.
-#[derive(Clone, Debug, PartialEq)]
-pub enum ViewError {
-    /// The required usage flag is not present in the image.
-    Usage(Usage),
-    /// The backend refused for some reason.
-    Unsupported,
-}
-
-impl fmt::Display for ViewError {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        let description = self.description();
-        match *self {
-            ViewError::Usage(usage) => write!(f, "{}: {:?}", description, usage),
-            _ => write!(f, "{}", description)
-        }
-    }
-}
-
-impl Error for ViewError {
-    fn description(&self) -> &str {
-        match *self {
-            ViewError::Usage(_) =>
-                "The required usage flag is not present in the image",
-            ViewError::Unsupported =>
-                "The backend refused for some reason",
-        }
-    }
+/// Error creating a buffer view.
+#[derive(Fail, Debug, Clone, PartialEq, Eq)]
+pub enum ViewCreationError {
+    /// Memory allocation on the host side failed.
+    /// This could be caused by a lack of memory.
+    #[fail(display = "Host memory allocation failed.")]
+    OutOfHostMemory,
+    /// Memory allocation on the device side failed.
+    /// This could be caused by a lack of memory.
+    #[fail(display = "Device memory allocation failed.")]
+    OutOfDeviceMemory,
+    /// Buffer view format is not supported.
+    #[fail(display = "Buffer view format unsupported ({:?}).", format)]
+    UnsupportedFormat {
+        /// Unsupported format passed on view creation.
+        format: Option<format::Format>,
+    },
 }
 
 bitflags!(
@@ -95,21 +81,27 @@ bitflags!(
 );
 
 impl Usage {
-    /// Can this buffer be used in transfer operations ?
+    /// Returns if the buffer can be used in transfer operations.
     pub fn can_transfer(&self) -> bool {
         self.intersects(Usage::TRANSFER_SRC | Usage::TRANSFER_DST)
     }
 }
 
 bitflags!(
-    /// Buffer state flags.
+    /// Buffer access flags.
+    ///
+    /// Access of buffers by the pipeline or shaders.
     #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
     pub struct Access: u32 {
-        ///
+        /// Read commands instruction for indirect execution.
         const INDIRECT_COMMAND_READ = 0x1;
+        /// Read index values for indexed draw commands.
         ///
+        /// See [`draw_indexed`](../command/trait.RawCommandBuffer.html#tymethod.draw_indexed)
+        /// and [`draw_indexed_indirect`](../command/trait.RawCommandBuffer.html#tymethod.draw_indexed_indirect).
         const INDEX_BUFFER_READ = 0x2;
-        ///
+        /// Read vertices from vertex buffer for draw commands in the [`VERTEX_INPUT`](
+        /// ../pso/struct.PipelineStage.html#associatedconstant.VERTEX_INPUT) stage.
         const VERTEX_BUFFER_READ = 0x4;
         ///
         const CONSTANT_BUFFER_READ = 0x8;
@@ -132,11 +124,10 @@ bitflags!(
     }
 );
 
-/// Buffer state
-pub type State = Access;
-
-/// Index buffer view for `bind_index_buffer`, slightly
-/// analogous to an index table into an array.
+/// Index buffer view for `bind_index_buffer`.
+///
+/// Defines a buffer slice used for acquiring the indicies on draw commands.
+/// Indices are used to lookup vertex indices in the vertex buffers.
 pub struct IndexBufferView<'a, B: Backend> {
     /// The buffer to bind.
     pub buffer: &'a B::Buffer,

src/hal/src/device.rs

@@ -1,3 +1,5 @@
+//! Logical device
+//!
 //! # Device
 //!
 //! This module exposes the `Device` trait, which provides methods for creating
@@ -306,7 +308,7 @@ pub trait Device<B: Backend>: Any + Send + Sync {
     ///
     fn create_buffer_view<R: RangeArg<u64>>(
         &self, buf: &B::Buffer, fmt: Option<format::Format>, range: R
-    ) -> Result<B::BufferView, buffer::ViewError>;
+    ) -> Result<B::BufferView, buffer::ViewCreationError>;
 
     ///
     fn destroy_buffer_view(&self, view: B::BufferView);