openframeworks / openFrameworks

openFrameworks is a community-developed cross platform toolkit for creative coding in C++.
http://openframeworks.cc
Other
9.98k stars 2.55k forks source link

[Idea] - Remove legacy math from Core #8178

Open dimitre opened 2 weeks ago

dimitre commented 2 weeks ago

This is a proposition for discussion. I was testing removing all legacy math (ofPoint, ofVecXf, ofMatrixXxX, ofQuaternion) from core, addons and examples. Now I have this updated PR where we can opt out from legacy math if desired.

I think today some interoperability between glm::vec2 -> glm::vec3 is achieved through some indirect mean like glm::vec2 -> ofVec2f -> ofVec3f -> glm::vec3 which is not ideal.

I think we should not use implicit conversion in OF Core, but if we need we can add operators to glm itself like this:

//--------------------------------------------------------------
inline glm::vec3 operator+(const glm::vec3 & v1, const glm::vec4 & v2){
    return v1 + glm::vec3(v2.x, v2.y, v2.z);
}

// addons/ofxGui/src/ofxButton.cpp:58:65
//--------------------------------------------------------------
inline glm::vec2 operator+(const glm::vec2 & v1, const glm::vec3 & v2){
    return v1 + glm::vec2(v2.x, v2.y);
}

Thoughts? Ideas? cc @ofTheo @NickHardeman

dimitre commented 2 weeks ago

related:

roymacdonald commented 2 weeks ago

As far as I can recall, there was a lot of work and though put into how to fully go into using glm only but not breaking projects using OF vec classes. The solution at which we got is quite nice and it works (thus, I dont see the need to remove at all if it is not bothering). Having #ifdef OF_USE_LEGACY_MATH is nice, but I would leave it enabled by default and only if someone really knew what they are doing to disable. I tend to work with old projects and it really becomes painful and annoying when things dont work as they used to.

As for the examples, all those should have been converted to use glm long ago and I dont see any issue on doing so. (I suggest making a different PR just with the updated examples). But you need to test these and compare, visually that the behavior is the same. I remember seeing odd stuff, like with matrix decomposition.

I would keep using the toOf and toGlm functions within the core in order to enforce their usage and still keep compatibility, which essentially implies dont change much.

I think we should not use implicit conversion in OF Core,

Implicit conversion is actually nice and useful, what problems do you see with it?

I think today some interoperability between glm::vec2 -> glm::vec3 is achieved through some indirect mean like glm::vec2 -> ofVec2f -> ofVec3f -> glm::vec3 which is not ideal.

if it is possible to remove this, and make it direct it would be great.

As of the addons. I think it is ok to update their internal use but I would not change their signatures (public and protected members and methods) as that can potentially break projects. I would change those only using OF_USE_LEGACY_MATH is undefined.

This is a BIG change which I really think it needs to be taken with calm. there is no need to rush. there is no need to break what is already working.

Last but not least: Please, make a new branch for this, it makes it a lot easier for other to check. And dont merge to master until agreed to do so.

dimitre commented 2 weeks ago

@roymacdonald This PR should not change any functionality. (it needs testing) what I did was finish porting all Core functions and examples to glm and having the option of not using "legacy math"

roymacdonald commented 2 weeks ago

I know what you did, I checked your commits, and while seems like a tempting idea, it really needs to be checked and tested.