Check image layout in try_gpu_lock (#816)

* [WIP] Check image layout in try_gpu_lock

* Finish implementation

* Add entry in CHANGELOG

* Fix wrong unlock with error recovery

* Improve CommandBufferExecError::AccessError to provide a hint

* Fix AttachmentImage wrong layout report

CHANGELOG.md

@@ -1,9 +1,12 @@
 # Unreleased (major)
 
+- Changed `ImageAccess::try_gpu_lock` and `unlock()` to verify whether the image layout is correct,
+  especially at the first usage of an image.
 - Changed `BufferAccess::conflict_*` and `ImageAccess::conflict_*` to forbid querying a specific
   range of the resource.
 - Changed `CpuBufferPool::next()` and `chunk()` to return a `Result` in case of an error when
   allocating or mapping memory.
+- Changed `CommandBufferExecError::AccessError` to provide a hint of where the error occurs.
 - Fixed `layers` argument validation in `Swapchain::new_inner`.
 - Added `vulkano::pipeline::vertex::BufferlessDefinition` and `BufferlessVertices` to enable
   bufferless drawing.

vulkano/src/command_buffer/synced/base.rs

@@ -811,6 +811,9 @@ struct ResourceFinalState {
 /// Equivalent to `Command`, but with less methods. Typically contains less things than the
 /// `Command` it comes from.
 pub trait FinalCommand {
+    // Returns a user-friendly name for the command, for error reporting purposes.
+    fn name(&self) -> &'static str;
+
     // Gives access to the `num`th buffer used by the command.
     fn buffer(&self, _num: usize) -> &BufferAccess {
         panic!()
@@ -820,9 +823,24 @@ pub trait FinalCommand {
     fn image(&self, _num: usize) -> &ImageAccess {
         panic!()
     }
+
+    // Returns a user-friendly name for the `num`th buffer used by the command, for error
+    // reporting purposes.
+    fn buffer_name(&self, _num: usize) -> Cow<'static, str> {
+        panic!()
+    }
+
+    // Returns a user-friendly name for the `num`th image used by the command, for error
+    // reporting purposes.
+    fn image_name(&self, _num: usize) -> Cow<'static, str> {
+        panic!()
+    }
 }
 
-impl FinalCommand for () {
+impl FinalCommand for &'static str {
+    fn name(&self) -> &'static str {
+        *self
+    }
 }
 
 // Equivalent of `BuilderKey` for a finished command buffer.
@@ -1059,12 +1077,13 @@ impl<P> SyncCommandBuffer<P> {
 
                     match (buf.try_gpu_lock(entry.exclusive, queue), prev_err) {
                         (Ok(_), _) => (),
-                        (Err(err), AccessCheckError::Unknown) => {
-                            ret_value = Err(err.into());
-                            break;
-                        },
-                        (_, AccessCheckError::Denied(err)) => {
-                            ret_value = Err(err.into());
+                        (Err(err), AccessCheckError::Unknown) | (_, AccessCheckError::Denied(err)) => {
+                            ret_value = Err(CommandBufferExecError::AccessError {
+                                error: err,
+                                command_name: cmd.name().into(),
+                                command_param: cmd.buffer_name(resource_index),
+                                command_offset: command_id,
+                            });
                             break;
                         },
                     };
@@ -1087,14 +1106,15 @@ impl<P> SyncCommandBuffer<P> {
                         Err(err) => err
                     };
 
-                    match (img.try_gpu_lock(entry.exclusive, queue), prev_err) {
+                    match (img.try_gpu_lock(entry.exclusive, entry.initial_layout), prev_err) {
                         (Ok(_), _) => (),
-                        (Err(err), AccessCheckError::Unknown) => {
-                            ret_value = Err(err.into());
-                            break;
-                        },
-                        (_, AccessCheckError::Denied(err)) => {
-                            ret_value = Err(err.into());
+                        (Err(err), AccessCheckError::Unknown) | (_, AccessCheckError::Denied(err)) => {
+                            ret_value = Err(CommandBufferExecError::AccessError {
+                                error: err,
+                                command_name: cmd.name().into(),
+                                command_param: cmd.image_name(resource_index),
+                                command_offset: command_id,
+                            });
                             break;
                         },
                     };
@@ -1132,7 +1152,7 @@ impl<P> SyncCommandBuffer<P> {
                         let cmd = &commands_lock[command_id];
                         let img = cmd.image(resource_index);
                         unsafe {
-                            img.unlock();
+                            img.unlock(None);
                         }
                     },
                 }
@@ -1155,7 +1175,7 @@ impl<P> SyncCommandBuffer<P> {
     pub unsafe fn unlock(&self) {
         let commands_lock = self.commands.lock().unwrap();
 
-        for key in self.resources.keys() {
+        for (key, val) in self.resources.iter() {
             let (command_id, resource_ty, resource_index) = match *key {
                 CbKey::Command {
                     command_id,
@@ -1177,7 +1197,12 @@ impl<P> SyncCommandBuffer<P> {
                 KeyTy::Image => {
                     let cmd = &commands_lock[command_id];
                     let img = cmd.image(resource_index);
-                    img.unlock();
+                    let trans = if val.final_layout != val.initial_layout {
+                        Some(val.final_layout)
+                    } else {
+                        None
+                    };
+                    img.unlock(trans);
                 },
             }
         }

vulkano/src/command_buffer/synced/commands.rs

@@ -83,9 +83,15 @@ impl<P> SyncCommandBufferBuilder<P> {
                 impl<F> FinalCommand for Fin<F>
                     where F: FramebufferAbstract + Send + Sync + 'static
                 {
+                    fn name(&self) -> &'static str {
+                        "vkCmdBeginRenderPass"
+                    }
                     fn image(&self, num: usize) -> &ImageAccess {
                         self.0.attached_image_view(num).unwrap().parent()
                     }
+                    fn image_name(&self, num: usize) -> Cow<'static, str> {
+                        format!("attachment {}", num).into()
+                    }
                 }
                 Box::new(Fin(self.framebuffer))
             }
@@ -157,10 +163,17 @@ impl<P> SyncCommandBufferBuilder<P> {
                 impl<B> FinalCommand for Fin<B>
                     where B: BufferAccess + Send + Sync + 'static
                 {
+                    fn name(&self) -> &'static str {
+                        "vkCmdBindIndexBuffer"
+                    }
                     fn buffer(&self, num: usize) -> &BufferAccess {
                         assert_eq!(num, 0);
                         &self.0
                     }
+                    fn buffer_name(&self, num: usize) -> Cow<'static, str> {
+                        assert_eq!(num, 0);
+                        "index buffer".into()
+                    }
                 }
                 Box::new(Fin(self.buffer))
             }
@@ -218,6 +231,9 @@ impl<P> SyncCommandBufferBuilder<P> {
                 impl<Gp> FinalCommand for Fin<Gp>
                     where Gp: Send + Sync + 'static
                 {
+                    fn name(&self) -> &'static str {
+                        "vkCmdBindPipeline"
+                    }
                 }
                 Box::new(Fin(self.pipeline))
             }
@@ -251,6 +267,9 @@ impl<P> SyncCommandBufferBuilder<P> {
                 impl<Cp> FinalCommand for Fin<Cp>
                     where Cp: Send + Sync + 'static
                 {
+                    fn name(&self) -> &'static str {
+                        "vkCmdBindPipeline"
+                    }
                 }
                 Box::new(Fin(self.pipeline))
             }
@@ -326,6 +345,9 @@ impl<P> SyncCommandBufferBuilder<P> {
                     where S: ImageAccess + Send + Sync + 'static,
                           D: ImageAccess + Send + Sync + 'static
                 {
+                    fn name(&self) -> &'static str {
+                        "vkCmdBlitImage"
+                    }
                     fn image(&self, num: usize) -> &ImageAccess {
                         if num == 0 {
                             &self.0
@@ -335,6 +357,15 @@ impl<P> SyncCommandBufferBuilder<P> {
                             panic!()
                         }
                     }
+                    fn image_name(&self, num: usize) -> Cow<'static, str> {
+                        if num == 0 {
+                            "source".into()
+                        } else if num == 1 {
+                            "destination".into()
+                        } else {
+                            panic!()
+                        }
+                    }
                 }
 
                 // Note: borrow checker somehow doesn't accept `self.source` and `self.destination`
@@ -441,10 +472,17 @@ impl<P> SyncCommandBufferBuilder<P> {
                 impl<I> FinalCommand for Fin<I>
                     where I: ImageAccess + Send + Sync + 'static
                 {
+                    fn name(&self) -> &'static str {
+                        "vkCmdClearColorImage"
+                    }
                     fn image(&self, num: usize) -> &ImageAccess {
                         assert_eq!(num, 0);
                         &self.0
                     }
+                    fn image_name(&self, num: usize) -> Cow<'static, str> {
+                        assert_eq!(num, 0);
+                        "target".into()
+                    }
                 }
 
                 // Note: borrow checker somehow doesn't accept `self.image` without using an Option.
@@ -522,6 +560,9 @@ impl<P> SyncCommandBufferBuilder<P> {
                     where S: BufferAccess + Send + Sync + 'static,
                           D: BufferAccess + Send + Sync + 'static
                 {
+                    fn name(&self) -> &'static str {
+                        "vkCmdCopyBuffer"
+                    }
                     fn buffer(&self, num: usize) -> &BufferAccess {
                         match num {
                             0 => &self.0,
@@ -529,6 +570,13 @@ impl<P> SyncCommandBufferBuilder<P> {
                             _ => panic!(),
                         }
                     }
+                    fn buffer_name(&self, num: usize) -> Cow<'static, str> {
+                        match num {
+                            0 => "source".into(),
+                            1 => "destination".into(),
+                            _ => panic!(),
+                        }
+                    }
                 }
                 // Note: borrow checker somehow doesn't accept `self.source` and `self.destination`
                 // without using an Option.
@@ -628,15 +676,25 @@ impl<P> SyncCommandBufferBuilder<P> {
                     where S: BufferAccess + Send + Sync + 'static,
                           D: ImageAccess + Send + Sync + 'static
                 {
+                    fn name(&self) -> &'static str {
+                        "vkCmdCopyBufferToImage"
+                    }
                     fn buffer(&self, num: usize) -> &BufferAccess {
                         assert_eq!(num, 0);
                         &self.0
                     }
-
+                    fn buffer_name(&self, num: usize) -> Cow<'static, str> {
+                        assert_eq!(num, 0);
+                        "source".into()
+                    }
                     fn image(&self, num: usize) -> &ImageAccess {
                         assert_eq!(num, 0);
                         &self.1
                     }
+                    fn image_name(&self, num: usize) -> Cow<'static, str> {
+                        assert_eq!(num, 0);
+                        "destination".into()
+                    }
                 }
 
                 // Note: borrow checker somehow doesn't accept `self.source` and `self.destination`
@@ -742,15 +800,25 @@ impl<P> SyncCommandBufferBuilder<P> {
                     where S: ImageAccess + Send + Sync + 'static,
                           D: BufferAccess + Send + Sync + 'static
                 {
+                    fn name(&self) -> &'static str {
+                        "vkCmdCopyImageToBuffer"
+                    }
                     fn buffer(&self, num: usize) -> &BufferAccess {
                         assert_eq!(num, 0);
                         &self.1
                     }
-
+                    fn buffer_name(&self, num: usize) -> Cow<'static, str> {
+                        assert_eq!(num, 0);
+                        "destination".into()
+                    }
                     fn image(&self, num: usize) -> &ImageAccess {
                         assert_eq!(num, 0);
                         &self.0
                     }
+                    fn image_name(&self, num: usize) -> Cow<'static, str> {
+                        assert_eq!(num, 0);
+                        "source".into()
+                    }
                 }
 
                 // Note: borrow checker somehow doesn't accept `self.source` and `self.destination`
@@ -832,7 +900,7 @@ impl<P> SyncCommandBufferBuilder<P> {
             }
 
             fn into_final_command(self: Box<Self>) -> Box<FinalCommand + Send + Sync> {
-                Box::new(())
+                Box::new("vkCmdDispatch")
             }
         }
 
@@ -865,10 +933,17 @@ impl<P> SyncCommandBufferBuilder<P> {
                 impl<B> FinalCommand for Fin<B>
                     where B: BufferAccess + Send + Sync + 'static
                 {
+                    fn name(&self) -> &'static str {
+                        "vkCmdDispatchIndirect"
+                    }
                     fn buffer(&self, num: usize) -> &BufferAccess {
                         assert_eq!(num, 0);
                         &self.0
                     }
+                    fn buffer_name(&self, num: usize) -> Cow<'static, str> {
+                        assert_eq!(num, 0);
+                        "indirect buffer".into()
+                    }
                 }
                 Box::new(Fin(self.buffer))
             }
@@ -925,7 +1000,7 @@ impl<P> SyncCommandBufferBuilder<P> {
             }
 
             fn into_final_command(self: Box<Self>) -> Box<FinalCommand + Send + Sync> {
-                Box::new(())
+                Box::new("vkCmdDraw")
             }
         }
 
@@ -964,7 +1039,7 @@ impl<P> SyncCommandBufferBuilder<P> {
             }
 
             fn into_final_command(self: Box<Self>) -> Box<FinalCommand + Send + Sync> {
-                Box::new(())
+                Box::new("vkCmdDrawIndexed")
             }
         }
 
@@ -1005,10 +1080,17 @@ impl<P> SyncCommandBufferBuilder<P> {
                 impl<B> FinalCommand for Fin<B>
                     where B: BufferAccess + Send + Sync + 'static
                 {
+                    fn name(&self) -> &'static str {
+                        "vkCmdDrawIndirect"
+                    }
                     fn buffer(&self, num: usize) -> &BufferAccess {
                         assert_eq!(num, 0);
                         &self.0
                     }
+                    fn buffer_name(&self, num: usize) -> Cow<'static, str> {
+                        assert_eq!(num, 0);
+                        "indirect buffer".into()
+                    }
                 }
                 Box::new(Fin(self.buffer))
             }
@@ -1073,10 +1155,17 @@ impl<P> SyncCommandBufferBuilder<P> {
                 impl<B> FinalCommand for Fin<B>
                     where B: BufferAccess + Send + Sync + 'static
                 {
+                    fn name(&self) -> &'static str {
+                        "vkCmdDrawIndexedIndirect"
+                    }
                     fn buffer(&self, num: usize) -> &BufferAccess {
                         assert_eq!(num, 0);
                         &self.0
                     }
+                    fn buffer_name(&self, num: usize) -> Cow<'static, str> {
+                        assert_eq!(num, 0);
+                        "indirect buffer".into()
+                    }
                 }
                 Box::new(Fin(self.buffer))
             }
@@ -1128,7 +1217,7 @@ impl<P> SyncCommandBufferBuilder<P> {
             }
 
             fn into_final_command(self: Box<Self>) -> Box<FinalCommand + Send + Sync> {
-                Box::new(())
+                Box::new("vkCmdEndRenderPass")
             }
         }
 
@@ -1173,10 +1262,16 @@ impl<P> SyncCommandBufferBuilder<P> {
                 impl<B> FinalCommand for Fin<B>
                     where B: BufferAccess + Send + Sync + 'static
                 {
+                    fn name(&self) -> &'static str {
+                        "vkCmdFillBuffer"
+                    }
                     fn buffer(&self, num: usize) -> &BufferAccess {
                         assert_eq!(num, 0);
                         &self.0
                     }
+                    fn buffer_name(&self, _: usize) -> Cow<'static, str> {
+                        "destination".into()
+                    }
                 }
                 Box::new(Fin(self.buffer))
             }
@@ -1225,7 +1320,7 @@ impl<P> SyncCommandBufferBuilder<P> {
             }
 
             fn into_final_command(self: Box<Self>) -> Box<FinalCommand + Send + Sync> {
-                Box::new(())
+                Box::new("vkCmdNextSubpass")
             }
         }
 
@@ -1267,6 +1362,9 @@ impl<P> SyncCommandBufferBuilder<P> {
                 impl<Pl> FinalCommand for Fin<Pl>
                     where Pl: Send + Sync + 'static
                 {
+                    fn name(&self) -> &'static str {
+                        "vkCmdPushConstants"
+                    }
                 }
                 Box::new(Fin(self.pipeline_layout))
             }
@@ -1309,6 +1407,9 @@ impl<P> SyncCommandBufferBuilder<P> {
             fn into_final_command(self: Box<Self>) -> Box<FinalCommand + Send + Sync> {
                 struct Fin(Arc<Event>);
                 impl FinalCommand for Fin {
+                    fn name(&self) -> &'static str {
+                        "vkCmdResetEvent"
+                    }
                 }
                 Box::new(Fin(self.event))
             }
@@ -1334,7 +1435,7 @@ impl<P> SyncCommandBufferBuilder<P> {
             }
 
             fn into_final_command(self: Box<Self>) -> Box<FinalCommand + Send + Sync> {
-                Box::new(())
+                Box::new("vkCmdSetBlendConstants")
             }
         }
 
@@ -1360,7 +1461,7 @@ impl<P> SyncCommandBufferBuilder<P> {
             }
 
             fn into_final_command(self: Box<Self>) -> Box<FinalCommand + Send + Sync> {
-                Box::new(())
+                Box::new("vkCmdSetDepthBias")
             }
         }
 
@@ -1389,7 +1490,7 @@ impl<P> SyncCommandBufferBuilder<P> {
             }
 
             fn into_final_command(self: Box<Self>) -> Box<FinalCommand + Send + Sync> {
-                Box::new(())
+                Box::new("vkCmdSetDepthBounds")
             }
         }
 
@@ -1416,6 +1517,9 @@ impl<P> SyncCommandBufferBuilder<P> {
             fn into_final_command(self: Box<Self>) -> Box<FinalCommand + Send + Sync> {
                 struct Fin(Arc<Event>);
                 impl FinalCommand for Fin {
+                    fn name(&self) -> &'static str {
+                        "vkCmdSetEvent"
+                    }
                 }
                 Box::new(Fin(self.event))
             }
@@ -1441,7 +1545,7 @@ impl<P> SyncCommandBufferBuilder<P> {
             }
 
             fn into_final_command(self: Box<Self>) -> Box<FinalCommand + Send + Sync> {
-                Box::new(())
+                Box::new("vkCmdSetLineWidth")
             }
         }
 
@@ -1474,7 +1578,7 @@ impl<P> SyncCommandBufferBuilder<P> {
             }
 
             fn into_final_command(self: Box<Self>) -> Box<FinalCommand + Send + Sync> {
-                Box::new(())
+                Box::new("vkCmdSetScissor")
             }
         }
 
@@ -1508,7 +1612,7 @@ impl<P> SyncCommandBufferBuilder<P> {
             }
 
             fn into_final_command(self: Box<Self>) -> Box<FinalCommand + Send + Sync> {
-                Box::new(())
+                Box::new("vkCmdSetViewport")
             }
         }
 
@@ -1546,10 +1650,16 @@ impl<P> SyncCommandBufferBuilder<P> {
                 impl<B> FinalCommand for Fin<B>
                     where B: BufferAccess + Send + Sync + 'static
                 {
+                    fn name(&self) -> &'static str {
+                        "vkCmdUpdateBuffer"
+                    }
                     fn buffer(&self, num: usize) -> &BufferAccess {
                         assert_eq!(num, 0);
                         &self.0
                     }
+                    fn buffer_name(&self, _: usize) -> Cow<'static, str> {
+                        "destination".into()
+                    }
                 }
                 Box::new(Fin(self.buffer))
             }
@@ -1634,6 +1744,9 @@ impl<'b, P> SyncCommandBufferBuilderBindDescriptorSets<'b, P> {
             fn into_final_command(self: Box<Self>) -> Box<FinalCommand + Send + Sync> {
                 struct Fin(SmallVec<[Box<DescriptorSet + Send + Sync>; 12]>);
                 impl FinalCommand for Fin {
+                    fn name(&self) -> &'static str {
+                        "vkCmdBindDescriptorSets"
+                    }
                     fn buffer(&self, mut num: usize) -> &BufferAccess {
                         for set in self.0.iter() {
                             if let Some(buf) = set.buffer(num) {
@@ -1643,6 +1756,16 @@ impl<'b, P> SyncCommandBufferBuilderBindDescriptorSets<'b, P> {
                         }
                         panic!()
                     }
+                    fn buffer_name(&self, mut num: usize) -> Cow<'static, str> {
+                        for (set_num, set) in self.0.iter().enumerate() {
+                            if let Some(buf) = set.buffer(num) {
+                                return format!("Buffer bound to descriptor {} of set {}", buf.1, set_num)
+                                    .into();
+                            }
+                            num -= set.num_buffers();
+                        }
+                        panic!()
+                    }
                     fn image(&self, mut num: usize) -> &ImageAccess {
                         for set in self.0.iter() {
                             if let Some(img) = set.image(num) {
@@ -1652,6 +1775,16 @@ impl<'b, P> SyncCommandBufferBuilderBindDescriptorSets<'b, P> {
                         }
                         panic!()
                     }
+                    fn image_name(&self, mut num: usize) -> Cow<'static, str> {
+                        for (set_num, set) in self.0.iter().enumerate() {
+                            if let Some(img) = set.image(num) {
+                                return format!("Image bound to descriptor {} of set {}", img.1, set_num)
+                                    .into();
+                            }
+                            num -= set.num_images();
+                        }
+                        panic!()
+                    }
                 }
                 Box::new(Fin(self.inner))
             }
@@ -1820,9 +1953,15 @@ impl<'a, P> SyncCommandBufferBuilderBindVertexBuffer<'a, P> {
             fn into_final_command(self: Box<Self>) -> Box<FinalCommand + Send + Sync> {
                 struct Fin(Vec<Box<BufferAccess + Send + Sync>>);
                 impl FinalCommand for Fin {
+                    fn name(&self) -> &'static str {
+                        "vkCmdBindVertexBuffers"
+                    }
                     fn buffer(&self, num: usize) -> &BufferAccess {
                         &self.0[num]
                     }
+                    fn buffer_name(&self, num: usize) -> Cow<'static, str> {
+                        format!("Buffer #{}", num).into()
+                    }
                 }
                 Box::new(Fin(self.buffers))
             }
@@ -1903,6 +2042,9 @@ impl<'a, P> SyncCommandBufferBuilderExecuteCommands<'a, P> {
             fn into_final_command(self: Box<Self>) -> Box<FinalCommand + Send + Sync> {
                 struct Fin(Vec<Box<Any + Send + Sync>>);
                 impl FinalCommand for Fin {
+                    fn name(&self) -> &'static str {
+                        "vkCmdExecuteCommands"
+                    }
                 }
                 Box::new(Fin(self.command_buffers))
             }

vulkano/src/command_buffer/traits.rs

@@ -7,6 +7,7 @@
 // notice may not be copied, modified, or distributed except
 // according to those terms.
 
+use std::borrow::Cow;
 use std::error;
 use std::fmt;
 use std::sync::Arc;
@@ -340,7 +341,12 @@ impl<F, Cb> Drop for CommandBufferExecFuture<F, Cb>
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub enum CommandBufferExecError {
     /// Access to a resource has been denied.
-    AccessError(AccessError),
+    AccessError {
+        error: AccessError,
+        command_name: Cow<'static, str>,
+        command_param: Cow<'static, str>,
+        command_offset: usize,
+    },
 
     /// The command buffer or one of the secondary command buffers it executes was created with the
     /// "one time submit" flag, but has already been submitted it the past.
@@ -357,7 +363,7 @@ impl error::Error for CommandBufferExecError {
     #[inline]
     fn description(&self) -> &str {
         match *self {
-            CommandBufferExecError::AccessError(_) => {
+            CommandBufferExecError::AccessError { .. } => {
                 "access to a resource has been denied"
             },
             CommandBufferExecError::OneTimeSubmitAlreadySubmitted => {
@@ -375,7 +381,7 @@ impl error::Error for CommandBufferExecError {
     #[inline]
     fn cause(&self) -> Option<&error::Error> {
         match *self {
-            CommandBufferExecError::AccessError(ref err) => Some(err),
+            CommandBufferExecError::AccessError { ref error, .. } => Some(error),
             _ => None,
         }
     }
@@ -387,10 +393,3 @@ impl fmt::Display for CommandBufferExecError {
         write!(fmt, "{}", error::Error::description(self))
     }
 }
-
-impl From<AccessError> for CommandBufferExecError {
-    #[inline]
-    fn from(err: AccessError) -> CommandBufferExecError {
-        CommandBufferExecError::AccessError(err)
-    }
-}

vulkano/src/image/attachment.rs

@@ -9,12 +9,12 @@
 
 use std::iter::Empty;
 use std::sync::Arc;
+use std::sync::atomic::AtomicBool;
 use std::sync::atomic::AtomicUsize;
 use std::sync::atomic::Ordering;
 
 use buffer::BufferAccess;
 use device::Device;
-use device::Queue;
 use format::ClearValue;
 use format::Format;
 use format::FormatDesc;
@@ -90,6 +90,10 @@ pub struct AttachmentImage<F = Format, A = PotentialDedicatedAllocation<StdMemor
     // Must be either "depth-stencil optimal" or "color optimal".
     attachment_layout: ImageLayout,
 
+    // If true, then the image is in the layout of `attachment_layout` (above). If false, then it
+    // is still `Undefined`.
+    initialized: AtomicBool,
+
     // Number of times this image is locked on the GPU side.
     gpu_lock: AtomicUsize,
 }
@@ -394,6 +398,7 @@ impl<F> AttachmentImage<F> {
                         } else {
                             ImageLayout::ColorAttachmentOptimal
                         },
+                        initialized: AtomicBool::new(false),
                         gpu_lock: AtomicUsize::new(0),
                     }))
     }
@@ -448,7 +453,29 @@ unsafe impl<F, A> ImageAccess for AttachmentImage<F, A>
     }
 
     #[inline]
-    fn try_gpu_lock(&self, _: bool, _: &Queue) -> Result<(), AccessError> {
+    fn try_gpu_lock(&self, _: bool, expected_layout: ImageLayout) -> Result<(), AccessError> {
+        if expected_layout != self.attachment_layout && expected_layout != ImageLayout::Undefined {
+            if self.initialized.load(Ordering::SeqCst) {
+                return Err(AccessError::UnexpectedImageLayout {
+                    requested: expected_layout,
+                    allowed: self.attachment_layout,
+                });
+            } else {
+                return Err(AccessError::UnexpectedImageLayout {
+                    requested: expected_layout,
+                    allowed: ImageLayout::Undefined,
+                });
+            }
+        }
+
+        if expected_layout != ImageLayout::Undefined {
+            if !self.initialized.load(Ordering::SeqCst) {
+                return Err(AccessError::ImageNotInitialized {
+                    requested: expected_layout,
+                });
+            }
+        }
+
         if self.gpu_lock.compare_and_swap(0, 1, Ordering::SeqCst) == 0 {
             Ok(())
         } else {
@@ -463,7 +490,12 @@ unsafe impl<F, A> ImageAccess for AttachmentImage<F, A>
     }
 
     #[inline]
-    unsafe fn unlock(&self) {
+    unsafe fn unlock(&self, new_layout: Option<ImageLayout>) {
+        if let Some(new_layout) = new_layout {
+            debug_assert_eq!(new_layout, self.attachment_layout);
+            self.initialized.store(true, Ordering::SeqCst);
+        }
+
         let prev_val = self.gpu_lock.fetch_sub(1, Ordering::SeqCst);
         debug_assert!(prev_val >= 1);
     }

vulkano/src/image/immutable.rs

@@ -308,7 +308,16 @@ unsafe impl<F, A> ImageAccess for ImmutableImage<F, A>
     }
 
     #[inline]
-    fn try_gpu_lock(&self, exclusive_access: bool, _: &Queue) -> Result<(), AccessError> {
+    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,
+            });
+        }
+
         if exclusive_access {
             return Err(AccessError::ExclusiveDenied);
         }
@@ -325,7 +334,8 @@ unsafe impl<F, A> ImageAccess for ImmutableImage<F, A>
     }
 
     #[inline]
-    unsafe fn unlock(&self) {
+    unsafe fn unlock(&self, new_layout: Option<ImageLayout>) {
+        debug_assert!(new_layout.is_none());
     }
 }
 
@@ -416,7 +426,14 @@ unsafe impl<F, A> ImageAccess for ImmutableImageInitialization<F, A>
     }
 
     #[inline]
-    fn try_gpu_lock(&self, _: bool, _: &Queue) -> Result<(), AccessError> {
+    fn try_gpu_lock(&self, _: bool, expected_layout: ImageLayout) -> Result<(), AccessError> {
+        if expected_layout != ImageLayout::Undefined {
+            return Err(AccessError::UnexpectedImageLayout {
+                requested: expected_layout,
+                allowed: ImageLayout::Undefined,
+            });
+        }
+
         if self.image.initialized.load(Ordering::Relaxed) {
             return Err(AccessError::AlreadyInUse);
         }
@@ -435,7 +452,8 @@ unsafe impl<F, A> ImageAccess for ImmutableImageInitialization<F, A>
     }
 
     #[inline]
-    unsafe fn unlock(&self) {
+    unsafe fn unlock(&self, new_layout: Option<ImageLayout>) {
+        assert_eq!(new_layout, Some(self.image.layout));
         self.image.initialized.store(true, Ordering::Relaxed);
     }
 }

vulkano/src/image/storage.rs

@@ -14,7 +14,6 @@ use std::sync::atomic::Ordering;
 
 use buffer::BufferAccess;
 use device::Device;
-use device::Queue;
 use format::ClearValue;
 use format::FormatDesc;
 use format::FormatTy;
@@ -214,7 +213,15 @@ unsafe impl<F, A> ImageAccess for StorageImage<F, A>
     }
 
     #[inline]
-    fn try_gpu_lock(&self, _: bool, _: &Queue) -> Result<(), AccessError> {
+    fn try_gpu_lock(&self, _: bool, expected_layout: ImageLayout) -> Result<(), AccessError> {
+        // TODO: handle initial layout transition
+        if expected_layout != ImageLayout::General && expected_layout != ImageLayout::Undefined {
+            return Err(AccessError::UnexpectedImageLayout {
+                requested: expected_layout,
+                allowed: ImageLayout::General,
+            });
+        }
+
         let val = self.gpu_lock.compare_and_swap(0, 1, Ordering::SeqCst);
         if val == 0 {
             Ok(())
@@ -230,7 +237,8 @@ unsafe impl<F, A> ImageAccess for StorageImage<F, A>
     }
 
     #[inline]
-    unsafe fn unlock(&self) {
+    unsafe fn unlock(&self, new_layout: Option<ImageLayout>) {
+        assert!(new_layout.is_none() || new_layout == Some(ImageLayout::General));
         self.gpu_lock.fetch_sub(1, Ordering::SeqCst);
     }
 }

vulkano/src/image/swapchain.rs

@@ -10,7 +10,6 @@
 use std::sync::Arc;
 
 use buffer::BufferAccess;
-use device::Queue;
 use format::ClearValue;
 use format::Format;
 use format::FormatDesc;
@@ -117,7 +116,7 @@ unsafe impl ImageAccess for SwapchainImage {
     }
 
     #[inline]
-    fn try_gpu_lock(&self, _: bool, _: &Queue) -> Result<(), AccessError> {
+    fn try_gpu_lock(&self, _: bool, _: ImageLayout) -> Result<(), AccessError> {
         // Swapchain image are only accessible after being acquired.
         Err(AccessError::SwapchainImageAcquireOnly)
     }
@@ -127,7 +126,8 @@ unsafe impl ImageAccess for SwapchainImage {
     }
 
     #[inline]
-    unsafe fn unlock(&self) {
+    unsafe fn unlock(&self, _: Option<ImageLayout>) {
+        // TODO: store that the image was initialized
     }
 }
 

vulkano/src/image/traits.rs

@@ -8,7 +8,6 @@
 // according to those terms.
 
 use buffer::BufferAccess;
-use device::Queue;
 use format::ClearValue;
 use format::Format;
 use format::PossibleDepthFormatDesc;
@@ -160,13 +159,23 @@ pub unsafe trait ImageAccess {
 
     /// 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
+    /// GPU operation requires an exclusive access to the image (which includes image layout
+    /// transitions) then `exlusive_access` should be true.
+    ///
+    /// The `expected_layout` is the layout we expect the image to be in when we lock it. If the
+    /// actual layout doesn't match this expected layout, then an error should be returned. If
+    /// `Undefined` is passed, that means that the caller doesn't care about the actual layout,
+    /// and that a layout mismatch shouldn't return an error.
+    ///
     /// This function exists to prevent the user from causing a data race by reading and writing
     /// to the same resource at the same time.
     ///
     /// If you call this function, you should call `unlock()` once the resource is no longer in use
     /// by the GPU. The implementation is not expected to automatically perform any unlocking and
     /// can rely on the fact that `unlock()` is going to be called.
-    fn try_gpu_lock(&self, exclusive_access: bool, queue: &Queue) -> Result<(), AccessError>;
+    fn try_gpu_lock(&self, exclusive_access: bool, expected_layout: ImageLayout)
+                    -> Result<(), AccessError>;
 
     /// Locks the resource for usage on the GPU. Supposes that the resource is already locked, and
     /// simply increases the lock by one.
@@ -180,10 +189,24 @@ pub unsafe trait ImageAccess {
 
     /// Unlocks the resource previously acquired with `try_gpu_lock` or `increase_gpu_lock`.
     ///
+    /// If the GPU operation that we unlock from transitionned the image to another layout, then
+    /// it should be passed as parameter.
+    ///
+    /// A layout transition requires exclusive access to the image, which means two things:
+    ///
+    /// - The implementation can panic if it finds out that the layout is not the same as it
+    ///   currently is and that it is not locked in exclusive mode.
+    /// - There shouldn't be any possible race between `unlock` and `try_gpu_lock`, since
+    ///   `try_gpu_lock` should fail if the image is already locked in exclusive mode.
+    ///
     /// # Safety
     ///
-    /// Must only be called once per previous lock.
-    unsafe fn unlock(&self);
+    /// - Must only be called once per previous lock.
+    /// - The transitionned layout must be supported by the image (eg. the layout shouldn't be
+    ///   `ColorAttachmentOptimal` if the image wasn't created with the `color_attachment` usage).
+    /// - The transitionned layout must not be `Undefined`.
+    ///
+    unsafe fn unlock(&self, transitionned_layout: Option<ImageLayout>);
 }
 
 /// Inner information about an image.
@@ -240,8 +263,10 @@ unsafe impl<T> ImageAccess for T
     }
 
     #[inline]
-    fn try_gpu_lock(&self, exclusive_access: bool, queue: &Queue) -> Result<(), AccessError> {
-        (**self).try_gpu_lock(exclusive_access, queue)
+    fn try_gpu_lock(&self, exclusive_access: bool, expected_layout: ImageLayout)
+                    -> Result<(), AccessError>
+    {
+        (**self).try_gpu_lock(exclusive_access, expected_layout)
     }
 
     #[inline]
@@ -250,8 +275,8 @@ unsafe impl<T> ImageAccess for T
     }
 
     #[inline]
-    unsafe fn unlock(&self) {
-        (**self).unlock()
+    unsafe fn unlock(&self, transitionned_layout: Option<ImageLayout>) {
+        (**self).unlock(transitionned_layout)
     }
 }
 
@@ -301,8 +326,10 @@ unsafe impl<I> ImageAccess for ImageAccessFromUndefinedLayout<I>
     }
 
     #[inline]
-    fn try_gpu_lock(&self, exclusive_access: bool, queue: &Queue) -> Result<(), AccessError> {
-        self.image.try_gpu_lock(exclusive_access, queue)
+    fn try_gpu_lock(&self, exclusive_access: bool, expected_layout: ImageLayout)
+                    -> Result<(), AccessError>
+    {
+        self.image.try_gpu_lock(exclusive_access, expected_layout)
     }
 
     #[inline]
@@ -311,8 +338,8 @@ unsafe impl<I> ImageAccess for ImageAccessFromUndefinedLayout<I>
     }
 
     #[inline]
-    unsafe fn unlock(&self) {
-        self.image.unlock()
+    unsafe fn unlock(&self, new_layout: Option<ImageLayout>) {
+        self.image.unlock(new_layout)
     }
 }