Skip to content

ofVectorMath -> glm, work in progress#4943

Closed
arturoc wants to merge 69 commits intoopenframeworks:masterfrom
arturoc:refactor-glm
Closed

ofVectorMath -> glm, work in progress#4943
arturoc wants to merge 69 commits intoopenframeworks:masterfrom
arturoc:refactor-glm

Conversation

@arturoc
Copy link
Member

@arturoc arturoc commented Feb 24, 2016

By now i've only ported ofVec2f -> glm::vec2. and added toOf and toGlm functions so it's easy to convert from one another

The things that will be most problematic:

  • glm works with radians instead of degrees, once everything is ported should work the same but if you mix things it produces subtle errors that are difficult to detect.
  • vectors don't have methods, everything is done through functions so:
ofVec2f v1, v2;
auto angle = v1.angle(v2);

now becomes:

glm::vec2 v1, v2;
auto angle = glm::angle(v1,v2);
  • there's subtle differences in some methods like our angle is glm's orientedAngle so the above should really be:
glm::vec2 v1, v2;
auto angle = glm::orientedAngle(v1,v2);
  • glm::vec*::length() is the only method for vectors but it's not the actual length but the dimension so for vec2 it'll always return 2. it's kind of easy to spot cause everything else is functions instead of methods but it's also something that will fail on run time and will be difficult to detect

@arturoc
Copy link
Member Author

arturoc commented Feb 24, 2016

Also do we want to keep the glm namespace? other posibilities would be:

  • reimport the classes as glm::vec2 -> ofVec2 glm::vec3 -> ofVec3 but that would be really confusing
  • reexport everything in glm to of:: and start having some things in our namespace. we could actually do the same for other libraries, like json, or boost::filesystem which right now are ofJson and std::filesystem

@ofTheo
Copy link
Member

ofTheo commented Feb 24, 2016

@arturoc -I actually assumed the plan here was to just use glm for the internals of ofVec2f and ofVec3f and the rest of the math stuff - but still keep the public facing API the same.

I think deprecating ofVec3f / ofVec2f would be a huge change and break almost every existing addon once the deprecation transitions to a complete removal.

If glm::vec2 / vec3 etc doesn't have methods - could ofVec2f / ofVec3f just extend them and then keep the methods that we currently have but use the glm functions internally?

I guess we could internally switch to using glm functions for core and examples - but I feel like we would need to keep backwards compatibility around for a really long time - so would it be worth it?

Another downside to using glm directly instead of ofVec3f / ofVec2f is that if we ever wanted to switch from GLM to another maths lib ( for some unknown reason ), it would break backwards compatibility in a big way.

I feel like the strength with OF vs Cinder is that for the most part it abstracts out the particular library being used, making swapping out APIs a fairly clean process ( renderer, videoplayer, videograbber, etc ).

So I guess I would be in favor of keeping ofVec2f/ofVec3f etc as is - using GLM internally and add the toGLM and toOF calls you described ( if needed ), so that people could easily work with GLM directly if needed.

@arturoc
Copy link
Member Author

arturoc commented Feb 24, 2016

My idea is to use glm all over the core but keep deprecated methods for anything that accepts ofVec_f or have ofVec_f auto convert to glm. I think it's doable to have an api that won't break addons or projects using ofVec and still use glm

I don't think wrapping the glm classes in ofVec is a good idea the main reason to swtich to glm is that it's become the defacto standard for graphics math so using it directly is almost equivalent to using std::vector in graphcis right now, you'll find a lots of examples on how to things using glm outside of the OF realm so it will be easier to adapt the code.

Another good reason to not wrap glm is that right now we have multiplication order like:

vec * mat

while the glsl standard and most books, documentation.... use:

mat * vec

if we wrapped glm into ofVec and switch the multiplication order it would break code in a really nasty way where it would be compile but it won't work and would be really difficult to fix.

The main advantage of glm is it's api and that it's become a standard, if we wrap it into something else i don't think there's any advantage of switching to it. the current classes work fine and are reasonably fast.

@charlesveasey
Copy link
Contributor

yes, best not to wrap. GLM has a clean api and is similar to GLSL,
simplifying the graphics math interface substantially. Also makes porting
code to oF much easier.

On Wed, Feb 24, 2016 at 9:14 AM, arturo notifications@github.com wrote:

My idea is to use glm all over the core but keep deprecated methods for
anything that accepts ofVec_f or have ofVec_f auto convert to glm. I
think it's doable to have an api that won't break addons or projects using
ofVec and still use glm

I don't think wrapping the glm classes in ofVec is a good idea the main
reason to swtich to glm is that it's become the defacto standard for
graphics math so using it directly is almost equivalent to using
std::vector in graphcis right now, you'll find a lots of examples on how to
things using glm outside of the OF realm so it will be easier to adapt the
code.

Another good reason to not wrap glm is that right now we have
multiplication order like:

vec * mat

while the glsl standard and most books, documentation.... use:

mat * vec

if we wrapped glm into ofVec and switch the multiplication order it would
break code in a really nasty way where it would be compile but it won't
work and would be really difficult to fix.

The main advantage of glm is it's api and that it's become a standard, if
we wrap it into something else i don't think there's any advantage of
switching to it. the current classes work fine and are reasonably fast.


Reply to this email directly or view it on GitHub
#4943 (comment)
.

@arturoc
Copy link
Member Author

arturoc commented Feb 25, 2016

Some more findings:

  • Adding constructors and cast operators to ofVec* allows for code like the following to keep working:
ofVec3f v = mesh.getVertex(i);
ofMatrix4x4 mat;
v = v * mat;
mesh.setVertex(i, v);
  • Adding some arithmetic operators also allows for code like the following to keep working:
ofVec3f offset(0,10,0);
mesh.getVertices() += offset;

That alone already covers a lot of cases, right now all of the the core in this branch is using glm::vec2 and vec3 instead of ofVec2f and ofVec3f and the 3DPrimitives example as well as others works without problem.

There's still some things that don't work like:

ofMesh mesh;
auto length2 = mesh.getVertex(i).squaredLength()

or anything similar where we call ofVec methods on return values now fails to compile. what's even worse:

ofMesh mesh;
auto length2 = mesh.getVertex(i).length()

still works but the vale is not correct cause that method in glm::vec3 always returns 3, the dimension.

Also something like:

std::vector<ofVec3f> vertices;
mesh.setVertices(vertices);

will fail to compile. this can be solved by adding a deprecated method to ofMesh that accepts vector of ofVec3f and transforms it into glm::vec3 internally which would slow down things slightly but it should be ok

but then something like:

std::vector<ofVec3f> vertices = mesh.getVertices();

can't be fixed in a similar way, we could have getOfVertices that would allocate a vector and make a copy but that would be really slow and still fail to compile.

a possible solution for this is to have a templated ofMesh, that way we could define which type it uses for vertices, normals, colors and texture coordinates like:

template<class V, class N, class C, class T>
class ofMesh_

#ifdef OF_LEGACY_MESH
using ofMesh = ofMesh_<ofVec3f, ofVec3f, ofFloatColor, ofVec2f>
#else
using ofMesh = ofMesh_<glm::vec3, glm::vec3, ofFloatColor, glm::vec2>
#endif

That will allow old projects to work right away for the 2 cases above just by changing a constant in ofConstants or even in the project config. it will require to recompile the core when you change from one another but i think it should be ok.

This will also propagate to other classes so you can also use ofVboMesh, ofVbo, of3dPrimitive in the same way.

Apart from compatibility for legacy projects this will also have other interesting usages like:

ofMesh_<glm::vec3, glm::vec3, ofColor, glm::vec4> meshWith4texcoords;

will create a mesh with 4 dimensional texture coordinates and byte colors instead of float which can be handy in some situations

@ofTheo
Copy link
Member

ofTheo commented Feb 25, 2016

@arturoc thanks for looking into what the changes would look like.

I think this one was the one I was most worried about:

std::vector<ofVec3f> vertices = mesh.getVertices();

I think that this method is used a ton across OF projects and addons too.

Playing devil's advocate for a second here:

I am curious though what are the main things in ofVec2f and ofVec3f that are really holding OF back?
If there are small bugs wouldn't it be easier to simply fix them? If they are missing some key functionality wouldn't it be simpler to add to them?

For beginners I think the current vector math approach would be a lot more friendly and easy to get into.
With ofVec3f etc you can easily see what operations you have access to via the class methods. If we brought glm into the core and included the namespace you would have no easy way to know via code completion what is available.

It would also be good to know what else we gain from GLM that we don't currently have. Looking through the api I don't see a ton of things that OF is missing.

@arturoc
Copy link
Member Author

arturoc commented Feb 25, 2016

No, there's not a really huge difference, it's mostly more similar to glsl math so when you write code in the c++ and the glsl side it looks the same, multiplication order is the same... the problem with multiplication order from people reading some other documentation, opengl books.... is pretty common in the forum and the answer is something like "OF math classes are weird, sorry"

Then it has some really nice features like swizzles so you can call v.xy() to get the first 2 coordinates of a vec3 for example. We'd also get a proper mat3 which we lack now.

The other main reason is that maintaining a math library is hard, we've got the original implementations from other places and although they are pretty ok nobody maintains them anymore because we broke with the originals by renaming the classes to of... instead of just using the original classes.

Bringing support for what is right now the defacto standard seems only logic. There's been an OF addon for glm almost since it came out but it's never been very useful cause the core still used ofVec.

I've been testing more today and i have something that seems to have 100% compatibility with old projects through the macro i described above. It requires a templated ofMesh and ofPolyline but it doesn't seem a big deal and introduces some flexibility on those classes that can be useful for other stuff, like having 4d texture coordinates for projective transformations, or 2d polylines for performance

One possibility would be to still default to ofVec by now while giving the possibility to use glm for people who want to do so, (they would just need to enable this macro in ofConstants to have full support) and all the internal math in OF would be done through glm so things should be slightly faster.

@tgfrerer
Copy link
Member

I think switching over to glm would be a great!

I totally get that the switch from our current way or vector maths feels painful, but i'd like to propose that in the long run, it could be very well worth it.

I think moving to GLM will save us from technical debt, it will bring our maths more in line with generally followed conventions, and this in the end, will make it more easier (and rewarding) to learn for beginners.

In the last couple of years, I've been seeing glm being more and more adopted as a de-facto standard, like @arturoc mentioned. The recent Vulkan SDK for example, uses it a lot, it's also used in many GL samples and tutorials.

With glm we'd have an API that is stable, and built to match GLSL, which is something that makes the move from CPU code to GPU code easier.

It's also heavily optimised to take advantage of underlaying CPU infrastructure, something that potentially could make a difference on Raspberry Pi and other weaker platforms, where doing the matrix math is where a lot of CPU time is spent.

The current implementation of our maths classes has some shortcomings, ofMatrix3x3 for example does not have feature parity with ofMatrix4x4, and with GLM we can profit from not having to write / fix / maintain this ourselves.

Although some things may appear easier for beginners with our current vector maths classes, intermediate and advanced users stumble over things that might be intuitive to beginners, but are deeply counter-intuitive if you come from a GL/GLSL or maths or other frameworks background.

An example for this would be our vector - matrix default multiplication order, as Arturo mentioned.

Because almost all maths books and OpenGL itself follows the other convention, this can be super confusing for advanced and less advanced people alike. Dog knows how long it took me when i started to figure out why everything i was looking at through a camera was moving in some weird direction... First you have to learn how matrices and vectors are multiplied, and then you have to learn and remember how openFrameworks does this. Imagine only having to learn this once =).

I'm not sure code completion would have trouble with GLM, XCode and Visual Studio usually don't have trouble with header only libraries. The other nice thing about GLM is that it's built after GLSL, so reading up on GLSL built-in classes gives you pretty neat documentation on what's available, and GLM has some really nice docs, too...

My suggestion would be to deprecate the current vector maths classes, but to invest a good chunk of time in figuring out how to make the transition / deprecation period as smooth as possible.

This is very close to my heart, and I'm more than happy to help with it.

ofConstants defines default types for vertices, colors, normals and
texture coordinates. Right now they still default to using ofVec.

That + the constructors, cast operators and arithmetic operators from
ofVec <-> glm seem to keep 100% compatibility with old projects.

Commenting OF_USE_LEGACY_MESH in ofConstants enables glm by default in
ofMesh and ofPolyline
@arturoc
Copy link
Member Author

arturoc commented Feb 25, 2016

about code completion i guess what theo is more concerned about is not being able to do vec. tab/esc and get the list of methods you can call on a vec but glm:: tab/esc will do the same. the syntax is also really nice from what i see, i haven't had any weird issue except at the very beginning with the length thing but other than that i've changed the whole core in almost one go and everything seems to keep working without problem.

@julapy
Copy link
Member

julapy commented Feb 25, 2016

we have a couple people at our studio who use Cinder,
and ive recently started updating some of my core addons so they work across OF and Cinder.

part of this was to replace all ofVectorMath used in the addons with GLM,
which was very straight forward… it just worked (which never happens :)

moving forward to GLM will definitely allow for more OF-Cinder addon cross pollination.

anyhoo, just my 2c worth

Lukasz Karluk // Code on Canvas
www.codeoncanvas.cc http://codeoncanvas.cc/
+61 (0)415 179 079 tel:+61-415-179-079
Suite 2, 36-42 Chippen St,
Chippendale 2008 NSW, AUS

On 26 Feb 2016, at 4:13 AM, arturo notifications@github.com wrote:

about code completion i guess what theo is more concerned about is not being able to do vec. tab/esc and get the list of methods you can call on a vec but glm:: tab/esc will do the same. the syntax is also really nice from what i see, i haven't had any weird issue except at the very beginning with the length thing but other than that i've changed the whole core in almost one go and everything seems to keep working without problem.


Reply to this email directly or view it on GitHub #4943 (comment).

@ofTheo
Copy link
Member

ofTheo commented Feb 26, 2016

@arturoc that is what I meant about the code completion.
so that would be an argument for not importing the glm namespace.
doing glm:: and then hitting esc would give you the list of all the calls.

however are we okay with users having to do?:

glm::vec3 pos
vs:
vec3 pos

which if you are writing a ton of code with vecs the glm:: namespace being at the beginning might be a bit tedious.

not importing the glm namespace I guess then makes it less like glsl
so we that would make the porting between the two more work.

this could be either glsl or glm if we import.

vec3 delta = p2-p1; 
float len = length(delta); 

but if we have the namespace as part of OF then we lose the code completion component.
hmmm. :)

@tgfrerer @arturoc I would love to talk more about the best way to transition between ofVec2f or ofVec3f and glm.
The OF vector math classes are used everywhere. Ideally there would be a way where both would work.
The #define approach seems clever but it would make it so that you could only use one or the other. If we could find a way that is less binary than that, then we could end up making the default be GLM a lot sooner.

@ofTheo
Copy link
Member

ofTheo commented Feb 26, 2016

@arturoc @tgfrerer - thinking about this more I think importing the namespace really hurts us in terms of teaching/education.

I think code completion is incredible helpful for people to get a sense of what operations they have access to.

Currently I can get do delta. and hit esc or wait and see all the methods I have available.

delta

It seems that we lose this a bit with glm as even if we do glm:: and hit esc we have the entire library, not just the functions relating to the data type we're working with.

So there is a loss of more context specific code completion.
Now it might be that this loss is worth it - for what we gain - but its something we should be aware of if we do switch.

@arturoc
Copy link
Member Author

arturoc commented Feb 26, 2016

@tgfrerer @arturoc I would love to talk more about the best way to transition between ofVec2f or ofVec3f and glm.
The OF vector math classes are used everywhere. Ideally there would be a way where both would work.
The #define approach seems clever but it would make it so that you could only use one or the other. If we could find a way that is less binary than that, then we could end up making the default be GLM a lot sooner.

I don't think there's another way around this if we want to have 100% compatibility with older projects. you can have auto-conversion for individual objects but as soon as you want to use a full collection of oVec3f with glm::vec3 there's no way to do it unless the container is defined as being of one type or another.

We could have something intermediate where things like:

vector<ofVec3f> vertices = mesh.getVertices();

will fail and there's an easy fix like:

vector<ofVec3f> vertices = toOf(mesh.getVertices());

The way see it, right now we could just switch to glm and for old projects you can enable the legacy macro which will give you total compatibility.

I was also going to add another macro so by enabling it nothing at all gets converted so you can easily spot the places where you have to change your code to convert it.

about the namespace, glm::vec3 and ofVec3f are just 2 letters difference, the ::. In any case we shouldn't import the glm namespace globally, the names in it are too generic and would for sure clash with other things. we could add using namespace glm by default to every cpp (in the PG template)

even if you import the full namespace you can still do:

vec3 a(3,0,2);
glm::cross(vec3(1.0), a);

so you get code completion for functions and use the shorter syntax for class names...

after using it for a couple of days i really don't think this is a big deal, the api is so much more consistent than the current one that it doesn't matter that much and it's also really intuitive.

i find my self having to look up how to things much more often with ofVec after years than with glm after a couple of days. like in ofVec/ofMatrix there's some methods that are static, some that you have to call on an instance and it's completely arbitrary. glm api feels much more intutitive.

You could argue that a beginner will need to look the autocompletion but if you don't know what cross, dor or perpendicular are then looking them up in the autocomplete won't help much and if you do that's exactly how they are called in glm :)

There's a couple of issues with glm though that i think are much more important than this, will post about it later.

@bakercp
Copy link
Member

bakercp commented Feb 26, 2016

I would argue against a seamless / invisible transition. To me, glm::vec2
is becoming as valid and widespread as std::vector or std::size_t and
hiding or making these glm vector classes behind an OF facade makes about
as much sense as replacing std::vector with ofVector. Embracing widely
accepted standards on their own terms is in the spirit of the unifying
nature of OF. I wouldn't import any namespaces (aren't we trying to revert
this in the core?) and make a hard break with ofVec3f, ofPoint, etc. To
help developers we need to communicate the benefits of the upgrade and give
tips/tricks for updating old code.

This standards adoption is working really well in the case of our
filesystem classes, and a switch to glm had been on the table / planned for
several years now as far as I can tell. The advantages (standard, widely
tested, varying precision, sync with glsl, external documentation, ease of
transition from cinder et al, etc) are too numerous to enumerate.

On Fri, Feb 26, 2016, 11:48 AM arturo notifications@github.com wrote:

@tgfrerer https://github.com/tgfrerer @arturoc
https://github.com/arturoc I would love to talk more about the best way
to transition between ofVec2f or ofVec3f and glm.
The OF vector math classes are used everywhere. Ideally there would be a
way where both would work.
The #define approach seems clever but it would make it so that you could
only use one or the other. If we could find a way that is less binary than
that, then we could end up making the default be GLM a lot sooner.

I don't think there's another way around this if we want to have 100%
compatibility with older projects. you can have auto-conversion for
individual objects but as soon as you want to use a full collection of
oVec3f with glm::vec3 there's no way to do it unless the container is
defined as being of one type or another.

We could have something intermediate where things like:

vector vertices = mesh.getVertices();

will fail and there's an easy fix like:

vector vertices = toOf(mesh.getVertices());

The way see it, right now we could just switch to glm and for old projects
you can enable the legacy macro which will give you total compatibility.

I was also going to add another macro so by enabling it nothing at all
gets converted so you can easily spot the places where you have to change
your code to convert it.

about the namespace, glm::vec3 and ofVec3f are just 2 letters difference,
the ::. In any case we shouldn't import the glm namespace globally, the
names in it are too generic and would for sure clash with other things. we
could add using namespace glm by default to every cpp (in the PG
template)

even if you import the full namespace you can still do:

vec3 a(3,0,2);
glm::cross(vec3(1.0), a);

so you get code completion for functions and use the shorter syntax for
class names...

after using it for a couple of days i really don't think this is a big
deal, the api is so much more consistent than the current one that it
doesn't matter that much and it's also really intuitive.

i find my self having to look up how to things much more often with ofVec
after years than with glm after a couple of days. like in ofVec/ofMatrix
there's some methods that are static, some that you have to call on an
instance and it's completely arbitrary. glm api feels much more intutitive.

You could argue that a beginner will need to look the autocompletion but
if you don't know what cross, dor or perpendicular are then looking them up
in the autocomplete won't help much and if you do that's exactly how they
are called in glm :)

There's a couple of issues with glm though that i think are much more
important than this, will post about it later.


Reply to this email directly or view it on GitHub
#4943 (comment)
.

@ofTheo
Copy link
Member

ofTheo commented Feb 26, 2016

@arturoc I should spend some time working with this branch just to get a feel.
@bakercp thanks for your feedback!

I do worry however that all the feedback we get on these sorts of issues are from the much more experienced, higher level OF user, who typically is very pro 'adding the latest and greatest' at the potential cost of compatibility and legibility. So I think its good to be a little cautious :)
By their nature beginner OF-ers are not likely to be submitting feedback regarding these sorts of changes, so we should just try and keep them in mind when we're discussing these things.

In this case I think there are some very strong arguments for the switch and @bakercp you bring up some good points. However I think a transition time will be important, or at least a clear way to be backwards compatible as @arturoc suggested. We don't want to be nuking say 25% of addons on ofxAddons.com from working with OF with a hard switch.

@arturoc
Copy link
Member Author

arturoc commented Feb 26, 2016

So, som problems i've seen:

  • glm has some inconsistencies with angles. Some functions like angle(vec,vec) return radians, or perspective(fov...) fov is in radians while rotate(mat, angle, axis) angle is in degrees! In most cases we wrap this but there might be some cases in which it can become confusing.
  • glm by design doesn't mutate anything so:
rotate(mat,angle,axis)

is not going to do anything you have to do:

mat = rotate(mat,angle,axis)

which is better in terms of avoiding side effects but will for sure trip up some people when using it for the first time. this is very common in modern apis, using immutable data... but being used to getRotated()/rotate it'll be confusing for some at the beginning. i've ported the whole core and have catched it every time right after writing it but have done it a couple of times.

  • glm is much more strict than ofVec in types conversion, this is safer but also harder sometimes, it's pretty similar to glsl so it's usually ok, but i've found one case where i've been trying to figure out what was going on for a while:
ofVec3f v;
ofMatrix4x4 m;
auto v2 = v * m;

now becomes

vec3 v;
mat4 m;
auto v2 = m * v;

except that you can't multiply vec3 * mat4, ofVec automatically does this but in glm you have to do it manually so you need to know that w=1.0 and that afterwards you have to divide everything by w:

vec3 v;
mat4 m;
auto v4 = m * vec4(v, 1.0);
auto v2 = v4.xyz() / v4.w;

For this we could just add an operator* that allows to multiply vec3 * mat4 and does what OF currently does

+ for dimensisons. this is more conformant with stl
some leftover uses of length as a member method changed to glm calls
@arturoc
Copy link
Member Author

arturoc commented Jul 15, 2016

I've been using this branch for a couple of weeks on a project with @prisonerjohn and it everything seems to work without problem.

All the methods that receive angles are now deprecated and there's versions with Rad and Deg as we spoke on the dev list

I'll merge this into master next week unless anyone has any suggestion before merging.

@ofTheo
Copy link
Member

ofTheo commented Jul 15, 2016

@arturoc - nice that you have been working with it on a project!

What is the macro approach for enabling old projects to work?

I see a OF_USE_LEGACY_MESH macro here: arturoc@2957052#diff-1d586ad83492545eb6a5dfcd87941db0R938 Is that the way of switching between old and new projects?

@arturoc
Copy link
Member Author

arturoc commented Jul 15, 2016

yes that's i've disabled it by now to catch more errors in addons... but it can we reenable it by default before merging into master or for next release

prisonerjohn and others added 3 commits July 18, 2016 13:55
…g it causes a GL error, which makes it a pain to debug other GL errors.
@arturoc arturoc closed this Sep 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants