changeset 4252:8475ff9c49ea

[xemacs-hg @ 2007-11-05 14:59:20 by didierv] Fix recent image related crashes
author didierv
date Mon, 05 Nov 2007 14:59:24 +0000
parents b45f331a659d
children 02b728d25575
files src/ChangeLog src/glyphs-gtk.c src/glyphs-x.c src/glyphs.c
diffstat 4 files changed, 105 insertions(+), 95 deletions(-) [+]
line wrap: on
line diff
--- a/src/ChangeLog	Sun Nov 04 22:50:59 2007 +0000
+++ b/src/ChangeLog	Mon Nov 05 14:59:24 2007 +0000
@@ -1,16 +1,27 @@
+2007-11-05  Didier Verna  <didier@xemacs.org>
+
+	* glyphs.c (potential_pixmap_file_instanciator): Fix comment
+	describing its behavior.
+	* glyphs.c (xbm_mask_file_munging): Update semantics of file and
+	mask_file arguments to reflect recent changes in
+	potential_pixmap_file_instanciator (see patch below).
+	* glyphs-x.c (autodetect_normalize): Update call to
+	xbm_mask_file_munging to reflect the above change.
+	* glyphs-gtk.c (autodetect_normalize): Ditto.
+
 2007-10-10  Didier Verna  <didier@xemacs.org>
 
-        * glyphs.c (potential_pixmap_file_instantiator): Make a difference
-        between not being able to locate a pixmap file, and not having a
-        console method to do so.
-        * glyphs.c (simple_image_type_normalize): Notice that difference,
-        and don't err when the method is unavailable.
-        * glyphs.c (xbm_normalize): Ditto.
-        * glyphs.c (xface_normalize): Ditto.
-        * glyphs.c (xpm_normalize): Ditto.
-        * glyphs-shared.c (shared_resource_normalize): Ditto.
-        * glyphs-x.c (x_locate_pixmap_file): Recognize ~ pathnames as
-        fully qualified.
+	* glyphs.c (potential_pixmap_file_instantiator): Make a difference
+	between not being able to locate a pixmap file, and not having a
+	console method to do so.
+	* glyphs.c (simple_image_type_normalize): Notice that difference,
+	and don't err when the method is unavailable.
+	* glyphs.c (xbm_normalize): Ditto.
+	* glyphs.c (xface_normalize): Ditto.
+	* glyphs.c (xpm_normalize): Ditto.
+	* glyphs-shared.c (shared_resource_normalize): Ditto.
+	* glyphs-x.c (x_locate_pixmap_file): Recognize ~ pathnames as
+	fully qualified.
 
 2007-10-03  Didier Verna  <didier@xemacs.org>
 
--- a/src/glyphs-gtk.c	Sun Nov 04 22:50:59 2007 +0000
+++ b/src/glyphs-gtk.c	Mon Nov 05 14:59:24 2007 +0000
@@ -162,7 +162,7 @@
 
 /************************************************************************/
 /* convert from a series of RGB triples to an XImage formated for the   */
-/* proper display 							*/
+/* proper display							*/
 /************************************************************************/
 static GdkImage *
 convert_EImage_to_GDKImage (Lisp_Object device, int width, int height,
@@ -215,7 +215,7 @@
       gdk_image_destroy (outimg);
       return NULL;
     }
-  
+
   if (vis->type == GDK_VISUAL_PSEUDO_COLOR)
     {
       unsigned long pixarray[256];
@@ -231,7 +231,7 @@
 	{
 	  GdkColor color;
 	  int res;
-	
+
 	  color.red = qtable->rm[i] ? qtable->rm[i] << 8 : 0;
 	  color.green = qtable->gm[i] ? qtable->gm[i] << 8 : 0;
 	  color.blue = qtable->bm[i] ? qtable->bm[i] << 8 : 0;
@@ -343,7 +343,7 @@
 #endif
 	    }
 	}
-    }  
+    }
   return outimg;
 }
 
@@ -885,7 +885,7 @@
 gtk_init_image_instance_from_eimage (struct Lisp_Image_Instance *ii,
 				     int width, int height,
 				     int slices,
-				     unsigned char *eimage, 
+				     unsigned char *eimage,
 				     int dest_mask,
 				     Lisp_Object instantiator,
 				     Lisp_Object pointer_fg,
@@ -1168,13 +1168,13 @@
 	  Fmake_color_instance
 	  (value, device, encode_error_behavior_flag (ERROR_ME_DEBUG_WARN));
       else
-        {
-          assert (COLOR_SPECIFIERP (value));
-          value = Fspecifier_instance (value, domain, Qnil, Qnil);
-        }
+	{
+	  assert (COLOR_SPECIFIERP (value));
+	  value = Fspecifier_instance (value, domain, Qnil, Qnil);
+	}
 
       if (NILP (value))
-        continue;
+	continue;
       results = noseeum_cons (noseeum_cons (name, value), results);
       i++;
     }
@@ -1188,7 +1188,7 @@
   for (j=0; j<i; j++)
     {
       Lisp_Object cons = XCAR (results);
-      colortbl[j].color = 
+      colortbl[j].color =
 	* COLOR_INSTANCE_GTK_COLOR (XCOLOR_INSTANCE (XCDR (cons)));
 
       colortbl[j].name = XSTRING_DATA (XCAR (cons));
@@ -1334,7 +1334,7 @@
 	IMAGE_INSTANCE_PIXMAP_HOTSPOT_X (ii) = 1;
 	IMAGE_INSTANCE_PIXMAP_HOTSPOT_Y (ii) = 1;
 
-     
+
       image_instance_convert_to_pointer (ii, instantiator, pointer_fg,
 					 pointer_bg);
       break;
@@ -1441,9 +1441,9 @@
 static void
 gtk_resource_validate (Lisp_Object instantiator)
 {
-  if ((NILP (find_keyword_in_vector (instantiator, Q_file)) 
+  if ((NILP (find_keyword_in_vector (instantiator, Q_file))
        &&
-       NILP (find_keyword_in_vector (instantiator, Q_resource_id))) 
+       NILP (find_keyword_in_vector (instantiator, Q_resource_id)))
       ||
       NILP (find_keyword_in_vector (instantiator, Q_resource_type)))
     sferror ("Must supply :file, :resource-id and :resource-type",
@@ -1513,11 +1513,11 @@
     iitype = IMAGE_POINTER;
   else if (dest_mask & IMAGE_COLOR_PIXMAP_MASK)
     iitype = IMAGE_COLOR_PIXMAP;
-  else 
+  else
     incompatible_image_types (instantiator, dest_mask,
-   			      IMAGE_COLOR_PIXMAP_MASK | IMAGE_POINTER_MASK);
+			      IMAGE_COLOR_PIXMAP_MASK | IMAGE_POINTER_MASK);
 #endif
-  
+
   /* mess with the keyword info we were provided with */
   gtk_initialize_pixmap_image_instance (ii, 1, type);
   c = gdk_cursor_new ((GdkCursorType) resource_name_to_resource (resource_id, type));
@@ -1614,7 +1614,7 @@
 	    alist = Fcons (Fcons (Q_hotspot_y, make_int (yhot)),
 			   alist);
 
-	  alist = xbm_mask_file_munging (alist, filename, Qnil, console_type);
+	  alist = xbm_mask_file_munging (alist, filename, Qt, console_type);
 
 	  {
 	    Lisp_Object result = alist_to_tagged_vector (Qxbm, alist);
@@ -1689,10 +1689,10 @@
 			  Qfile_name);
 
       if (cursor_name_to_index (name_ext) != -1)
-        {
-          result = alist_to_tagged_vector (Qcursor_font, alist);
-          is_cursor_font = 1;
-        }
+	{
+	  result = alist_to_tagged_vector (Qcursor_font, alist);
+	  is_cursor_font = 1;
+	}
     }
 
   if (!is_cursor_font)
@@ -1851,7 +1851,7 @@
     {
 	/* Need to initialize the array */
 	/* Supposedly since this array is static it should be
-           initialized to NULLs for us, but I'm very paranoid. */
+	   initialized to NULLs for us, but I'm very paranoid. */
 	for (i = 0; i < GDK_NUM_GLYPHS; i++)
 	{
 	    the_gdk_cursors[i] = NULL;
@@ -2210,7 +2210,7 @@
       GdkColor *fcolor, *bcolor;
 
       style = gtk_style_copy (style);
-  
+
       /* Update the foreground. */
       pixel = FACE_FOREGROUND (IMAGE_INSTANCE_WIDGET_FACE (ii), domain);
       fcolor = COLOR_INSTANCE_GTK_COLOR (XCOLOR_INSTANCE (pixel));
@@ -2592,7 +2592,7 @@
 	 this possibility. */
       if (IMAGE_INSTANCEP (image_instance))
 	XIMAGE_INSTANCE_WIDGET_ACTION_OCCURRED (image_instance) = 1;
-      
+
       if (!NILP (callback_ex) && !UNBOUNDP (callback_ex))
 	{
 	  event = Fmake_event (Qnil, Qnil);
@@ -3009,4 +3009,3 @@
     return 1;
   }
 }
-
--- a/src/glyphs-x.c	Sun Nov 04 22:50:59 2007 +0000
+++ b/src/glyphs-x.c	Mon Nov 05 14:59:24 2007 +0000
@@ -159,7 +159,7 @@
 
 /************************************************************************/
 /* convert from a series of RGB triples to an XImage formated for the   */
-/* proper display 							*/
+/* proper display							*/
 /************************************************************************/
 static XImage *
 convert_EImage_to_XImage (Lisp_Object device, int width, int height,
@@ -591,18 +591,18 @@
     Ibyte *path = egetenv ("XBMLANGPATH");
     if (path)
       {
-        Extbyte *pathext;
+	Extbyte *pathext;
 	SubstitutionRec subs[1];
 	subs[0].match = 'B';
 	LISP_STRING_TO_EXTERNAL (name, subs[0].substitution, Qfile_name);
 	C_STRING_TO_EXTERNAL (path, pathext, Qfile_name);
 	/* #### Motif uses a big hairy default if $XBMLANGPATH isn't set.
-           We don't.  If you want it used, set it. */
+	   We don't.  If you want it used, set it. */
 	if (pathext &&
 	    (pathext = XtResolvePathname (display, "bitmaps", 0, 0, pathext,
 					  subs, XtNumber (subs), 0)))
-          {
-            name = build_ext_string (pathext, Qfile_name);
+	  {
+	    name = build_ext_string (pathext, Qfile_name);
 	    XtFree (pathext);
 	    return (name);
 	  }
@@ -1311,12 +1311,12 @@
 	  Fmake_color_instance
 	    (value, device, encode_error_behavior_flag (ERROR_ME_DEBUG_WARN));
       else
-        {
-          assert (COLOR_SPECIFIERP (value));
-          value = Fspecifier_instance (value, domain, Qnil, Qnil);
-        }
+	{
+	  assert (COLOR_SPECIFIERP (value));
+	  value = Fspecifier_instance (value, domain, Qnil, Qnil);
+	}
       if (NILP (value))
-        continue;
+	continue;
       results = noseeum_cons (noseeum_cons (name, value), results);
       i++;
     }
@@ -1562,7 +1562,7 @@
 	IMAGE_INSTANCE_PIXMAP_HOTSPOT_X (ii) = make_int (xpmattrs.x_hotspot);
       if (xpmattrs.valuemask & XpmHotspot)
 	IMAGE_INSTANCE_PIXMAP_HOTSPOT_Y (ii) = make_int (xpmattrs.y_hotspot);
-      
+
       image_instance_convert_to_pointer (ii, instantiator, pointer_fg,
 					 pointer_bg);
       break;
@@ -1718,7 +1718,7 @@
 	    alist = Fcons (Fcons (Q_hotspot_y, make_int (yhot)),
 			   alist);
 
-	  alist = xbm_mask_file_munging (alist, filename, Qnil, console_type);
+	  alist = xbm_mask_file_munging (alist, filename, Qt, console_type);
 
 	  {
 	    Lisp_Object result = alist_to_tagged_vector (Qxbm, alist);
@@ -1789,10 +1789,10 @@
       const char *name_ext;
       LISP_STRING_TO_EXTERNAL (data, name_ext, Qfile_name);
       if (XmuCursorNameToIndex (name_ext) != -1)
-        {
-          result = alist_to_tagged_vector (Qcursor_font, alist);
-          is_cursor_font = 1;
-        }
+	{
+	  result = alist_to_tagged_vector (Qcursor_font, alist);
+	  is_cursor_font = 1;
+	}
     }
 
   if (!is_cursor_font)
@@ -2078,7 +2078,7 @@
       /* Since we are being unmapped we want the enclosing frame to
 	 get focus. The losing with simple scrolling but is the safest
 	 thing to do. */
-      emacs_Xt_handle_widget_losing_focus 
+      emacs_Xt_handle_widget_losing_focus
 	( XFRAME (IMAGE_INSTANCE_FRAME (p)),
 	  IMAGE_INSTANCE_X_WIDGET_ID (p));
       XtUnmapWidget (IMAGE_INSTANCE_X_CLIPWIDGET (p));
@@ -2175,7 +2175,7 @@
 	 changes we make to the values we get back will look like they
 	 have already been applied. If we rebuild the widget tree then
 	 we may lose properties. */
-      wv = copy_widget_value_tree (lw_get_all_values 
+      wv = copy_widget_value_tree (lw_get_all_values
 				   (IMAGE_INSTANCE_X_WIDGET_LWID (p)),
 				   NO_CHANGE);
     }
@@ -2236,7 +2236,7 @@
       Arg al[2];
       XtSetArg (al [0], XtNx, &IMAGE_INSTANCE_X_WIDGET_XOFFSET (p));
       XtSetArg (al [1], XtNy, &IMAGE_INSTANCE_X_WIDGET_YOFFSET (p));
-      XtGetValues (FRAME_X_TEXT_WIDGET 
+      XtGetValues (FRAME_X_TEXT_WIDGET
 		   (XFRAME (IMAGE_INSTANCE_FRAME (p))), al, 2);
     }
 
@@ -2977,7 +2977,7 @@
   IIFORMAT_HAS_METHOD (autodetect, validate);
   IIFORMAT_HAS_METHOD (autodetect, normalize);
   IIFORMAT_HAS_METHOD (autodetect, possible_dest_types);
-  /* #### autodetect is flawed IMO: 
+  /* #### autodetect is flawed IMO:
   1. It makes the assumption that you can detect whether the user
   wanted a cursor or a string based on the data, since the data is a
   string you have to prioritise cursors. Instead we will force users
--- a/src/glyphs.c	Sun Nov 04 22:50:59 2007 +0000
+++ b/src/glyphs.c	Mon Nov 05 14:59:24 2007 +0000
@@ -474,8 +474,8 @@
   for (len -= 2; len >= 1; len -= 2)
     {
       /* Keyword comparisons can be done with eq, the value must be
-         done with equal.
-         #### Note that this does not optimize re-ordering. */
+	 done with equal.
+	 #### Note that this does not optimize re-ordering. */
       if (!EQ (elt[len], old_elt[len])
 	  || !internal_equal (elt[len+1], old_elt[len+1], 0))
 	alist = Fcons (Fcons (elt[len], elt[len+1]), alist);
@@ -914,7 +914,7 @@
   { XD_LISP_OBJECT, offsetof (Lisp_Image_Instance, name) },
   { XD_LISP_OBJECT, offsetof (Lisp_Image_Instance, parent) },
   { XD_LISP_OBJECT, offsetof (Lisp_Image_Instance, instantiator) },
-  { XD_UNION, offsetof (struct Lisp_Image_Instance, u), 
+  { XD_UNION, offsetof (struct Lisp_Image_Instance, u),
     XD_INDIRECT (0, 0), { &image_instance_data_description } },
   { XD_END }
 };
@@ -993,7 +993,7 @@
 
   if (print_readably)
     printing_unreadable_object ("#<image-instance 0x%x>",
-           ii->header.uid);
+	   ii->header.uid);
   write_fmt_string_lisp (printcharfun, "#<image-instance (%s) ", 1,
 			 Fimage_instance_type (obj));
   if (!NILP (ii->name))
@@ -1095,7 +1095,7 @@
 	  write_c_string (printcharfun, "dead");
 	else
 	  write_c_string (printcharfun,
-	                  DEVICE_TYPE_NAME (XDEVICE (FRAME_DEVICE (f))));
+			  DEVICE_TYPE_NAME (XDEVICE (FRAME_DEVICE (f))));
       }
       write_c_string (printcharfun, "-frame>");
       write_fmt_string (printcharfun, " 0x%p",
@@ -1293,7 +1293,7 @@
 
     case IMAGE_WIDGET:
       /* We need the hash to be equivalent to what should be
-         displayed. */
+	 displayed. */
       hash = HASH5 (hash,
 		    LISP_HASH (IMAGE_INSTANCE_WIDGET_TYPE (i)),
 		    internal_hash (IMAGE_INSTANCE_WIDGET_PROPS (i), depth + 1),
@@ -2236,7 +2236,7 @@
 			   Lisp_Object data)
 {
   signal_error_1 (Qimage_conversion_error,
-                list3 (build_msg_string (string1),
+		list3 (build_msg_string (string1),
 		       build_msg_string (string2),
 		       data));
 }
@@ -2246,7 +2246,7 @@
 			     Lisp_Object data1, Lisp_Object data2)
 {
   signal_error_1 (Qimage_conversion_error,
-                list4 (build_msg_string (string1),
+		list4 (build_msg_string (string1),
 		       build_msg_string (string2),
 		       data1, data2));
 }
@@ -2530,9 +2530,10 @@
 /*                        pixmap file functions                         */
 /************************************************************************/
 
-/* If INSTANTIATOR refers to inline data, return Qnil.
+/* If INSTANTIATOR refers to inline data, return Qt.
    If INSTANTIATOR refers to data in a file, return the full filename
-   if it exists; otherwise, return a cons of (filename).
+   if it exists, Qnil if there's no console method for locating the file, or
+   (filename) if there was an error locating the file.
 
    FILE_KEYWORD and DATA_KEYWORD are symbols specifying the
    keywords used to look up the file and inline data,
@@ -2556,21 +2557,21 @@
   if (!NILP (file) && NILP (data))
     {
       struct console_methods *meths
-        = decode_console_type(console_type, ERROR_ME);
+	= decode_console_type(console_type, ERROR_ME);
 
       if (HAS_CONTYPE_METH_P (meths, locate_pixmap_file))
-        {
-          Lisp_Object retval
-            = CONTYPE_METH (meths, locate_pixmap_file, (file));
-
-      if (!NILP (retval))
-	return retval;
-      else
-	return Fcons (file, Qnil); /* should have been file */
+	{
+	  Lisp_Object retval
+	    = CONTYPE_METH (meths, locate_pixmap_file, (file));
+
+	  if (!NILP (retval))
+	    return retval;
+	  else
+	    return Fcons (file, Qnil); /* should have been file */
+	}
+      else /* method unavailable */
+	return Qnil;
     }
-      else /* method unavailable */
-  return Qnil;
-}
 
   return Qt;
 }
@@ -2751,10 +2752,10 @@
   /* This is unclean but it's fairly standard -- a number of the
      bitmaps in /usr/include/X11/bitmaps use it -- so we support
      it. */
-  if (NILP (mask_file)
+  if (EQ (mask_file, Qt)
       /* don't override explicitly specified mask data. */
       && NILP (assq_no_quit (Q_mask_data, alist))
-      && !NILP (file))
+      && !EQ (file, Qt))
     {
       mask_file = MAYBE_LISP_CONTYPE_METH
 	(decode_console_type(console_type, ERROR_ME),
@@ -2834,7 +2835,6 @@
 		       alist);
     }
 
-  /* #### FIXME: Hmmm... what about mask being Qt ?? -- dvl */
   alist = xbm_mask_file_munging (alist, file, mask_file, console_type);
 
   {
@@ -2904,11 +2904,11 @@
   if (EQ (file, Qt) && EQ (mask_file, Qt)) /* no conversion necessary */
     RETURN_UNGCPRO (inst);
 
-
-  /* #### FIXME: and what about file / mask being Qt ? -- dvl */
   alist = tagged_vector_to_alist (inst);
 
   {
+    /* #### FIXME: what if EQ (file, Qt) && !EQ (mask, Qt) ? Is that possible?
+       If so, we have a problem... -- dvl */
     Lisp_Object data = make_string_from_file (file);
     alist = remassq_no_quit (Q_file, alist);
     /* there can't be a :data at this point. */
@@ -3382,7 +3382,7 @@
 	ABORT ();	/* We're not allowed anything else currently. */
 
       /* If we don't have an instance at this point then create
-         one. */
+	 one. */
       if (UNBOUNDP (instance))
 	{
 	  Lisp_Object locative =
@@ -3429,12 +3429,12 @@
       else if (NILP (instance))
 	gui_error ("Can't instantiate image (probably cached)", instantiator);
       /* We found an instance. However, because we are using the glyph
-         as the hash key instead of the instantiator, the current
-         instantiator may not be the same as the original. Thus we
-         must update the instance based on the new
-         instantiator. Preserving instance identity like this is
-         important to stop excessive window system widget creation and
-         deletion - and hence flashing. */
+	 as the hash key instead of the instantiator, the current
+	 instantiator may not be the same as the original. Thus we
+	 must update the instance based on the new
+	 instantiator. Preserving instance identity like this is
+	 important to stop excessive window system widget creation and
+	 deletion - and hence flashing. */
       else
 	{
 	  /* #### This function should be able to cope with *all*
@@ -4284,7 +4284,7 @@
 
 
 /*****************************************************************************
- *                     glyph cachel functions                         	     *
+ *                     glyph cachel functions	     *
  *****************************************************************************/
 
 /* #### All of this is 95% copied from face cachels.  Consider
@@ -4457,7 +4457,7 @@
 
 
 /*****************************************************************************
- *                     subwindow cachel functions                     	     *
+ *                     subwindow cachel functions	     *
  *****************************************************************************/
 /* Subwindows are curious in that you have to physically unmap them to
    not display them. It is problematic deciding what to do in
@@ -4825,7 +4825,7 @@
   register_ignored_expose (f, IMAGE_INSTANCE_DISPLAY_X (ii),
 			   IMAGE_INSTANCE_DISPLAY_Y (ii),
 			   IMAGE_INSTANCE_DISPLAY_WIDTH (ii),
-			   IMAGE_INSTANCE_DISPLAY_HEIGHT (ii));  
+			   IMAGE_INSTANCE_DISPLAY_HEIGHT (ii));
   IMAGE_INSTANCE_SUBWINDOW_DISPLAYEDP (ii) = 0;
 
   MAYBE_DEVMETH (XDEVICE (IMAGE_INSTANCE_DEVICE (ii)),
@@ -5101,7 +5101,7 @@
 	    {
 	      /* Increment the index of the image slice we are currently
 		 viewing. */
-	      IMAGE_INSTANCE_PIXMAP_SLICE (ii) = 
+	      IMAGE_INSTANCE_PIXMAP_SLICE (ii) =
 		(IMAGE_INSTANCE_PIXMAP_SLICE (ii) + 1)
 		% IMAGE_INSTANCE_PIXMAP_MAXSLICE (ii);
 	      /* We might need to kick redisplay at this point - but we