-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
[Idea] - Remove legacy math from Core #8178
base: master
Are you sure you want to change the base?
Conversation
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 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.
Implicit conversion is actually nice and useful, what problems do you see with it?
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: |
@roymacdonald This PR should not change any functionality. (it needs testing) |
I know what you did, I checked your commits, and while seems like a tempting idea, it really needs to be checked and tested. |
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:
Thoughts? Ideas? cc @ofTheo @NickHardeman