GNOME.org
 

[cogl/wip/rib/cogl-1.12: 90/139] Don't use eglGetProcAddress to retrieve core functions

commit df77e8565e9822bfcdeb88b380f64f0f303aaef6
Author: Neil Roberts <neil linux intel com>
Date:   Wed Jun 20 12:42:31 2012 +0100

    Don't use eglGetProcAddress to retrieve core functions
    
    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.
    
    Reviewed-by: Robert Bragg <robert linux intel com>
    
    (cherry picked from commit 72089730ad06ccdd38a344279a893965ae68cec1)
    
      Since we aren't able to break API on the 1.12 branch
      cogl_get_proc_address is still supported but isn't easily able to
      determine whether the given name corresponds to a core symbol or
      not.  For now we just assume the symbol being queried isn't part
      of the core GL api and update the documentation accordingly.

 cogl/cogl-feature-private.c       |   12 ++++++++++--
 cogl/cogl-renderer-private.h      |    3 ++-
 cogl/cogl-renderer.c              |    5 +++--
 cogl/cogl.c                       |    2 +-
 cogl/cogl1-context.h              |    4 ++++
 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 ++++++++++--
 14 files changed, 70 insertions(+), 17 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/cogl.c b/cogl/cogl.c
index a1fa5a7..6ab38c5 100644
--- a/cogl/cogl.c
+++ b/cogl/cogl.c
@@ -94,7 +94,7 @@ cogl_get_proc_address (const char* name)
 {
   _COGL_GET_CONTEXT (ctx, NULL);
 
-  return _cogl_renderer_get_proc_address (ctx->display->renderer, name);
+  return _cogl_renderer_get_proc_address (ctx->display->renderer, name, FALSE);
 }
 
 CoglBool
diff --git a/cogl/cogl1-context.h b/cogl/cogl1-context.h
index 49744ac..f9cf946 100644
--- a/cogl/cogl1-context.h
+++ b/cogl/cogl1-context.h
@@ -88,6 +88,10 @@ cogl_features_available (CoglFeatureFlags features);
  * as a wrapper around glXGetProcAddress() or whatever is the
  * appropriate function for the current backend.
  *
+ * <note>This function should not be used to query core opengl API
+ * symbols since eglGetProcAddress for example doesn't allow this and
+ * and may return a junk pointer if you do.</note>
+ *
  * Return value: a pointer to the requested function or %NULL if the
  *   function is not available.
  */
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 289bc80..800ebce 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)
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]