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

pygame.examples.glcube updated to use 'modern' API. #1730

Merged
merged 3 commits into from May 23, 2020

Conversation

MyreMylar
Copy link
Contributor

My attempt to update the gl cube example to show how you can switch the context profile.

So it actually works I also included a modern gl version of the gl cube as close as I could make it to the original.

This finishes fixing issue #85

@lgtm-com
Copy link

lgtm-com bot commented May 14, 2020

This pull request introduces 2 alerts when merging aa98938 into ea4a028 - view on LGTM.com

new alerts:

  • 2 for Unused import

@robertpfeiffer
Copy link
Contributor

Looks good. I ported glcube to modernGL, but your example seems to be the better way to go.

Copy link
Member

@illume illume left a comment

Choose a reason for hiding this comment

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

Nice one.

I wonder if we should just remove the old OpenGL style code in here so it's easier to read? Then the _modern parts could be removed.

I guess OpenGL.arrays.vbo could be used for the vertex buffers instead to avoid the ctypes import. Probably could use a few doc strings/comments in those transform functions.

@MyreMylar
Copy link
Contributor Author

OpenGL.arrays.vbo

I looked at the source code for this and it imports ctypes:
https://bazaar.launchpad.net/~mcfletch/pyopengl/trunk/view/head:/OpenGL/arrays/vbo.py#L38

So I'm not sure we'd be gaining anything by switching - it seems to be pretty much the same code in a wrapper. Not an openGL expert though.

I wonder if we should just remove the old OpenGL style code in here so it's easier to read? Then the _modern parts could be removed.

The reason I kept the old parts is because the original point of updating the gl cube example was to show that you could switch the core context profile - in these lines:

gl_version = (2, 0)  # GL Version number (Major, Minor)

# By setting these attributes we can choose which Open GL Profile
# to use, profiles greater than 3.2 use a different rendering path
pg.display.gl_set_attribute(pg.GL_CONTEXT_MAJOR_VERSION, gl_version[0])
pg.display.gl_set_attribute(pg.GL_CONTEXT_MINOR_VERSION, gl_version[1])
pg.display.gl_set_attribute(pg.GL_CONTEXT_PROFILE_MASK,
                            pg.GL_CONTEXT_PROFILE_CORE)

..and I noticed that switching it higher than 3.1 didn't work because of the change between old gl and modern gl. If we remove the old style code, won't we just have the same problem in the other direction, where if you set the profile below 3 it won't work?

Probably could use a few doc strings/comments in those transform functions.

Yes, I will add some.

Copy link
Member

@illume illume left a comment

Choose a reason for hiding this comment

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

Awesome :)

@illume illume changed the title Updated gl cube example pygame.examples.glcube updated to use 'modern' API. May 23, 2020
@illume illume merged commit bd3e4f4 into pygame:master May 23, 2020
@MyreMylar MyreMylar deleted the update-gl-cube-example branch June 5, 2020 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants