Fix image layout transitions (#1171)

vulkano/src/command_buffer/synced/base.rs

@@ -510,7 +510,7 @@ impl<P> SyncCommandBufferBuilder<P> {
                     // collision. But since the pipeline barrier is going to be submitted before
                     // the flushed commands, it would be a mistake if `collision_cmd_id` hasn't
                     // been flushed yet.
-                    if collision_cmd_id >= first_unflushed_cmd_id {
+                    if collision_cmd_id >= first_unflushed_cmd_id || entry.current_layout != start_layout {
                         unsafe {
                             // Flush the pending barrier.
                             self.inner.pipeline_barrier(&self.pending_barrier);
@@ -625,6 +625,7 @@ impl<P> SyncCommandBufferBuilder<P> {
                 let mut actually_exclusive = exclusive;
                 let mut actual_start_layout = start_layout;
 
+
                 if !self.is_secondary && resource_ty == KeyTy::Image &&
                     start_layout != ImageLayout::Undefined &&
                     start_layout != ImageLayout::Preinitialized
@@ -633,9 +634,11 @@ impl<P> SyncCommandBufferBuilder<P> {
                     let img = commands_lock.commands[latest_command_id].image(resource_index);
                     let initial_layout_requirement = img.initial_layout_requirement();
 
-                    if initial_layout_requirement != start_layout {
-                        actually_exclusive = true;
-                        actual_start_layout = initial_layout_requirement;
+                    // Checks if the image is initialized and transitions it
+                    // if it isn't
+                    let is_layout_initialized = img.is_layout_initialized();
+
+                    if initial_layout_requirement != start_layout || !is_layout_initialized {
 
                         // Note that we transition from `bottom_of_pipe`, which means that we
                         // wait for all the previous commands to be entirely finished. This is
@@ -648,6 +651,19 @@ impl<P> SyncCommandBufferBuilder<P> {
                         //   suboptimal in some cases, in the general situation it will be ok.
                         //
                         unsafe {
+                            let from_layout = if is_layout_initialized {
+                                actually_exclusive = true;
+                                initial_layout_requirement
+                            } else {
+                                if img.preinitialized_layout() {
+                                    ImageLayout::Preinitialized
+                                } else {
+                                    ImageLayout::Undefined
+                                }
+                            };
+                            if  initial_layout_requirement != start_layout {
+                                actual_start_layout = initial_layout_requirement;
+                            }
                             let b = &mut self.pending_barrier;
                             b.add_image_memory_barrier(img,
                                                        0 .. img.mipmap_levels(),
@@ -661,8 +677,9 @@ impl<P> SyncCommandBufferBuilder<P> {
                                                        access,
                                                        true,
                                                        None,
-                                                       initial_layout_requirement,
+                                                       from_layout,
                                                        start_layout);
+                            img.layout_initialized();
                         }
                     }
                 }

vulkano/src/framebuffer/macros.rs

@@ -312,18 +312,6 @@ macro_rules! ordered_passes_renderpass {
                 })*
 
                 $(if $atch_name == num {
-                    // If the clear OP is Clear or DontCare, default to the Undefined layout.
-                    if initial_layout == Some(ImageLayout::DepthStencilAttachmentOptimal) ||
-                        initial_layout == Some(ImageLayout::ColorAttachmentOptimal) ||
-                        initial_layout == Some(ImageLayout::TransferDstOptimal)
-                    {
-                        if $crate::framebuffer::LoadOp::$load == $crate::framebuffer::LoadOp::Clear ||
-                            $crate::framebuffer::LoadOp::$load == $crate::framebuffer::LoadOp::DontCare
-                        {
-                            initial_layout = Some(ImageLayout::Undefined);
-                        }
-                    }
-
                     $(initial_layout = Some($init_layout);)*
                     $(final_layout = Some($final_layout);)*
                 })*

vulkano/src/image/attachment.rs

@@ -497,6 +497,16 @@ unsafe impl<F, A> ImageAccess for AttachmentImage<F, A>
         let prev_val = self.gpu_lock.fetch_sub(1, Ordering::SeqCst);
         debug_assert!(prev_val >= 1);
     }
+
+    #[inline]
+    unsafe fn layout_initialized(&self) {
+       self.initialized.store(true, Ordering::SeqCst);
+    }
+
+    #[inline]
+    fn is_layout_initialized(&self) -> bool {
+       self.initialized.load(Ordering::SeqCst)
+    }
 }
 
 unsafe impl<F, A> ImageClearValue<F::ClearValue> for Arc<AttachmentImage<F, A>>

vulkano/src/image/swapchain.rs

@@ -82,6 +82,16 @@ impl<W> SwapchainImage<W> {
     fn my_image(&self) -> ImageInner {
         self.swapchain.raw_image(self.image_offset).unwrap()
     }
+
+    #[inline]
+    fn layout_initialized(&self) {
+        self.swapchain.image_layout_initialized(self.image_offset);
+    }
+
+    #[inline]
+    fn is_layout_initialized(&self) -> bool {
+       self.swapchain.is_image_layout_initialized(self.image_offset)
+    }
 }
 
 unsafe impl<W> ImageAccess for SwapchainImage<W> {
@@ -121,6 +131,16 @@ unsafe impl<W> ImageAccess for SwapchainImage<W> {
         Err(AccessError::SwapchainImageAcquireOnly)
     }
 
+    #[inline]
+    unsafe fn layout_initialized(&self) {
+        self.layout_initialized();
+    }
+
+    #[inline]
+    fn is_layout_initialized(&self) -> bool{
+        self.is_layout_initialized()
+    }
+
     #[inline]
     unsafe fn increase_gpu_lock(&self) {
     }

vulkano/src/image/sys.rs

@@ -67,6 +67,7 @@ pub struct UnsafeImage {
 
     // `vkDestroyImage` is called only if `needs_destruction` is true.
     needs_destruction: bool,
+    preinitialized_layout: bool,
 }
 
 impl UnsafeImage {
@@ -551,6 +552,7 @@ impl UnsafeImage {
             mipmaps: mipmaps,
             format_features: format_features,
             needs_destruction: true,
+            preinitialized_layout,
         };
 
         Ok((image, mem_reqs))
@@ -580,6 +582,7 @@ impl UnsafeImage {
             mipmaps: mipmaps,
             format_features: output.optimalTilingFeatures,
             needs_destruction: false, // TODO: pass as parameter
+            preinitialized_layout: false, // TODO: Maybe this should be passed in?
         }
     }
 
@@ -778,6 +781,11 @@ impl UnsafeImage {
     pub fn usage_input_attachment(&self) -> bool {
         (self.usage & vk::IMAGE_USAGE_INPUT_ATTACHMENT_BIT) != 0
     }
+
+    #[inline]
+    pub fn preinitialized_layout(&self) -> bool {
+        self.preinitialized_layout
+    }
 }
 
 unsafe impl VulkanObject for UnsafeImage {

vulkano/src/image/traits.rs

@@ -92,6 +92,27 @@ pub unsafe trait ImageAccess {
         self.inner().image.supports_blit_destination()
     }
 
+    /// When images are created their memory layout is initially `Undefined` or `Preinitialized`.
+    /// This method allows the image memory barrier creation process to signal when an image
+    /// has been transitioned out of its initial `Undefined` or `Preinitialized` state. This
+    /// allows vulkano to avoid creating unnecessary image memory barriers between future
+    /// uses of the image.
+    ///
+    /// ## Unsafe
+    ///
+    /// If a user calls this method outside of the intended context and signals that the layout
+    /// is no longer `Undefined` or `Preinitialized` when it is still in an `Undefined` or
+    /// `Preinitialized` state, this may result in the vulkan implementation attempting to use
+    /// an image in an invalid layout. The same problem must be considered by the implementer
+    /// of the method.
+    unsafe fn layout_initialized(&self) {}
+
+    fn is_layout_initialized(&self) -> bool {false}
+
+    unsafe fn preinitialized_layout(&self) -> bool {
+        self.inner().image.preinitialized_layout()
+    }
+
     /// Returns the layout that the image has when it is first used in a primary command buffer.
     ///
     /// The first time you use an image in an `AutoCommandBufferBuilder`, vulkano will suppose that
@@ -277,6 +298,16 @@ unsafe impl<T> ImageAccess for T
     unsafe fn unlock(&self, transitioned_layout: Option<ImageLayout>) {
         (**self).unlock(transitioned_layout)
     }
+
+    #[inline]
+    unsafe fn layout_initialized(&self) {
+        (**self).layout_initialized();
+    }
+
+    #[inline]
+    fn is_layout_initialized(&self) -> bool {
+        (**self).is_layout_initialized()
+    }
 }
 
 /// Wraps around an object that implements `ImageAccess` and modifies the initial layout

vulkano/src/swapchain/swapchain.rs

@@ -555,6 +555,24 @@ impl <W> Swapchain<W> {
     pub fn clipped(&self) -> bool {
         self.clipped
     }
+
+    // This method is necessary to allow `SwapchainImage`s to signal when they have been
+    // transitioned out of their initial `undefined` image layout.
+    //
+    // See the `ImageAccess::layout_initialized` method documentation for more details.
+    pub(crate) fn image_layout_initialized(&self, image_offset: usize) {
+        let image_entry = self.images.get(image_offset);
+        if let Some(ref image_entry) = image_entry {
+            image_entry.undefined_layout.store(false, Ordering::SeqCst);
+        }
+    }
+
+    pub(crate) fn is_image_layout_initialized(&self, image_offset: usize) -> bool {
+        let image_entry = self.images.get(image_offset);
+        if let Some(ref image_entry) = image_entry {
+            !image_entry.undefined_layout.load(Ordering::SeqCst)
+        } else { false }
+    }
 }
 
 unsafe impl<W> VulkanObject for Swapchain<W> {