GLM_FORCE_CTOR_INIT needed for preprocessor macros

[neilmendoza]

I just updated to the latest version of OF and commit a9e962c is giving me some strange issues. In the example for ofxTinynurbs, if I add...

glm::vec3 a, b;
glm::vec3 c = 0.5f * (a + b);

...at the start of ofApp::setup(), the data in ofxTinynurbs gets corrupted with OF commit a9e962c or later (where GLM_FORCE_CTOR_INIT is added).

[ofTheo]

hmm.
#define GLM_FORCE_CTOR_INIT

Is meant to set a and b to 0,0,0.

can you cout a and b and share?
both with GLM_FORCE_CTOR_INIT defined and commented out?

I am assuming you have the glm libs updated from apothecary or from the latest release?

[bakercp]

So, I ran into this problem and it took me several days to figure it out.

I found that it happened in my addons where I was including glm headers without including ofConstants.h (where GLM_FORCE_CTOR_INIT is defined) immediately before those includes. Further, I found that depending on the compile state, that wasn't a reliable reliable solution and I had to include #include "ofVectorMath.h" in all addons where glm was used directly.

Anyway, it was a huge headache and I'd strongly advocate that we not define GLM_FORCE_CTOR_INIT in OF, but rather make sure we manually initialize glm elements within the core and document that addons need to do the same as modern glm requires.

This is all very odd because I usually include ofMain.h at the beginning of my ofApp.h or main.cpp before anything else. ofMain.h includes ofVectorMath.h which includes ofConstants.h etc. so it's not clear to me where the inclusion chain broke down, but anyway the solution was to not include glm elements myself without first including ofVectorMath.h.

[ofTheo]

Initially I thought we shouldn't include GLM_FORCE_CTOR_INIT. But in the OF core it took forever to track down and fix bugs related to quats and vecs not being set. I think it could bring a bunch of misery to people's projects and break addons ( which are 0.11.0 compatible but aren't being updated regularly ) to not have that include.

At the same time I understand that you'll end up with two types of glm code - some which assumes auto initialization and some which doesn't. I am not sure if there is a graceful way to transition from it ( like deprecation )? Maybe there is?

The code @neilmendoza posted above wouldn't be valid without it:

glm::vec3 a, b;
glm::vec3 c = 0.5f * (a + b);

[neilmendoza]

It also took me all day to track down this bug. That code actually has a and b initialized in the project I'm working on that I copied it from. I don't care about the values of a, b and c being garbage though, that's just the minimal code I could give as an example that is causing memory corruptions elsewhere (in the tinynurbs library in this case) when GLM_FORCE_CTOR_INIT is defined. Makes sense that is might be due to different defines for glm in different files based on includes. Will have a look later.

[bakercp]

Yeah, I understand it's a tough call. I think you probably made the right one for the sake of addons. We should probably document it more clearly somewhere in a document of "known issues" or something.

…

On Tue, Jan 28, 2020, 6:41 PM Neil Mendoza ***@***.***> wrote: It also took me all day to track down this bug. That code actually has *a* and *b* initialized in the project I'm working on that I copied it from. I don't care about the values of *a*, *b* and *c* being garbage though, that's just the minimal code I could give as an example that is causing memory corruptions elsewhere (in the tinylib library in this case) when GLM_FORCE_CTOR_INIT is defined. Makes sense that is might be due to different defines for glm in different files based on includes. Will have a look later. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#6530?email_source=notifications&email_token=AACJLRDKHMEI6CVBKUYJ62LRADGFLA5CNFSM4KM3F3D2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKFRRYQ#issuecomment-579541218>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACJLREFHMJOCPJG2J7HR3TRADGFLANCNFSM4KM3F3DQ> .

[neilmendoza]

Yeah, agree it's a tough choice. Seems like this opens the door to some pretty gnarly bugs if you can't use anything that depends on glm without including the OF headers in the correct place. Would be nicer if GLM_FORCE_CTOR_INIT could be defined in the project rather than an include file.

[arturoc]

Adding this to projects seems the best solution,

[danomatika]

This bit ofxLua for a while. I didn't know what was going on until someone pointed out the glm define. The main thing it seemed to effect was causing a runtime crash only in Debug builds: danomatika/ofxLua#58

[neilmendoza]

I just came across this memory corruption issue again and wasted another few hours. It would really be much better to get the project generator to include this as a preprocessor macro. If not, you end up with memory corruptions every time you include a glm header directly without ofConstants.h first. Seems like the people on this thread that tracked down why their code is misbehaving are all pretty experienced OF users. I'm guessing there are a lot of other people that never manage to figure out why their code randomly does unexpected things.

[dimitre]

Nice @neilmendoza I think it is a good idea. what do you think @ofTheo?
Which projectGenerator function would help with that?
I suppose it is addDefine()
at least I can see this adding GCC_PREPROCESSOR_DEFINITIONS to XCode project.

[ofTheo]

Yeah, totally agree. Honestly feel it shouldn't almost be a patch to GLM or part of apothecary but I'd be down to adding it to pre processor macros for all platforms.

[NickHardeman]

Working on adding this to preprocessor definitions. I noticed that GLM_ENABLE_EXPERIMENTAL is also defined a the top of a lot of files. Should that also get added to the preprocessor definitions?
For example:

openFrameworks/libs/openFrameworks/3d/ofCamera.cpp

Line 6 in cc177eb

#define GLM_ENABLE_EXPERIMENTAL

@ofTheo @dimitre

[ofTheo]

Yes I think so!
Thanks @NickHardeman

[dimitre]

What I did in my fork was totally remove GLM_FORCE_CTOR_INIT by initializing all mat4 by { 1.0f }
and left experimental only in one file.

It is a way of avoid GLM including files it won't use or doing unnecessary operations.

[ofTheo]

@dimitre we need GLM_FORCE_CTOR_INIT for 0.12.1 and possibly beyond.

Too many addons will break and the ways that they are break are really subtle and hard to track down.

The small benefit is not worth the collective time that will be wasted across projects.
( been through a couple of rounds of this over the years :) )

[dimitre]

@ofTheo ok
those are the only places needing GLM_ENABLE_EXPERIMENTAL to have the include <glm/gtx/vector_angle.hpp> available

[ofTheo]

Thanks @dimitre

Also, after 0.12.1 we can do a good check of the OF core and make sure all glm variable inside OF are initialized and just have
GLM_FORCE_CTOR_INIT as a project define.

We'll over engineer it for now with Nick's PR and then do a more thorough cleanup post 0.12.1

[NickHardeman]

Closed by #8352