waffle-gl / waffle

A C library for selecting an OpenGL API and window system at runtime
https://waffle.freedesktop.org
BSD 2-Clause "Simplified" License
35 stars 16 forks source link

wflinfo: use waffle_dl_sym to get glGetStringi #20

Closed fjhenigman closed 10 years ago

fjhenigman commented 10 years ago

Wflinfo tries to use glGetStringi when the gl version >= 3.0, but eglGetProcAddress cannot be relied on to get core functions like glGetStringi unless the egl version is 1.5 or higher. Dlsym works with lower egl versions. This lets wflinfo work on platforms where egl < 1.5 and gl >= 3.0, e.g. mali.

fjhenigman commented 10 years ago

This is a quick fix, but perhaps enough, unless there's some reason I can't guess why waffle_get_proc_address was used here originally, which is quite likely. I tested with mali and mesa. Is there any reason waffle can't have an address-getting function that figures out all this crap for the user? It could check the gl and egl version, determine if the function is core or extension, and then decide to use eglGetProcAddress or dlsym. If all that is really necessary.

djkurtz commented 10 years ago

Pedantically: eglGetProcAddress() was originally spec'ed to only work for EGL and GL "extension" functions. The "core" functions were explicitly prohibited. Support for "extension and non-extension" functions was added to EGL 1.4 via EGL_KHR_get_all_proc_addresses [0]. This extension became part of EGL 1.5. [0] https://www.khronos.org/registry/egl/extensions/KHR/EGL_KHR_get_all_proc_addresses.txt

GL dispatch is non-trivial; take a look at piglit's dispatch. The "long term goal" for piglit is to used libepoxy [1], but the epoxy conversion has been held up for almost a year due to lack of motivation to make it work with MSVC, afaict. [1] https://github.com/anholt/libepoxy/

BTW, libepoxy itself had this same eglGetProcAddress issue [2], but it should be fixed now. [2] https://github.com/anholt/libepoxy/issues/21

evelikov commented 10 years ago

@fjhenigman When I converted glGetError, glGetIntegerv and glGetString to dl_sym I (wrongly) assumed that glGetStringi is not part of the (OpenGL 1.0) Core specification. After checking it seems that I've got it wrong and your commit looks like the correct thing to do.

@chadversary feel free to add to the patch Reviewed-by: Emil Velikov emil.l.velikov@gmail.com

@djkurtz Over the summer I've added WGL support for waffle, which I'm rebasing atm. Afaict adding GL dispatch within waffle is still open, hopefully @chadversary will to find some time to get the work merged. First I would like to nuke all the explicit dependencies (use dlopen when possible), and then I'll start looking at GL dispatch. Patches welcome :)

linyaa-kiwi commented 10 years ago

@fjhenigman said:

Is there any reason waffle can't have an address-getting function that figures out all this crap for the user? It could check the gl and egl version, determine if the function is core or extension, and then decide to use eglGetProcAddress or dlsym. If all that is really necessary.

More is necessary. The native platform (EGL/CGL/WGL/GLX) is also a factor in determining when dlsym vs GetProcAddress is legal.

For Waffle to make that decision and "figure out all that crap" for the user, it would need to reproduce all the internal logic of piglit-dispatch or libepoxy. I rewrote that logic for Piglit earlier this year, and it wouldn't be difficult to do it again for Waffle. But Waffle doesn't have that logic today because, when I first wrote Waffle, Khronos had not yet published an XML registry for OpenGL ES. There was no clean way to generate that logic. The cleanest solution was ad-hoc parsing of Khronos headers with a heavy dose of ugly workarounds.

Now that Khronos has published the gl.xml in the registry, it shouldn't be too much work to implement a more sane waffle_get_proc_address().

@fjhenigman Both the code in Waffle and in your pull request is incorrect. Your fix has the potential to break other platforms (WGL and non-Mesa GL drivers) because glGetStringi was not promoted to OpenGL until GL 3.0. It's technically illegal on GLX to dlsym() any symbol (core or extension) that doesn not belong in OpenGL 1.2. WGL has similar restrictions, but I don't recall the cutoff GL version.

I think the correct fix should look like this:

if (EGL and (OpenGL ES >= 3.0 or OpenGL >= 3.0) { // EGL 1.4 doesn't allow eglGetProcAddress on non-extension symbols. glGetStringi = waffle_dl_sym(..., "glGetStringi") } else { // TODO: Explain this crazy mess. glGetStringi = waffle_get_proc_address("glGetStringi") }

linyaa-kiwi commented 10 years ago

@fjhenigman I'ts been several days since the last comment... If you prefer, I can implement the correct fix myself and submit it to you for review and testing on Mali.

fjhenigman commented 10 years ago

Rather than add code everywhere symbol lookup is needed, it makes more sense to have a smart lookup function, yes? I can wait for that, and work around this until then. I'd like to help with the lookup problem, but realistically may not be able to in the near future.

linyaa-kiwi commented 10 years ago

@fjhenigman I agree with you. Waffle should have a "smart lookup" function rather than ad-hoc logic at every GL symbol lookup. But that smart function is a very large. At a minimum, it takes several hundred lines of Python and another several hundred lines of generated C code. When you're ready to help with it, please let me know.

In the interim, before Waffle gets that lookup functionality, we should still apply local ad-hoc fixes when needed. If I can find tomorrow, I'll write a well-documented patch for that.

linyaa-kiwi commented 10 years ago

Patch discussion is on the mailing list. See thread [http://www.mail-archive.com/waffle%40lists.freedesktop.org/msg00713.html].