Simple game engine

[gpx1000]

This is the next big tutorial effort. I added the conclusion to the base line tutorial, and this is the "next thing." The game engine presented here is one example of what the tutorial describes and not a 1-1 mapping of the source code to the implementation in the attachments folder.
Future work will add new features to the tutorial in the Vulkan Samples project. This is meant to give the course to describe basics of the game engine and Samples is meant to describe features as they are implemented. This can be used as a template for a starting off point for any project as it is designed to be self contained.

Core features in this tutorial/engine:

• HRTF (positional) audio in compute pipeline.
• Physics basic in compute pipeline.
• PBR gltf loading.
• KTX2 with BasisU compression.
• ImGUI

Several features are left out intentionally as a method of what we'll implement and feature in the Samples including parallel loading of the assets. Bistro takes a few moments to load as it stands here.

[SaschaWillems]

Also need to check all external links. Afair already mentioned that and there are still links that are either wrong or just dead like https://developer.download.nvidia.com/books/HTML/gpugems/gpugems_ch17.html

[asuessenbach]

Some more comments...

[asuessenbach]

The log level is just some kind of categorization with the generated output, you can't filter for e.g. warnings and above, right?

[gpx1000]

Correct, although I might change that. I've been debating about doing that change. Might make things easier for people wanting to get used to Android development to have it work the same everywhere.

[asuessenbach]

MEASURE_END -> MEASURE_STOP ?

[asuessenbach]

Is Cleanup called from somewhere else than the destructor?

[asuessenbach]

Can the vertexBuffers be non-empty before reserving here?
Maybe an assert(vertexBuffers.empty()); would be appropriate, instead.

[asuessenbach]

Why do you use std::arrays of size 1, instead of just one value?

[asuessenbach]

Maybe use designated initializers and use default values for irrelevant settings here?

[asuessenbach]

Don't you have some struct holding this data?

[asuessenbach]

Why do you explicitly clean up the old buffers? Aren't they automatically cleaned when you assign some new buffers?

[asuessenbach]

You should either check for drawData->TotalVtxCount != vertexCounts[frameIndex], or move this assignment out of this block.

[asuessenbach]

Some more comments...

[asuessenbach]

Throughout the code, you're sometimes using TimeDelta, and sometimes using std::chrono::milliseconds. Maybe you should use just one of them?

[asuessenbach]

Why do you erase std::next(it), not it?

[asuessenbach]

No find and check needed, just try_emplace an empty pool.

[asuessenbach]

Is it reasonable to cache the models in the ModelLoader?

[asuessenbach]

light_scale is EMISSIVE_SCALE_FACTOR, no matter what generator is used?

[asuessenbach]

A const function should return a const *.

[asuessenbach]

What if type is none of "directional", "point", or "spot"?

[asuessenbach]

No default color?

[asuessenbach]

You already had the very same lambda, but called it calculateNodeTransform up at line 668.

[asuessenbach]

I don't know, what's the expected node to light node ratio.
Is it worth to calculate the world transform for each node, and then use those from light nodes only?

[asuessenbach]

And some more comments...

[asuessenbach]

The materialndex is the index into the gltfModel::materials, and the materialName is the key for the ModelLoader::materials, right?
Why don't you hold the ModelLoader::materials as an array, corresponding to the gltfModel::materials? Then you would just need that index.

[asuessenbach]

How are all those texture paths related to those in the Material, referenced by the materialName?

[asuessenbach]

Would it simplify things if you added some default InstanceData for the not instanced meshes?
Resulting in a one-element vector for the non-instanced meshes, and a multi-element vector for the instanced ones.

[asuessenbach]

Should matIndex be renamed to materialIndex?
In the context of matrix handling, matIndex might be misleading.

[asuessenbach]

... and why have the instances a materialIndex?
I would have expected that everything in a MaterialMesh is related to the very same material.
Maybe you should decouple Mesh (vertices and indices) and Material?

[asuessenbach]

Designated initializers, maybe?

[asuessenbach]

If all those resources are ordered correctly in the vulkanResources, you would just need to waitForIdle and to unmap the memories. All others would be automatically done.

[asuessenbach]

Instead of comparing with VK_NULL_HANDLE, you could as well check for !*vulkanResources.physicsBuffer, etc.

[asuessenbach]

What does this comment mean?
You still initialize with zero.

[asuessenbach]

Again this hardcoded 0.0335f.
Maybe at least some constexpr somewhere?

[asuessenbach]

Some more comments...

[asuessenbach]

Currently, there's just one class deriving from RigidBody: ConcreteRigidBody.
Do you plan to introduce more classes later on, or why do you introduce this pure virtual base class?

[asuessenbach]

Where is the RigidBody removed from?
As it's the counterpart to CreateRigidBody, you might name it DestroyRigidBody?

However, one could argue whether creating a RigidBody should be part of the PhysicsSystem at all. Maybe you should instead change CreateRigidBody to AddRigidBody(std::unique_ptr<RigidBody> rb);.

[asuessenbach]

If the destructor does nothing, you just don't need it.

[asuessenbach]

This is the alternative to createPBRDescriptorSetLayout?
Then you might name it more specific, like createPhongDescriptorSetLayout?

[asuessenbach]

Are two separate shader modules out of two separate but identical shader codes necessary?

[asuessenbach]

As said above: no need to set members to their default value.

[asuessenbach]

Again: two separate shader modules out of two separate but identical shader codes?

[asuessenbach]

Again: use a std::array?

[asuessenbach]

Again: no need to set pViewports and pScissors for dynamic viewport and scissor

[asuessenbach]

createGraphicsPipeline, createPBRPipeline, and createLightingPipeline are some lengthy functions that in large parts are identical. Worth to add some helper function(s)?

[SaschaWillems]

@gpx1000 : Still can't build this on Windows with MSVC:

C:\VulkanSDK\1.4.321.1\include\glm/gtx/matrix_decompose.hpp(24,10): fatal error C1189: #error:  "GLM: GLM_GTX_matrix_de
compose is an experimental extension and may change in the future. Use #define GLM_ENABLE_EXPERIMENTAL before including
 it, if you really want to use it." [V:\Vulkan-Docs-Site\Vulkan-Tutorial\attachments\simple_engine\build\SimpleEngine.v
cxproj]

V:\Vulkan-Docs-Site\Vulkan-Tutorial\attachments\simple_engine\renderer_resources.cpp(16,10): fatal error C1083: Datei (
Include) kann nicht ge÷ffnet werden: "ktxvulkan.h": No such file or directory [V:\Vulkan-Docs-Site\Vulkan-Tutorial\atta
chments\simple_engine\build\SimpleEngine.vcxproj]
C:\VulkanSDK\1.4.321.1\include\glm\gtx\matrix_decompose.hpp(24,10): error C1189: #error:  "GLM: GLM_GTX_matrix_decompos
e is an experimental extension and may change in the future. Use #define GLM_ENABLE_EXPERIMENTAL before including it, i
f you really want to use it." [V:\Vulkan-Docs-Site\Vulkan-Tutorial\attachments\simple_engine\build\SimpleEngine.vcxproj
]

V:\Vulkan-Docs-Site\Vulkan-Tutorial\attachments\simple_engine\renderer_resources.cpp(16,10): error C1083: Datei (Includ
e) kann nicht geöffnet werden: "ktxvulkan.h": No such file or directory [V:\Vulkan-Docs-Site\Vulkan-Tutorial\attachment
s\simple_engine\build\SimpleEngine.vcxproj]

And some more errors.

Would like to run this at least once for a review.

Any idea on how to fix?

[gpx1000]

I haven't touched windows compiling yet. I'll try to get that addressed ASAP.

[SaschaWillems]

To get it compile with MSVC I did this:

• Manually add ktx_vulkan.h, for me it's not included with the libktx that vcpkg is downloading
• Use math.h and define _USE_MATH_DEFINES to get M_PI
• Remove constexpr from static constexpr std::vector emptyVector; in model_loader

[SaschaWillems]

While it does compile and run with above adjustments, it doesn't work properly.

This is how it looks for me on Win11 + with NV RTX 4070:

• No colors
• No audio
• All objects are transparent
• Only 10~20 fps
• Lots of validation errors

Looks like it can't load any of the images of that model. Looking at the folder where the bistro scene gets downloaded, I can't see a single KTX2 file?

[gpx1000]

That explains it. looks like the KTX2 file being missing and it's using the hand coded default files. That's the root cause of the no colors.

The audio, select default ping and hit play. You should hear something. I don't think I checked in the wav I've been using for debug as I don't think that's a good one to use.
Objects transparency is due to the texture issue mentioned.

Validation is clean here. give me a sample of them.

frame rate here is around 16fps. The intention is to have culling be part of the samples version of the game engine but that wasn't introduced here for this one so while I want to do things like improve the loading speed of the model, I'm trying to hold off and keep it as simple as possible yet be something we can build off of for the samples version. I chose optimization as a topic for the Samples version when it comes to getting it to work in Android / iOS.

[gpx1000]

Grab the latest of the bistro and you should have textures.

[gpx1000]

@SaschaWillems if you want, the changes you did for Windows, please make a PR against my branch here and I'll merge in your changes. That way we can keep a history and enable any further help you might be willing to give.

[SaschaWillems]

Textures now work. I guess the validation errors were caused by them missing, though I di get descriptor freeing related errors when resizing.

But lighting looks off when starting the app:

The basic lighting setting looks better, but the sun position slider doesn't do anything.

IMO optimizations are a must for this. It's doing ~12fps for me in fullscreen mode. I don't think we should leave it at that.

[SaschaWillems]

For reference, here is the Bistro scene in one of my renderers:

That renderer doesn't cull either, but you can see some visual differences like normal mapping, discard for alpha-masked objects, etc.

[gpx1000]

For what it's worth, the model you're using is slightly different. You're using a version of the bistro that's a lot simpler in that it doesn't come from the NVIDIA one that opens up the bistro and lets you go inside. There's also other differences. But I think a key one is the blender specified lumans value which does make the scene too bright during the noon day. Yes, there's still plenty of bugs in the rendering that I'm still trying to work out. But I'm still not entirely sure how much of it is due to rendering bugs from the punctual light source (sun) vs not. And the time of day slider is more for debugging the PBR pipeline. I'm still struggling with a few things in it. The phong version doesn't have the slider support because I haven't found a need to validate it there yet, but maybe it makes sense to keep the slider as it is pretty neat and add support to the phong path.

[gpx1000]

Main issue was not treating Spec-Gloss extension correctly.

However, still have less than optimal treatment of the glassware:

[asuessenbach]

Some more comments...

[asuessenbach]

The code for APP_CMD_INIT_WINDOW and APP_CMD_WINDOW_RESIZED are identical?

[asuessenbach]

Is it correct that a query (HasWindowResized) is not const and potentially modifies the queried state?

[asuessenbach]

Use [[maybe_unused]] instead?

[asuessenbach]

And in this implementation, you don't do that because of?

[asuessenbach]

Is it worth to have both, GetDevice() and GetRAIIDevice()?
Wouldn't getting the vk::raii::Device const & be sufficient, as you get to the corresponding vk::Device that easy?

[asuessenbach]

To keep the API consistent, you could pass in a vk::raii::CommandBuffer const & and a vk::raii::Fence const & here.

[asuessenbach]

Pass in a vk::raii::Image const & here?

[asuessenbach]

In this file, some functions get/return a vk::Handle, some get/return a vk::raii::Handle.
Maybe it would be more consistent to always use a vk::raii::Handle.

[asuessenbach]

No need to copy stuff into a one-element vector.
You could just call
commandBufferRaii.bindDescriptorSets(vk::PipelineBindPoint::eCompute, *computePipelineLayout, 0, *computeDescriptorSets[0], {});

[asuessenbach]

Where will the rawCommandBuffer be destroyed, then?

[SaschaWillems]

Looking better now, but the image is very noisy. I'm not sure where these come from, but I can see a lot of artifacts like these, changing when moving around:

And it looks like there is no mip mapping,

Update: This seems to be caused by over-saturation. Changing lighting so the affected surfaces get darker, the artifacts go away.

[SaschaWillems]

A code related question, that Andreas might've already asked (hard to see, github hides older comments): What is the idea behind converting (Vulkan-hpp) exceptions into bools? See e.g. Pipeline::createGraphicsPipeline. It catches exceptions from Vulkan-hpp and "converts" them into a bool. At work I would consider that bad practice, as the conversion removes important information (the exception).

Since this is exclusively using Vulkan-hpp and C++, why not stick to exceptions. So in above case, try, catch, log inside the function and raise the exception up to the calling code.