eos icon indicating copy to clipboard operation
eos copied to clipboard

A collection of fixes to allow compilation in a c++11 environment (openFrameworks)

Open bakercp opened this issue 6 years ago • 3 comments

First, thanks for your work on this. It's really great!

I'm working on an addon that enables openFrameworks users to use this code. Here are some of the updates I had to make to get it running. You can keep track of progress here:

https://github.com/bakercp/ofxEos

bakercp avatar Mar 16 '18 20:03 bakercp

Hey that sounds really cool! Thank you very much!

And thank you very much for the PR. I think some of these we can definitely merge, particularly the ones that are fixes! (missing inline etc.). We don't want to support too old compilers though, as this repo is rather moving towards C++17 than supporting older C++11 compilers. The rule is sort-of that we move forward with all features that are supported on the latest Xcode (currently 9.2 - and Xcode is usually the deal-braker with respect to new C++ features...), the latest VS (currently 2017.5), and a modestly recent g++/clang (we test on g++7 but it may actually still work on g++-5).

I will have a closer look at the PR next week when I've got a bit more time :)

Oh and the roadmap is actually to move away completely from glm (and also get rid of the last occurences of cv::Vec*-types in the rendering). The plan is to use Eigen for all that. We don't really need glm for much actually and Eigen can do most of it, and it's really not too good to have vector types from 3 different libraries in eos. Also, using Eigen everywhere will make the python bindings even more awesome, since Eigen works perfect together with pybind11. Since I've got the opportunity to ask you, I'm curious, if you have an opinion about that. Do you think glm has a reason to stay?

PS: If you don't mind, would you rename "EOS" in your project(s) to "eos"? We usually use the lower-case spelling. :-) That would be awesome!

patrikhuber avatar Mar 16 '18 21:03 patrikhuber

Thanks for taking a look!

Regarding the CI errors -- we use glm all over openFrameworks, so it must have picked up the glm::clamp function elsewhere. I'll leave it alone though until you take a look.

We recently moved everything in openFrameworks from DIY vec/matrix to glm since it better mirrors glsl, is well-supported, documented, etc. Since a lot of the ways that we use it in openFrameworks are graphics-focused it makes sense for us. Personally I really like GLM. The only downside I've found is that it can get slow to compile if you don't watch your includes (but that's not really GLM's fault). I don't have a strong opinion on Eigen. For eos purposes it might be nice to have all of the linear algebra routines (SVD, etc) that come with Eigen. GLM has a lot of great linear algebra tools useful for graphics programming, but not so much on the analytical side. So it really depends I suppose.

Regarding C++17, we still only support up to C++11 in openFrameworks since we support so many platforms, so that's our least common denominator (and it took forever to get even there!). I totally understand your perspective though.

I'll change my addon name to ofxEos (without the capital, it looks funny next to all of the other openframeworks addons, as they all have a capital after the ofx - http://ofxaddons.com/). I'll refer to it as eos in my readme, links and repos etc.

Keep me posted on any bits of the PR you'd like to include. I'm happy to resubmit a clean PR when the time comes.

Thanks for sharing the project!

bakercp avatar Mar 16 '18 21:03 bakercp

Hi again :)

Thank you for adjusting the name!

As you've already seen, I've modified your commits a bit and added the optional-tweak for pre-C++17 compilers. I've also now done the same with std::clamp. I've also cherry-picked the inline and #include fixes. Thank you very much. The "use newer glm functions" (2f8881fb89642e533bda705a7a821b96a891c0df) I think I'll merge as well soon but I want to have a quick look at that first.

That leaves the auto-return-types and generic lambdas. I'm a bit hesitant to cherry-pick/merge these, as they are C++14 syntax, and eos is a throughout C++14 project. We realised over the last few weeks that we still need to compromise on C++17 and support C++14 - but we definitely don't want to go below that. Particularly as C++14 was more of a "bugfix" release to C++11 and compiler support is very widespread by now.

Thank you very much for your input on glm! I totally agree with you. Mostly the glm::project, ortho and other "projection" matrix functions would be sorely missing without glm, and glm's "compatibility" with OpenGL in that regard is great. But these we can probably reimplement with Eigen types and it's not really worth keeping around the whole of glm (and a second set of vector/matrix types...) just for that. Let's see where it goes.

Could you please have a quick look at my review-comment above regarding the default template argument (b6f4975)? It strikes me really weird that that's not working and to remove the default argument there.

Thank you again and I'll get to the remaining things soon!

patrikhuber avatar Mar 21 '18 21:03 patrikhuber

Hi again, it's been a long while! I'm guessing you might not be working on/with ofx-eos anymore? In any case - it looks like ofx now has C++17 support or is very close to getting there (https://github.com/openframeworks/openFrameworks/issues/6585), so I think I can close this PR. I'm also thinking about moving eos to C++17 and dropping support for pre-C++17 in the future.

Thank you again for the nice interaction back then, and let me know in case you're still working in the area and would like to discuss anything!

patrikhuber avatar Feb 18 '23 16:02 patrikhuber