Vk pipeline initialization #55
Conversation
There was a problem hiding this comment.
Some code changes requested. Additionally, since the app now relies on loading data from the filesystem, we need to establish systems and/or conventions for it to be able to locate that data when debugging. Usually debuggers will use the project's root directory as the working directory, but may also use the directory where the executable was built. I've been using the project root in my .vscode/launch.json, but that fails to load the shaders since they are inside ./editor. Changing the pwd to that works, but it's currently undocumented in the repo.
| GEN_NODISCARD std::vector<vk::Image> const & getSwapChainImages() const { return m_swapChainImages; } | ||
| GEN_NODISCARD std::vector<vk::UniqueImageView> const & getSwapChainImageViews() const { return m_swapChainImageViews; } |
There was a problem hiding this comment.
Please use std::span<T const> instead of std::vector<T> const& (in general, everywhere).
| @@ -69,6 +76,8 @@ namespace gen | |||
| vk::UniqueSurfaceKHR m_surface{}; | |||
| Gpu m_gpu{}; | |||
| vk::UniqueDevice m_device{}; | |||
| vk::UniqueCommandPool m_commandPool{}; | |||
| vk::UniqueCommandBuffer m_commandBuffer{}; | |||
There was a problem hiding this comment.
Command buffers are owned by command pools, we don't need RAII here. (AKA just use vk::CommandBuffer.)
| vk::Viewport viewport{}; | ||
| vk::Rect2D scissor{}; |
There was a problem hiding this comment.
These should be dynamic states (vk::DynamicState). Otherwise you'll need a new pipeline every time the window resizes.
There was a problem hiding this comment.
Related: config is missing vk::PipelineDynamicStateCreateInfo.
| vk::Viewport viewport{}; | ||
| vk::Rect2D scissor{}; | ||
| vk::PipelineInputAssemblyStateCreateInfo inputAssemblyInfo{}; | ||
| vk::PipelineTessellationStateCreateInfo tessellationInfo{}; |
There was a problem hiding this comment.
Not required, we won't be using tesselation.
| m_commandBuffer = | ||
| std::move(m_device->allocateCommandBuffersUnique(vk::CommandBufferAllocateInfo(m_commandPool.get(), vk::CommandBufferLevel::ePrimary, 1)).front()); |
There was a problem hiding this comment.
VulkanHPP offers overloads for such functions that take a pointer and count instead of returning a vector. It's not a big deal here as this only happens once, but later on we'll be creating command buffers on the fly for various stuff, where it will add needless allocation overhead.
| configInfo.viewport.x = 0.0F; | ||
| configInfo.viewport.y = 0.0F; | ||
| configInfo.viewport.width = static_cast<float>(extent.x); | ||
| configInfo.viewport.height = static_cast<float>(extent.y); |
There was a problem hiding this comment.
Recommend flipping the viewport.
|
|
||
| std::array<vk::PipelineShaderStageCreateInfo, 2> shaderStages = {vertShaderStageInfo, fragShaderStageInfo}; | ||
|
|
||
| std::vector<vk::DynamicState> dynamicStates = {vk::DynamicState::eViewport, vk::DynamicState::eScissor}; |
There was a problem hiding this comment.
Why vector if you know you want just two entries?
|
Putting this as blocking until restructure is finished. |
Rinzii
added
Status: Blocked
This PR is primarily focused on the creation of the Vulkan pipeline, but there is also some small additional add ons bundled with the PR for the future.