[vulkan] simplify image transition code. (#441)

This CL splits the ChangeLayout method into a method to change to Draw
mode and a method to change to Probe mode. The framebuffer is always
transitioned to Draw mode after initialization. The RenderPassGuard now
transitions the framebuffer into draw layout on initialize and out of
draw layout on shutdown.

5 files changed, 83 insertions, 85 deletions

diff --git a/src/vulkan/frame_buffer.cc b/src/vulkan/frame_buffer.cc
index 33be51c..412a0d3 100644
--- a/src/vulkan/frame_buffer.cc
+++ b/src/vulkan/frame_buffer.cc
@@ -43,7 +43,8 @@ FrameBuffer::~FrameBuffer() {
   }
 }
 
-Result FrameBuffer::Initialize(VkRenderPass render_pass,
+Result FrameBuffer::Initialize(CommandBuffer* command_buffer,
+                               VkRenderPass render_pass,
                                const Format& depth_format) {
   std::vector<VkImageView> attachments;
 
@@ -108,43 +109,13 @@ Result FrameBuffer::Initialize(VkRenderPass render_pass,
     return Result("Vulkan::Calling vkCreateFramebuffer Fail");
   }
 
+  ChangeFrameToDrawLayout(command_buffer);
   return {};
 }
 
-Result FrameBuffer::ChangeFrameImageLayout(CommandBuffer* command,
-                                           FrameImageState layout) {
-  if (layout == FrameImageState::kInit) {
-    return Result(
-        "FrameBuffer::ChangeFrameImageLayout new layout cannot be kInit");
-  }
-
-  if (layout == frame_image_layout_)
-    return {};
-
-  if (layout == FrameImageState::kProbe) {
-    if (frame_image_layout_ != FrameImageState::kClearOrDraw) {
-      return Result(
-          "FrameBuffer::ChangeFrameImageLayout new layout cannot be kProbe "
-          "from kInit");
-    }
-
-    for (auto& img : color_images_) {
-      img->ChangeLayout(command, VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL,
-                        VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL,
-                        VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT,
-                        VK_PIPELINE_STAGE_TRANSFER_BIT);
-    }
-    if (depth_image_) {
-      depth_image_->ChangeLayout(
-          command, VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL,
-          VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL,
-          VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT,
-          VK_PIPELINE_STAGE_TRANSFER_BIT);
-    }
-
-    frame_image_layout_ = FrameImageState::kProbe;
-    return {};
-  }
+void FrameBuffer::ChangeFrameToDrawLayout(CommandBuffer* command) {
+  if (frame_image_layout_ == FrameImageState::kClearOrDraw)
+    return;
 
   VkImageLayout old_layout = frame_image_layout_ == FrameImageState::kInit
                                  ? VK_IMAGE_LAYOUT_UNDEFINED
@@ -156,24 +127,46 @@ Result FrameBuffer::ChangeFrameImageLayout(CommandBuffer* command,
           : VK_PIPELINE_STAGE_TRANSFER_BIT;
 
   for (auto& img : color_images_) {
-    img->ChangeLayout(command, old_layout,
-                      VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL, source_stage,
+    auto barrier = img->CreateBarrier(old_layout,
+                                      VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL);
+    img->ImageBarrier(command, barrier, source_stage,
                       VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT);
   }
   if (depth_image_) {
-    depth_image_->ChangeLayout(
-        command, old_layout, VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL,
-        source_stage, VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT);
+    auto barrier = depth_image_->CreateBarrier(
+        old_layout, VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL);
+    depth_image_->ImageBarrier(command, barrier, source_stage,
+                               VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT);
   }
   frame_image_layout_ = FrameImageState::kClearOrDraw;
-  return {};
 }
 
-void FrameBuffer::CopyColorImagesToHost(CommandBuffer* command) {
+void FrameBuffer::ChangeFrameToProbeLayout(CommandBuffer* command) {
+  if (frame_image_layout_ == FrameImageState::kProbe)
+    return;
+
   for (auto& img : color_images_) {
-    ChangeFrameImageLayout(command, FrameImageState::kProbe);
-    img->CopyToHost(command);
+    auto barrier = img->CreateBarrier(VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL,
+                                      VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL);
+    img->ImageBarrier(command, barrier,
+                      VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT,
+                      VK_PIPELINE_STAGE_TRANSFER_BIT);
+  }
+  if (depth_image_) {
+    auto barrier = depth_image_->CreateBarrier(
+        VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL,
+        VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL);
+    depth_image_->ImageBarrier(command, barrier,
+                               VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT,
+                               VK_PIPELINE_STAGE_TRANSFER_BIT);
   }
+
+  frame_image_layout_ = FrameImageState::kProbe;
+}
+
+void FrameBuffer::TransferColorImagesToHost(CommandBuffer* command) {
+  for (auto& img : color_images_)
+    img->CopyToHost(command);
 }
 
 void FrameBuffer::CopyImagesToBuffers() {
diff --git a/src/vulkan/frame_buffer.h b/src/vulkan/frame_buffer.h
index 2f21c73..691be2b 100644
--- a/src/vulkan/frame_buffer.h
+++ b/src/vulkan/frame_buffer.h
@@ -42,9 +42,12 @@ class FrameBuffer {
       uint32_t height);
   ~FrameBuffer();
 
-  Result Initialize(VkRenderPass render_pass, const Format& depth_format);
+  Result Initialize(CommandBuffer* command_buffer,
+                    VkRenderPass render_pass,
+                    const Format& depth_format);
 
-  Result ChangeFrameImageLayout(CommandBuffer* command, FrameImageState layout);
+  void ChangeFrameToDrawLayout(CommandBuffer* command);
+  void ChangeFrameToProbeLayout(CommandBuffer* command);
 
   VkFramebuffer GetVkFrameBuffer() const { return frame_; }
   const void* GetColorBufferPtr(size_t idx) const {
@@ -54,7 +57,7 @@ class FrameBuffer {
   // Only record the command for copying the image that backs this
   // framebuffer to the host accessible buffer. The actual submission
   // of the command must be done later.
-  void CopyColorImagesToHost(CommandBuffer* command);
+  void TransferColorImagesToHost(CommandBuffer* command);
 
   void CopyImagesToBuffers();
 
@@ -70,6 +73,7 @@ class FrameBuffer {
   uint32_t width_ = 0;
   uint32_t height_ = 0;
   uint32_t depth_ = 1;
+
   FrameImageState frame_image_layout_ = FrameImageState::kInit;
 };
 
diff --git a/src/vulkan/graphics_pipeline.cc b/src/vulkan/graphics_pipeline.cc
index ab1c046..74a2ef8 100644
--- a/src/vulkan/graphics_pipeline.cc
+++ b/src/vulkan/graphics_pipeline.cc
@@ -347,35 +347,32 @@ VkBlendOp ToVkBlendOp(BlendOp op) {
 class RenderPassGuard {
  public:
   explicit RenderPassGuard(GraphicsPipeline* pipeline) : pipeline_(pipeline) {
-    auto* frame = pipeline->GetFrameBuffer();
-    auto* cmd = pipeline->GetCommandBuffer();
-    result_ = frame->ChangeFrameImageLayout(cmd, FrameImageState::kClearOrDraw);
-    if (!result_.IsSuccess())
-      return;
+    auto* frame = pipeline_->GetFrameBuffer();
+    auto* cmd = pipeline_->GetCommandBuffer();
+    frame->ChangeFrameToDrawLayout(cmd);
 
     VkRenderPassBeginInfo render_begin_info = VkRenderPassBeginInfo();
     render_begin_info.sType = VK_STRUCTURE_TYPE_RENDER_PASS_BEGIN_INFO;
-    render_begin_info.renderPass = pipeline->GetVkRenderPass();
+    render_begin_info.renderPass = pipeline_->GetVkRenderPass();
     render_begin_info.framebuffer = frame->GetVkFrameBuffer();
     render_begin_info.renderArea = {{0, 0},
                                     {frame->GetWidth(), frame->GetHeight()}};
-    pipeline->GetDevice()->GetPtrs()->vkCmdBeginRenderPass(
+    pipeline_->GetDevice()->GetPtrs()->vkCmdBeginRenderPass(
         cmd->GetVkCommandBuffer(), &render_begin_info,
         VK_SUBPASS_CONTENTS_INLINE);
   }
 
-  bool IsActive() const { return result_.IsSuccess(); }
-  Result GetResult() const { return result_; }
-
   ~RenderPassGuard() {
-    if (result_.IsSuccess()) {
-      pipeline_->GetDevice()->GetPtrs()->vkCmdEndRenderPass(
-          pipeline_->GetCommandBuffer()->GetVkCommandBuffer());
-    }
+    auto* cmd = pipeline_->GetCommandBuffer();
+
+    pipeline_->GetDevice()->GetPtrs()->vkCmdEndRenderPass(
+        cmd->GetVkCommandBuffer());
+
+    auto* frame = pipeline_->GetFrameBuffer();
+    frame->ChangeFrameToProbeLayout(cmd);
   }
 
  private:
-  Result result_;
   GraphicsPipeline* pipeline_;
 };
 
@@ -692,15 +689,20 @@ Result GraphicsPipeline::Initialize(uint32_t width,
   if (!r.IsSuccess())
     return r;
 
+  CommandBufferGuard guard(GetCommandBuffer());
+  if (!guard.IsRecording())
+    return guard.GetResult();
+
   frame_ = MakeUnique<FrameBuffer>(device_, color_buffers_, width, height);
-  r = frame_->Initialize(render_pass_, depth_stencil_format_);
+  r = frame_->Initialize(GetCommandBuffer(), render_pass_,
+                         depth_stencil_format_);
   if (!r.IsSuccess())
     return r;
 
   frame_width_ = width;
   frame_height_ = height;
 
-  return {};
+  return guard.Submit(GetFenceTimeout());
 }
 
 Result GraphicsPipeline::SendVertexBufferDataIfNeeded(
@@ -789,8 +791,6 @@ Result GraphicsPipeline::ClearBuffer(const VkClearValue& clear_value,
 
   {
     RenderPassGuard render_pass_guard(this);
-    if (!render_pass_guard.IsActive())
-      return render_pass_guard.GetResult();
 
     VkClearAttachment clear_attachment = VkClearAttachment();
     clear_attachment.aspectMask = aspect;
@@ -806,7 +806,7 @@ Result GraphicsPipeline::ClearBuffer(const VkClearValue& clear_value,
         command_->GetVkCommandBuffer(), 1, &clear_attachment, 1, &clear_rect);
   }
 
-  frame_->CopyColorImagesToHost(command_.get());
+  frame_->TransferColorImagesToHost(command_.get());
 
   Result r = cmd_buf_guard.Submit(GetFenceTimeout());
   if (!r.IsSuccess())
@@ -850,8 +850,6 @@ Result GraphicsPipeline::Draw(const DrawArraysCommand* command,
 
     {
       RenderPassGuard render_pass_guard(this);
-      if (!render_pass_guard.IsActive())
-        return render_pass_guard.GetResult();
 
       BindVkDescriptorSets(pipeline_layout);
 
@@ -896,7 +894,7 @@ Result GraphicsPipeline::Draw(const DrawArraysCommand* command,
       }
     }
 
-    frame_->CopyColorImagesToHost(command_.get());
+    frame_->TransferColorImagesToHost(command_.get());
 
     r = cmd_buf_guard.Submit(GetFenceTimeout());
     if (!r.IsSuccess())
diff --git a/src/vulkan/transfer_image.cc b/src/vulkan/transfer_image.cc
index 3f50326..91370de 100644
--- a/src/vulkan/transfer_image.cc
+++ b/src/vulkan/transfer_image.cc
@@ -34,12 +34,11 @@ const VkImageCreateInfo kDefaultImageInfo = {
     1,                                   /* arrayLayers */
     VK_SAMPLE_COUNT_1_BIT,               /* samples */
     VK_IMAGE_TILING_OPTIMAL,             /* tiling */
-    VK_IMAGE_USAGE_TRANSFER_SRC_BIT |
-        VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT, /* usage */
-    VK_SHARING_MODE_EXCLUSIVE,               /* sharingMode */
-    0,                                       /* queueFamilyIndexCount */
-    nullptr,                                 /* pQueueFamilyIndices */
-    VK_IMAGE_LAYOUT_UNDEFINED,               /* initialLayout */
+    0,                                   /* usage */
+    VK_SHARING_MODE_EXCLUSIVE,           /* sharingMode */
+    0,                                   /* queueFamilyIndexCount */
+    nullptr,                             /* pQueueFamilyIndices */
+    VK_IMAGE_LAYOUT_UNDEFINED,           /* initialLayout */
 };
 
 }  // namespace
@@ -180,11 +179,8 @@ void TransferImage::CopyToHost(CommandBuffer* command) {
   MemoryBarrier(command);
 }
 
-void TransferImage::ChangeLayout(CommandBuffer* command,
-                                 VkImageLayout old_layout,
-                                 VkImageLayout new_layout,
-                                 VkPipelineStageFlags from,
-                                 VkPipelineStageFlags to) {
+VkImageMemoryBarrier TransferImage::CreateBarrier(VkImageLayout old_layout,
+                                                  VkImageLayout new_layout) {
   VkImageMemoryBarrier barrier = VkImageMemoryBarrier();
   barrier.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER;
   barrier.oldLayout = old_layout;
@@ -254,7 +250,13 @@ void TransferImage::ChangeLayout(CommandBuffer* command,
       barrier.dstAccessMask = 0;
       break;
   }
+  return barrier;
+}
 
+void TransferImage::ImageBarrier(CommandBuffer* command,
+                                 VkImageMemoryBarrier barrier,
+                                 VkPipelineStageFlags from,
+                                 VkPipelineStageFlags to) const {
   device_->GetPtrs()->vkCmdPipelineBarrier(command->GetVkCommandBuffer(), from,
                                            to, 0, 0, NULL, 0, NULL, 1,
                                            &barrier);
diff --git a/src/vulkan/transfer_image.h b/src/vulkan/transfer_image.h
index 7a3a4bb..bbea7a2 100644
--- a/src/vulkan/transfer_image.h
+++ b/src/vulkan/transfer_image.h
@@ -39,11 +39,12 @@ class TransferImage : public Resource {
   Result Initialize(VkImageUsageFlags usage);
   VkImageView GetVkImageView() const { return view_; }
 
-  void ChangeLayout(CommandBuffer* command,
-                    VkImageLayout old_layout,
-                    VkImageLayout new_layout,
+  VkImageMemoryBarrier CreateBarrier(VkImageLayout old_layout,
+                                     VkImageLayout new_layout);
+  void ImageBarrier(CommandBuffer* command,
+                    VkImageMemoryBarrier barrier,
                     VkPipelineStageFlags from,
-                    VkPipelineStageFlags to);
+                    VkPipelineStageFlags to) const;
 
   // Only record the command for copying this image to its secondary
   // host-accessible buffer. The actual submission of the command