master 8a7df412a6 2/2: Improve color handling on colormapped displays

[Po Lu]

branch: master
commit 8a7df412a640c8b2334b78ec0ca872a6d11e8b0e
Author: Po Lu <luangruo@yahoo.com>
Commit: Po Lu <luangruo@yahoo.com>

    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.
---
 src/xfns.c  | 18 ++++++++++++++----
 src/xterm.c | 43 ++++++++++++++++++++++++-------------------
 src/xterm.h |  6 ++++++
 3 files changed, 44 insertions(+), 23 deletions(-)

diff --git a/src/xfns.c b/src/xfns.c
index e288f797fb..3d1fa92609 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 30229c45a4..6682d3c9a4 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 dd510ae257..846df03277 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)