From 8c8281c2ea8ed1865f978a2cc1cc3e2d29cca316 Mon Sep 17 00:00:00 2001 From: msiglreith Date: Sat, 16 Jun 2018 16:36:21 +0200 Subject: [PATCH] hal: Improve buffer documentation and cleanup error handling --- src/backend/dx11/src/device.rs | 2 +- src/backend/dx12/src/device.rs | 4 +- src/backend/empty/src/lib.rs | 2 +- src/backend/gl/src/device.rs | 7 +- src/backend/metal/src/device.rs | 10 +-- src/backend/vulkan/src/device.rs | 2 +- src/hal/src/buffer.rs | 127 ++++++++++++++----------------- src/hal/src/device.rs | 4 +- 8 files changed, 75 insertions(+), 83 deletions(-) diff --git a/src/backend/dx11/src/device.rs b/src/backend/dx11/src/device.rs index f547f030c7e..eb1061d0102 100644 --- a/src/backend/dx11/src/device.rs +++ b/src/backend/dx11/src/device.rs @@ -666,7 +666,7 @@ impl hal::Device for Device { buffer: &Buffer, format: Option, range: R, - ) -> Result { + ) -> Result { unimplemented!() } diff --git a/src/backend/dx12/src/device.rs b/src/backend/dx12/src/device.rs index 38ba29e51ce..76d18f27c0a 100644 --- a/src/backend/dx12/src/device.rs +++ b/src/backend/dx12/src/device.rs @@ -1910,14 +1910,14 @@ impl d::Device for Device { buffer: &n::Buffer, format: Option, range: R, - ) -> Result { + ) -> Result { 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); diff --git a/src/backend/empty/src/lib.rs b/src/backend/empty/src/lib.rs index 7f2d9d39f9f..93775ab818e 100644 --- a/src/backend/empty/src/lib.rs +++ b/src/backend/empty/src/lib.rs @@ -177,7 +177,7 @@ impl hal::Device for Device { unimplemented!() } - fn create_buffer_view>(&self, _: &(), _: Option, _: R) -> Result<(), buffer::ViewError> { + fn create_buffer_view>(&self, _: &(), _: Option, _: R) -> Result<(), buffer::ViewCreationError> { unimplemented!() } diff --git a/src/backend/gl/src/device.rs b/src/backend/gl/src/device.rs index f5961d34c1e..25d54d7f84c 100644 --- a/src/backend/gl/src/device.rs +++ b/src/backend/gl/src/device.rs @@ -672,8 +672,7 @@ impl d::Device for Device { ) -> Result { 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 { @@ -681,7 +680,7 @@ impl d::Device for Device { } 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 for Device { fn create_buffer_view>( &self, _: &n::Buffer, _: Option, _: R - ) -> Result { + ) -> Result { unimplemented!() } diff --git a/src/backend/metal/src/device.rs b/src/backend/metal/src/device.rs index e8a3470776f..0baad0c8b95 100644 --- a/src/backend/metal/src/device.rs +++ b/src/backend/metal/src/device.rs @@ -166,7 +166,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 { @@ -1667,22 +1667,22 @@ impl hal::Device for Device { fn create_buffer_view>( &self, buffer: &n::Buffer, format_maybe: Option, range: R - ) -> Result { + ) -> Result { 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); diff --git a/src/backend/vulkan/src/device.rs b/src/backend/vulkan/src/device.rs index 2da5c7aaa37..6512c370510 100644 --- a/src/backend/vulkan/src/device.rs +++ b/src/backend/vulkan/src/device.rs @@ -930,7 +930,7 @@ impl d::Device for Device { fn create_buffer_view>( &self, buffer: &n::Buffer, format: Option, range: R - ) -> Result { + ) -> Result { let (offset, size) = conv::map_range_arg(&range); let info = vk::BufferViewCreateInfo { s_type: vk::StructureType::BufferViewCreateInfo, diff --git a/src/hal/src/buffer.rs b/src/hal/src/buffer.rs index 9abbf7097dc..e965a7793bf 100644 --- a/src/hal/src/buffer.rs +++ b/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, + }, } 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, diff --git a/src/hal/src/device.rs b/src/hal/src/device.rs index 9214ea44d39..95852519346 100644 --- a/src/hal/src/device.rs +++ b/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: Any + Send + Sync { /// fn create_buffer_view>( &self, buf: &B::Buffer, fmt: Option, range: R - ) -> Result; + ) -> Result; /// fn destroy_buffer_view(&self, view: B::BufferView);