From 18f5ae987730381a79b900cdaebcb613362b0410 Mon Sep 17 00:00:00 2001 From: Jaebaek Seo Date: Mon, 10 Dec 2018 14:57:07 -0500 Subject: Vulkan: change framebuffer image layout correctly (#156) `oldLayout` of `VkImageMemoryBarrier` when calling `vkCmdPipelineBarrier()` must match previous image layout. This commit changes image layout of framebuffer correctly and fixes bugs of incorrect drawing results in Intel GPU. Fixes #155 --- src/vulkan/descriptor.h | 5 ----- src/vulkan/frame_buffer.cc | 22 +++++++++++++++++----- src/vulkan/graphics_pipeline.cc | 37 +++++++++++++++++++++++-------------- src/vulkan/graphics_pipeline.h | 2 +- 4 files changed, 41 insertions(+), 25 deletions(-) (limited to 'src') diff --git a/src/vulkan/descriptor.h b/src/vulkan/descriptor.h index 4bc26db..8bb09ac 100644 --- a/src/vulkan/descriptor.h +++ b/src/vulkan/descriptor.h @@ -60,11 +60,6 @@ class Descriptor { uint32_t GetDescriptorSet() const { return descriptor_set_; } uint32_t GetBinding() const { return binding_; } - bool operator<(const Descriptor& r) { - if (descriptor_set_ == r.descriptor_set_) - return binding_ < r.binding_; - return descriptor_set_ < r.descriptor_set_; - } DescriptorType GetType() const { return type_; } diff --git a/src/vulkan/frame_buffer.cc b/src/vulkan/frame_buffer.cc index 8177405..85f4c9b 100644 --- a/src/vulkan/frame_buffer.cc +++ b/src/vulkan/frame_buffer.cc @@ -89,10 +89,22 @@ Result FrameBuffer::ChangeFrameImageLayout(VkCommandBuffer command, "FrameBuffer::ChangeFrameImageLayout new layout cannot be kProbe " "from kInit"); } - // Note that we set the final layout of RenderPass as - // VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, thus we must not change - // image layout of frame buffer from color attachment to transfer - // source here. + + if (color_image_) { + color_image_->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_COLOR_ATTACHMENT_OUTPUT_BIT, + VK_PIPELINE_STAGE_TRANSFER_BIT); + } + frame_image_layout_ = FrameImageState::kProbe; return {}; } @@ -113,7 +125,7 @@ Result FrameBuffer::ChangeFrameImageLayout(VkCommandBuffer command, VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT, VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT); } - frame_image_layout_ = layout; + frame_image_layout_ = FrameImageState::kClearOrDraw; return {}; } diff --git a/src/vulkan/graphics_pipeline.cc b/src/vulkan/graphics_pipeline.cc index 36444bf..e74e8c9 100644 --- a/src/vulkan/graphics_pipeline.cc +++ b/src/vulkan/graphics_pipeline.cc @@ -131,6 +131,10 @@ Result GraphicsPipeline::CreateRenderPass() { if (color_format_ != VK_FORMAT_UNDEFINED) { attachment_desc.push_back(kDefaultAttachmentDesc); attachment_desc.back().format = color_format_; + attachment_desc.back().initialLayout = + VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL; + attachment_desc.back().finalLayout = + VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL; color_refer.attachment = static_cast(attachment_desc.size() - 1); color_refer.layout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL; @@ -142,6 +146,10 @@ Result GraphicsPipeline::CreateRenderPass() { if (depth_stencil_format_ != VK_FORMAT_UNDEFINED) { attachment_desc.push_back(kDefaultAttachmentDesc); attachment_desc.back().format = depth_stencil_format_; + attachment_desc.back().initialLayout = + VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL; + attachment_desc.back().finalLayout = + VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL; depth_refer.attachment = static_cast(attachment_desc.size() - 1); depth_refer.layout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL; @@ -358,9 +366,14 @@ Result GraphicsPipeline::SendBufferDataIfNeeded() { memory_properties_); } -void GraphicsPipeline::ActivateRenderPassIfNeeded() { +Result GraphicsPipeline::ActivateRenderPassIfNeeded() { if (render_pass_state_ == RenderPassState::kActive) - return; + return {}; + + Result r = frame_->ChangeFrameImageLayout(command_->GetCommandBuffer(), + FrameImageState::kClearOrDraw); + if (!r.IsSuccess()) + return r; VkRenderPassBeginInfo render_begin_info = {}; render_begin_info.sType = VK_STRUCTURE_TYPE_RENDER_PASS_BEGIN_INFO; @@ -370,6 +383,7 @@ void GraphicsPipeline::ActivateRenderPassIfNeeded() { vkCmdBeginRenderPass(command_->GetCommandBuffer(), &render_begin_info, VK_SUBPASS_CONTENTS_INLINE); render_pass_state_ = RenderPassState::kActive; + return {}; } void GraphicsPipeline::DeactivateRenderPassIfNeeded() { @@ -447,15 +461,10 @@ Result GraphicsPipeline::ClearBuffer(const VkClearValue& clear_value, if (!r.IsSuccess()) return r; - r = frame_->ChangeFrameImageLayout(command_->GetCommandBuffer(), - FrameImageState::kClearOrDraw); + r = ActivateRenderPassIfNeeded(); if (!r.IsSuccess()) return r; - // TODO(jaebaek): When multiple clear and draw commands exist, handle - // begin/end render pass properly. - ActivateRenderPassIfNeeded(); - VkClearAttachment clear_attachment = {}; clear_attachment.aspectMask = aspect; clear_attachment.colorAttachment = 0; @@ -507,7 +516,6 @@ Result GraphicsPipeline::Draw(const DrawArraysCommand* command) { if (!r.IsSuccess()) return r; - // TODO(jaebaek): Handle primitive topology. if (pipeline_ == VK_NULL_HANDLE) { r = CreateVkGraphicsPipeline(ToVkTopology(command->GetTopology())); if (!r.IsSuccess()) @@ -522,10 +530,9 @@ Result GraphicsPipeline::Draw(const DrawArraysCommand* command) { if (!r.IsSuccess()) return r; - frame_->ChangeFrameImageLayout(command_->GetCommandBuffer(), - FrameImageState::kClearOrDraw); - - ActivateRenderPassIfNeeded(); + r = ActivateRenderPassIfNeeded(); + if (!r.IsSuccess()) + return r; BindVkDescriptorSets(); BindVkPipeline(); @@ -548,7 +555,9 @@ Result GraphicsPipeline::ProcessCommands() { if (!r.IsSuccess()) return r; - ActivateRenderPassIfNeeded(); + r = ActivateRenderPassIfNeeded(); + if (!r.IsSuccess()) + return r; DeactivateRenderPassIfNeeded(); r = frame_->CopyColorImageToHost(command_->GetCommandBuffer()); diff --git a/src/vulkan/graphics_pipeline.h b/src/vulkan/graphics_pipeline.h index 4142482..5515316 100644 --- a/src/vulkan/graphics_pipeline.h +++ b/src/vulkan/graphics_pipeline.h @@ -85,7 +85,7 @@ class GraphicsPipeline : public Pipeline { Result CreateVkGraphicsPipeline(VkPrimitiveTopology topology); Result CreateRenderPass(); - void ActivateRenderPassIfNeeded(); + Result ActivateRenderPassIfNeeded(); void DeactivateRenderPassIfNeeded(); // Send vertex and index buffers. -- cgit v1.2.3