-
-
Notifications
You must be signed in to change notification settings - Fork 238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove implementation files, make same API for inlining and shared library #135
base: master
Are you sure you want to change the base?
Conversation
- Remove all include/cglm/call.h, include/cglm/call/*.h and src/*.c files, while keeping option to compile and use CGLM as a library. - Have ONE API ONLY, and switch between compiling it inline or using library by just defining CGLM_LIB. - Simplify maintenance and reduce number of files. Details: - Build library by compiling src/cglm.c. This defines CGLM_DLL - Use library by defining -DCGLM_LIB and link with created lib (dll or so). Main task was done using the regular expressions in Notepad++ below. Only first function in io.h done manually. Logic added/improved in include/cglm/common.h find 1: CGLM_INLINE repl 1: CGLM_DECL find 2: \) \{\r\n(#if) repl 2: \) CGLM_ENDD\r\n#ifndef CGLM_LIB\r\n\{\r\n$1 find 3: \) \{\r\n( [^ ]) repl 3: \) CGLM_ENDD\r\n#ifndef CGLM_LIB\r\n\{\r\n$1 find 4: ^\}\r\n\r\n repl 4: \}\r\n#endif\r\n\r\n
Hi @tylo-work Thank you for contributing to cglm 🤗 I don't know, this is a GOOD IDEA but brings a HUGE change to the design and code base and conflicts with the idea that provide both inline and compile version. For instance users can use both I don't know, let's get more reviews, more feedbacks here. |
No problem, I see the issues that you mention, yes it does remove existing API, and you can no longer pick which function to call from library.
Ideally though, one should not need to pick and choose functions to be called from library vs inline, so “small” functions should always be defined inline, and the longer should be library functions when you choose this configuration.
I have started creating my own personal smaller c-library (using some of your code and c++ glm), so I can make it perfect for my needs.
Btw, I have a suggestion for new swizzle functions. Speed should be around as fast as yours, but this is more user friendly and flexible, imo:
CGLM_INLINE void
glm_swizzle4(vec4 v, const char *swz, vec4 dest) {
dest[0] = v[(swz[0] - 'x') & 3];
dest[1] = v[(swz[1] - 'x') & 3];
dest[2] = v[(swz[2] - 'x') & 3];
dest[3] = v[(swz[3] - 'x') & 3];
}
CGLM_INLINE void
glm_swizzle3(vec4 v, const char *swz, vec3 dest) {
dest[0] = v[(swz[0] - 'x') & 3];
dest[1] = v[(swz[1] - 'x') & 3];
dest[2] = v[(swz[2] - 'x') & 3];
}
CGLM_INLINE void
glm_swizzle2(vec4 v, const char *swz, vec2 dest) {
dest[0] = v[(swz[0] - 'x') & 3];
dest[1] = v[(swz[1] - 'x') & 3];
}
// Examples:
vec4 v1 = {1, 2, 3, 4}, v2;
vec3 u1, u2;
vec2 p;
glm_swizzle4(v1, “wxyy”, v2);
glm_swizzle3(v2, “xyw”, u1);
glm_swizzle3(u1, “zyx”, u2);
glm_swizzle2(u2, “zx”, p);
glm_swizzle4(p, “xxyy”, v2);
Ps: Your following defines are wrong, should be (3, 2, 1, 0), although you won’t need them much if you use my swizzle functions 😉
#define GLM_WZYX GLM_SHUFFLE4(0, 1, 2, 3)
#define GLM_ZYX GLM_SHUFFLE3(0, 1, 2)
Cheers,
Tyge Løvset
Seniorforsker
Senior Researcher
+47 932 67 779
Fantoftvegen 38, 5072 Bergen, Norway
[cid:norce_426cc920-64ab-4893-95d0-c29d5dab670c.png]
NORCE Norwegian Research Centre AS
norceresearch.no<https://www.norceresearch.no/>
From: Recep Aslantas <[email protected]>
Sent: Monday, April 13, 2020 20:26
To: recp/cglm <[email protected]>
Cc: Tyge Løvset <[email protected]>; Mention <[email protected]>
Subject: Re: [recp/cglm] Remove implementation files, make same API for inlining and shared library (#135)
Hi @tylo-work<https://github.com/tylo-work>
Thank you for contributing to cglm 🤗
…________________________________
I don't know, this is a GOOD IDEA but brings a HUGE change to the design and code base and conflicts with the idea that provide both inline and compile version.
For instance users can use both glm_mat4_mul() (inline) and glmc_mat4_mul() (library-call) at the same time. glm_ is always inline (tries to be) and glmc_ is always library-called / compiled version. So there is no confusing.
I don't know, let's get more reviews, more feedbacks here.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#135 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AOLYYGX3X4B5GZKFUP5X5V3RMNKJ3ANCNFSM4MGBTBGQ>.
|
I think this PR must stay open if you like to get more feedbacks from others. In the future we may want to bring this changes... Feel free to contribute cglm for other bug fixes and features... ;) Some places may require some functions like
this is interesting approach. I must learn how this My goal was to use SIMD here but I couldn't :'(, that would be fastest one.
I think I have written tests for them. Nothing may be wrong, you may want to check |
Details:
FIXES/changes in include/cglm/common.h:
Note: It should be trivial to extend common.h so that you could e.g. define CGLM_STATIC_LIB as a third compilation option in addition to inline and shared library.
Tested on windows gcc (tdm64) 9.2, Visual studio 2019, Tiny C.
Code changes were done in Notepad++ using the regular expressions below (on all open files). Only the first function in io.h and the tests needed to be edited manually.
find 1: CGLM_INLINE
repl 1: CGLM_DECL
find 2: ) {\r\n(#if)
repl 2: ) CGLM_ENDD\r\n#ifndef CGLM_LIB\r\n{\r\n$1
find 3: ) {\r\n( [^ ])
repl 3: ) CGLM_ENDD\r\n#ifndef CGLM_LIB\r\n{\r\n$1
find 4: ^}\r\n\r\n
repl 4: }\r\n#endif\r\n\r\n