Remove function definitions from header files

[sven-luepke]

Let's move some function implementations that don't have to be inlined from .hpp headers to .cpp implementation files. This would have the following benefits:

• Clearer header files and easier to understand class structures
• Faster partial recompilation

[Lachei]

I think that's a good idea looking at the large classes right now. Just for completeness here the reasons why this was not done from the get go: As the codebase should mainly be a blueprint with a lot of setup code already written and be used by simply copy pasting an existing blueprint for a raytracing pipeline and adopting it to one needs, i thought with the vsg library in the back the classes for the raytracing pipeline etc. would stay small. Using header only files would then have the benefit of having the class in a single file and thus no matter the changes, only a single file has to be touched. And as the classes are meant for prototyping, a lot of changes will occur.

Nevertheless i think separating the source files is preferable. The source files then should definitely be grouped into src and include folders for a better project structure.

Would you do the rewriting for the current files?

[sven-luepke]

I think that's a good idea looking at the large classes right now. Just for completeness here the reasons why this was not done from the get go: As the codebase should mainly be a blueprint with a lot of setup code already written and be used by simply copy pasting an existing blueprint for a raytracing pipeline and adopting it to one needs, i thought with the vsg library in the back the classes for the raytracing pipeline etc. would stay small. Using header only files would then have the benefit of having the class in a single file and thus no matter the changes, only a single file has to be touched. And as the classes are meant for prototyping, a lot of changes will occur.

Nevertheless i think separating the source files is preferable. The source files then should definitely be grouped into src and include folders for a better project structure.

Would you do the rewriting for the current files?

Sure, I can do it.

[sven-luepke]

While working on this I came accross an issue with the include directives. Let's use Defines.hpp as an example, which currently looks like this:

#pragma once

using uint = uint32_t;

It uses the type uint32_t from the <cstdint> header even though no header is included here. But how is it possible that this header works just fine in the current code?

The explanation for this is, that the compiler (and probably also the code analyser of your IDE) does not care whether you include the cstdint header IN THIS FILE. The only thing that matters is that after the preprocessor is done, the definition of uint32_t is somewhere above its usage.

To see how this works in our case we can take a look at the first few lines of VulkanPBRT.cpp:

#include <iostream>
#include <set>
#include "Defines.hpp"

Somewhere down the line both of the standard library headers include cstdint. This means that after preprocessing, uint32_t is defined before its usage in Defines.hpp. And thats why the compiler sees no problem here.

However if we for some reason removed the standard library headers here, it would break. Or if you try to move some code from .hpp to .cpp files like I did. And probably any other bigger refactor of the code.

But there is a way we can avoid such a fragile construction by ordering our include directives from the most specific headers at the top to the most general headers at the bottom:

#include <RatherSpecificCustomHeaders>
...
#include <RatherGeneralCustomHeaders>
...
#include <HeadersOfOtherLibraries>
...
#include <StdLibHeaders>

[Lachei]

Yes we can definitely impose such an ordering. Just as a funny side note: On linux the uint data type is actually defined somewhere in the hardware specific window creation headers, the using was added afterwards to be able to port the program without much change to windows. Preferably we should get rid of the uint's and exchange them with the "normal" uint32_t type as this is the one normally used.

I am going to test the Draft in the afternoon and am going to exchange all uint's.