Skip to content

Commit

Permalink
Vulkan: Fix image layout issues after compute shader uploads.
Browse files Browse the repository at this point in the history
We're already in GENERAL so probably not worth to transfer to DST just
to do even more transfers due to the silliness of GenerateMip.

I'm planning to rework the whole texture upload thing to be far more
optimal with some kind of TextureUploadManager

Fixes #13987
  • Loading branch information
hrydgard committed Jan 30, 2021
1 parent de02c7e commit c48bdf7
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 8 deletions.
5 changes: 5 additions & 0 deletions Common/GPU/Vulkan/VulkanDebug.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ VKAPI_ATTR VkBool32 VKAPI_CALL VulkanDebugUtilsCallback(
// UNASSIGNED-CoreValidation-Shader-OutputNotConsumed - benign perf warning
return false;
}
if (messageCode == 1303270965) {
// Benign perf warning, image blit using GENERAL layout.
// UNASSIGNED
return false;
}

const char *pLayerPrefix = "";
if (messageSeverity & VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT) {
Expand Down
15 changes: 10 additions & 5 deletions Common/GPU/Vulkan/VulkanImage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,8 @@ void VulkanTexture::ClearMip(VkCommandBuffer cmd, int mip, uint32_t value) {
vkCmdClearColorImage(cmd, image_, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, &clearVal, 1, &range);
}

void VulkanTexture::GenerateMip(VkCommandBuffer cmd, int mip) {
// Low-quality mipmap generation by bilinear blit, but works okay.
void VulkanTexture::GenerateMip(VkCommandBuffer cmd, int mip, VkImageLayout imageLayout) {
_assert_msg_(mip != 0, "Cannot generate the first level");
_assert_msg_(mip < numMips_, "Cannot generate mipmaps past the maximum created (%d vs %d)", mip, numMips_);
VkImageBlit blit{};
Expand All @@ -214,16 +215,20 @@ void VulkanTexture::GenerateMip(VkCommandBuffer cmd, int mip) {
blit.dstOffsets[1].y = height_ >> mip;
blit.dstOffsets[1].z = 1;

// TODO: We could do better with the image transitions - would be enough with one per level
// for the memory barrier, then one final one for the whole stack when done. This function
// currently doesn't have a global enough view, though.
// We should also coalesce barriers across multiple texture uploads in a frame and all kinds of other stuff, but...

TransitionImageLayout2(cmd, image_, mip - 1, 1, VK_IMAGE_ASPECT_COLOR_BIT,
VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL,
imageLayout, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL,
VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT,
VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_TRANSFER_READ_BIT);

// Low-quality mipmap generation, but works okay.
vkCmdBlitImage(cmd, image_, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, image_, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 1, &blit, VK_FILTER_LINEAR);
vkCmdBlitImage(cmd, image_, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, image_, imageLayout, 1, &blit, VK_FILTER_LINEAR);

TransitionImageLayout2(cmd, image_, mip - 1, 1, VK_IMAGE_ASPECT_COLOR_BIT,
VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, imageLayout,
VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT,
VK_ACCESS_TRANSFER_READ_BIT, VK_ACCESS_TRANSFER_WRITE_BIT);
}
Expand Down
2 changes: 1 addition & 1 deletion Common/GPU/Vulkan/VulkanImage.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class VulkanTexture {
bool CreateDirect(VkCommandBuffer cmd, VulkanDeviceAllocator *allocator, int w, int h, int numMips, VkFormat format, VkImageLayout initialLayout, VkImageUsageFlags usage = VK_IMAGE_USAGE_TRANSFER_DST_BIT | VK_IMAGE_USAGE_SAMPLED_BIT, const VkComponentMapping *mapping = nullptr);
void ClearMip(VkCommandBuffer cmd, int mip, uint32_t value);
void UploadMip(VkCommandBuffer cmd, int mip, int mipWidth, int mipHeight, VkBuffer buffer, uint32_t offset, size_t rowLength); // rowLength is in pixels
void GenerateMip(VkCommandBuffer cmd, int mip);
void GenerateMip(VkCommandBuffer cmd, int mip, VkImageLayout imageLayout);
void EndCreate(VkCommandBuffer cmd, bool vertexTexture = false, VkImageLayout layout = VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL);

// When loading mips from compute shaders, you need to pass VK_IMAGE_LAYOUT_GENERAL to the above function.
Expand Down
2 changes: 1 addition & 1 deletion Common/GPU/Vulkan/thin3d_vulkan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,7 @@ bool VKTexture::Create(VkCommandBuffer cmd, VulkanPushBuffer *push, const Textur
}
// Generate the rest of the mips automatically.
for (; i < mipLevels_; i++) {
vkTex_->GenerateMip(cmd, i);
vkTex_->GenerateMip(cmd, i, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL);
}
}
vkTex_->EndCreate(cmd, false);
Expand Down
3 changes: 2 additions & 1 deletion GPU/Vulkan/TextureCacheVulkan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -930,6 +930,7 @@ void TextureCacheVulkan::BuildTexture(TexCacheEntry *const entry) {
void *data;
bool dataScaled = true;
if (replaced.Valid()) {
// Directly load the replaced image.
data = drawEngine_->GetPushBufferForTextureData()->PushAligned(size, &bufferOffset, &texBuf, pushAlignment);
replaced.Load(i, data, stride);
entry->vkTex->UploadMip(cmdInit, i, mipWidth, mipHeight, texBuf, bufferOffset, stride / bpp);
Expand Down Expand Up @@ -1010,7 +1011,7 @@ void TextureCacheVulkan::BuildTexture(TexCacheEntry *const entry) {

// Generate any additional mipmap levels.
for (int level = maxLevel + 1; level <= maxLevelToGenerate; level++) {
entry->vkTex->GenerateMip(cmdInit, level);
entry->vkTex->GenerateMip(cmdInit, level, computeUpload ? VK_IMAGE_LAYOUT_GENERAL : VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL);
}

if (maxLevel == 0) {
Expand Down

0 comments on commit c48bdf7

Please sign in to comment.