-
Notifications
You must be signed in to change notification settings - Fork 219
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
Allocate DescriptorBuffer memory from pools #943
Conversation
Allocate buffers that usually contain uniform data from memory pools, instead of directly calling vkAllocateMemory. Some drivers have very low limits on the number of Vulkan memory objects that can be allocated. This fixes vsg-dev#942.
} | ||
else | ||
{ | ||
warn("DescriptorBuffer::compile(..) unable to allocate buffer within associated DeviceMemory."); | ||
throw Exception{"Error: DescriptorBuffer::compile(..) unable to allocate buffer."}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are to throw an exception then the returning a relevant Vulkan error would be helpful.
Alternatively we could use vsg::fatal().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are to throw an exception then the returning a relevant Vulkan error would be helpful.
I agree. That would require changing the memory pool interface, but perhaps that is desirable.
Alternatively we could use vsg::fatal().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This close to a release I want to avoid getting diverted into lots of other work - occasionally I need to get some paid work done :-)
For testing purposes and help refine the code I have created a timoore-buffer-memory-pools branch: https://github.com/vsg-dev/VulkanSceneGraph/tree/timoore-buffer-memory-pools I have also changed the way the flags are passed into the reserveMemory(..) method to help make the code more readable and easier to adapt the settings: 00138ad We may way to change to using device local storage for the DescriptorBuffer and than host visible. I am wary of making such a change off the the cuff though so I'l leave as is and we can optimize later. |
I have looked at other places that the reserveMemory(..) is used and have standardized the wording and add the VK_ERROR_OUT_OF_DEVICE_MEMORY to be more consistent with the other usage: 6f6e375 I will do some testing now to make sure everything is working as intended. |
Testing looks fine, so I have now merged the timoore-buffer-memory-pools branch with VSG master., |
I'd hold off on that too; it's a complex subject. It seems to be very common usage to allocate uniform buffers in host visible memory. On the other hand, device local memory on AMD integrated APUs is a scarce resource. You may have seen this useful link: https://asawicki.info/news_1740_vulkan_memory_types_on_pc_and_how_to_use_them . |
I hadn't come across the article, thanks for the link. The explanation of how AMD APUs are managing device local is particularly relevant as I am using one right now! I will need to reflect on the topic. Further done the line it might be worth having some scheme to set preferences and fallbacks to cope with how different drivers/hardware are best used w.r.t memory allocations. |
Allocate buffers that usually contain uniform data from memory pools, instead of directly calling vkAllocateMemory. Some drivers have very low limits on the number of Vulkan memory objects that can be allocated.
This fixes #942.