-
-
Notifications
You must be signed in to change notification settings - Fork 163
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
Implement display.gl_get_proc
#3285
base: main
Are you sure you want to change the base?
Conversation
- SDL3 compat - Use WINFUNCTYPE (stdcall) on win32 - Use ctypes._FuncPointer in stubs
Thoughts: Maybe it would be simpler to import ctypes instead of a custom module into display.c, and then use PyObject_call stuff and ifdef WIN32 to do the same logic as the custom ffi.py module Maybe we should give the pointer only, and let the user worry about ctypes, since they're going to need to worry about ctypes anyway to extract any value (as I understand it). Plus it makes it simpler code for this seemingly-niche use case. |
- Import ctypes directly in display.c - Don't use SDL_GetError when SDL_GL_GetProcAddress returns NULL
|
||
| :sl:`Get an OpenGL function by name` | ||
| :sg:`gl_get_proc(proc_name) -> ctypes._FuncPointer` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is an uncommon function it could be explained with a bit more detail, for example mentioning that it will try to import ctypes if it is not imported, the fact that it can fail and raise errors, and if it's relevant roughly how to use the return value with an embedded example.
[assuming the "changes" are addressed/discussed and that the code actually works] I think it's ok for it to return a ctypes object since it is imported only on first function call. if we didn't use ctypes the return value wouldn't be much of an abstraction, so it would be less useful, I think. |
Trys to fix #3284
Example:
WARNING
This PR introduces a new dependency:
ctypes
This PR is experimental. If you have better idea, feel free to open another PR.