kbale / osgocean

An ocean rendering nodekit for OpenSceneGraph
GNU Lesser General Public License v3.0
109 stars 56 forks source link

Port osgOcean to OSG 3.4.x - use osg/Version to keep backwards compatibility #52

Closed fcami closed 6 years ago

fcami commented 7 years ago

These changes make osgOcean work against OpenSceneGraph 3.4.1 and should preserve backwards compatibility.

fcami commented 6 years ago

Hi @kbale - could you please merge this PR? Or tell me what you'd like changed.

romainreignier commented 6 years ago

Hi @fcami

I have quickly tried your changes on Linux with the oceanExample because it is the only app I had on hand. Everything seems to work fine, much better than my attempt at upgrading to 3.4!

So for me, this PR is fine.

But, in the code, I would prefer:

#if OSG_VERSION_LESS_THAN(3,2,2)

instead of

#if OSG_VERSION_GREATER_THAN(3,2,1)

Because if the API change in the future, we would have to nest the preprocessor macros, which would be less readable.

fcami commented 6 years ago

Hi @romainreignier & @kbale Thanks for testing! I'll update the PR soon, I agree with you.

fcami commented 6 years ago

PR updated & I think I got the versions right this time. @romainreignier & @kbale please let me know what you think.

romainreignier commented 6 years ago

Ok, I prefer that version. I saw that you change the version number according to the change. I did not check these but I assume you checked them.

fcami commented 6 years ago

I checked them and they should be right. Note "should": time permitting I would have built those versions and double-checked (and I didn't). If anyone complains I'll fix that as well. @kbale ? :)

whatnick commented 6 years ago

Successfully compiled on MSVC 2015 using OSG 3.4.0 binaries. Would vote for a merge.

romainreignier commented 6 years ago

@whatnick Did you try the oceanExample application? Because on Windows, I used to find some artifacts on Windows while trying to port to OSG 3.4. I do not have my Windows setup anymore so it would be nice if you could try it (going underwater in the demo). Thanks.

whatnick commented 6 years ago

This is what I have underwater: image

fcami commented 6 years ago

@whatnick @romainreignier maybe you're hitting a shader issue: https://github.com/kbale/osgocean/issues/53 ?

Everything I get underwater seems to be uniform (white on my graphical stack).

romainreignier commented 6 years ago

@whatnick Thanks for taking the time to test so it seems OK.

@kbale This PR seems great and should be merged.

@fcami Yes, it may be related. I think that on OSG 3.2, I only had this issue on Intel GPU, using the second GPU on my laptop, NVidia 750M, there were no issue.

whatnick commented 6 years ago

Hence issues are due to shader limitations on embedded GPU on Intel.

On Feb 7, 2018 18:47, "Romain Reignier" notifications@github.com wrote:

@whatnick https://github.com/whatnick Thanks for taking the time to test so it seems OK.

@kbale https://github.com/kbale This PR seems great and should be merged.

@fcami https://github.com/fcami Yes, it may be related. I think that on OSG 3.2, I only had this issue on Intel GPU, using the second GPU on my laptop, NVidia 750M, there were no issue.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kbale/osgocean/pull/52#issuecomment-363690864, or mute the thread https://github.com/notifications/unsubscribe-auth/AAd_hNGJU0uxPQRBRofHu9Sw1t1u_va_ks5tSVwEgaJpZM4QMjED .

psi29a commented 6 years ago

Where are we on this? :) OpenMW is interested in including this and we're based on osg3.4

kbale commented 6 years ago

Hi @fcami

My apologies for not merging this sooner, I've recently had a baby and I really haven't had time to keep up with any changes to osgOcean, it's been some time since I worked on this project. None the less, it's in now, thank you for your contribution.

psi29a commented 6 years ago

@kbale Congratulations! Welcome to the no-sleep club. :P

fcami commented 6 years ago

hi @kbale, no worries, thanks for merging!

lucasamparo commented 2 years ago

Very useful pull request for OSG migration. Thanks.

fcami commented 2 years ago

Thanks @lucasamparo , that kind of feedback is gold to me.