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.

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

Asserts on Android 9 wth vkGetPhysicalDeviceMemoryProperties2KHR #410

Open
alecazam opened this issue Feb 23, 2024 · 2 comments
Open

Asserts on Android 9 wth vkGetPhysicalDeviceMemoryProperties2KHR #410

alecazam opened this issue Feb 23, 2024 · 2 comments

Comments

@alecazam
Copy link

alecazam commented Feb 23, 2024

Hitting another crash on our 1.1 Android 9 devices. This code seems to just assert on this extension, but we weren't loading the extension nor was that documented for using VMA. Also the KHR extension isn't needed on 1.2+. So this is similar to the failure with the 1.3 asserts, but there the code conditionally test.

Samsung SM-g955f w/Mali-G71 running Android 9, Android 28, Galaxy S8+, Vulkan 1.1?

For now, we'll just remove this assert. I'm assuming this was a Vulkan 1.1 part, and so it should have had the vkGetPhysicalDeviceMemoryProperties2 which the code sets vkGetPhysicalDeviceMemoryProperties2KHR to.

Code that sets up the function ptr.

#if VMA_MEMORY_BUDGET || VMA_VULKAN_VERSION >= 1001000
    if(m_VulkanApiVersion >= VK_MAKE_VERSION(1, 1, 0))
    {
        m_VulkanFunctions.vkGetPhysicalDeviceMemoryProperties2KHR = (PFN_vkGetPhysicalDeviceMemoryProperties2)vkGetPhysicalDeviceMemoryProperties2;
    }
#endif

This is the assert that fails at startup. I'll dig through vulkan.gpuinfo.org, since this is a player device, and not one that I have.

#if VMA_MEMORY_BUDGET || VMA_VULKAN_VERSION >= 1001000
	// This is also asserting on 1.1, why not make these optional
	// KRH requires the caller to have loaded the extension, but 1.1 should have the function.
        // also should this be check for == instead of >=, there 
    if(m_UseExtMemoryBudget || m_VulkanApiVersion >= VK_MAKE_VERSION(1, 1, 0))
    {
        VMA_ASSERT(m_VulkanFunctions.vkGetPhysicalDeviceMemoryProperties2KHR != VMA_NULL);
    }
#endif
@adam-sawicki-a
Copy link
Contributor

I made some fixes in importing m_VulkanFunctions.vkGetPhysicalDeviceMemoryProperties2KHR for various cases of Vulkan version and extension availability, but these could result in compilation errors rather than the assert failing.

What method of importing Vulkan functions do you use? VMA_STATIC_VULKAN_FUNCTIONS, VMA_DYNAMIC_VULKAN_FUNCTIONS, or you provide VmaVulkanFunctions?

What suspected bug in VMA do you report here?

Function vkGetPhysicalDeviceMemoryProperties2KHR should be set whenever:

  • You recognized Vulkan >= 1.1 is available and you enabled such version, indicated by VmaAllocatorCreateInfo::vulkanApiVersion (fetched as "vkGetPhysicalDeviceMemoryProperties2") or
  • You recognized VK_KHR_get_physical_device_properties2 instance extension is available and you enabled it, along with VK_EXT_memory_budget, indicated by VMA_ALLOCATOR_CREATE_EXT_MEMORY_BUDGET_BIT (fetched as "vkGetPhysicalDeviceMemoryProperties2KHR").

And this is what this assert checks, so I think it is correct.

@kusma
Copy link

kusma commented Jul 30, 2024

I'm seeing the same here. I'm using Volk, so both VMA_STATIC_VULKAN_FUNCTIONS and VMA_DYNAMIC_VULKAN_FUNCTIONS are disabled, and indeed using VmaVulkanFunctions.

However, it seems to work fine if I pass the pointer to vkGetPhysicalDeviceMemoryProperties2 instead of vkGetPhysicalDeviceMemoryProperties2KHR to VmaVulkanFunctions... Strange, because those should AFAICT be aliases of each other.

I'm using RADV from Mesa, in case that matters.

kusma added a commit to kusma/even-laster-engine that referenced this issue Aug 25, 2024
For some reason, I need to use vkGetPhysicalDeviceMemoryProperties2
instead of vkGetPhysicalDeviceMemoryProperties2KHR here. See this
issue for details:

GPUOpen-LibrariesAndSDKs/VulkanMemoryAllocator#410
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants