Monomorphized Vertex Layout

[Cons-Cat]

One issue I've personally had with using Liblava is that, while it provides such great abstractions for presenting vertices, they're only available if you stick to using the provided lava::vertex. That's a reasonable vertex layout, but I think practically any graphics project that isn't absolutely trivial needs a different vertex layout at some point.

My proposal is to introduce a placeholder templated struct named generic_mesh<> (and generic_mesh_data<>) for expressing the same behaviors. These take default template arguments that are equivalent to using mesh and mesh_data, so factoring non-generic code into generic code should be nearly frictionless. But these generic versions are capable of expressing more.

I say placeholder, because in my opinion there is too much code duplication introduced by this, and since generic meshes are intended to be as easy to use as the current meshes, they could just replace the non-generic structs entirely.

I made a new demo named generic triangle, and added GLSL shaders for it in the res folder. This presents 3 triangles: a triangle using lava::vertex, a triangle using a custom vertex with 3 ints for position, and a triangle using a custom vertex with 3 doubles for position.

These can each call the move and scale functions, which I tried to make generic and ergonomic. Originally I tried doing this with preprocessor macros, but it got insanely complex quickly, so it's templates.

I'm also thinking this can open the door to a constexpr abstraction over graphics_pipeline::set_vertex_attributes. There could be a new templated method that takes only the byte offsets and struct fields of data that you want to send into GPU, and it could compile-time expand into complete instances of VkVertexInputAttributeDescriptions.

This is a draft because there is still a bunch to do, has not been thoroughly tested, the code is messy etc. and I wanted to discuss what you think if it's an okay idea. Is this a reasonable approach to the feature? How do you want to proceed?

[TheLavaBlock]

Interesting approach. I also like your triangle demo - much better than the current 🔺

A generic solution for handling other vertex layouts makes a lot of sense.

I'll have a closer look at your adjustments in the coming days and will try to adapt it.

We can discuss it here together. Maybe the others have an opinion about it as well.

[Cons-Cat]

Building Custom Rule C:/projects/liblava/CMakeLists.txt generic_triangle.cpp C:\projects\liblava\liblava-demo\generic_triangle.cpp(156,1): fatal error C1001: Internal compiler error. [C:\projects\liblava\build\lava-generic-triangle.vcxproj] (compiler file 'D:\agent\_work\13\s\src\vctools\Compiler\CxxFE\sl\p1\c\ParseTreeActions.cpp', line 6592) To work around this problem, try simplifying or changing the program near the locations listed above.

To be honest, I don't really know what to do about this CI error. The specific source code line that errors (generic_triangle.cpp(156,1)) appears be this:

    };

And the error message doesn't tell me anything that I know how to work with. This seems to compile fine for me with GCC 11 on Arch Linux. I can try building this on a Windows 10 system later and maybe get a better idea what's going on, but for now I'm very confused.

[Cons-Cat]

Otherwise, I've improved the code a lot in the past day, so I think it can be scrutinized now. My immediate priority regarding this pr is extending generic_create_mesh() to be capable of generating primitives other than just triangles.

I'm not sure what should be done after that. I need to add more Doxygen comments, but as far as features go, I'm not sure whether the generic meshes will be missing anything that non-generic meshes have. Maybe I should make the OBJ loader build generic vertex meshes? Do you know what else this is still missing?

[TheLavaBlock]

And the error message doesn't tell me anything that I know how to work with. This seems to compile fine for me with GCC 11 on Arch Linux. I can try building this on a Windows 10 system later and maybe get a better idea what's going on, but for now I'm very confused.

I also tried to build it locally and changed the complexity of the method a bit, only after removing 2 pipelines (int and double, completely!) the error disappeared - really a pity that it is not clear exactly where the problem comes from. Perhaps we will find the cause of this while refactoring.

[TheLavaBlock]

I'm not sure whether the generic meshes will be missing anything that non-generic meshes have. Maybe I should make the OBJ loader build generic vertex meshes? Do you know what else this is still missing?

I would suggest that we first provide mesh and generic_mesh at the same time. So that we gradually support both, if it makes sense. And then finally convert the old mesh into a generic one.

The best thing would be if the users don't notice this change/enhancement at all (or as little as possible). The OBJ loader also has to be switched over at a later stage.

[TheLavaBlock]

Generally a good approach with the generic_mesh. Thanks for your effort. I appreciate very much.

[Cons-Cat]

I feel like this PR is now basically done, except that I'm not sure what the best way to fix the Windows build is. I'm thinking maybe I can have CMake set a #define (or an if constexpr value) that skips compiling the offending code if using the Visual Studio generator? I specify the generator rather than operating system, because theoretically I think MinGW should still work, but I haven't tested. I'm almost certain this is a bug in MSVC / MS Build, and I feel like the use of this C++20 requires it doesn't like is a very pleasant solution to making create_mesh() generate generic primitives.

My idea is that on Visual Studio, until Microsoft fixes this, create_mesh() can force programmers to manually initialize their primitives if they aren't using lava::vertex. This should be a forwards-compatible solution. It would look something like this:

mesh_template<my_struct>::ptr my_mesh = create_mesh<my_mesh>(device, mesh_type::triangle);
for (auto& vert : my_mesh->get_vertices()) {
    // Colors not automatically generated.
    vert.color = std::array<3, float>{ 1, 1, 1 };
}

And when Microsoft fixes this issue with the non-preview release of C++20 (presumably), this code could be removed and all projects should still work.

What do you think?

[Cons-Cat]

Part of why CI still fails is that MSVC is also failing to compile a C++17 feature that I used to fix the broken C++20 feature, haha. I'm starting to hate this compiler.

[Cons-Cat]

I've been really stumped on this problem! Through a combination of minimal pre-processor macros and a C++ proposal that has been accepted for C++23, I would be able to make a reasonably elegant hack (doing everything at runtime in MSVC instead of compile time), but as it stands all I can think to do now would be writing some ridiculously unreadable macros to make that transformation possible.

CI passes because I have effectively disabled create_mesh from generating anything except position data when using MSVC, regardless of using lava::vertex or not. That's a regression, so not a good solution imo. I'll keep thinking on the problem.

[Cons-Cat]

I've got a solution now. On MSVC compilation, additional template parameters are required to specify that the inputted struct does not contain the fields color, normal, and/or uv. This is backwards compatible, and it is somewhat forwards-compatible. If MSVC improves, this function can be a deprecated call into another function that has a simpler template parameter list.

[TheLavaBlock]

Alright, I have made a few adjustments...

Thanks for your work - A lot of effort has already gone into it! 👍

I'm checking out the next days, how we can possibly improve it, so that we don't have something like this:

create_mesh<int_vertex, false, true, false, true, false, false>(app.device, mesh_type::triangle);

(maybe we can get around the MSVC problem)

🔍 If anyone has any suggestions, please feel free to contribute.

---

As said, we will continue with the demos in the next step. I already have some 💡 ideas that we could try out

[Cons-Cat]

I'm checking out the next days, how we can possibly improve it, so that we don't have something like this:

create_mesh<int_vertex, false, true, false, true, false, false>(app.device, mesh_type::triangle);

(maybe we can get around the MSVC problem)

The Microsoft bug report I linked is, as of today, "Under Investigation". If we wait a bit, it's possible they'll fix it soon.

[TheLavaBlock]

You are right. Let's wait and see. If there are any changes, we can always refer to #59

But first of all the merge 🔨