Resolve "Context Functionality"

Closes #7 (closed)

changed milestone to %First Triangle

added 🍪 ToDo 😶 Critical labels

added 1 commit

• 47fe8a97 - [#7 (closed)] add new context functionalities

Compare with previous version

@agauggel @tfrisch @awasmut @thollmann Wir haben mal alle ToDos aus #7 (closed) umgesetzt. Wir bräuchten jetzt nur noch einen/mehrere Reviewer zum drübergucken :D

removed 🍪 ToDo label

added 🍪 ToDo 🙏 Review labels

removed 🍪 ToDo label

In Context.hpp:

• Zeile 44 ist noch der Klassen-Name in der Definition! (ist tatsächlich ein Compiler-Fehler, der behoben werden muss)

In Context.cpp:

• Zeile 4 bis 6 sollte in die create() Funktion, da es nur darin verwendet wird. Es sollten keinerlei weitere Vulkan-Calls kommen, welche die Validation-Layer-Namen benötigen.
• Zeile 33 wird zur Laufzeit überprüft, ob validation-layer genutzt werden sollen. Dafür ist kein preprocessor-command in der .cpp-Datei. Ich persönlich hätte auch kein Problem damit sowas in die .cpp zu packen und dafür könnten die Abfragen, Übergaben etc. der Validation-Layer zur Compile-Zeit passieren (würde potentiell auch die globalen Variablen sparen). Am besten wir diskutieren das jetzt und machen es konsistent. Ich denke, beides hat Vorteile.
• Zeile 134 funktioniert so leider nicht (phyDevice liegt immer auf dem Stack und hat eine valide Addresse), aber man könnte stattdessen max_score überprüfen.
• Zeile 171 wäre es vermutlich besser das else-if rauszunehmen und die Bedingung in den else-Zweig einzubauen.
• Zeile 210 bis 233 kann man vermutlich noch was vereinfachen mit std::min().

Ansonsten noch zu einigen Compiler-Warnungen, die auftreten: Ich denke, ihr habt vor allem online die C-API calls zu z.B. enumerate...() Funktionen gesehen und daher übernommen. Diese geben normalerweise noch ein Result zurück, welches überprüft werden muss. Man kann sich das allerdings sparen, wenn man die vereinfachte Überladung von der HPP nutzt.

const std::vector<vk::PhysicalDevice>& devices = instance.enumeratePhysicalDevices();
const uint32_t deviceCount = devices.size();

...statt...

uint32_t deviceCount = 0;
instance.enumeratePhysicalDevices(&deviceCount, nullptr);

std::vector<vk::PhysicalDevice> devices(deviceCount);
instance.enumeratePhysicalDevices(&deviceCount, devices.data());

Das ist an einigen Stellen noch ähnlich und die Variante, direkt einen fertigen vector zu übernehmen sollte meistens weniger Probleme bereiten und spart Code.

Ansonsten passt es soweit und alles scheint wie gewünscht zu funktionieren.

mentioned in commit 0e8cdfb5

added 1 commit

• 0e8cdfb5 - [#7 (closed)][Fix] Simplify and correct Context class

Compare with previous version

@tfrisch Wir haben soweit versucht deine Anmerkungen umzusetzen. Bei einer Sache waren wir uns aber noch unsicher, und zwar bei den queue priorities in Zeile 222. Eigentlich hatten wir die bzgl. der vulkan.h so definiert z.B. float queuePrios[] = { 1.0f, 1.0f, 1.0f, 1.0f }; und dann deviceQueueCreateInfo.pQueuePriorities = queuePrios; Das führt allerdings in der Verwendung mit der vulkan.hpp zu validation errors. Wenn man das mit einem new löst, wie in Z.222, gibt es keine Validation errors, aber wir konnten das dann auch nicht mehr deleten, was uns bei der Lösung nicht so gefällt. Hast du da eine Idee/einen Tipp?

marked this merge request as ready

requested review from @tfrisch

added 1 commit

• 9beed6c5 - [#7 (closed)] replaced queue priority array by a std::vector

Compare with previous version

added 9 commits

• 9beed6c5...ae7d8fad - 8 commits from branch develop
• c1de6145 - Merge branch 'develop' into 7-context-functionality

Compare with previous version

added 1 commit

• 644f7410 - [#7 (closed)] change glfw calls to CoreManager calls

Compare with previous version

resolved all threads

approved this merge request

added 2 commits

• 80ff3da6 - [#7 (closed)] fixed some compiler warnings
• 898eb191 - [#7 (closed)] fixed cross-compatible debug-flags

Compare with previous version

mentioned in commit 97cfabc3

merged