Improve color handling on colormapped displays

* src/xfns.c (select_visual): Set `visual_info' field whenever
appropriate.
(x_create_tip_frame, XDisplayCells): Don't access private fields
of Visual.

* src/xterm.c (x_color_cells, x_alloc_nearest_color_1): Use
colormap_size instead of default cell count.
(XTflash, x_bitmap_icon, x_term_init):
* src/xterm.h (struct x_display_info, FRAME_X_VISUAL_INFO): Stop
accessing private fields of Visual.

3 files changed, 44 insertions, 23 deletions

diff --git a/src/xfns.c b/src/xfns.c
index e288f797fbd..3d1fa926096 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -6605,6 +6605,7 @@ select_visual (struct x_display_info *dpyinfo)
                SSDATA (ENCODE_SYSTEM (value)));
 
       dpyinfo->visual = vinfo.visual;
+      dpyinfo->visual_info = vinfo;
     }
   else
     {
@@ -6638,6 +6639,7 @@ select_visual (struct x_display_info *dpyinfo)
                 {
                   dpyinfo->n_planes = vinfo[i].depth;
                   dpyinfo->visual = vinfo[i].visual;
+                  dpyinfo->visual_info = vinfo[i];
                   dpyinfo->pict_format = format;
 
                   XFree (vinfo);
@@ -6658,7 +6660,7 @@ select_visual (struct x_display_info *dpyinfo)
                               &vinfo_template, &n_visuals);
       if (n_visuals <= 0)
         fatal ("Can't get proper X visual info");
-
+      dpyinfo->visual_info = *vinfo;
       dpyinfo->n_planes = vinfo->depth;
       XFree (vinfo);
     }
@@ -7540,8 +7542,8 @@ x_create_tip_frame (struct x_display_info *dpyinfo, Lisp_Object parms)
 
     if (FRAME_DISPLAY_INFO (f)->n_planes == 1)
       disptype = Qmono;
-    else if (FRAME_DISPLAY_INFO (f)->visual->class == GrayScale
-             || FRAME_DISPLAY_INFO (f)->visual->class == StaticGray)
+    else if (FRAME_X_VISUAL_INFO (f)->class == GrayScale
+             || FRAME_X_VISUAL_INFO (f)->class == StaticGray)
       disptype = intern ("grayscale");
     else
       disptype = intern ("color");
@@ -9019,7 +9021,15 @@ XkbFreeNames (XkbDescPtr xkb, unsigned int which, Bool free_map)
 int
 XDisplayCells (Display *dpy, int screen_number)
 {
-  return 1677216;
+  struct x_display_info *dpyinfo = x_display_info_for_display (dpy);
+
+  if (!dpyinfo)
+    emacs_abort ();
+
+  /* Not strictly correct, since the display could be using a
+     non-default visual, but it satisfies the callers we need to care
+     about.  */
+  return dpyinfo->visual_info.colormap_size;
 }
 #endif
 
diff --git a/src/xterm.c b/src/xterm.c
index 30229c45a47..6682d3c9a46 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -3641,8 +3641,7 @@ x_color_cells (Display *dpy, int *ncells)
 
   if (dpyinfo->color_cells == NULL)
     {
-      Screen *screen = dpyinfo->screen;
-      int ncolor_cells = XDisplayCells (dpy, XScreenNumberOfScreen (screen));
+      int ncolor_cells = dpyinfo->visual_info.colormap_size;
       int i;
 
       dpyinfo->color_cells = xnmalloc (ncolor_cells,
@@ -3841,7 +3840,7 @@ x_alloc_nearest_color_1 (Display *dpy, Colormap cmap, XColor *color)
   eassume (dpyinfo);
   rc = XAllocColor (dpy, cmap, color) != 0;
 
-  if (dpyinfo->visual->class == DirectColor)
+  if (dpyinfo->visual_info.class == DirectColor)
     return rc;
 
   if (rc == 0)
@@ -3900,7 +3899,7 @@ x_alloc_nearest_color_1 (Display *dpy, Colormap cmap, XColor *color)
           retry = true;
           xfree (dpyinfo->color_cells);
 
-          ncolor_cells = XDisplayCells (dpy, XScreenNumberOfScreen (dpyinfo->screen));
+          ncolor_cells = dpyinfo->visual_info.colormap_size;
 
           dpyinfo->color_cells = xnmalloc (ncolor_cells,
                                            sizeof *dpyinfo->color_cells);
@@ -5735,7 +5734,7 @@ XTflash (struct frame *f)
 
   block_input ();
 
-  if (FRAME_X_VISUAL (f)->class == TrueColor)
+  if (FRAME_X_VISUAL_INFO (f)->class == TrueColor)
     {
       values.function = GXxor;
       values.foreground = (FRAME_FOREGROUND_PIXEL (f)
@@ -5821,7 +5820,7 @@ XTflash (struct frame *f)
                     flash_left, FRAME_INTERNAL_BORDER_WIDTH (f),
                     width, height - 2 * FRAME_INTERNAL_BORDER_WIDTH (f));
 
-  if (FRAME_X_VISUAL (f)->class == TrueColor)
+  if (FRAME_X_VISUAL_INFO (f)->class == TrueColor)
     XFreeGC (FRAME_X_DISPLAY (f), gc);
   x_flush (f);
 
@@ -13986,11 +13985,17 @@ x_bitmap_icon (struct frame *f, Lisp_Object file)
             }
 
 #elif defined (HAVE_XPM) && defined (HAVE_X_WINDOWS)
-
-          rc = x_create_bitmap_from_xpm_data (f, gnu_xpm_bits);
-          if (rc != -1)
-            FRAME_DISPLAY_INFO (f)->icon_bitmap_id = rc;
-
+          /* This allocates too many colors.  */
+          if (FRAME_X_VISUAL_INFO (f)->class == TrueColor
+              /* That pixmap needs about 240 colors, and we should
+                 also leave some more space for other colors as
+                 well.  */
+              || FRAME_X_VISUAL_INFO (f)->colormap_size >= (240 * 4))
+            {
+              rc = x_create_bitmap_from_xpm_data (f, gnu_xpm_bits);
+              if (rc != -1)
+                FRAME_DISPLAY_INFO (f)->icon_bitmap_id = rc;
+            }
 #endif
 
           /* If all else fails, use the (black and white) xbm image. */
@@ -17306,7 +17311,7 @@ x_term_init (Lisp_Object display_name, char *xrm_option, char *resource_name)
   /* See if a private colormap is requested.  */
   if (dpyinfo->visual == DefaultVisualOfScreen (dpyinfo->screen))
     {
-      if (dpyinfo->visual->class == PseudoColor)
+      if (dpyinfo->visual_info.class == PseudoColor)
         {
           AUTO_STRING (privateColormap, "privateColormap");
           AUTO_STRING (PrivateColormap, "PrivateColormap");
@@ -17324,13 +17329,13 @@ x_term_init (Lisp_Object display_name, char *xrm_option, char *resource_name)
                                      dpyinfo->visual, AllocNone);
 
   /* See if we can construct pixel values from RGB values.  */
-  if (dpyinfo->visual->class == TrueColor)
+  if (dpyinfo->visual_info.class == TrueColor)
     {
-      get_bits_and_offset (dpyinfo->visual->red_mask,
+      get_bits_and_offset (dpyinfo->visual_info.red_mask,
                            &dpyinfo->red_bits, &dpyinfo->red_offset);
-      get_bits_and_offset (dpyinfo->visual->blue_mask,
+      get_bits_and_offset (dpyinfo->visual_info.blue_mask,
                            &dpyinfo->blue_bits, &dpyinfo->blue_offset);
-      get_bits_and_offset (dpyinfo->visual->green_mask,
+      get_bits_and_offset (dpyinfo->visual_info.green_mask,
                            &dpyinfo->green_bits, &dpyinfo->green_offset);
 
 #ifdef HAVE_XRENDER
@@ -17357,9 +17362,9 @@ x_term_init (Lisp_Object display_name, char *xrm_option, char *resource_name)
           if (XAllocColor (dpyinfo->display,
                            dpyinfo->cmap, &xc) != 0)
             {
-              alpha_mask = xc.pixel & ~(dpyinfo->visual->red_mask
-                                        | dpyinfo->visual->blue_mask
-                                        | dpyinfo->visual->green_mask);
+              alpha_mask = xc.pixel & ~(dpyinfo->visual_info.red_mask
+                                        | dpyinfo->visual_info.blue_mask
+                                        | dpyinfo->visual_info.green_mask);
 
               if (alpha_mask)
                 get_bits_and_offset (alpha_mask, &dpyinfo->alpha_bits,
diff --git a/src/xterm.h b/src/xterm.h
index dd510ae2576..846df03277c 100644
--- a/src/xterm.h
+++ b/src/xterm.h
@@ -283,6 +283,9 @@ struct x_display_info
   /* The Visual being used for this display.  */
   Visual *visual;
 
+  /* The visual information corresponding to VISUAL.  */
+  XVisualInfo visual_info;
+
 #ifdef HAVE_XRENDER
   /* The picture format for this display.  */
   XRenderPictFormat *pict_format;
@@ -1031,6 +1034,9 @@ extern void x_mark_frame_dirty (struct frame *f);
 /* This is the Visual which frame F is on.  */
 #define FRAME_X_VISUAL(f) FRAME_DISPLAY_INFO (f)->visual
 
+/* And its corresponding visual info.  */
+#define FRAME_X_VISUAL_INFO(f) (&FRAME_DISPLAY_INFO (f)->visual_info)
+
 #ifdef HAVE_XRENDER
 #define FRAME_X_PICTURE_FORMAT(f) FRAME_DISPLAY_INFO (f)->pict_format
 #define FRAME_X_PICTURE(f) ((f)->output_data.x->picture)