LukasBanana / LLGL

Low Level Graphics Library (LLGL) is a thin abstraction layer for the modern graphics APIs OpenGL, Direct3D, Vulkan, and Metal
BSD 3-Clause "New" or "Revised" License
2.03k stars 135 forks source link

Allow for aggregate initialization in descriptors #84

Open IyadHamid opened 1 year ago

IyadHamid commented 1 year ago

I noticed that there are a lot of structures with unnecessary constructors which just emulate aggregate initialization. Would it be possible to remove the various constructors/assignment operators to allow for aggregate initialization. This would also allow for C++20's designated initialization if compiling with C++20.

LukasBanana commented 1 year ago

It's true there are several constructors to emulate aggregate initialization, but the reason is that aggregate initialization does not work in conjunction with default initializers, at least not in C++11. According to cppreference.com, a struct with default member initializers is not an aggregate until C++14: https://en.cppreference.com/w/cpp/language/aggregate_initialization

The idea for default initializers in the public interfaces is purely for stability. Are there any particular structs that bother you to have such constructors or are there any structs missing such constructors?

Perhaps utilizing C++14 feature __cpp_aggregate_nsdmi is worth considering. On the other hand, though, explicit constructors allow to specify how many members should be initialized at once, for instance Viewport v = { x, y, width } with a default value for height would make no sense, but with a constructor you can declare that all four members have to be initialized at once.

Having said that, some structs would definitely benefit from C++20's designed initialization (a feature that exists since C99 btw), like ClearValue or TextureDescriptor for instance.

IyadHamid commented 1 year ago

Ah, I didn't realize that default member initialization prevented it being an aggregate until C++14. In my project I am using C++20 to see how the modern features would play with low level graphics. Luckily most of the large descriptors (e.g. GraphicsPipelineDescriptor) are considered aggregates and allow for greater readability if I am constructing the descriptor within the function call.

The descriptors I noted not having aggregation in C++20 were ResourceHeapDescriptor, BindingDescriptor, and ShaderDescriptor; although none of them were much of a bother.

Using explicit constructors to specify how many members should be initialized at once is a good idea, however, to keep with aggregate initialization with this constraint, you could make helper utility functions to construct it so designated initializers can still be made to whoever wants to (much like VertexBufferDesc and similar).

Overall I think the library is great and thanks for making it.

LukasBanana commented 1 year ago

Thank you for your feedback. VertexBufferDesc is one of the utiliy functions, but perhaps it's not a bad idea to switch to more of those instead of holding aggregate initialization for C++20 compilers back. I might also consider just bumping up the minimum C++ version to C++14 at some point.