search

recp / cglm

📽 Highly Optimized 2D / 3D Graphics Math (glm) for C
MIT License
2.34k stars 231 forks source link

Introduce `box`/`boxs` types instead of passing an array of two vec3(s) around #359

Open v1993 opened 12 months ago

v1993 commented 12 months ago

While a typedef vec3[2] box would already be nice to have in array API since it's more explicit about meaning of a value, struct API would benefit from this change the most. Currently, due to lack of a dedicated structure, functions that return a box utilize an out parameter instead of actually returning the struct, e.g.

https://github.com/recp/cglm/blob/1fdc1c86757f50438434528ecddd62e41940e41d/include/cglm/struct/box.h#L49-L51

This largely runs contrary to the whole idea of struct API, which tries to avoid out parameters and instead return results.

By introducing a boxs struct this could be changed to far more natural

CGLM_INLINE
boxs
glms_aabb_(merge)(boxs box1, boxs box2)

However, unlike typedef in array API, this would be a breaking change; thus I'm unsure what's the best course of action here.

P.S.: I suppose types could be called aabb/aabbs instead to avoid clashing with more generic box term and match type function prefixes as well.

recp commented 12 months ago

Hi @v1993,

You are right, struct api could return a type but I'm not sure for bringing new type since engines may have an AABB type internally, introducing a new one could lead to conflicts with existing typenames

Still, we can consider adding a new type, such as 'aabb' or 'glm_aabb_t', maybe just for struct api. Let's get more feedback to see if there are any concerns or suggestions.

v1993 commented 12 months ago

I think that it would be good to have it in both APIs, at very least for consistency. Not quite sure about type name conflicts - cglm already defines its types without any prefixes, so I'd probably go for consistent, if somewhat conflict-inducing, naming scheme.

If you're worried about possible conflicts, then it might be a good idea to "scope" all types, e.g. name them glm_<name>_t with an (most likely enabled by default) option to typedef them to unscoped names. That way, users who prefer a little extra verbosity over possibilty of a conflict can disable shorthand names and use full ones.

That said, I'm just voicing my thoughs here - I'm actually working on Vala bindings for cglm (struct API turned out to be incredible for this use case), so C names don't really matter for me - only function signatures do.

duarm commented 12 months ago

One of the things I like about cglm is that it doesn't prefix types, it's too much verbosity for the sake of avoiding a possible conflict, and everyone pays the price. It should be optional if it's a concern.