Combine _cogl_context_check_gl_version and update_features into one

The _cogl_context_check_gl_version function is meant to be called once
Cogl has a GL context so that it can check whether the context found
is supported by Cogl. However, only the stub winsys was calling this
and it was doing it before Cogl had a chance to retrieve the function
pointer for glString so it would just crash. This patch combines the
two functions into one so that _cogl_context_update_features returns a
gboolean and a GError. Then it can just check the context itself.

https://bugzilla.gnome.org/show_bug.cgi?id=654440

Reviewed-by: Robert Bragg <robert@linux.intel.com>

cogl/cogl-context-private.h

@@ -288,17 +288,15 @@ _cogl_context_get_default ();
 const CoglWinsysVtable *
 _cogl_context_get_winsys (CoglContext *context);
 
-/* Check whether the current GL context is supported by Cogl */
-gboolean
-_cogl_context_check_gl_version (CoglContext *context,
-                                GError **error);
-
 /* Query the GL extensions and lookup the corresponding function
  * pointers. Theoretically the list of extensions can change for
  * different GL contexts so it is the winsys backend's responsiblity
- * to know when to re-query the GL extensions. */
+ * to know when to re-query the GL extensions. The backend should also
-void
+ * check whether the GL context is supported by Cogl. If not it should
-_cogl_context_update_features (CoglContext *context);
+ * return FALSE and set @error */
+gboolean
+_cogl_context_update_features (CoglContext *context,
+                               GError **error);
 
 /* Obtains the context and returns retval if NULL */
 #define _COGL_GET_CONTEXT(ctxvar, retval) \

cogl/cogl-context.c

@@ -509,35 +509,16 @@ cogl_egl_context_get_egl_display (CoglContext *context)
 #endif
 
 gboolean
-_cogl_context_check_gl_version (CoglContext *context,
+_cogl_context_update_features (CoglContext *context,
-                                GError **error)
+                               GError **error)
 {
 #ifdef HAVE_COGL_GL
   if (context->driver == COGL_DRIVER_GL)
-    return _cogl_gl_check_gl_version (context, error);
+    return _cogl_gl_update_features (context, error);
 #endif
 
 #if defined(HAVE_COGL_GLES) || defined(HAVE_COGL_GLES2)
-  return _cogl_gles_check_gl_version (context, error);
+  return _cogl_gles_update_features (context, error);
-#endif
-
-  g_assert_not_reached ();
-}
-
-void
-_cogl_context_update_features (CoglContext *context)
-{
-#ifdef HAVE_COGL_GL
-  if (context->driver == COGL_DRIVER_GL)
-    {
-      _cogl_gl_update_features (context);
-      return;
-    }
-#endif
-
-#if defined(HAVE_COGL_GLES) || defined(HAVE_COGL_GLES2)
-  _cogl_gles_update_features (context);
-  return;
 #endif
 
   g_assert_not_reached ();

cogl/cogl-private.h

@@ -27,18 +27,12 @@
 G_BEGIN_DECLS
 
 gboolean
-_cogl_gl_check_gl_version (CoglContext *context,
+_cogl_gl_update_features (CoglContext *context,
-                           GError **error);
+                          GError **error);
-
-void
-_cogl_gl_update_features (CoglContext *context);
 
 gboolean
-_cogl_gles_check_gl_version (CoglContext *context,
+_cogl_gles_update_features (CoglContext *context,
-                             GError **error);
+                            GError **error);
-
-void
-_cogl_gles_update_features (CoglContext *context);
 
 gboolean
 _cogl_check_extension (const char *name, const char *ext);

cogl/driver/gl/cogl-gl.c

@@ -71,9 +71,9 @@ _cogl_get_gl_version (int *major_out, int *minor_out)
   return TRUE;
 }
 
-gboolean
+static gboolean
-_cogl_gl_check_gl_version (CoglContext *ctx,
+check_gl_version (CoglContext *ctx,
-                           GError **error)
+                  GError **error)
 {
   int major, minor;
   const char *gl_extensions;
@@ -120,12 +120,9 @@ _cogl_gl_check_gl_version (CoglContext *ctx,
   return TRUE;
 }
 
-/* Query the GL extensions and lookup the corresponding function
+gboolean
- * pointers. Theoretically the list of extensions can change for
+_cogl_gl_update_features (CoglContext *context,
- * different GL contexts so it is the winsys backend's responsiblity
+                          GError **error)
- * to know when to re-query the GL extensions. */
-void
-_cogl_gl_update_features (CoglContext *context)
 {
   CoglPrivateFeatureFlags private_flags = 0;
   CoglFeatureFlags flags = 0;
@@ -134,7 +131,7 @@ _cogl_gl_update_features (CoglContext *context)
   int num_stencil_bits = 0;
   int gl_major = 0, gl_minor = 0;
 
-  _COGL_GET_CONTEXT (ctx, NO_RETVAL);
+  _COGL_GET_CONTEXT (ctx, FALSE);
 
   /* We have to special case getting the pointer to the glGetString
      function because we need to use it to determine what functions we
@@ -143,6 +140,9 @@ _cogl_gl_update_features (CoglContext *context)
     (void *) _cogl_get_proc_address (_cogl_context_get_winsys (context),
                                      "glGetString");
 
+  if (!check_gl_version (context, error))
+    return FALSE;
+
   COGL_NOTE (WINSYS,
              "Checking features\n"
              "  GL_VENDOR: %s\n"
@@ -231,4 +231,6 @@ _cogl_gl_update_features (CoglContext *context)
   /* Cache features */
   context->private_feature_flags |= private_flags;
   context->feature_flags |= flags;
+
+  return TRUE;
 }

cogl/driver/gles/cogl-gles.c

@@ -33,18 +33,8 @@
 #include "cogl-feature-private.h"
 
 gboolean
-_cogl_gles_check_gl_version (GError **error)
+_cogl_gles_update_features (CoglContext *context,
-{
+                            GError **error)
-  /* The GLES backend doesn't have any particular version requirements */
-  return TRUE;
-}
-
-/* Query the GL extensions and lookup the corresponding function
- * pointers. Theoretically the list of extensions can change for
- * different GL contexts so it is the winsys backend's responsiblity
- * to know when to re-query the GL extensions. */
-void
-_cogl_gles_update_features (CoglContext *context)
 {
   CoglPrivateFeatureFlags private_flags = 0;
   CoglFeatureFlags flags = 0;
@@ -135,4 +125,6 @@ _cogl_gles_update_features (CoglContext *context)
   /* Cache features */
   context->private_feature_flags |= private_flags;
   context->feature_flags |= flags;
+
+  return TRUE;
 }

cogl/winsys/cogl-winsys-egl.c

@@ -503,19 +503,20 @@ error:
   return FALSE;
 }
 
-static void
+static gboolean
-update_winsys_features (CoglContext *context)
+update_winsys_features (CoglContext *context, GError **error)
 {
   CoglDisplayEGL *egl_display = context->display->winsys;
   CoglRendererEGL *egl_renderer = context->display->renderer->winsys;
 
-  g_return_if_fail (egl_display->egl_context);
+  g_return_val_if_fail (egl_display->egl_context, FALSE);
 
   memset (context->winsys_features, 0, sizeof (context->winsys_features));
 
   check_egl_extensions (context->display->renderer);
 
-  _cogl_context_update_features (context);
+  if (!_cogl_context_update_features (context, error))
+    return FALSE;
 
 #if defined (COGL_HAS_EGL_PLATFORM_POWERVR_X11_SUPPORT) || \
     defined (COGL_HAS_EGL_PLATFORM_WAYLAND_SUPPORT)
@@ -532,6 +533,8 @@ update_winsys_features (CoglContext *context)
       COGL_FLAGS_SET (context->winsys_features,
                       COGL_WINSYS_FEATURE_SWAP_REGION_THROTTLE, TRUE);
     }
+
+  return TRUE;
 }
 
 #ifdef COGL_HAS_EGL_PLATFORM_POWERVR_X11_SUPPORT
@@ -1114,9 +1117,7 @@ _cogl_winsys_context_init (CoglContext *context, GError **error)
                                  event_filter_cb,
                                  context);
 #endif
-  update_winsys_features (context);
+  return update_winsys_features (context, error);
-
-  return TRUE;
 }
 
 static void

cogl/winsys/cogl-winsys-glx.c

@@ -395,8 +395,8 @@ error:
   return FALSE;
 }
 
-static void
+static gboolean
-update_winsys_features (CoglContext *context)
+update_winsys_features (CoglContext *context, GError **error)
 {
   CoglGLXDisplay *glx_display = context->display->winsys;
   CoglXlibRenderer *xlib_renderer = context->display->renderer->winsys;
@@ -405,9 +405,10 @@ update_winsys_features (CoglContext *context)
   int default_screen;
   int i;
 
-  g_return_if_fail (glx_display->glx_context);
+  g_return_val_if_fail (glx_display->glx_context, FALSE);
 
-  _cogl_context_update_features (context);
+  if (!_cogl_context_update_features (context, error))
+    return FALSE;
 
   memset (context->winsys_features, 0, sizeof (context->winsys_features));
 
@@ -475,6 +476,8 @@ update_winsys_features (CoglContext *context)
       _cogl_winsys_has_feature (COGL_WINSYS_FEATURE_VBLANK_WAIT))
     COGL_FLAGS_SET (context->winsys_features,
                     COGL_WINSYS_FEATURE_SWAP_REGION_THROTTLE, TRUE);
+
+  return TRUE;
 }
 
 /* It seems the GLX spec never defined an invalid GLXFBConfig that
@@ -763,9 +766,7 @@ _cogl_winsys_context_init (CoglContext *context, GError **error)
   cogl_xlib_renderer_add_filter (context->display->renderer,
                                  glx_event_filter_cb,
                                  context);
-  update_winsys_features (context);
+  return update_winsys_features (context, error);
-
-  return TRUE;
 }
 
 static void

cogl/winsys/cogl-winsys-stub.c

@@ -83,11 +83,9 @@ _cogl_winsys_context_init (CoglContext *context, GError **error)
 {
   context->winsys = &_cogl_winsys_stub_dummy_ptr;
 
-  if (!_cogl_context_check_gl_version (context, error))
+  if (!_cogl_context_update_features (context, error))
     return FALSE;
 
-  _cogl_context_update_features (context);
-
   memset (context->winsys_features, 0, sizeof (context->winsys_features));
 
   return TRUE;

cogl/winsys/cogl-winsys-wgl.c

@@ -542,17 +542,18 @@ get_wgl_extensions_string (HDC dc)
   return NULL;
 }
 
-static void
+static gboolean
-update_winsys_features (CoglContext *context)
+update_winsys_features (CoglContext *context, GError **error)
 {
   CoglDisplayWgl *wgl_display = context->display->winsys;
   CoglRendererWgl *wgl_renderer = context->display->renderer->winsys;
   const char *wgl_extensions;
   int i;
 
-  g_return_if_fail (wgl_display->wgl_context);
+  g_return_val_if_fail (wgl_display->wgl_context, FALSE);
 
-  _cogl_context_update_features (context);
+  if (!_cogl_context_update_features (context, error))
+    return FALSE;
 
   memset (context->winsys_features, 0, sizeof (context->winsys_features));
 
@@ -581,6 +582,8 @@ update_winsys_features (CoglContext *context)
                               TRUE);
           }
     }
+
+  return TRUE;
 }
 
 static gboolean
@@ -594,9 +597,7 @@ _cogl_winsys_context_init (CoglContext *context, GError **error)
                                   win32_event_filter_cb,
                                   context);
 
-  update_winsys_features (context);
+  return update_winsys_features (context, error);
-
-  return TRUE;
 }
 
 static void