-
Notifications
You must be signed in to change notification settings - Fork 0
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
Rendering pipeline #33
Conversation
# Conflicts: # editor/src/main.cpp # engine/header_list.cmake
main merge
I'd like a review of how I'm handling instance creation inside of the device class. relevant commit is a13d01e |
Also light review of the vkHelpers is also fine |
engine/include/graphics/device.hpp
Outdated
std::optional<u32> graphicsFamily; | ||
std::optional<u32> presentFamily; |
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.
Are you sure you want to get into multi-queue right now? It's 10x more complicated than dealing with a single queue.
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.
All the resources I’ve been using has been using multi but if its far more complicated then I’ll prob just pull it back to a single queue.
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.
Which resources use multiple queues? vkguide uses only one, so does Vulkan tutorial.
We are requesting a graphics queue, which supports all we need for the guide.
Why are all checks consistently failing? |
The title of the PR is misleading, this work sets up a Vulkan instance, no pipelines or rendering. |
This PR is a draft that was originally meant to be reviewed for the entire rendering pipeline but since I’ve brought the work load to have more reviews it is no longer correct. I just reused it instead of closing the PR and opening a new one though I likely should of just closed and restarted it. |
No PCH issues. My bad on my part forgot to test without PCH. Will correct all of those issues before re-review! |
* Begin cleanup of vk * Allow logging in release builds * Clean up layout * Clean up layout * CleaFix issue with extensions * remove old header * fix missing conditional compilation * Add missing include * Add missing include * Minor clean up * formatting * make windowing more safe * clean up the pipeline as it'ss not as relevant yet * Improve safety of devices and make things cleaner * Cleanup includes * More cleanup * Auto-format and gitignore changes. * Add log category to formatted output. * Put logger and application inside of the try catch * Use Delete struct with unique_ptr over shared_ptr approach * Use system/win32/windowsHeader.hpp. * Incorrect ordering of includes * Address code review * Remove app based validation layers * Completely remove all validation layers * Redo api layout for title * Small cleanup * Minor cleanups --------- Co-authored-by: Karn Kaul <[email protected]>
No description provided.