feat: replace deletion_queue with smart pointers#98
Merged
Conversation
anunknowperson
approved these changes
May 29, 2025
There was a problem hiding this comment.
Pull Request Overview
This PR replaces manual deletion queues with RAII smart pointer wrappers for Vulkan resources to simplify cleanup and reduce memory leaks. Key changes include the introduction of Vulkan smart wrapper classes for fences, semaphores, command pools, images, and buffers, as well as updates throughout the engine to use these wrappers in place of raw Vulkan handles.
- Introduced RAII wrapper classes in vk_smart_wrappers.h
- Updated vk_engine.h and vk_engine.cpp to use smart pointers for resource management
- Modified command buffer and pipeline initialization to work with the new smart wrappers
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/include/graphics/vulkan/vk_smart_wrappers.h | New RAII wrappers for Vulkan objects ensuring automatic cleanup |
| src/include/graphics/vulkan/vk_engine.h | Switched deletion queue to smart pointers for frame data cleanup |
| src/graphics/vulkan/vk_loader.cpp | Updated image retrieval to use the smart pointer getters |
| src/graphics/vulkan/vk_engine.cpp | Replaced raw handles with smart pointer calls across resource creation, submission, and cleanup |
| src/graphics/vulkan/vk_command_buffers.cpp | Updated command pool and fence creation to use smart pointers |
| src/graphics/vulkan/pipelines.cpp | Adapted pipeline resource usage to work with smart pointers |
Comments suppressed due to low confidence (1)
src/graphics/vulkan/vk_engine.cpp:625
- Manual destruction of command pools in the cleanup routine is redundant since the VulkanCommandPool destructor already handles resource deletion. Removing this manual call will help prevent potential double free errors.
if (_frame._commandPool) { vkDestroyCommandPool(_device, _frame._commandPool->get(), nullptr); }
Tydik42
approved these changes
Jun 2, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace raw Vulkan handles with smart wrapper classes for automatic resource management. Updates command pools, images, and fences to use RAII pattern, eliminating manual cleanup code and reducing memory leaks.