From d4225b22626edab0b4eba785dfecd0769a9dddd8 Mon Sep 17 00:00:00 2001 From: baldurk Date: Thu, 9 Nov 2017 12:03:31 +0000 Subject: [PATCH] Add some hefty padding to reported image required size on AMD. Refs #795 * AMD reports image required sizes with some variation even for the same creation parameters. This can mean the same image is reported with less required size during capture compared to replay, meaning memory binds won't work. * To get around this, we pad up the requirements to alignment * 4 and fudge to try and encompass any possible variation. --- .../driver/vulkan/wrappers/vk_get_funcs.cpp | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/renderdoc/driver/vulkan/wrappers/vk_get_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_get_funcs.cpp index 3ca68405c..cb4ec7ed8 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_get_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_get_funcs.cpp @@ -160,6 +160,30 @@ void WrappedVulkan::vkGetImageMemoryRequirements(VkDevice device, VkImage image, for(uint32_t i = 0; i < VK_MAX_MEMORY_TYPES; i++) if(memIdxMap[i] < 32U && (bits & (1U << memIdxMap[i]))) pMemoryRequirements->memoryTypeBits |= (1U << i); + + // AMD can have some variability in the returned size, so we need to pad the reported size to + // allow for this. The variability isn't quite clear, but for now we assume aligning size to + // alignment * 4 should be sufficient (adding on a fixed padding won't help the problem as it + // won't remove the variability, nor will adding then aligning for the same reason). + if(GetDriverVersion().IsAMD()) + { + VkMemoryRequirements &memreq = *pMemoryRequirements; + + VkDeviceSize oldsize = memreq.size; + memreq.size = AlignUp(memreq.size, memreq.alignment * 4); + + // if it's already 'super aligned', then bump it up a little. We assume that this case + // represents the low-end of the variation range, and other variations will be a little higher. + // The other alternative is the variations are all lower and this one happened to be super + // aligned, which I think (arbitrarily really) is less likely. + if(oldsize == memreq.size) + memreq.size = AlignUp(memreq.size + 1, memreq.alignment * 4); + + RDCDEBUG( + "Padded image memory requirements from %llu to %llu (base alignment %llu) (%f%% increase)", + oldsize, memreq.size, memreq.alignment, + (100.0 * double(memreq.size - oldsize)) / double(oldsize)); + } } void WrappedVulkan::vkGetImageSparseMemoryRequirements( @@ -218,6 +242,30 @@ void WrappedVulkan::vkGetImageMemoryRequirements2KHR(VkDevice device, for(uint32_t i = 0; i < VK_MAX_MEMORY_TYPES; i++) if(memIdxMap[i] < 32U && (bits & (1U << memIdxMap[i]))) pMemoryRequirements->memoryRequirements.memoryTypeBits |= (1U << i); + + // AMD can have some variability in the returned size, so we need to pad the reported size to + // allow for this. The variability isn't quite clear, but for now we assume aligning size to + // alignment * 4 should be sufficient (adding on a fixed padding won't help the problem as it + // won't remove the variability, nor will adding then aligning for the same reason). + if(GetDriverVersion().IsAMD()) + { + VkMemoryRequirements &memreq = pMemoryRequirements->memoryRequirements; + + VkDeviceSize oldsize = memreq.size; + memreq.size = AlignUp(memreq.size, memreq.alignment * 4); + + // if it's already 'super aligned', then bump it up a little. We assume that this case + // represents the low-end of the variation range, and other variations will be a little higher. + // The other alternative is the variations are all lower and this one happened to be super + // aligned, which I think (arbitrarily really) is less likely. + if(oldsize == memreq.size) + memreq.size = AlignUp(memreq.size + 1, memreq.alignment * 4); + + RDCDEBUG( + "Padded image memory requirements from %llu to %llu (base alignment %llu) (%f%% increase)", + oldsize, memreq.size, memreq.alignment, + (100.0 * double(memreq.size - oldsize)) / double(oldsize)); + } } void WrappedVulkan::vkGetImageSparseMemoryRequirements2KHR(