From 01fcd53faa6980d2bfbaf1542afeecabccb74d87 Mon Sep 17 00:00:00 2001 From: Rua Date: Wed, 19 May 2021 18:06:56 +0200 Subject: [PATCH] Check draw_indirect count against limit, minor doc fixes (#1574) --- CHANGELOG_VULKANO.md | 8 ++- vulkano/src/command_buffer/auto.rs | 61 ++++++++++++++++--- .../validity/indirect_buffer.rs | 19 +++++- vulkano/src/descriptor/mod.rs | 4 +- vulkano/src/image/mod.rs | 2 +- vulkano/src/query.rs | 2 +- 6 files changed, 75 insertions(+), 21 deletions(-) diff --git a/CHANGELOG_VULKANO.md b/CHANGELOG_VULKANO.md index 2315ad58..05f187a2 100644 --- a/CHANGELOG_VULKANO.md +++ b/CHANGELOG_VULKANO.md @@ -6,13 +6,15 @@ - **Breaking** Vulkano-shaders now checks if the device supports the shader's SPIR-V version, when loading the shader. - **Breaking** (but unlikely) Vulkano-shaders now compiles to SPIR-V 1.0 by default. If your shader needs features only available in a higher version, you can specify the target version on the `shader!` macro with the new `vulkan_version: "major.minor"` and `spirv_version: "major.minor"` arguments. +- **Breaking** change to `ImageFormatProperties::sample_counts` field. + - `sample_counts` field is originaly represented as u32 type, which is now represented by `SampleCounts` struct-type which is a boolean collection of supported `sample_counts`. + - Added conversion function between SampleCountFlagBits (u32-type) and `SampleCounts` type. - Added `DeviceExtensions::khr_spirv_1_4`, which allows SPIR-V 1.4 shaders in Vulkan 1.1. - Added `FunctionPointers::api_version` to query the highest supported instance version. - Added `Instance::api_version` and `Device::api_version` to return the actual supported Vulkan version. These may differ between instance and device, and be lower than what `FunctionPointers::api_version` and `PhysicalDevice::api_version` return (currently never higher than 1.1, but this may change in the future). - Fixed the issue when creating a buffer with exportable fd on Linux(see to #1545). -- **Breaking**, change to `ImageFormatProperties::sample_counts` field. - - `sample_counts` field is originaly represented as u32 type, which is now represented by `SampleCounts` struct-type which is a boolean collection of supported `sample_counts`. - - Added conversion function between SampleCountFlagBits (u32-type) and `SampleCounts` type. +- The `draw_indirect` and `draw_indexed_indirect` commands on `AutoCommandBufferBuilder` now check the draw count against the `max_draw_indirect_count` limit. +- Fixed a few documentation errors. # Version 0.23.0 (2021-04-10) diff --git a/vulkano/src/command_buffer/auto.rs b/vulkano/src/command_buffer/auto.rs index 50708857..8e1549d1 100644 --- a/vulkano/src/command_buffer/auto.rs +++ b/vulkano/src/command_buffer/auto.rs @@ -1214,8 +1214,14 @@ impl AutoCommandBufferBuilder { } } - /// Perform multiple draw operations using a graphics pipeline. One draw is performed for each - /// `vulkano::command_buffer::DrawIndirectCommand` struct in `indirect_buffer`. + /// Perform multiple draw operations using a graphics pipeline. + /// + /// One draw is performed for each [`DrawIndirectCommand`] struct in `indirect_buffer`. + /// The maximum number of draw commands in the buffer is limited by the + /// [`max_draw_indirect_count`](crate::instance::Limits::max_draw_indirect_count) limit. + /// This limit is 1 unless the + /// [`multi_draw_indirect`](crate::device::Features::multi_draw_indirect) feature has been + /// enabled. /// /// `vertex_buffer` is a set of vertex and/or instance buffers used to provide input. It is /// used for every draw operation. @@ -1254,7 +1260,22 @@ impl AutoCommandBufferBuilder { check_descriptor_sets_validity(&pipeline, &sets)?; let vb_infos = check_vertex_buffers(&pipeline, vertex_buffer)?; - let draw_count = indirect_buffer.len() as u32; + let requested = indirect_buffer.len() as u32; + let limit = self + .device() + .physical_device() + .limits() + .max_draw_indirect_count(); + + if requested > limit { + return Err( + CheckIndirectBufferError::MaxDrawIndirectCountLimitExceeded { + limit, + requested, + } + .into(), + ); + } if let StateCacherOutcome::NeedChange = self.state_cacher.bind_graphics_pipeline(&pipeline) @@ -1284,7 +1305,7 @@ impl AutoCommandBufferBuilder { self.inner.draw_indirect( indirect_buffer, - draw_count, + requested, mem::size_of::() as u32, )?; Ok(self) @@ -1372,9 +1393,14 @@ impl AutoCommandBufferBuilder { } } - /// Perform multiple draw operations using a graphics pipeline, using an index buffer. One - /// draw is performed for for each `vulkano::command_buffer::DrawIndirectCommand` struct in - /// `indirect_buffer`. + /// Perform multiple draw operations using a graphics pipeline, using an index buffer. + /// + /// One draw is performed for each [`DrawIndirectCommand`] struct in `indirect_buffer`. + /// The maximum number of draw commands in the buffer is limited by the + /// [`max_draw_indirect_count`](crate::instance::Limits::max_draw_indirect_count) limit. + /// This limit is 1 unless the + /// [`multi_draw_indirect`](crate::device::Features::multi_draw_indirect) feature has been + /// enabled. /// /// `vertex_buffer` is a set of vertex and/or instance buffers used to provide input. /// `index_buffer` is a buffer containing indices into the vertex buffer that should be @@ -1418,7 +1444,22 @@ impl AutoCommandBufferBuilder { check_descriptor_sets_validity(&pipeline, &sets)?; let vb_infos = check_vertex_buffers(&pipeline, vertex_buffer)?; - let draw_count = indirect_buffer.len() as u32; + let requested = indirect_buffer.len() as u32; + let limit = self + .device() + .physical_device() + .limits() + .max_draw_indirect_count(); + + if requested > limit { + return Err( + CheckIndirectBufferError::MaxDrawIndirectCountLimitExceeded { + limit, + requested, + } + .into(), + ); + } if let StateCacherOutcome::NeedChange = self.state_cacher.bind_graphics_pipeline(&pipeline) @@ -1454,7 +1495,7 @@ impl AutoCommandBufferBuilder { self.inner.draw_indexed_indirect( indirect_buffer, - draw_count, + requested, mem::size_of::() as u32, )?; Ok(self) @@ -1629,7 +1670,7 @@ impl AutoCommandBufferBuilder { /// Adds a command that copies the results of a range of queries to a buffer on the GPU. /// - /// [`query_pool.ty().data_size()`](crate::query::QueryType::data_size) elements + /// [`query_pool.ty().result_size()`](crate::query::QueryType::result_size) elements /// will be written for each query in the range, plus 1 extra element per query if /// [`QueryResultFlags::with_availability`] is enabled. /// The provided buffer must be large enough to hold the data. diff --git a/vulkano/src/command_buffer/validity/indirect_buffer.rs b/vulkano/src/command_buffer/validity/indirect_buffer.rs index f106ae18..3acd4b2a 100644 --- a/vulkano/src/command_buffer/validity/indirect_buffer.rs +++ b/vulkano/src/command_buffer/validity/indirect_buffer.rs @@ -15,12 +15,12 @@ use std::error; use std::fmt; /// Checks whether an indirect buffer can be bound. -pub fn check_indirect_buffer( +pub fn check_indirect_buffer( device: &Device, - buffer: &Ib, + buffer: &Inb, ) -> Result<(), CheckIndirectBufferError> where - Ib: BufferAccess + Send + Sync + 'static, + Inb: BufferAccess + Send + Sync + 'static, { assert_eq!( buffer.inner().buffer.device().internal_object(), @@ -39,6 +39,13 @@ where pub enum CheckIndirectBufferError { /// The "indirect buffer" usage must be enabled on the indirect buffer. BufferMissingUsage, + /// The maximum number of indirect draws has been exceeded. + MaxDrawIndirectCountLimitExceeded { + /// The limit that must be fulfilled. + limit: u32, + /// What was requested. + requested: u32, + }, } impl error::Error for CheckIndirectBufferError {} @@ -53,6 +60,12 @@ impl fmt::Display for CheckIndirectBufferError { CheckIndirectBufferError::BufferMissingUsage => { "the indirect buffer usage must be enabled on the indirect buffer" } + CheckIndirectBufferError::MaxDrawIndirectCountLimitExceeded { + limit, + requested, + } => { + "the maximum number of indirect draws has been exceeded" + } } ) } diff --git a/vulkano/src/descriptor/mod.rs b/vulkano/src/descriptor/mod.rs index 236f10d1..f50c7e58 100644 --- a/vulkano/src/descriptor/mod.rs +++ b/vulkano/src/descriptor/mod.rs @@ -66,9 +66,7 @@ //! //! ## Creating a descriptor set //! -//! ```ignore -//! // TODO: write example for: PersistentDescriptorSet::start(layout.clone()).add_buffer(data_buffer.clone()) -//! ``` +//! TODO: write example for: PersistentDescriptorSet::start(layout.clone()).add_buffer(data_buffer.clone()) //! //! ## Passing the descriptor set when drawing //! diff --git a/vulkano/src/image/mod.rs b/vulkano/src/image/mod.rs index 5934c804..d6b26a17 100644 --- a/vulkano/src/image/mod.rs +++ b/vulkano/src/image/mod.rs @@ -31,7 +31,7 @@ //! # High-level wrappers //! //! In the vulkano library, an image is any object that implements the [`ImageAccess`] trait. You -//! can create a view by wrapping them in an [`ImageView`]. +//! can create a view by wrapping them in an [`ImageView`](crate::image::view::ImageView). //! //! Since the `ImageAccess` trait is low-level, you are encouraged to not implement it yourself but //! instead use one of the provided implementations that are specialized depending on the way you diff --git a/vulkano/src/query.rs b/vulkano/src/query.rs index f9238930..40193294 100644 --- a/vulkano/src/query.rs +++ b/vulkano/src/query.rs @@ -253,7 +253,7 @@ impl<'a> QueriesRange<'a> { /// Copies the results of this range of queries to a buffer on the CPU. /// - /// [`self.pool().ty().data_size()`](QueryType::data_size) elements + /// [`self.pool().ty().result_size()`](QueryType::result_size) elements /// will be written for each query in the range, plus 1 extra element per query if /// [`QueryResultFlags::with_availability`] is enabled. /// The provided buffer must be large enough to hold the data.