Skip to content

Vulkan: Avoid allocating depth images for stuff like temp copies, depal buffers etc. #16099

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

Merged
merged 2 commits into from
Sep 25, 2022
Merged

Vulkan: Avoid allocating depth images for stuff like temp copies, depal buffers etc. #16099

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
753fd13
Vulkan: Avoid allocating depth images for stuff like temp copies, dep…
hrydgard Sep 24, 2022
70c5ca6
Remove debug log. Add some new debug log though, unrelated to this PR…
hrydgard Sep 25, 2022
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
4 changes: 4 additions & 0 deletions Common/GPU/Vulkan/VulkanBarrier.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ class VulkanBarrier {
VkAccessFlags srcAccessMask, VkAccessFlags dstAccessMask,
VkPipelineStageFlags srcStageMask, VkPipelineStageFlags dstStageMask
) {
_dbg_assert_(image != VK_NULL_HANDLE);

srcStageMask_ |= srcStageMask;
dstStageMask_ |= dstStageMask;
dependencyFlags_ |= VK_DEPENDENCY_BY_REGION_BIT;
Expand Down Expand Up @@ -46,6 +48,8 @@ class VulkanBarrier {
void TransitionImageAuto(
VkImage image, int baseMip, int numMipLevels, VkImageAspectFlags aspectMask, VkImageLayout oldImageLayout, VkImageLayout newImageLayout
) {
_dbg_assert_(image != VK_NULL_HANDLE);

VkAccessFlags srcAccessMask = 0;
VkAccessFlags dstAccessMask = 0;
switch (oldImageLayout) {
Expand Down
9 changes: 7 additions & 2 deletions Common/GPU/Vulkan/VulkanQueueRunner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1323,7 +1323,7 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c
iter.targetLayout
);
iter.fb->color.layout = iter.targetLayout;
} else if ((iter.aspect & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT)) && iter.fb->depth.layout != iter.targetLayout) {
} else if (iter.fb->depth.image != VK_NULL_HANDLE && (iter.aspect & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT)) && iter.fb->depth.layout != iter.targetLayout) {
recordBarrier_.TransitionImageAuto(
iter.fb->depth.image,
0,
Expand Down Expand Up @@ -1367,7 +1367,7 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c
VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT
);
}
if (step.render.framebuffer->depth.layout == VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL) {
if (step.render.framebuffer->depth.image != VK_NULL_HANDLE && step.render.framebuffer->depth.layout == VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL) {
recordBarrier_.TransitionImage(
step.render.framebuffer->depth.image,
0,
Expand Down Expand Up @@ -1750,6 +1750,8 @@ void VulkanQueueRunner::PerformCopy(const VKRStep &step, VkCommandBuffer cmd) {
vkCmdCopyImage(cmd, src->color.image, src->color.layout, dst->color.image, dst->color.layout, 1, &copy);
}
if (step.copy.aspectMask & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT)) {
_dbg_assert_(src->depth.image != VK_NULL_HANDLE);
_dbg_assert_(dst->depth.image != VK_NULL_HANDLE);
copy.srcSubresource.aspectMask = step.copy.aspectMask & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT);
copy.dstSubresource.aspectMask = step.copy.aspectMask & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT);
vkCmdCopyImage(cmd, src->depth.image, src->depth.layout, dst->depth.image, dst->depth.layout, 1, &copy);
Expand Down Expand Up @@ -1804,6 +1806,9 @@ void VulkanQueueRunner::PerformBlit(const VKRStep &step, VkCommandBuffer cmd) {

// We can't copy only depth or only stencil unfortunately.
if (step.blit.aspectMask & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT)) {
_dbg_assert_(src->depth.image != VK_NULL_HANDLE);
_dbg_assert_(dst->depth.image != VK_NULL_HANDLE);

if (src->depth.layout != VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL) {
SetupTransitionToTransferSrc(src->depth, VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT, &recordBarrier_);
}
Expand Down
12 changes: 8 additions & 4 deletions Common/GPU/Vulkan/VulkanRenderManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,16 +146,21 @@ bool VKRComputePipeline::Create(VulkanContext *vulkan) {
return success;
}

VKRFramebuffer::VKRFramebuffer(VulkanContext *vk, VkCommandBuffer initCmd, VKRRenderPass *compatibleRenderPass, int _width, int _height, const char *tag) : vulkan_(vk), tag_(tag) {
VKRFramebuffer::VKRFramebuffer(VulkanContext *vk, VkCommandBuffer initCmd, VKRRenderPass *compatibleRenderPass, int _width, int _height, bool createDepthStencilBuffer, const char *tag) : vulkan_(vk), tag_(tag) {
width = _width;
height = _height;

_dbg_assert_(tag);

CreateImage(vulkan_, initCmd, color, width, height, VK_FORMAT_R8G8B8A8_UNORM, VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL, true, tag);
CreateImage(vulkan_, initCmd, depth, width, height, vulkan_->GetDeviceInfo().preferredDepthStencilFormat, VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL, false, tag);
vulkan_->SetDebugName(color.image, VK_OBJECT_TYPE_IMAGE, StringFromFormat("fb_color_%s", tag).c_str());
if (createDepthStencilBuffer) {
CreateImage(vulkan_, initCmd, depth, width, height, vulkan_->GetDeviceInfo().preferredDepthStencilFormat, VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL, false, tag);
vulkan_->SetDebugName(depth.image, VK_OBJECT_TYPE_IMAGE, StringFromFormat("fb_depth_%s", tag).c_str());
}

// We create the actual framebuffer objects on demand, because some combinations might not make sense.
// Framebuffer objects are just pointers to a set of images, so no biggie.
}

VkFramebuffer VKRFramebuffer::Get(VKRRenderPass *compatibleRenderPass, RenderPassType rpType) {
Expand All @@ -170,6 +175,7 @@ VkFramebuffer VKRFramebuffer::Get(VKRRenderPass *compatibleRenderPass, RenderPas

views[0] = color.imageView;
if (hasDepth) {
_dbg_assert_(depth.imageView != VK_NULL_HANDLE);
views[1] = depth.imageView;
}
fbci.renderPass = compatibleRenderPass->Get(vulkan_, rpType);
Expand All @@ -183,8 +189,6 @@ VkFramebuffer VKRFramebuffer::Get(VKRRenderPass *compatibleRenderPass, RenderPas
_assert_(res == VK_SUCCESS);

if (!tag_.empty() && vulkan_->Extensions().EXT_debug_utils) {
vulkan_->SetDebugName(color.image, VK_OBJECT_TYPE_IMAGE, StringFromFormat("fb_color_%s", tag_.c_str()).c_str());
vulkan_->SetDebugName(depth.image, VK_OBJECT_TYPE_IMAGE, StringFromFormat("fb_depth_%s", tag_.c_str()).c_str());
vulkan_->SetDebugName(framebuf[(int)rpType], VK_OBJECT_TYPE_FRAMEBUFFER, StringFromFormat("fb_%s", tag_.c_str()).c_str());
}

Expand Down
6 changes: 3 additions & 3 deletions Common/GPU/Vulkan/VulkanRenderManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,15 @@ void CreateImage(VulkanContext *vulkan, VkCommandBuffer cmd, VKRImage &img, int

class VKRFramebuffer {
public:
VKRFramebuffer(VulkanContext *vk, VkCommandBuffer initCmd, VKRRenderPass *compatibleRenderPass, int _width, int _height, const char *tag);
VKRFramebuffer(VulkanContext *vk, VkCommandBuffer initCmd, VKRRenderPass *compatibleRenderPass, int _width, int _height, bool createDepthStencilBuffer, const char *tag);
~VKRFramebuffer();

VkFramebuffer Get(VKRRenderPass *compatibleRenderPass, RenderPassType rpType);

int width = 0;
int height = 0;
VKRImage color{};
VKRImage depth{};
VKRImage color{}; // color.image is always there.
VKRImage depth{}; // depth.image is allowed to be VK_NULL_HANDLE.

const char *Tag() const {
return tag_.c_str();
Expand Down
4 changes: 1 addition & 3 deletions Common/GPU/Vulkan/thin3d_vulkan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1496,9 +1496,7 @@ class VKFramebuffer : public Framebuffer {

Framebuffer *VKContext::CreateFramebuffer(const FramebufferDesc &desc) {
VkCommandBuffer cmd = renderManager_.GetInitCmd();
// TODO: We always create with depth here, even when it's not needed (such as color temp FBOs).
// Should optimize those away.
VKRFramebuffer *vkrfb = new VKRFramebuffer(vulkan_, cmd, renderManager_.GetQueueRunner()->GetCompatibleRenderPass(), desc.width, desc.height, desc.tag);
VKRFramebuffer *vkrfb = new VKRFramebuffer(vulkan_, cmd, renderManager_.GetQueueRunner()->GetCompatibleRenderPass(), desc.width, desc.height, desc.z_stencil, desc.tag);
return new VKFramebuffer(vkrfb);
}

Expand Down
4 changes: 4 additions & 0 deletions GPU/Common/TextureCacheCommon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,10 @@ TexCacheEntry *TextureCacheCommon::SetTexture() {

nextTexture_ = nullptr;
nextNeedsRebuild_ = false;

if (hasClutGPU) {
WARN_LOG_ONCE(clut_fb, G3D, "Render-to-CLUT combined with framebuffer texture at %08x - Not yet supported", texaddr);
}
SetTextureFramebuffer(bestCandidate); // sets curTexture3D
return nullptr;
}
Expand Down