Skip to content
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

Optional glbinding as OpenGL binding #4

Closed
wants to merge 7 commits into from
Closed

Optional glbinding as OpenGL binding #4

wants to merge 7 commits into from

Conversation

scheibel
Copy link
Contributor

This week my team published an alternative OpenGL binding beside GLEW.
As a simple test we tried to replace the OpenGL binding of known open-source software with ours: glbinding.

For SuperTux this was a pleasingly simple task.
I just want you to be able to use our work we have done while integrating glbinding.
If you pull this PR into your branch, the build defaults to the exact same setting as without this PR.

That has been done:

  • add glbinding as optional dependency
  • add an option that glbinding can be used instead of GLEW
  • add another option to enable per OpenGL-function tracing during runtime
  • adapt OpenGL-dependent code so that it works with both GLEW and glbinding

I know this is just a mirror, but if you're interested in the code you can merge this PR in your internally used git-Repository.

@@ -72,6 +72,8 @@ PKG_SEARCH_MODULE(SDL2IMAGE REQUIRED SDL2_image>=2.0.0)
SET(HAVE_SDL TRUE)

OPTION(ENABLE_OPENGL "Enable OpenGL support" ON)
OPTION(GLBINDING_ENABLED "Use glbinding instead of GLEW" OFF)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am no expert in CMake, but shouldn't this option be called USE_GLBINDING since we're referencing USE_GLBINDING later in the code (l. 84)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally, I named this option like this, but I changed it to GLBINDING_ENABLED when I added the second glbinding option, so that they get grouped by the cmake gui.
I'm not attached to the exact naming of the options, they can be renamed to whatever you like.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, but they should be the same in line 75 and line 84, so that this actually works, right? In any case: Many thanks for your work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, they have to have the same name, I fixed this.

@tobbi
Copy link
Member

tobbi commented Aug 16, 2014

I can merge this, but I keep getting a crash when I try to start SuperTux with glbinding:
http://pastebin.com/Gf8TeqrT

@scheibel
Copy link
Contributor Author

I see that you have used OS X as operating system.
I tested my code on Ubuntu 14.04 where all works just fine.
It seems like a bug in glbinding on OS X where function addresses aren't resolved correctly but I'm pretty sure that we fixed this bug a long time ago.
I might be able to test it on OS X on monday and try to fix this issue.

Thanks for the extensive crash report.

@scheibel
Copy link
Contributor Author

I moved the glbinding initialization code behind the context creation code, where it belongs.
One of our team members has reproduced the error on his OS X machine and is about to test this fix.

@tobbi
Copy link
Member

tobbi commented Aug 19, 2014

I squashed all commits and applied the pull request on our repository with git revision f266dcd. Changes should appear on this mirror within the hour. In case there are any problems arising, I will get back to you.

Thanks for your contribution! :-)

@tobbi tobbi closed this Aug 19, 2014
@tobbi
Copy link
Member

tobbi commented Aug 19, 2014

Hmm, sorry, this was quick, but I accidentally wasn't compiling with glbinding. It still crashes at startup on my Mac 10.9. Please see pastebin for the error message:
http://pastebin.com/a4wb40vS

@tobbi tobbi reopened this Aug 19, 2014
@scheibel
Copy link
Contributor Author

It seems that on OS X an OpenGL context below version 3.0 is used (I assume 2.1), so querying extensions fails.
Therefor we integrated a special test for OpenGL version < 3.0 and return an empty list instead of querying the OpenGL driver.

This is implemented in the no_extensions_below_3_0 branch of glbinding.
Does the current SuperTux code work with the specific branch of glbinding?
If so, we will merge the branch into the master so this piece of code is integrated into the 1.1 release.

@tobbi
Copy link
Member

tobbi commented Aug 22, 2014

The abovementioned branch works, albeit being rather slow. I can upload the log which contains the opengl calls from starting supertux to loading the first worldmap, however, it's 95 MB big, which is too big for any pastebin service.

@scheibel
Copy link
Contributor Author

Of course, with ongoing write to disk, the program is much times slower.
You used the debugging mode, which we used to analyse the OpenGL calls made by SuperTux.
There is the CMake option GLBINDING_DEBUG_OUTPUT that can be turned off in order to reduce the functionality of glbinding to a bare OpenGl function binding.
This should be as fast as using GLEW.

@tobbi
Copy link
Member

tobbi commented Aug 22, 2014

You're right. Disabling that debug statement fixed it. Done the final change in commit 8415881 and closing this. Many thanks, again!

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.

None yet

2 participants