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

[renderer] Remove redundant arguments which are available through device wrapper. #245

Merged
merged 1 commit into from
Sep 13, 2020

Conversation

IAmNotHanni
Copy link
Member

@IAmNotHanni IAmNotHanni commented Sep 12, 2020

Closes #238

This pull requests takes the essential work from my overloaded draft pr #241.
In this pull request, I removed redundant arguments which are already available through the device wrapper's get methods.

This code change also fixes an issues with queue management. The problem is that sometimes we passed the graphics queue to a constructor which names it internally as data_transfer_queue. Because both are just VkQueue, the constructor accepts it and passes it on even further down to the members.

For example this:

m_textures.emplace_back(*m_vkdevice, m_vkdevice->physical_device(),
                                       m_vkdevice->allocator(), texture_file,
                                       texture_name, m_vkdevice->graphics_queue(),
                                       m_vkdevice->graphics_queue_family_index());

will be passed on to:

Texture::Texture(const wrapper::Device &device, const VkPhysicalDevice graphics_card,
                 const VmaAllocator vma_allocator, const std::string &file_name,
                 const std::string &name, const VkQueue data_transfer_queue,
                 const std::uint32_t data_transfer_queue_family_index)

This new code has been changed so it is the constructor call to CommandBuffer which specifies the queue which is used.

EDIT: I forgot to mention I also forward-declared most variables in the RAII wrapper header files.

@IAmNotHanni IAmNotHanni added the cat:refactor refactor/clean up/simplifications/etc. label Sep 12, 2020
@IAmNotHanni IAmNotHanni self-assigned this Sep 12, 2020
@IAmNotHanni
Copy link
Member Author

This pull request already contains the squashed changes @yeetari requested previously in my draft pull request.
Feel free to review again :)

@IAmNotHanni IAmNotHanni merged commit 741c3ee into master Sep 13, 2020
@IAmNotHanni IAmNotHanni deleted the hanni/device_wrapper_refactoring2 branch September 13, 2020 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat:refactor refactor/clean up/simplifications/etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not pass data which is already contained in wrapper::Device
4 participants