Switch leaks from VMA_ASSERT to VMA_ASSERT_LEAK.

[alecazam]

These are all of the asserts that I turned into VMA_ASSERT_LEAK. While I agree that leaks are important to track, when the app is shutting down, I really don't want it to abort. So these go to a non-asserting abort. But still want the rest of the allocator to assert if there is an issue.

Now I see all the leaks at shutdown, instead of having the app abort() at the first leak. And I still see the rest of my shutdown leaks and issues.

[adam-sawicki-a]

I think this is a great idea to introduce a separate macro VMA_ASSERT_LEAK. I added it. Thank you for the suggestion.

I am not sure about these two asserts checking if m_hMemory == VK_NULL_HANDLE. Are you sure they are false in case of memory leaks? I think they are more about validating internal consistency of the library, e.g. whether VmaDeviceMemoryBlock::Destroy was called before destructor. I left them as VMA_ASSERT.

[alecazam]

Pretty sure those were firing, but I had to be safe than sorry. And I caught and fixed another one in our crash reports that's not listed above. Here's all my current mods to vk_mem_alloc.h if they help. There was a mod to logging too.

vk_mem_alloc.zip

[adam-sawicki-a]

OK, I made some more changes based on your code.

I can see you use some old version of the library. I recommend merging latest from the "master" branch, as it contains some bug fixes.

[alecazam]

Okay, I updated our vk_mem_alloc.h to latest.

1. There are too many untagged pointers for these to work with clang. I always have to turn these off.
2. Also there are still lots of uninitialized class member variables amidst initialized ones.
3. I have all %llu replaced with %" PRIu64 to allow Win clang to properly compile these. There is an open issue on this still

// The VMA_NULLABLE macro is defined to be _Nullable when compiling with Clang.
// see: https://clang.llvm.org/docs/AttributeReference.html#nullable
#ifndef VMA_NULLABLE
    #if 0 // def __clang__
        #define VMA_NULLABLE _Nullable
    #else
        #define VMA_NULLABLE
    #endif
#endif

// The VMA_NOT_NULL macro is defined to be _Nonnull when compiling with Clang.
// see: https://clang.llvm.org/docs/AttributeReference.html#nonnull
#ifndef VMA_NOT_NULL
    #if 0 // def __clang__
        #define VMA_NOT_NULL _Nonnull
    #else
        #define VMA_NOT_NULL
    #endif
#endif

Also in my version, there is this construct. This allows the leaks to log but not all the other spam from VMA_DEBUG_LOG. Not sure that the VMA_DEBUG_LOG_FORMAT was needed. Our log works with varargs, and does printflike testing of incoming arguments. The default printf should do the same, so you should see problem 3 above. This is due to x86 vs x64, and also x64 win using long int for uint64_t, and everyone else using long long.

#ifndef VMA_DEBUG_LOG_LEAK
    #define VMA_DEBUG_LOG_LEAK VMA_DEBUG_LOG_FORMAT
#endif

void VmaBlockMetadata::DebugLogAllocation(VkDeviceSize offset, VkDeviceSize size, void* userData) const
{
    if (IsVirtual())
    {
        VMA_DEBUG_LOG_LEAK("UNFREED VIRTUAL ALLOCATION; Offset: %" PRIu64 "; Size: %" PRIu64 "; UserData: %p", offset, size, userData);
    }
    else
    {
        VMA_ASSERT(userData != VMA_NULL);
        VmaAllocation allocation = reinterpret_cast<VmaAllocation>(userData);

        userData = allocation->GetUserData();
        const char* name = allocation->GetName();

#if VMA_STATS_STRING_ENABLED
        VMA_DEBUG_LOG_LEAK("UNFREED ALLOCATION; Offset: %" PRIu64 "; Size: %" PRIu64 "; UserData: %p; Name: %s; Type: %s; Usage: %u",
            offset, size, userData, name ? name : "vma_empty",
            VMA_SUBALLOCATION_TYPE_NAMES[allocation->GetSuballocationType()],
            allocation->GetBufferImageUsage());
#else
        VMA_DEBUG_LOG_LEAK("UNFREED ALLOCATION; Offset: % " PRIu64 "; Size: %" PRIu64 "; UserData: %p; Name: %s; Type: %u",
            offset, size, userData, name ? name : "vma_empty",
            (uint32_t)allocation->GetSuballocationType());
#endif // VMA_STATS_STRING_ENABLED
    }

}

[alecazam]

Also I saw a comment about old constructs on Android in our version, and we had a workaround. Android jumped from Vulkan 1.1 headers to 1.3, but only in NDK 25. Should really move to NDK 26, since they finally added weak symbols like on macOS/iOS. So can then call ahead to newer NDK calls without all the hassle.

Looks like code is using the older version of these calls now, so this fix isn't needed.

#ifdef __ANDROID__ // TODO: should check vulkan define here for newer calls
                // hack - use the old macros, not sure why Android isn't using newer headers to build
                json.ContinueString(VK_VERSION_MAJOR(deviceProperties.apiVersion));
                json.ContinueString(".");
                json.ContinueString(VK_VERSION_MINOR(deviceProperties.apiVersion));
                json.ContinueString(".");
                json.ContinueString(VK_VERSION_PATCH(deviceProperties.apiVersion));
#else
                json.ContinueString(VK_API_VERSION_MAJOR(deviceProperties.apiVersion));
                json.ContinueString(".");
                json.ContinueString(VK_API_VERSION_MINOR(deviceProperties.apiVersion));
                json.ContinueString(".");
                json.ContinueString(VK_API_VERSION_PATCH(deviceProperties.apiVersion));
#endif

[alecazam]

Also our code wasn't building on Android due to use of VK_ERROR_UKNOWN instead of VK_ERROR_UNKNOWN_COPY in 2 spots.

VkResult VmaAllocator_T::BindBufferMemory(
    VmaAllocation hAllocation,
    VkDeviceSize allocationLocalOffset,
    VkBuffer hBuffer,
    const void* pNext)
{
    VkResult res = VK_ERROR_UNKNOWN_COPY;


VkResult VmaAllocator_T::BindImageMemory(
    VmaAllocation hAllocation,
    VkDeviceSize allocationLocalOffset,
    VkImage hImage,
    const void* pNext)
{
    VkResult res = VK_ERROR_UNKNOWN_COPY;

[adam-sawicki-a]

Thank you for reporting these issues. I made some fixes as you requested.