PacktPublishing / Mastering-Graphics-Programming-with-Vulkan

MIT License
514 stars 65 forks source link

gltf Models Not Loading #29

Closed OptimisticMonkey closed 1 year ago

OptimisticMonkey commented 1 year ago

There is an issue loading gltf model other than Sponza.

Its easy to reproduce, just try to load any other model - duck.gltf for example.

It looks like the problem is related to the following comment at line 501:main.cpp // NOTE(marco): for now we expect all three textures to be defined. In the next chapter // we'll relax this constraint thanks to bindless rendering!

This approach eventually causes skipping of mesh_draws.push( mesh_draw ); for many gltf models.

I cannot load models in Chapter 2 either.

Would be great if you guys could fix this so people starting out could load other models.

theWatchmen commented 1 year ago

I think we might have fixed some of this in later chapters and could be just a matter of back-porting the fix. It might be a while before we look at it though as we are in the middle of preparing for a conference :)

OptimisticMonkey commented 1 year ago

Thanks - When starting to read any book that builds up code, its always easier to move up from a working base. If I knew more I would try to to fix for you, but that is why I am reading the book :-)

At least the model that does load is Sponza :-)

Radagan commented 1 year ago

I'm getting a lot of validation errors as well... Is Vulkan 1.3 supported by these examples or do we need to use 1.2?

jatobu commented 1 year ago

All chapters seem to load by default the Sponza glft file, and the book doesn't mention what glft files should it be used to test each chapter's project with, and whatever glft file I try to pass as argument it will never load correctly (or at all)... no matter what chapter am I trying to run.

theWatchmen commented 1 year ago

@Radagan We switched to Vulkan 1.3 at some point and I don't think we went back to make sure all chapters were validation error free. We also tested on devices that supported all the extensions we use. While we tried to safeguard unsupported extension use, some cases might have slipped through. Some chapters (i.e. the ones on ray-tracing) do need the extensions we request and won't work on devices that don't support those extensions.

@jatobu Sponza is the default model and the one we tested more extensively with. Flight Helmet is another one, but we didn't go through all of the glTF models in the sample repo. It's hard to show the impact of some techniques on some of the smaller models, which is why we default to Sponza.

For both cases, please feel free to report the issues you encounter or create a PR to fix the issue. We'll get to them as soon as we can.

Radagan commented 1 year ago

@theWatchmen Thank you. I just wanted to make sure 1.3 was okay. I agree with @OptimisticMonkey that with a topic like Vulkan working examples are critical to validating what is being learned, or risk obscuring with doubt that you have learned the lesson correctly!

OptimisticMonkey commented 1 year ago

@theWatchmen Let us know when you have time to fix it with other models... I was planning to use this as a starting point for some other stuff, but dont want to start with it broken :-(

I would personally prefer a solid working Chapter 1-4 base than fixes for later chapters

jatobu commented 1 year ago

@theWatchmen Oh, I see... at the end of Chapter1 the book says we should see by default the Flight Helmet and after running it and seeing Sponza made me think something was wrong, and after going through all the other chapters and seeing the same Sponza I thought something was broken somewhere, and I should be seeing different models showcasing each chapter's topic.

Anyway, after adding the Flight Helmet manually and changeing scale I finally can see it as intended.

@OptimisticMonkey pass the Flight Helmet path as an argument, and change these lines in Chapter1's code and should do the trick:

576:    vec3s eye = vec3s{ 0.0f, 1.5f, 2.0f };
583:    float model_scale = 3.50f;
OptimisticMonkey commented 1 year ago

Thanks @jatobu ! I will try it with the helmet! I tried several other models - most of the models wont work if you try them - glad to know there is at least one other one. Out of curiosity, do you know if the helmet has three textures defined on it?

OptimisticMonkey commented 1 year ago

For those interested, I stumbled onto a great vulkan gltf viewer:

https://github.com/SaschaWillems/Vulkan-glTF-PBR

BTW - Looks like the gltf format itself specifies the correct camera/scene/scale stuff in it. That way you can read in the correct eye position, scale, and projection stuff without guessing

https://github.com/KhronosGroup/glTF#overview

theWatchmen commented 1 year ago

@jatobu You don't need to modify the code to change the global scale, there is an imgui widget in the first chapter to do that :)

@OptimisticMonkey we did fix it in later chapters (can't remember if from chapter 2 or later), so the models that do work will have the right scale. We'll backport these fixes to the early chapters as soon as we can.

Radagan commented 1 year ago

@OptimisticMonkey, Yes, Sascha's repos are really helpful. But he uses tinygltfloader, which I had a lot of trouble using. It parses the format fine, but I had difficulty making basic scale, position, and rotation changes to composite more than one gltf object on the screen. I'm following the advice in this book and trying to decide between cgltf and libgltf. Not sure which is the best to use for my modern C++ project. Any and all advice welcome!

@jatobu , Thank you for the scaling tip!

@theWatchmen, Hum... clearly I need to re-read the first chapter... RenderDoc?

theWatchmen commented 1 year ago

I fixed chapter 1 to load as many glTF models as possible. We still don't handle the following:

Transparency has been added in later chapters, an the other cases are relatively rare or used only for very simple tests.

I am going to consider this as resolved, but feel free to re-open if needed.

OptimisticMonkey commented 1 year ago

Thanks @theWatchmen ! Worked great with the gltf files I tested :-)