See https://discourse.libsdl.org/t/android-update-breaks-opengl-es-1-0/23709

Tagging a bunch of bugs with "target-2.0.10" so we have a clear list of things to address before a 2.0.10 release.

Please note that "addressing" one of these bugs might mean deciding to defer on it until after 2.0.10, or resolving it as WONTFIX, etc. This is just here to tell us we should look at it carefully, and soon.

If you have new information or feedback on this issue, this is a good time to add it to the conversation, as we're likely to be paying attention to this specific report in the next few days/weeks.

Thanks!

--ryan.

Sylvain, can you double check this?

I tried to simply remove the conditional pre-processor:
  "#if !defined(SDL_VIDEO_DRIVER_ANDROID)"
so that SDL_EGL_GetProcAddress() can use eglGetProcAddress.

I run a minimal test-case to use "glGetString" and display enum GL_RENDERER (0x1F01):

          typedef char * (*ff_t)(int);
          const char *f = "glGetString";
          ff_t ret = NULL;
          ret = (ff_t) SDL_GL_GetProcAddress(f);
          SDL_Log("SDL load of %s, ret=%p", f, ret);
          SDL_Log("SDL getstring=%s", ret(0x1F01));


- It works on Android 9 and display some Mali-version.

- On a Nexus 10 with Android 4.2.2 it returns a valid handle but it crashes. 
  when it's nonactivated, it would display some Mali-another-version. So we can't enable this code like this.

NB: I am trying with glGetString() because this is the original issue in bug 1290.
(glGetString() is used by SDL_GL_ExtensionSupported().)


From online docs, it seems that eglGetProcAddress() should be used for extension function, not core functions.
And glGetString is a core function.

Maybe glGetString should be loaded with SDL_LoadFunction() ?
https://hg.libsdl.org/SDL/file/06fa9a25d2a0/src/video/SDL_video.c#l2939


Why do we need eglGetProcAddress if we have dlsym ?  
Can't we rewrite by reversing steps:

SDL_EGL_GetProcAddress() {
  1/ try SDL_LoadFunction
  2/ try eglGetProcAddress
}

The spec specifically says not to use dlsym() in case the correct functions for the current context come from an internal driver table rather than globally accessible functions. This has in practice crashed when Mesa and hardware accelerated drivers are available in the same application.

From the Khronos documentation:
https://www.khronos.org/registry/EGL/sdk/docs/man/html/eglGetProcAddress.xhtml

"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)."

Title says it's for extensions: "eglGetProcAddress — return a GL or an EGL extension function"
but the text you quote says it's for any function.

Maybe because of the notes:
"If the EGL version is not 1.5 or greater, only queries of EGL and client API extension functions will succeed."

That would explain the original issue: querying "glGetString" was failing because EGL version was below 1.5.
Which would still be valid on my old Nexus 10 4.2.2 (which is 2012, whereas EGL 1.5 is from 2014).

Yes, that makes sense.

So the original Android bug was that a valid extension would be reported as available, but the entry points for it would report NULL through eglGetProcAddress(), and this was fixed a million years ago.

I suspect the hesitation after that was probably the same issue as Bug #4794, and I believe this patch should resolve both of these bugs, allowing us to remove the #ifdef ANDROID check:

https://hg.libsdl.org/SDL/rev/2e673c68ab97

I don't have an Android device set up at the moment; Sylvain, if you get a moment, I'd appreciate if you'd try the latest in revision control with those #ifdefs removed. If it works, I'd say just commit the #ifdef removal and resolve this bug.

--ryan.

EGL version wasn't retrieved. Fixed in https://hg.libsdl.org/SDL/rev/9bed6495d9ac

(Was added for KMS/DRM https://hg.libsdl.org/SDL/rev/cbc6a8a5b701#l6.131 )

Two devices gave me version 1.4 (One old and one recent, strange ?).

So I may be hit the case where the version cannot be retrieved with "NO_DISPLAY"
( "A display of EGL_NO_DISPLAY is supported only if the EGL version is 1.5 or greater. "
https://www.khronos.org/registry/EGL/sdk/docs/man/html/eglQueryString.xhtml )

(I copy-pasted it for offscreen, but not tested)

So I removed the #ifdef because this isn't crashing my old android nexus10
https://hg.libsdl.org/SDL/rev/18d7c9aa4a56

The test-case:

   /* Core function */    
   {   
       typedef char * (*func_type_t)(int);
       const char *func_name = "glGetString";
       func_type_t func = NULL;
       func = (func_type_t) SDL_GL_GetProcAddress(func_name);
       SDL_Log("SDL load of %s, func=%p", func_name, func);
       if (func) {
           SDL_Log("SDL getstring=%s", func(0x1F01));
       } else {
           SDL_Log("SDL cannot load function %s", func_name);
       }   
   }   

   /* Extension function */
   {   
       /* EGLBoolean eglQueryDevicesEXT(EGLint max_devices, EGLDeviceEXT *devices, EGLint *num_devices); */
       typedef int (*func_type_t)(int, void **, int *); 
       const char *func_name = "eglQueryDevicesEXT";
       func_type_t func = NULL;
       int max_devices = 10;
       void *devices[10];
       int num_devices = 0;
       func = (func_type_t) SDL_GL_GetProcAddress(func_name);
       SDL_Log("SDL load of %s, func=%p", func_name, func);
       if (func) {
           int ret = func(max_devices, devices, &num_devices);
           SDL_Log("SDL ret=%d num_devices=%d", ret, num_devices);
       } else {
           SDL_Log("SDL cannot load function %s", func_name);
       }   
   }

But actually SDL_LoadFunction() is a dlsym, and Sam commented: 

"The spec specifically says not to use dlsym() in case the correct functions for the current context come from an internal driver table rather than globally accessible functions. This has in practice crashed when Mesa and hardware accelerated drivers are available in the same application."

(In reply to Sylvain from comment #11)
> But actually SDL_LoadFunction() is a dlsym, and Sam commented: 
> 
> "The spec specifically says not to use dlsym() in case the correct functions
> for the current context come from an internal driver table rather than
> globally accessible functions. This has in practice crashed when Mesa and
> hardware accelerated drivers are available in the same application."

Yes, but before we removed those ifdefs, it was exclusively using dlsym.

The hope is that 1.5 devices will do the right thing, and legacy devices will survive with the dlsym call like they have so far.

The more appropriate solution is to have a list of core symbols, and only use dlsym if it’s one of those and we aren’t on 1.5, but that’s a huge hassle if we can get away with not doing it.

--ryan.

Yes that's true !

Created attachment 3988 [details]
list of GLES core symbols.

(In reply to Ryan C. Gordon from comment #12)
> The more appropriate solution is to have a list of core symbols, and only
> use dlsym if it’s one of those and we aren’t on 1.5, but that’s a huge
> hassle if we can get away with not doing it.

Here's the list if we want to do this, through GLES 3.2.  358 symbols.

--ryan.

But this is too much symbols !

Only if we're <= EGL 1.4, we need to check that the name is within the set of names of <= EGL 1.4.

(there may be a lot of symbol anyway, and to be super rigorous: if we're in version 1.x (<= 4), we need to check for the set 1.x).

From src/video/khronos/EGL/egl.h  

that would be?

EGL 1.0:
eglChooseConfig
eglCopyBuffers
eglCreateContext
eglCreatePbufferSurface
eglCreatePixmapSurface
eglCreateWindowSurface
eglDestroyContext
eglDestroySurface
eglGetConfigAttrib
eglGetConfigs
eglGetCurrentDisplay
eglGetCurrentSurface
eglGetDisplay
eglGetError
eglGetProcAddress
eglInitialize
eglMakeCurrent
eglQueryContext
eglQueryString
eglQuerySurface
eglSwapBuffers
eglTerminate
eglWaitGL
eglWaitNative

1.1:
eglBindTexImage
eglReleaseTexImage
eglSurfaceAttrib
eglSwapInterval

1.2:
eglBindAPI
eglQueryAPI
eglCreatePbufferFromClientBuffer
eglReleaseThread
eglWaitClient

1.3:
nothing

1.4:
eglGetCurrentContext

(In reply to Sylvain from comment #15)
> But this is too much symbols !

It's only the EGL symbols themselves we're talking about? I assumed it meant "core" GLES (glActiveTexture, etc, as opposed to GLES extensions).

--ryan.

Oops, I'm not sure, you're know better *GL* than me and are probably right!
But re-reading:

eglGetProcAddress — return a GL or an EGL extension function 

https://www.khronos.org/registry/EGL/sdk/docs/man/html/eglGetProcAddress.xhtml

Because of the note:
"If the EGL version is not 1.5 or greater, only queries of EGL and client API extension functions will succeed."


In fact, for EGL < 1.5, it only allows: 
- "EGL" functions
and
- "client API extensions" function

So core EGL functions are also ok ?! So we could just check for that the prefix is "egl" ?!

Re-reading, I think you are totally correct :)

Closing the issue because this is fixed as we could.
I removed also the android busted comment because this was solved almost 10 years ago ...
and it's not so much related (glGetString finding extension that eglGetProcAddress could not resolved) 
https://issuetracker.google.com/issues/36916350
https://hg.libsdl.org/SDL/rev/fdf2fe9ca0b1