[Cogl] [PATCH 1/2] Don't use eglGetProcAddress to retrieve core functions

Neil Roberts neil at linux.intel.com
Wed Jun 20 08:54:05 PDT 2012


According to the EGL spec, eglGetProcAddress should only be used to
retrieve extension functions. It also says that returning non-NULL
does not mean the extension is available so you could interpret this
as saying that the function is allowed to return garbage for core
functions. This seems to happen at least for the Android
implementation of EGL.

To workaround this the winsys's are now passed down a flag to say
whether the function is from the core API. This information is already
in the gl-prototypes headers as the minimum core GL version and as a
pair of flags to specify whether it is available in core GLES1 and
GLES2. If the function is in core the EGL winsys will now avoid using
eglGetProcAddress and always fallback to querying the library directly
with the GModule API.

The GLX winsys is left alone because glXGetProcAddress apparently
supports querying core API and extension functions.

The WGL winsys could ideally be changed because wglGetProcAddress
should also only be used for extension functions but the situation is
slightly different because WGL considers anything from GL > 1.1 to be
an extension so it would need a bit more information to determine
whether to query the function directly from the library.

The SDL winsys is also left alone because it's not as easy to portably
determine which GL library SDL has chosen to load in order to resolve
the symbols directly.
---
 cogl/cogl-feature-private.c       |   12 ++++++++++--
 cogl/cogl-renderer-private.h      |    3 ++-
 cogl/cogl-renderer.c              |    5 +++--
 cogl/driver/gl/cogl-gl.c          |    3 ++-
 cogl/driver/gles/cogl-gles.c      |    3 ++-
 cogl/winsys/cogl-winsys-egl.c     |    8 +++++---
 cogl/winsys/cogl-winsys-glx.c     |    7 ++++++-
 cogl/winsys/cogl-winsys-private.h |    3 ++-
 cogl/winsys/cogl-winsys-sdl.c     |   10 ++++++++++
 cogl/winsys/cogl-winsys-sdl2.c    |   12 +++++++++++-
 cogl/winsys/cogl-winsys-stub.c    |    3 ++-
 cogl/winsys/cogl-winsys-wgl.c     |   12 ++++++++++--
 12 files changed, 65 insertions(+), 16 deletions(-)

diff --git a/cogl/cogl-feature-private.c b/cogl/cogl-feature-private.c
index e6f0bd3..7883b3c 100644
--- a/cogl/cogl-feature-private.c
+++ b/cogl/cogl-feature-private.c
@@ -45,6 +45,7 @@ _cogl_feature_check (CoglRenderer *renderer,
 {
   const char *suffix = NULL;
   int func_num;
+  CoglBool in_core;
 
   /* First check whether the functions should be directly provided by
      GL */
@@ -55,7 +56,10 @@ _cogl_feature_check (CoglRenderer *renderer,
        (data->gles_availability & COGL_EXT_IN_GLES)) ||
       (driver == COGL_DRIVER_GLES2 &&
        (data->gles_availability & COGL_EXT_IN_GLES2)))
-    suffix = "";
+    {
+      suffix = "";
+      in_core = TRUE;
+    }
   else
     {
       /* Otherwise try all of the extensions */
@@ -107,6 +111,8 @@ _cogl_feature_check (CoglRenderer *renderer,
               break;
             }
         }
+
+      in_core = FALSE;
     }
 
   /* If we couldn't find anything that provides the functions then
@@ -122,7 +128,9 @@ _cogl_feature_check (CoglRenderer *renderer,
 
       full_function_name = g_strconcat (data->functions[func_num].name,
                                         suffix, NULL);
-      func = _cogl_renderer_get_proc_address (renderer, full_function_name);
+      func = _cogl_renderer_get_proc_address (renderer,
+                                              full_function_name,
+                                              in_core);
       g_free (full_function_name);
 
       if (func == NULL)
diff --git a/cogl/cogl-renderer-private.h b/cogl/cogl-renderer-private.h
index 808102c..72ea9d0 100644
--- a/cogl/cogl-renderer-private.h
+++ b/cogl/cogl-renderer-private.h
@@ -92,6 +92,7 @@ _cogl_renderer_remove_native_filter (CoglRenderer *renderer,
 
 void *
 _cogl_renderer_get_proc_address (CoglRenderer *renderer,
-                                 const char *name);
+                                 const char *name,
+                                 CoglBool in_core);
 
 #endif /* __COGL_RENDERER_PRIVATE_H */
diff --git a/cogl/cogl-renderer.c b/cogl/cogl-renderer.c
index fa84625..82e45b6 100644
--- a/cogl/cogl-renderer.c
+++ b/cogl/cogl-renderer.c
@@ -509,11 +509,12 @@ cogl_renderer_get_winsys_id (CoglRenderer *renderer)
 
 void *
 _cogl_renderer_get_proc_address (CoglRenderer *renderer,
-                                 const char *name)
+                                 const char *name,
+                                 CoglBool in_core)
 {
   const CoglWinsysVtable *winsys = _cogl_renderer_get_winsys (renderer);
 
-  return winsys->renderer_get_proc_address (renderer, name);
+  return winsys->renderer_get_proc_address (renderer, name, in_core);
 }
 
 int
diff --git a/cogl/driver/gl/cogl-gl.c b/cogl/driver/gl/cogl-gl.c
index bc1e5ec..3976d5a 100644
--- a/cogl/driver/gl/cogl-gl.c
+++ b/cogl/driver/gl/cogl-gl.c
@@ -321,7 +321,8 @@ _cogl_driver_update_features (CoglContext *ctx,
      can expect */
   ctx->glGetString =
     (void *) _cogl_renderer_get_proc_address (ctx->display->renderer,
-                                              "glGetString");
+                                              "glGetString",
+                                              TRUE);
 
   if (!check_gl_version (ctx, error))
     return FALSE;
diff --git a/cogl/driver/gles/cogl-gles.c b/cogl/driver/gles/cogl-gles.c
index b570fea..9e46d12 100644
--- a/cogl/driver/gles/cogl-gles.c
+++ b/cogl/driver/gles/cogl-gles.c
@@ -171,7 +171,8 @@ _cogl_driver_update_features (CoglContext *context,
      can expect */
   context->glGetString =
     (void *) _cogl_renderer_get_proc_address (context->display->renderer,
-                                              "glGetString");
+                                              "glGetString",
+                                              TRUE);
 
   COGL_NOTE (WINSYS,
              "Checking features\n"
diff --git a/cogl/winsys/cogl-winsys-egl.c b/cogl/winsys/cogl-winsys-egl.c
index 871eea1..17ed67b 100644
--- a/cogl/winsys/cogl-winsys-egl.c
+++ b/cogl/winsys/cogl-winsys-egl.c
@@ -118,11 +118,13 @@ get_error_string (void)
 
 static CoglFuncPtr
 _cogl_winsys_renderer_get_proc_address (CoglRenderer *renderer,
-                                        const char *name)
+                                        const char *name,
+                                        CoglBool in_core)
 {
-  void *ptr;
+  void *ptr = NULL;
 
-  ptr = eglGetProcAddress (name);
+  if (!in_core)
+    ptr = eglGetProcAddress (name);
 
   /* eglGetProcAddress doesn't support fetching core API so we need to
      get that separately with GModule */
diff --git a/cogl/winsys/cogl-winsys-glx.c b/cogl/winsys/cogl-winsys-glx.c
index 6650106..c733ec4 100644
--- a/cogl/winsys/cogl-winsys-glx.c
+++ b/cogl/winsys/cogl-winsys-glx.c
@@ -129,10 +129,15 @@ static const CoglFeatureData winsys_feature_data[] =
 
 static CoglFuncPtr
 _cogl_winsys_renderer_get_proc_address (CoglRenderer *renderer,
-                                        const char *name)
+                                        const char *name,
+                                        CoglBool in_core)
 {
   CoglGLXRenderer *glx_renderer = renderer->winsys;
 
+  /* The GLX_ARB_get_proc_address extension documents that this should
+   * work for core functions too so we don't need to do anything
+   * special with in_core */
+
   return glx_renderer->glXGetProcAddress ((const GLubyte *) name);
 }
 
diff --git a/cogl/winsys/cogl-winsys-private.h b/cogl/winsys/cogl-winsys-private.h
index cd9ca2e..a39cbd6 100644
--- a/cogl/winsys/cogl-winsys-private.h
+++ b/cogl/winsys/cogl-winsys-private.h
@@ -70,7 +70,8 @@ typedef struct _CoglWinsysVtable
 
   CoglFuncPtr
   (*renderer_get_proc_address) (CoglRenderer *renderer,
-                                const char *name);
+                                const char *name,
+                                CoglBool in_core);
 
   CoglBool
   (*renderer_connect) (CoglRenderer *renderer, GError **error);
diff --git a/cogl/winsys/cogl-winsys-sdl.c b/cogl/winsys/cogl-winsys-sdl.c
index 58c177a..39843fb 100644
--- a/cogl/winsys/cogl-winsys-sdl.c
+++ b/cogl/winsys/cogl-winsys-sdl.c
@@ -54,6 +54,16 @@ static CoglFuncPtr
 _cogl_winsys_renderer_get_proc_address (CoglRenderer *renderer,
                                         const char *name)
 {
+  /* XXX: It's not totally clear whether it's safe to call this for
+   * core functions. From the code it looks like the implementations
+   * will fall back to using some form of dlsym if the winsys
+   * GetProcAddress function returns NULL. Presumably this will work
+   * in most cases apart from EGL platforms that return invalid
+   * pointers for core functions. It's awkward for this code to get a
+   * handle to the GL module that SDL has chosen to load so just
+   * calling SDL_GL_GetProcAddress is probably the best we can do
+   * here. */
+
 #ifdef COGL_HAS_SDL_GLES_SUPPORT
   if (renderer->driver != COGL_DRIVER_GL)
     return SDL_GLES_GetProcAddress (name);
diff --git a/cogl/winsys/cogl-winsys-sdl2.c b/cogl/winsys/cogl-winsys-sdl2.c
index 44843ad..f9b94e9 100644
--- a/cogl/winsys/cogl-winsys-sdl2.c
+++ b/cogl/winsys/cogl-winsys-sdl2.c
@@ -61,8 +61,18 @@ typedef struct _CoglOnscreenSdl2
 
 static CoglFuncPtr
 _cogl_winsys_renderer_get_proc_address (CoglRenderer *renderer,
-                                        const char *name)
+                                        const char *name,
+                                        CoglBool in_core)
 {
+  /* XXX: It's not totally clear whether it's safe to call this for
+   * core functions. From the code it looks like the implementations
+   * will fall back to using some form of dlsym if the winsys
+   * GetProcAddress function returns NULL. Presumably this will work
+   * in most cases apart from EGL platforms that return invalid
+   * pointers for core functions. It's awkward for this code to get a
+   * handle to the GL module that SDL has chosen to load so just
+   * calling SDL_GL_GetProcAddress is probably the best we can do
+   * here. */
   return SDL_GL_GetProcAddress (name);
 }
 
diff --git a/cogl/winsys/cogl-winsys-stub.c b/cogl/winsys/cogl-winsys-stub.c
index 02ebb8b..2648622 100644
--- a/cogl/winsys/cogl-winsys-stub.c
+++ b/cogl/winsys/cogl-winsys-stub.c
@@ -47,7 +47,8 @@ static int _cogl_winsys_stub_dummy_ptr;
 
 static CoglFuncPtr
 _cogl_winsys_renderer_get_proc_address (CoglRenderer *renderer,
-                                        const char *name)
+                                        const char *name,
+                                        CoglBool in_core)
 {
   static GModule *module = NULL;
 
diff --git a/cogl/winsys/cogl-winsys-wgl.c b/cogl/winsys/cogl-winsys-wgl.c
index 7851404..61cd335 100644
--- a/cogl/winsys/cogl-winsys-wgl.c
+++ b/cogl/winsys/cogl-winsys-wgl.c
@@ -127,14 +127,22 @@ static const CoglFeatureData winsys_feature_data[] =
 
 static CoglFuncPtr
 _cogl_winsys_renderer_get_proc_address (CoglRenderer *renderer,
-                                        const char *name)
+                                        const char *name,
+                                        CoglBool in_core)
 {
   CoglRendererWgl *wgl_renderer = renderer->winsys;
   void *proc = wglGetProcAddress ((LPCSTR) name);
 
   /* The documentation for wglGetProcAddress implies that it only
      returns pointers to extension functions so if it fails we'll try
-     resolving the symbol directly from the the GL library */
+     resolving the symbol directly from the the GL library. We could
+     completely avoid using wglGetProcAddress if in_core is TRUE but
+     on WGL any function that is in GL > 1.1 is considered an
+     extension and is not directly exported from opengl32.dll.
+     Therefore we currently just assume wglGetProcAddress will return
+     NULL for GL 1.1 functions and we can fallback to querying them
+     directly from the library */
+
   if (proc == NULL)
     {
       if (wgl_renderer->gl_module == NULL)
-- 
1.7.3.16.g9464b



More information about the Cogl mailing list