Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vulkan: Add missing barrier between multiple passes to the same target. #12224

Merged
merged 2 commits into from
Aug 8, 2019
Merged

Vulkan: Add missing barrier between multiple passes to the same target. #12224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3b0624f
Some changes and asserts.
hrydgard Aug 8, 2019
213e2cc
Vulkan: Add missing barrier when rendering to a target that's already…
hrydgard Aug 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,3 +110,6 @@ debian/ppsspp/

# YouCompleteMe file
.ycm_extra_conf.pyc

# RenderDoc
*.rdc
32 changes: 31 additions & 1 deletion ext/native/thin3d/VulkanQueueRunner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,14 +379,18 @@ void VulkanQueueRunner::RunSteps(VkCommandBuffer cmd, std::vector<VKRStep *> &st
// Planned optimizations:
// * Create copies of render target that are rendered to multiple times and textured from in sequence, and push those render passes
// as early as possible in the frame (Wipeout billboards).
// * Merge subsequent render passes to the same target that are interspersed with unrelated draws to other render targets (God of War).

for (int j = 0; j < (int)steps.size() - 1; j++) {
for (int j = 0; j < (int)steps.size(); j++) {
if (steps[j]->stepType == VKRStepType::RENDER &&
steps[j]->render.framebuffer &&
steps[j]->render.finalColorLayout == VK_IMAGE_LAYOUT_UNDEFINED) {
// Just leave it at color_optimal.
steps[j]->render.finalColorLayout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
}
}

for (int j = 0; j < (int)steps.size() - 1; j++) {
// Push down empty "Clear/Store" renderpasses, and merge them with the first "Load/Store" to the same framebuffer.
// Actually let's just bother with the first one for now. This affects Wipeout Pure.
if (steps.size() > 1 && steps[j]->stepType == VKRStepType::RENDER &&
Expand Down Expand Up @@ -458,6 +462,11 @@ void VulkanQueueRunner::RunSteps(VkCommandBuffer cmd, std::vector<VKRStep *> &st
case VKRStepType::RENDER_SKIP:
break;
}
}

// Deleting all in one go should be easier on the instruction cache than deleting
// them as we go - and easier to debug because we can look backwards in the frame.
for (size_t i = 0; i < steps.size(); i++) {
delete steps[i];
}
}
Expand Down Expand Up @@ -788,6 +797,9 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c
VkPipelineStageFlags dstStage{};
switch (barrier.oldLayout) {
case VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL:
barrier.srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_COLOR_ATTACHMENT_READ_BIT;
srcStage = VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT;
break;
case VK_IMAGE_LAYOUT_UNDEFINED:
barrier.srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_COLOR_ATTACHMENT_READ_BIT;
srcStage = VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT;
Expand Down Expand Up @@ -829,6 +841,22 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c
return;
}

if (step.render.framebuffer && step.render.framebuffer->color.layout == VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL) {
VkImageMemoryBarrier barrier{};
barrier.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER;
barrier.oldLayout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
barrier.newLayout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
barrier.subresourceRange.layerCount = 1;
barrier.subresourceRange.levelCount = 1;
barrier.image = step.render.framebuffer->color.image;
barrier.srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT;
barrier.dstAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_COLOR_ATTACHMENT_READ_BIT;
barrier.subresourceRange.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT;
barrier.srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED;
barrier.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED;
vkCmdPipelineBarrier(cmd, VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT, VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT, 0, 0, nullptr, 0, nullptr, 1, &barrier);
}

// This is supposed to bind a vulkan render pass to the command buffer.
PerformBindFramebufferAsRenderTarget(step, cmd);

Expand Down Expand Up @@ -947,6 +975,8 @@ void VulkanQueueRunner::PerformBindFramebufferAsRenderTarget(const VKRStep &step
int w;
int h;
if (step.render.framebuffer) {
_dbg_assert_(G3D, step.render.finalColorLayout != VK_IMAGE_LAYOUT_UNDEFINED);

VKRFramebuffer *fb = step.render.framebuffer;
framebuf = fb->framebuf;
w = fb->width;
Expand Down
14 changes: 10 additions & 4 deletions ext/native/thin3d/VulkanRenderManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,8 @@ void VulkanRenderManager::BindFramebufferAsRenderTarget(VKRFramebuffer *fb, VKRR
return;
}
}

// More redundant bind elimination.
if (curRenderStep_ && curRenderStep_->commands.size() == 0 && curRenderStep_->render.color == VKRRenderPassAction::KEEP && curRenderStep_->render.depth == VKRRenderPassAction::KEEP && curRenderStep_->render.stencil == VKRRenderPassAction::KEEP) {
// Can trivially kill the last empty render step.
assert(steps_.back() == curRenderStep_);
Expand All @@ -409,10 +411,9 @@ void VulkanRenderManager::BindFramebufferAsRenderTarget(VKRFramebuffer *fb, VKRR
}

VKRStep *step = new VKRStep{ VKRStepType::RENDER };
// This is what queues up new passes, and can end previous ones.
step->render.framebuffer = fb;
step->render.color = color;
step->render.depth= depth;
step->render.depth = depth;
step->render.stencil = stencil;
step->render.clearColor = clearColor;
step->render.clearDepth = clearDepth;
Expand All @@ -423,8 +424,13 @@ void VulkanRenderManager::BindFramebufferAsRenderTarget(VKRFramebuffer *fb, VKRR
steps_.push_back(step);

curRenderStep_ = step;
curWidth_ = fb ? fb->width : vulkan_->GetBackbufferWidth();
curHeight_ = fb ? fb->height : vulkan_->GetBackbufferHeight();
if (fb) {
curWidth_ = fb->width;
curHeight_ = fb->height;
} else {
curWidth_ = vulkan_->GetBackbufferWidth();
curHeight_ = vulkan_->GetBackbufferHeight();
}
}

bool VulkanRenderManager::CopyFramebufferToMemorySync(VKRFramebuffer *src, int aspectBits, int x, int y, int w, int h, Draw::DataFormat destFormat, uint8_t *pixels, int pixelStride) {
Expand Down