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

Rendering pipeline #33

Closed
wants to merge 32 commits into from
Closed

Rendering pipeline #33

wants to merge 32 commits into from

Conversation

Rinzii
Copy link
Owner

@Rinzii Rinzii commented Jul 23, 2023

No description provided.

@Rinzii
Copy link
Owner Author

Rinzii commented Aug 3, 2023

I'd like a review of how I'm handling instance creation inside of the device class. relevant commit is a13d01e

@Rinzii Rinzii requested a review from karnkaul August 3, 2023 14:57
@Rinzii
Copy link
Owner Author

Rinzii commented Aug 3, 2023

Also light review of the vkHelpers is also fine

editor/src/main.cpp Outdated Show resolved Hide resolved
engine/include/application.hpp Outdated Show resolved Hide resolved
engine/include/application.hpp Outdated Show resolved Hide resolved
engine/include/graphics/apiVersion.hpp Outdated Show resolved Hide resolved
Comment on lines 27 to 28
std::optional<u32> graphicsFamily;
std::optional<u32> presentFamily;
Copy link
Collaborator

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.

Copy link
Owner Author

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.

Copy link
Collaborator

@karnkaul karnkaul Aug 4, 2023

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.

engine/src/graphics/vkHelpers.cpp Outdated Show resolved Hide resolved
engine/src/graphics/vkHelpers.cpp Outdated Show resolved Hide resolved
engine/src/logger/log.cpp Outdated Show resolved Hide resolved
engine/src/windowing/window.cpp Outdated Show resolved Hide resolved
engine/src/windowing/window.cpp Outdated Show resolved Hide resolved
@karnkaul
Copy link
Collaborator

karnkaul commented Aug 3, 2023

Why are all checks consistently failing?

@karnkaul
Copy link
Collaborator

karnkaul commented Aug 3, 2023

The title of the PR is misleading, this work sets up a Vulkan instance, no pipelines or rendering.

@Rinzii
Copy link
Owner Author

Rinzii commented Aug 3, 2023

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.

@Rinzii
Copy link
Owner Author

Rinzii commented Aug 3, 2023

Why are all checks consistently failing?

No PCH issues. My bad on my part forgot to test without PCH. Will correct all of those issues before re-review!

Rinzii and others added 3 commits August 4, 2023 10:53
* 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]>
@Rinzii Rinzii closed this Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants