We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 4794 - SDL_GL_GetProcAddress() calls eglGetProcAddress() for non-core GLES functions on EGL 1.4
Summary: SDL_GL_GetProcAddress() calls eglGetProcAddress() for non-core GLES functions...
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: video (show other bugs)
Version: 2.0.10
Hardware: x86 Windows 10
: P2 major
Assignee: Ryan C. Gordon
QA Contact: Sam Lantinga
URL:
Keywords: target-2.0.12
Depends on:
Blocks:
 
Reported: 2019-09-06 22:57 UTC by Charles Huber
Modified: 2019-10-21 15:51 UTC (History)
1 user (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Charles Huber 2019-09-06 22:57:28 UTC 
When I use SDL against an EGL 1.4 implementation like SwiftShader[1] SDL tries to grab core OpenGL ES functions like glGetString() out of eglGetProcAddress() instead of using SDL_LoadFunction().

That behavior is valid in EGL 1.5:

(page 86, second paragraph)
"eglGetProcAddress may be queried for all EGL and client API functions supported by the implementation (whether those functions are extensions or not, and whether they are supported by the current client API context or not)."

But not EGL 1.4:

(page 56, first paragraph)
"eglGetProcAddress may not be queried for core (non-extension) functions in EGL or client APIs."


[1]: https://swiftshader.googlesource.com/SwiftShader; you can grab binaries from a 32-bit Chrome install under Application\<version>\swiftshader
Comment 1 Sylvain 2019-09-07 05:41:31 UTC 
Something similar in bug 4040
Comment 2 Ryan C. Gordon 2019-09-20 20:47:35 UTC 
We're changing how we do SDL release versions; now releases will be even numbers (2.0.10, 2.0.12, etc), and as soon as we tag a release, we'll move the internal version number to an odd number (2.0.12 ships, we tag the latest in revision control as 2.0.13 immediately, which will become 2.0.14 on release, etc).

As such, I'm moving the bugs tagged with target-2.0.11 to target 2.0.12. Sorry if you get a lot of email from this change!

Thanks,
--ryan.
Comment 3 Ryan C. Gordon 2019-09-20 20:48:39 UTC 
We're changing how we do SDL release versions; now releases will be even numbers (2.0.10, 2.0.12, etc), and as soon as we tag a release, we'll move the internal version number to an odd number (2.0.12 ships, we tag the latest in revision control as 2.0.13 immediately, which will become 2.0.14 on release, etc).

As such, I'm moving the bugs tagged with target-2.0.11 to target 2.0.12. Sorry if you get a lot of email from this change!

Thanks,
--ryan.
Comment 4 Ryan C. Gordon 2019-10-16 23:26:35 UTC 
Ok, so what it appears to do right now is call eglGetProcAddress() (if not on Android, see Bug #4040), and if that fails, it falls back to SDL_LoadFunction().

Is the problem that eglGetProcAddress() returns a bogus non-NULL pointer for missing functions (as Mesa's glXGetProcAddress() does)?

And if so, is there a convenient way to decide if a symbol is part of core or not?

--ryan.
Comment 5 Sylvain 2019-10-17 10:00:17 UTC 
On Android, (from bug 4040), Device Nexus 10 (old EGL), trying to load the core "eglGetString" gives a bogus non-NULL pointer.


Maybe we need also distinction between EGL version, < 1.5 and >= 1.5.

If the function if part of libGLES, this is a core function ? or can this also be an extension ?
Or could we hard-code the list of core functions ?
Comment 6 Charles Huber 2019-10-17 21:38:20 UTC 
Might be able to get away with doing dlsym() first, then falling back to eglGetProcAddress():

https://github.com/Dav1dde/glad/issues/62#issuecomment-241137825
Comment 7 Ryan C. Gordon 2019-10-18 04:10:53 UTC 
(In reply to Charles Huber from comment #6)
> Might be able to get away with doing dlsym() first, then falling back to
> eglGetProcAddress():
> 
> https://github.com/Dav1dde/glad/issues/62#issuecomment-241137825

I've taken a swing at this, in https://hg.libsdl.org/SDL/rev/2e673c68ab97 ... it's a bit of a tapdance, but it _should_ cover all cases.

The only thing it doesn't do is keep a list of core symbols, but I'm hoping we can avoid that.

I'd appreciate if someone could test this patch, as I'm shooting blind on this one.

--ryan.
Comment 8 Charles Huber 2019-10-18 14:56:03 UTC 
Excellent!  That patch fixes the stack corruption warnings I was seeing with a pre-38a9b3c417 SwiftShader on Windows that liked to send cdecl functions out of eglGetProcAddress() instead of proper stdcall ones[1].

You might also consider modifying SDL_EGL_LoadLibrary() so that it (re)captures the EGL major/minor version numbers from eglInitialize() instead of only parsing them out of the EGL_VERSION string.  That way we could have reliable EGL version numbers for sub-EGL 1.5 implementations (for logging or what have you), right now anything that doesn't support getting the EGL_VERSION string ends up as EGL 0.0 :)

Something like this:

// around line 505 of SDL_egl.c:
----------------------------------------
if (_this->egl_data->eglInitialize(_this->egl_data->egl_display, &egl_version_major, &egl_version_minor) != EGL_TRUE) {
    _this->gl_config.driver_loaded = 0;
    *_this->gl_config.driver_path = '\0';
    return SDL_SetError("Could not initialize EGL");
}
_this->egl_data->egl_version_major = egl_version_major;
_this->egl_data->egl_version_minor = egl_version_minor;
----------------------------------------


[1]: https://issuetracker.google.com/issues/140700303
Comment 9 Sylvain 2019-10-18 19:59:20 UTC 
@Charles: this has just been added before your comment. Maybe fetch the latest mercurial sources. You would get the version for version below 1.5

(not by using the parameters from eglInitialize, but with eglQueryString)
Comment 10 Charles Huber 2019-10-21 15:51:11 UTC 
@Sylvain: Ah, thanks for the heads-up!

Yup, that fixes all my issues for this bug, marking as fixed.