From d90106d4dfa67e9c19b22a116863e6d5788762b0 Mon Sep 17 00:00:00 2001 From: qnope Date: Sat, 19 Dec 2020 04:08:48 +0100 Subject: [PATCH] [Vulkano-1441] Adding the possibility to generate mipmaps at image (#1451) creation To do so, we created a SubImage that implements the ImageAccess trait This SubImage will decorate an ImageAccess image and will represent one or more mip level / array layer level --- CHANGELOG_VULKANO.md | 1 + examples/src/bin/debug.rs | 2 + examples/src/bin/image/main.rs | 3 +- vulkano/src/command_buffer/synced/base.rs | 21 +- vulkano/src/image/attachment.rs | 10 + vulkano/src/image/immutable.rs | 231 +++++++++++++++++++++- vulkano/src/image/mod.rs | 29 ++- vulkano/src/image/storage.rs | 10 + vulkano/src/image/swapchain.rs | 10 + vulkano/src/image/traits.rs | 22 +++ 10 files changed, 321 insertions(+), 18 deletions(-) diff --git a/CHANGELOG_VULKANO.md b/CHANGELOG_VULKANO.md index d118c8ab..9e35ffcc 100644 --- a/CHANGELOG_VULKANO.md +++ b/CHANGELOG_VULKANO.md @@ -1,5 +1,6 @@ # Unreleased +- **Breaking** The `ImmutableImage::from_iter` and `ImmutableImage::from_buffer` can build Mipmaps - **Breaking** `CpuAccessibleBuffer` now uses `RwLock` from `parking_lot`. - **Breaking** The `Kind` and `SubpassContents` types have been moved to the root of the `command_buffer` module. - **Breaking** On `AutoCommandBufferBuilder`, the methods `begin_render_pass` and `next_subpass` now take `SubpassContents` instead of a boolean value. diff --git a/examples/src/bin/debug.rs b/examples/src/bin/debug.rs index c4a9eb4a..3253a227 100644 --- a/examples/src/bin/debug.rs +++ b/examples/src/bin/debug.rs @@ -11,6 +11,7 @@ use vulkano::device::{Device, DeviceExtensions}; use vulkano::format::Format; use vulkano::image::Dimensions; use vulkano::image::ImmutableImage; +use vulkano::image::MipmapsCount; use vulkano::instance; use vulkano::instance::debug::{DebugCallback, MessageSeverity, MessageType}; use vulkano::instance::{Instance, InstanceExtensions, PhysicalDevice}; @@ -130,6 +131,7 @@ fn main() { let _ = ImmutableImage::from_iter( DATA.iter().cloned(), dimensions, + MipmapsCount::One, pixel_format, queue.clone(), ) diff --git a/examples/src/bin/image/main.rs b/examples/src/bin/image/main.rs index 9ea3b8be..ecfb227b 100644 --- a/examples/src/bin/image/main.rs +++ b/examples/src/bin/image/main.rs @@ -13,7 +13,7 @@ use vulkano::descriptor::descriptor_set::PersistentDescriptorSet; use vulkano::device::{Device, DeviceExtensions}; use vulkano::format::Format; use vulkano::framebuffer::{Framebuffer, FramebufferAbstract, RenderPassAbstract, Subpass}; -use vulkano::image::{Dimensions, ImageUsage, ImmutableImage, SwapchainImage}; +use vulkano::image::{Dimensions, ImageUsage, ImmutableImage, MipmapsCount, SwapchainImage}; use vulkano::instance::{Instance, PhysicalDevice}; use vulkano::pipeline::viewport::Viewport; use vulkano::pipeline::GraphicsPipeline; @@ -163,6 +163,7 @@ fn main() { ImmutableImage::from_iter( image_data.iter().cloned(), dimensions, + MipmapsCount::One, Format::R8G8B8A8Srgb, queue.clone(), ) diff --git a/vulkano/src/command_buffer/synced/base.rs b/vulkano/src/command_buffer/synced/base.rs index 834b3ee5..c68ca1dd 100644 --- a/vulkano/src/command_buffer/synced/base.rs +++ b/vulkano/src/command_buffer/synced/base.rs @@ -321,7 +321,13 @@ impl

Hash for BuilderKey

{ } KeyTy::Image => { let c = &commands_lock.commands[self.command_ids.borrow()[0]]; - c.image(self.resource_index).conflict_key().hash(state) + c.image(self.resource_index).conflict_key().hash(state); + c.image(self.resource_index) + .current_miplevels_access() + .hash(state); + c.image(self.resource_index) + .current_layer_levels_access() + .hash(state); } } } @@ -557,6 +563,7 @@ impl

SyncCommandBufferBuilder

{ { let cmd1 = &commands_lock.commands[*collision_cmd_id]; let cmd2 = &commands_lock.commands[latest_command_id]; + return Err(SyncCommandBufferBuilderError::Conflict { command1_name: cmd1.name(), command1_param: match entry_key_resource_ty { @@ -617,8 +624,8 @@ impl

SyncCommandBufferBuilder

{ let b = &mut self.pending_barrier; b.add_image_memory_barrier( img, - 0..img.mipmap_levels(), - 0..img.dimensions().array_layers(), + img.current_miplevels_access(), + img.current_layer_levels_access(), entry.stages, entry.access, stages, @@ -701,8 +708,8 @@ impl

SyncCommandBufferBuilder

{ let b = &mut self.pending_barrier; b.add_image_memory_barrier( img, - 0..img.mipmap_levels(), - 0..img.dimensions().array_layers(), + img.current_miplevels_access(), + img.current_layer_levels_access(), PipelineStages { bottom_of_pipe: true, ..PipelineStages::none() @@ -774,8 +781,8 @@ impl

SyncCommandBufferBuilder

{ barrier.add_image_memory_barrier( img, - 0..img.mipmap_levels(), - 0..img.dimensions().array_layers(), + img.current_miplevels_access(), + img.current_layer_levels_access(), state.stages, state.access, PipelineStages { diff --git a/vulkano/src/image/attachment.rs b/vulkano/src/image/attachment.rs index db3d15f9..8dc38a04 100644 --- a/vulkano/src/image/attachment.rs +++ b/vulkano/src/image/attachment.rs @@ -583,6 +583,16 @@ where fn is_layout_initialized(&self) -> bool { self.initialized.load(Ordering::SeqCst) } + + #[inline] + fn current_miplevels_access(&self) -> std::ops::Range { + 0..self.mipmap_levels() + } + + #[inline] + fn current_layer_levels_access(&self) -> std::ops::Range { + 0..1 + } } unsafe impl ImageClearValue for Arc> diff --git a/vulkano/src/image/immutable.rs b/vulkano/src/image/immutable.rs index 268b6772..ff3c902b 100644 --- a/vulkano/src/image/immutable.rs +++ b/vulkano/src/image/immutable.rs @@ -47,6 +47,7 @@ use memory::pool::MemoryPoolAlloc; use memory::pool::PotentialDedicatedAllocation; use memory::pool::StdMemoryPoolAlloc; use memory::DedicatedAlloc; +use sampler::Filter; use sync::AccessError; use sync::NowFuture; use sync::Sharing; @@ -65,10 +66,110 @@ pub struct ImmutableImage, + mip_levels_access: std::ops::Range, + layer_levels_access: std::ops::Range, + layout: ImageLayout, +} + +impl SubImage +{ + pub fn new( + image: Arc, + mip_level: u32, + mip_level_count: u32, + layer_level: u32, + layer_level_count: u32, + layout: ImageLayout, + ) -> Arc { + debug_assert!(mip_level + mip_level_count <= image.mipmap_levels()); + debug_assert!(layer_level + layer_level_count <= image.dimensions().array_layers()); + + let last_level = mip_level + mip_level_count; + let mip_levels_access = mip_level..last_level; + + let last_level = layer_level + layer_level_count; + let layer_levels_access = layer_level..last_level; + + Arc::new(SubImage { + image, + mip_levels_access, + layer_levels_access, + layout: ImageLayout::ShaderReadOnlyOptimal, + }) + } +} + // Must not implement Clone, as that would lead to multiple `used` values. pub struct ImmutableImageInitialization> { image: Arc>, used: AtomicBool, + mip_levels_access: std::ops::Range, + layer_levels_access: std::ops::Range, +} + +fn has_mipmaps(mipmaps: MipmapsCount) -> bool { + match mipmaps { + MipmapsCount::One => false, + MipmapsCount::Log2 => true, + MipmapsCount::Specific(x) => x > 1 + } +} + +fn generate_mipmaps( + cbb: &mut AutoCommandBufferBuilder, + image: Arc, + dimensions: Dimensions, + layout: ImageLayout, +) where + Img: ImageAccess + Send + Sync + 'static, +{ + let img_dim = dimensions.to_image_dimensions(); + for level in 1..image.mipmap_levels() { + let [xs, ys, ds] = img_dim + .mipmap_dimensions(level - 1) + .unwrap() + .width_height_depth(); + let [xd, yd, dd] = img_dim + .mipmap_dimensions(level) + .unwrap() + .width_height_depth(); + + let src = SubImage::new( + image.clone(), + level - 1, + 1, + 0, + img_dim.array_layers(), + layout, + ); + + let dst = SubImage::new(image.clone(), level, 1, 0, img_dim.array_layers(), layout); + + cbb.blit_image( + src, //source + [0, 0, 0], //source_top_left + [xs as i32, ys as i32, ds as i32], //source_bottom_right + 0, //source_base_array_layer + level - 1, //source_mip_level + dst, //destination + [0, 0, 0], //destination_top_left + [xd as i32, yd as i32, dd as i32], //destination_bottom_right + 0, //destination_base_array_layer + level, //destination_mip_level + 1, //layer_count + Filter::Linear, //filter + ) + .expect("failed to blit a mip map to image!"); + } } impl ImmutableImage { @@ -211,18 +312,19 @@ impl ImmutableImage { let init = ImmutableImageInitialization { image: image.clone(), used: AtomicBool::new(false), + mip_levels_access: 0..image.mipmap_levels(), + layer_levels_access: 0..image.dimensions().array_layers(), }; Ok((image, init)) } /// Construct an ImmutableImage from the contents of `iter`. - /// - /// TODO: Support mipmaps #[inline] pub fn from_iter( iter: I, dimensions: Dimensions, + mipmaps: MipmapsCount, format: F, queue: Arc, ) -> Result< @@ -244,15 +346,14 @@ impl ImmutableImage { false, iter, )?; - ImmutableImage::from_buffer(source, dimensions, format, queue) + ImmutableImage::from_buffer(source, dimensions, mipmaps, format, queue) } /// Construct an ImmutableImage containing a copy of the data in `source`. - /// - /// TODO: Support mipmaps pub fn from_buffer( source: B, dimensions: Dimensions, + mipmaps: MipmapsCount, format: F, queue: Arc, ) -> Result< @@ -268,23 +369,34 @@ impl ImmutableImage { F: FormatDesc + AcceptsPixels

+ 'static + Send + Sync, Format: AcceptsPixels

, { + let need_to_generate_mipmaps = has_mipmaps(mipmaps); let usage = ImageUsage { transfer_destination: true, + transfer_source: need_to_generate_mipmaps, sampled: true, ..ImageUsage::none() }; let layout = ImageLayout::ShaderReadOnlyOptimal; - let (buffer, init) = ImmutableImage::uninitialized( + let (image, initializer) = ImmutableImage::uninitialized( source.device().clone(), dimensions, format, - MipmapsCount::One, + mipmaps, usage, layout, source.device().active_queue_families(), )?; + let init = SubImage::new( + Arc::new(initializer), + 0, + 1, + 0, + 1, + ImageLayout::ShaderReadOnlyOptimal, + ); + let mut cbb = AutoCommandBufferBuilder::new(source.device().clone(), queue.family())?; cbb.copy_buffer_to_image_dimensions( source, @@ -296,14 +408,26 @@ impl ImmutableImage { 0, ) .unwrap(); + + if need_to_generate_mipmaps { + generate_mipmaps( + &mut cbb, + image.clone(), + image.dimensions, + ImageLayout::ShaderReadOnlyOptimal, + ); + } + let cb = cbb.build().unwrap(); let future = match cb.execute(queue) { Ok(f) => f, - Err(_) => unreachable!(), + Err(e) => unreachable!("{:?}", e) }; - Ok((buffer, future)) + image.initialized.store(true, Ordering::Relaxed); + + Ok((image, future)) } } @@ -392,6 +516,16 @@ where unsafe fn unlock(&self, new_layout: Option) { debug_assert!(new_layout.is_none()); } + + #[inline] + fn current_miplevels_access(&self) -> std::ops::Range { + 0..self.mipmap_levels() + } + + #[inline] + fn current_layer_levels_access(&self) -> std::ops::Range { + 0..self.dimensions().array_layers() + } } unsafe impl ImageContent

for ImmutableImage @@ -449,6 +583,75 @@ where } } +unsafe impl ImageAccess for SubImage +{ + #[inline] + fn inner(&self) -> ImageInner { + self.image.inner() + } + + #[inline] + fn initial_layout_requirement(&self) -> ImageLayout { + self.image.initial_layout_requirement() + } + + #[inline] + fn final_layout_requirement(&self) -> ImageLayout { + self.image.final_layout_requirement() + } + + #[inline] + fn conflicts_buffer(&self, other: &dyn BufferAccess) -> bool { + false + } + + #[inline] + fn conflicts_image(&self, other: &dyn ImageAccess) -> bool { + self.conflict_key() == other.conflict_key() + && self.current_miplevels_access() == other.current_miplevels_access() + && self.current_layer_levels_access() == other.current_layer_levels_access() + } + + fn current_miplevels_access(&self) -> std::ops::Range { + self.mip_levels_access.clone() + } + + fn current_layer_levels_access(&self) -> std::ops::Range { + self.layer_levels_access.clone() + } + + #[inline] + fn conflict_key(&self) -> u64 { + self.image.conflict_key() + } + + #[inline] + fn try_gpu_lock( + &self, + exclusive_access: bool, + expected_layout: ImageLayout, + ) -> Result<(), AccessError> { + if expected_layout != self.layout && expected_layout != ImageLayout::Undefined { + return Err(AccessError::UnexpectedImageLayout { + requested: expected_layout, + allowed: self.layout, + }); + } + + Ok(()) + } + + #[inline] + unsafe fn increase_gpu_lock(&self) { + self.image.increase_gpu_lock() + } + + #[inline] + unsafe fn unlock(&self, new_layout: Option) { + self.image.unlock(new_layout) + } +} + impl PartialEq for ImmutableImage where F: 'static + Send + Sync, @@ -536,6 +739,16 @@ where assert_eq!(new_layout, Some(self.image.layout)); self.image.initialized.store(true, Ordering::Relaxed); } + + #[inline] + fn current_miplevels_access(&self) -> std::ops::Range { + self.mip_levels_access.clone() + } + + #[inline] + fn current_layer_levels_access(&self) -> std::ops::Range { + self.layer_levels_access.clone() + } } impl PartialEq for ImmutableImageInitialization diff --git a/vulkano/src/image/mod.rs b/vulkano/src/image/mod.rs index 40e497f5..971b917a 100644 --- a/vulkano/src/image/mod.rs +++ b/vulkano/src/image/mod.rs @@ -685,6 +685,10 @@ impl ImageDimensions { #[cfg(test)] mod tests { use image::ImageDimensions; + use image::ImmutableImage; + use image::Dimensions; + use image::MipmapsCount; + use format; #[test] fn max_mipmaps() { @@ -797,4 +801,27 @@ mod tests { ); assert_eq!(dims.mipmap_dimensions(9), None); } -} + + #[test] + fn mipmap_working_immutable_image() { + let (device, queue) = gfx_dev_and_queue!(); + + let dimensions = Dimensions::Dim2d{width: 512, height: 512}; + { + let mut vec = Vec::new(); + + vec.resize(512 * 512, 0u8); + + let (image, _) = ImmutableImage::from_iter(vec.into_iter(), dimensions, MipmapsCount::One, format::R8Unorm, queue.clone()).unwrap(); + assert_eq!(image.mipmap_levels(), 1); + } + { + let mut vec = Vec::new(); + + vec.resize(512 * 512, 0u8); + + let (image, _) = ImmutableImage::from_iter(vec.into_iter(), dimensions, MipmapsCount::Log2, format::R8Unorm, queue.clone()).unwrap(); + assert_eq!(image.mipmap_levels(), 10); + } + } +} \ No newline at end of file diff --git a/vulkano/src/image/storage.rs b/vulkano/src/image/storage.rs index 03781f8e..04308b23 100644 --- a/vulkano/src/image/storage.rs +++ b/vulkano/src/image/storage.rs @@ -264,6 +264,16 @@ where assert!(new_layout.is_none() || new_layout == Some(ImageLayout::General)); self.gpu_lock.fetch_sub(1, Ordering::SeqCst); } + + #[inline] + fn current_miplevels_access(&self) -> std::ops::Range { + 0..self.mipmap_levels() + } + + #[inline] + fn current_layer_levels_access(&self) -> std::ops::Range { + 0..self.dimensions().array_layers() + } } unsafe impl ImageClearValue for StorageImage diff --git a/vulkano/src/image/swapchain.rs b/vulkano/src/image/swapchain.rs index 8cf97654..9f87587f 100644 --- a/vulkano/src/image/swapchain.rs +++ b/vulkano/src/image/swapchain.rs @@ -157,6 +157,16 @@ unsafe impl ImageAccess for SwapchainImage { unsafe fn unlock(&self, _: Option) { // TODO: store that the image was initialized } + + #[inline] + fn current_miplevels_access(&self) -> std::ops::Range { + 0..self.mipmap_levels() + } + + #[inline] + fn current_layer_levels_access(&self) -> std::ops::Range { + 0..1 + } } unsafe impl ImageClearValue<::ClearValue> for SwapchainImage { diff --git a/vulkano/src/image/traits.rs b/vulkano/src/image/traits.rs index 57996a94..592ba75b 100644 --- a/vulkano/src/image/traits.rs +++ b/vulkano/src/image/traits.rs @@ -187,6 +187,12 @@ pub unsafe trait ImageAccess { /// verify whether they actually overlap. fn conflict_key(&self) -> u64; + /// Returns the current mip level that is accessed by the gpu + fn current_miplevels_access(&self) -> std::ops::Range; + + /// Returns the current layer level that is accessed by the gpu + fn current_layer_levels_access(&self) -> std::ops::Range; + /// Locks the resource for usage on the GPU. Returns an error if the lock can't be acquired. /// /// After this function returns `Ok`, you are authorized to use the image on the GPU. If the @@ -324,6 +330,14 @@ where fn is_layout_initialized(&self) -> bool { (**self).is_layout_initialized() } + + fn current_miplevels_access(&self) -> std::ops::Range { + (**self).current_miplevels_access() + } + + fn current_layer_levels_access(&self) -> std::ops::Range { + (**self).current_layer_levels_access() + } } impl PartialEq for dyn ImageAccess + Send + Sync { @@ -406,6 +420,14 @@ where unsafe fn unlock(&self, new_layout: Option) { self.image.unlock(new_layout) } + + fn current_miplevels_access(&self) -> std::ops::Range { + self.image.current_miplevels_access() + } + + fn current_layer_levels_access(&self) -> std::ops::Range { + self.image.current_layer_levels_access() + } } impl PartialEq for ImageAccessFromUndefinedLayout