[gtk/wip/otte/gleanup: 41/73] x11: Rework Visual selection

[Chun-wei Fan <fanchunwei src gnome org>]

commit 5a96c4dc7794132e72a92410d6732337f715d88a
Author: Benjamin Otte <otte redhat com>
Date:   Sun Jun 13 17:34:55 2021 +0200

    x11: Rework Visual selection
    
    Instead of going via GdkVisual, doing a preselection and letting the GL
    initialization improve it, let the GL initialization pick an X Visual
    directly using X Visual code directly.
    
    The code should select the same visuals as before as it tries to apply
    the same logic, but it's a rewrite, so I expect I messed something up.

 gdk/x11/gdkdisplay-x11.c   |  63 ++++++++----
 gdk/x11/gdkdisplay-x11.h   |   3 +
 gdk/x11/gdkglcontext-egl.c |   6 +-
 gdk/x11/gdkglcontext-glx.c | 233 +++++++++++++++++----------------------------
 gdk/x11/gdkglcontext-x11.c |  18 ++--
 gdk/x11/gdkglcontext-x11.h |  12 ++-
 6 files changed, 160 insertions(+), 175 deletions(-)
---
diff --git a/gdk/x11/gdkdisplay-x11.c b/gdk/x11/gdkdisplay-x11.c
index de0a014146..334f33eb6f 100644
--- a/gdk/x11/gdkdisplay-x11.c
+++ b/gdk/x11/gdkdisplay-x11.c
@@ -1339,6 +1339,40 @@ set_sm_client_id (GdkDisplay  *display,
                      gdk_x11_get_xatom_by_name_for_display (display, "SM_CLIENT_ID"));
 }
 
+void
+gdk_x11_display_query_default_visual (GdkX11Display  *self,
+                                      Visual        **out_visual,
+                                      int            *out_depth)
+{
+  XVisualInfo template, *visinfo;
+  int n_visuals;
+  Display *dpy;
+
+  dpy = gdk_x11_display_get_xdisplay (GDK_DISPLAY (self));
+
+  template.screen = self->screen->screen_num;
+  template.depth = 32;
+  template.red_mask  = 0xff0000;
+  template.green_mask = 0x00ff00;
+  template.blue_mask = 0x0000ff;
+
+  visinfo = XGetVisualInfo (dpy,
+                            VisualScreenMask | VisualDepthMask
+                            | VisualRedMaskMask | VisualGreenMaskMask | VisualBlueMaskMask,
+                            &template,
+                            &n_visuals);
+  if (visinfo != NULL)
+    {
+      *out_visual = visinfo[0].visual;
+      *out_depth = visinfo[0].depth;
+      XFree (visinfo);
+      return;
+    }
+
+  *out_visual = DefaultVisual (dpy, self->screen->screen_num);
+  *out_depth = DefaultDepth (dpy, self->screen->screen_num);
+}
+
 /**
  * gdk_x11_display_open:
  * @display_name: (nullable): name of the X display.
@@ -1410,27 +1444,14 @@ gdk_x11_display_open (const char *display_name)
    * as we care about GLX details such as alpha/depth/stencil depth,
    * stereo and double buffering
    */
-  gdk_x11_display_init_gl (display_x11);
-
-  if (display_x11->screen->rgba_visual)
-    {
-      Visual *xvisual = GDK_X11_VISUAL (display_x11->screen->rgba_visual)->xvisual;
-
-      display_x11->window_depth = display_x11->screen->rgba_visual->depth;
-      display_x11->window_visual = xvisual;
-      display_x11->window_colormap = XCreateColormap (xdisplay,
-                                                      DefaultRootWindow (xdisplay),
-                                                      xvisual,
-                                                      AllocNone);
-      gdk_display_set_rgba (display, TRUE);
-    }
-  else
-    {
-      display_x11->window_depth = DefaultDepth (xdisplay, DefaultScreen (xdisplay)),
-      display_x11->window_visual = DefaultVisual (xdisplay, DefaultScreen (xdisplay));
-      display_x11->window_colormap = DefaultColormap (xdisplay, DefaultScreen (xdisplay));
-      gdk_display_set_rgba (display, FALSE);
-    }
+  if (!gdk_x11_display_init_gl (display_x11, &display_x11->window_visual, &display_x11->window_depth))
+    gdk_x11_display_query_default_visual (display_x11, &display_x11->window_visual, 
&display_x11->window_depth);
+
+  display_x11->window_colormap = XCreateColormap (xdisplay,
+                                                  DefaultRootWindow (xdisplay),
+                                                  display_x11->window_visual,
+                                                  AllocNone);
+  gdk_display_set_rgba (display, display_x11->window_depth == 32);
 
   /* We need to initialize events after we have the screen
    * structures in places
diff --git a/gdk/x11/gdkdisplay-x11.h b/gdk/x11/gdkdisplay-x11.h
index 977c4ac4ac..7ef252f0bd 100644
--- a/gdk/x11/gdkdisplay-x11.h
+++ b/gdk/x11/gdkdisplay-x11.h
@@ -177,6 +177,9 @@ struct _GdkX11DisplayClass
                                                                  const XEvent           *event);
 };
 
+void            gdk_x11_display_query_default_visual            (GdkX11Display          *self,
+                                                                 Visual                **out_visual,
+                                                                 int                    *out_depth);
 void            _gdk_x11_display_error_event                    (GdkDisplay             *display,
                                                                  XErrorEvent            *error);
 gsize           gdk_x11_display_get_max_request_size            (GdkDisplay             *display);
diff --git a/gdk/x11/gdkglcontext-egl.c b/gdk/x11/gdkglcontext-egl.c
index 4ecdcf3fbd..cad79305bb 100644
--- a/gdk/x11/gdkglcontext-egl.c
+++ b/gdk/x11/gdkglcontext-egl.c
@@ -593,7 +593,9 @@ gdk_x11_gl_context_egl_init (GdkX11GLContextEGL *self)
 }
 
 gboolean
-gdk_x11_display_init_egl (GdkX11Display *self)
+gdk_x11_display_init_egl (GdkX11Display  *self,
+                          Visual        **out_visual,
+                          int            *out_depth)
 {
   GdkDisplay *display = GDK_DISPLAY (self);
   Display *dpy;
@@ -664,6 +666,8 @@ gdk_x11_display_init_egl (GdkX11Display *self)
                                self->has_egl_swap_buffers_with_damage ? "yes" : "no",
                                self->has_egl_surfaceless_context ? "yes" : "no"));
 
+  gdk_x11_display_query_default_visual (self, out_visual, out_depth);
+
   return TRUE;
 }
 
diff --git a/gdk/x11/gdkglcontext-glx.c b/gdk/x11/gdkglcontext-glx.c
index 4ebc30bd89..04af52483f 100644
--- a/gdk/x11/gdkglcontext-glx.c
+++ b/gdk/x11/gdkglcontext-glx.c
@@ -836,167 +836,112 @@ out:
 
 #undef MAX_GLX_ATTRS
 
-struct glvisualinfo {
-  int supports_gl;
-  int double_buffer;
-  int stereo;
-  int alpha_size;
-  int depth_size;
-  int stencil_size;
-  int num_multisample;
-  int visual_caveat;
-};
-
-static gboolean
-visual_compatible (const GdkX11Visual *a, const GdkX11Visual *b)
-{
-  return a->type == b->type &&
-    a->depth == b->depth &&
-    a->red_mask == b->red_mask &&
-    a->green_mask == b->green_mask &&
-    a->blue_mask == b->blue_mask &&
-    a->colormap_size == b->colormap_size &&
-    a->bits_per_rgb == b->bits_per_rgb;
-}
-
 static gboolean
-visual_is_rgba (const GdkX11Visual *visual)
+visual_is_rgba (XVisualInfo *visinfo)
 {
   return
-    visual->depth == 32 &&
-    visual->red_mask   == 0xff0000 &&
-    visual->green_mask == 0x00ff00 &&
-    visual->blue_mask  == 0x0000ff;
+    visinfo->depth == 32 &&
+    visinfo->visual->red_mask   == 0xff0000 &&
+    visinfo->visual->green_mask == 0x00ff00 &&
+    visinfo->visual->blue_mask  == 0x0000ff;
 }
 
-/* This picks a compatible (as in has the same X visual details) visual
-   that has "better" characteristics on the GL side */
-static GdkX11Visual *
-pick_better_visual_for_gl (GdkX11Screen *x11_screen,
-                           struct glvisualinfo *gl_info,
-                           GdkX11Visual *compatible)
-{
-  GdkX11Visual *visual;
-  int i;
-  gboolean want_alpha = visual_is_rgba (compatible);
-
-  /* First look for "perfect match", i.e:
-   * supports gl
-   * double buffer
-   * alpha iff visual is an rgba visual
-   * no unnecessary stuff
-   */
-  for (i = 0; i < x11_screen->nvisuals; i++)
-    {
-      visual = x11_screen->visuals[i];
-      if (visual_compatible (visual, compatible) &&
-          gl_info[i].supports_gl &&
-          gl_info[i].double_buffer &&
-          !gl_info[i].stereo &&
-          (want_alpha ? (gl_info[i].alpha_size > 0) : (gl_info[i].alpha_size == 0)) &&
-          (gl_info[i].depth_size == 0) &&
-          (gl_info[i].stencil_size == 0) &&
-          (gl_info[i].num_multisample == 0) &&
-          (gl_info[i].visual_caveat == GLX_NONE_EXT))
-        return visual;
-    }
-
-  if (!want_alpha)
-    {
-      /* Next, allow alpha even if we don't want it: */
-      for (i = 0; i < x11_screen->nvisuals; i++)
-        {
-          visual = x11_screen->visuals[i];
-          if (visual_compatible (visual, compatible) &&
-              gl_info[i].supports_gl &&
-              gl_info[i].double_buffer &&
-              !gl_info[i].stereo &&
-              (gl_info[i].depth_size == 0) &&
-              (gl_info[i].stencil_size == 0) &&
-              (gl_info[i].num_multisample == 0) &&
-              (gl_info[i].visual_caveat == GLX_NONE_EXT))
-            return visual;
-        }
-    }
-
-  /* Next, allow depth and stencil buffers: */
-  for (i = 0; i < x11_screen->nvisuals; i++)
-    {
-      visual = x11_screen->visuals[i];
-      if (visual_compatible (visual, compatible) &&
-          gl_info[i].supports_gl &&
-          gl_info[i].double_buffer &&
-          !gl_info[i].stereo &&
-          (gl_info[i].num_multisample == 0) &&
-          (gl_info[i].visual_caveat == GLX_NONE_EXT))
-        return visual;
-    }
-
-  /* Next, allow multisample: */
-  for (i = 0; i < x11_screen->nvisuals; i++)
-    {
-      visual = x11_screen->visuals[i];
-      if (visual_compatible (visual, compatible) &&
-          gl_info[i].supports_gl &&
-          gl_info[i].double_buffer &&
-          !gl_info[i].stereo &&
-          (gl_info[i].visual_caveat == GLX_NONE_EXT))
-        return visual;
-    }
-
-  return compatible;
-}
-
-static void
-gdk_x11_screen_update_visuals_for_glx (GdkX11Screen *x11_screen)
+static gboolean
+gdk_x11_display_select_visual_for_glx (GdkX11Display  *display_x11,
+                                       Visual        **out_visual,
+                                       int            *out_depth)
 {
-  GdkDisplay *display;
-  GdkX11Display *display_x11;
+  GdkDisplay *display = GDK_DISPLAY (display_x11);
   Display *dpy;
-  struct glvisualinfo *gl_info;
-  int i;
+  XVisualInfo visual_template, *visual_list;
+  int i, n_visuals, best;
+  enum {
+    NO_VISUAL_FOUND,
+    WITH_MULTISAMPLING,
+    WITH_STENCIL_AND_DEPTH_BUFFER,
+    NO_ALPHA,
+    NO_ALPHA_VISUAL
+  } best_features;
 
-  display = x11_screen->display;
-  display_x11 = GDK_X11_DISPLAY (display);
   dpy = gdk_x11_display_get_xdisplay (display);
 
-  gl_info = g_new0 (struct glvisualinfo, x11_screen->nvisuals);
+  visual_template.screen = display_x11->screen->screen_num;
+  visual_list = XGetVisualInfo (dpy, VisualScreenMask, &visual_template, &n_visuals);
+  if (visual_list == NULL)
+    return FALSE;
+
+  best = -1;
+  best_features = NO_VISUAL_FOUND;
 
-  for (i = 0; i < x11_screen->nvisuals; i++)
+  for (i = 0; i < n_visuals; i++)
     {
-      XVisualInfo *visual_list;
-      XVisualInfo visual_template;
-      int nxvisuals;
-
-      visual_template.screen = x11_screen->screen_num;
-      visual_template.visualid = gdk_x11_visual_get_xvisual (x11_screen->visuals[i])->visualid;
-      visual_list = XGetVisualInfo (x11_screen->xdisplay, VisualIDMask| VisualScreenMask, &visual_template, 
&nxvisuals);
+      int tmp;
 
-      if (visual_list == NULL)
+      if (glXGetConfig (dpy, &visual_list[i], GLX_USE_GL, &tmp) != 0 || tmp != True ||
+          glXGetConfig (dpy, &visual_list[i], GLX_DOUBLEBUFFER, &tmp) != 0 || tmp != True ||
+          glXGetConfig (dpy, &visual_list[i], GLX_STEREO, &tmp) != 0 || tmp != False ||
+          (display_x11->has_glx_visual_rating &&
+           (glXGetConfig (dpy, &visual_list[i], GLX_VISUAL_CAVEAT_EXT, &tmp) != 0 || tmp != GLX_NONE_EXT)))
         continue;
 
-      glXGetConfig (dpy, &visual_list[0], GLX_USE_GL, &gl_info[i].supports_gl);
-      glXGetConfig (dpy, &visual_list[0], GLX_DOUBLEBUFFER, &gl_info[i].double_buffer);
-      glXGetConfig (dpy, &visual_list[0], GLX_STEREO, &gl_info[i].stereo);
-      glXGetConfig (dpy, &visual_list[0], GLX_ALPHA_SIZE, &gl_info[i].alpha_size);
-      glXGetConfig (dpy, &visual_list[0], GLX_DEPTH_SIZE, &gl_info[i].depth_size);
-      glXGetConfig (dpy, &visual_list[0], GLX_STENCIL_SIZE, &gl_info[i].stencil_size);
+      if (display_x11->has_glx_multisample &&
+          (glXGetConfig (dpy, &visual_list[i], GLX_SAMPLE_BUFFERS_ARB, &tmp) != 0 || tmp != 0))
+        {
+          if (best_features < WITH_MULTISAMPLING)
+            {
+              best_features = WITH_MULTISAMPLING;
+              best = i;
+            }
+          continue;
+        }
 
-      if (display_x11->has_glx_multisample)
-        glXGetConfig(dpy, &visual_list[0], GLX_SAMPLE_BUFFERS_ARB, &gl_info[i].num_multisample);
+      if (glXGetConfig (dpy, &visual_list[i], GLX_DEPTH_SIZE, &tmp) != 0 || tmp != 0 ||
+          glXGetConfig (dpy, &visual_list[i], GLX_STENCIL_SIZE, &tmp) != 0 || tmp != 0)
+        {
+          if (best_features < WITH_STENCIL_AND_DEPTH_BUFFER)
+            {
+              best_features = WITH_STENCIL_AND_DEPTH_BUFFER;
+              best = i;
+            }
+          continue;
+        }
 
-      if (display_x11->has_glx_visual_rating)
-        glXGetConfig(dpy, &visual_list[0], GLX_VISUAL_CAVEAT_EXT, &gl_info[i].visual_caveat);
-      else
-        gl_info[i].visual_caveat = GLX_NONE_EXT;
+      if (glXGetConfig (dpy, &visual_list[i], GLX_ALPHA_SIZE, &tmp) != 0 || tmp == 0)
+        {
+          if (best_features < NO_ALPHA)
+            {
+              best_features = NO_ALPHA;
+              best = i;
+            }
+          continue;
+        }
+
+      if (!visual_is_rgba (&visual_list[i]))
+        {
+          if (best_features < NO_ALPHA_VISUAL)
+            {
+              best_features = NO_ALPHA_VISUAL;
+              best = i;
+            }
+          continue;
+        }
 
+      /* everything is perfect */
+      best = i;
+      break;
+    }
+
+  if (best_features == NO_VISUAL_FOUND)
+    {
       XFree (visual_list);
+      return FALSE;
     }
 
-  if (x11_screen->rgba_visual)
-    x11_screen->rgba_visual = pick_better_visual_for_gl (x11_screen, gl_info, x11_screen->rgba_visual);
+  *out_visual = visual_list[best].visual;
+  *out_depth = visual_list[best].depth;
 
-  g_free (gl_info);
+  XFree (visual_list);
+  return TRUE;
 }
 
 GdkX11GLContext *
@@ -1122,6 +1067,8 @@ gdk_x11_display_get_glx_version (GdkDisplay *display,
 /*< private >
  * gdk_x11_display_init_glx:
  * @display_x11: an X11 display that has not been inited yet. 
+ * @out_visual: set to the Visual to be used with the returned config
+ * @out_depth: set to the depth to be used with the returned config
  *
  * Initializes the cached GLX state for the given @screen.
  *
@@ -1130,7 +1077,9 @@ gdk_x11_display_get_glx_version (GdkDisplay *display,
  * Returns: %TRUE if GLX was initialized
  */
 gboolean
-gdk_x11_display_init_glx (GdkX11Display *display_x11)
+gdk_x11_display_init_glx (GdkX11Display  *display_x11,
+                          Visual        **out_visual,
+                          int            *out_depth)
 {
   GdkDisplay *display = GDK_DISPLAY (display_x11);
   Display *dpy;
@@ -1235,7 +1184,5 @@ gdk_x11_display_init_glx (GdkX11Display *display_x11)
                      display_x11->has_glx_multisample ? "yes" : "no",
                      display_x11->has_glx_visual_rating ? "yes" : "no"));
 
-  gdk_x11_screen_update_visuals_for_glx (display_x11->screen);
-
-  return TRUE;
+  return gdk_x11_display_select_visual_for_glx (display_x11, out_visual, out_depth);
 }
diff --git a/gdk/x11/gdkglcontext-x11.c b/gdk/x11/gdkglcontext-x11.c
index 58a6c11ca4..b493c34e29 100644
--- a/gdk/x11/gdkglcontext-x11.c
+++ b/gdk/x11/gdkglcontext-x11.c
@@ -103,22 +103,26 @@ gdk_x11_display_make_gl_context_current (GdkDisplay   *display,
   return FALSE;
 }
 
-void
-gdk_x11_display_init_gl (GdkX11Display *self)
+gboolean
+gdk_x11_display_init_gl (GdkX11Display  *self,
+                         Visual        **out_visual,
+                         int            *out_depth)
 {
   GdkDisplay *display G_GNUC_UNUSED = GDK_DISPLAY (self);
 
   if (GDK_DISPLAY_DEBUG_CHECK (display, GL_DISABLE))
-    return;
+    return FALSE;
 
   if (!GDK_DISPLAY_DEBUG_CHECK (display, GL_GLX))
     {
       /* We favour EGL */
-      if (gdk_x11_display_init_egl (self))
-        return;
+      if (gdk_x11_display_init_egl (self, out_visual, out_depth))
+        return TRUE;
     }
 
-  if (gdk_x11_display_init_glx (self))
-    return;
+  if (gdk_x11_display_init_glx (self, out_visual, out_depth))
+    return TRUE;
+
+  return FALSE;
 }
 
diff --git a/gdk/x11/gdkglcontext-x11.h b/gdk/x11/gdkglcontext-x11.h
index 78377afc49..e9a349613c 100644
--- a/gdk/x11/gdkglcontext-x11.h
+++ b/gdk/x11/gdkglcontext-x11.h
@@ -60,7 +60,9 @@ struct _GdkX11GLContextClass
   void (* bind_for_frame_fence) (GdkX11GLContext *self);
 };
 
-void                    gdk_x11_display_init_gl                 (GdkX11Display *self);
+gboolean                gdk_x11_display_init_gl                 (GdkX11Display *self,
+                                                                 Visual       **out_visual,
+                                                                 int           *out_depth);
 
 GdkGLContext *          gdk_x11_surface_create_gl_context       (GdkSurface    *window,
                                                                  gboolean       attached,
@@ -76,7 +78,9 @@ gboolean                gdk_x11_display_make_gl_context_current (GdkDisplay    *
 
 typedef struct _GdkX11GLContextGLX      GdkX11GLContextGLX;
 
-gboolean                gdk_x11_display_init_glx                (GdkX11Display *display_x11);
+gboolean                gdk_x11_display_init_glx                (GdkX11Display *display_x11,
+                                                                 Visual       **out_visual,
+                                                                 int           *out_depth);
 
 GType                   gdk_x11_gl_context_glx_get_type         (void) G_GNUC_CONST;
 GdkX11GLContext *       gdk_x11_gl_context_glx_new              (GdkSurface    *surface,
@@ -94,7 +98,9 @@ gboolean                gdk_x11_gl_context_glx_make_current     (GdkDisplay    *
 
 typedef struct _GdkX11GLContextEGL      GdkX11GLContextEGL;
 
-gboolean                gdk_x11_display_init_egl                (GdkX11Display *display_x11);
+gboolean                gdk_x11_display_init_egl                (GdkX11Display *display_x11,
+                                                                 Visual       **out_visual,
+                                                                 int           *out_depth);
 void                    gdk_x11_surface_destroy_egl_surface     (GdkX11Surface *self);
 
 GType                   gdk_x11_gl_context_egl_get_type         (void) G_GNUC_CONST;