Subject: bug#14969: closed (Re: bug#14969: XBM image properties :foreground :background do not work on OSX)

From: [email-masked] (GNU bug Tracking System)
Date: Fri, 15 May 2015 09:37:02 +0000

Your bug report

#14969: XBM image properties :foreground :background do not work on OSX

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
14969: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=14969
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#14969: XBM image properties :foreground :background do not work on OSX
From: "Jan D." <[email-masked]>
To: Glenn Morris <[email-masked]>
Cc: [email-masked], Willem Rein Oudshoorn <[email-masked]>
Date: Fri, 15 May 2015 11:36:30 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

Glenn Morris skrev den 2013-07-31 17:59:
> "Jan D." wrote:
>
>> Can somebody say if this requires copyright assignment?
>
> I think so. Since the OP already said he was willing to complete one,
> I'll send the form off-list.
>

No assignment seen.  I fixed this in a different way.

	Jan D.

Subject: XBM image properties :foreground :background do not work on OSX
From: Willem Rein Oudshoorn <[email-masked]>
To: [email-masked]
Date: Sat, 27 Jul 2013 22:12:14 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

As in the subject, the XBM image properties :foreground :background 
have no effect on OSX. 

This is easily tested with:

=========
(defun make-test-image (foreground background)
  `(image :type xbm
	  :data
	  ,(let ((data (make-bool-vector (* 10 10) nil))
		 (index 0))
	     (while (< index (* 10 10))
	       (aset data index (= 0 (% index 3)))
	       (setq index (+ 1 index)))
	     data)
	  :height 10
	  :width 10
	  :foreground ,foreground
	  :background ,background))

(defun test-images ()
  (interactive)
  (insert-image (make-test-image nil nil))
  (insert-image (make-test-image "red" nil))
  (insert-image (make-test-image nil "green"))
  (insert-image (make-test-image "blue" "orange")))
=========

After running `test-images' I expect differently colored images, but
they are all monochrome. (black and white).

After applying the following patch, it works for nextstep based terminal
systems.

I have tried to adher to the style already present in the file,
and I do not the setup is the most elegant.  But changing that
would require a more intrusive patch.

Wim Oudshoorn.

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2013-07-27 13:08:03 +0000
+++ src/ChangeLog	2013-07-27 20:04:19 +0000
@@ -1,3 +1,21 @@
+2013-07-27  Willem Rein Oudshoorn  <[email-masked]>
+
+	* nsterm.h: added declaration of new method and function.
+
+	* nsimage.m (ns_image_convert_mono_to_color): New function to
+	set the background and foreground color of XBM bitmaps.
+	(setXBMColor:): Use new method
+	-setForegroundToRed:green:blue:alpha:andBackgroundToRed:green:blue:alpha:drawBackground:
+
+	(setForegroundToRed:green:blue:alpha:andBackgroundToRed:green:blue:alpha:drawBackground:):
+	New method to set foreground and background colors in monochrome bitmaps.
+
+	* image.c (Create_Pixmap_From_Bitmap_Data): Call
+	ns_image_convert_mono_to_color in case of non default colors,
+	similarly to the win32 code.  This fixes the problem that the
+	:foreground and :background properties of XBM images are ignored
+	on nextstep like platforms.
+
 2013-07-27  Eli Zaretskii  <[email-masked]>
 
 	* w32term.c (w32_read_socket) <WM_KILLFOCUS>: Call

=== modified file 'src/image.c'
--- src/image.c	2013-07-20 19:20:33 +0000
+++ src/image.c	2013-07-27 16:00:08 +0000
@@ -2674,7 +2674,8 @@
 
 #elif defined (HAVE_NS)
   img->pixmap = ns_image_from_XBM (data, img->width, img->height);
-
+  if (non_default_colors)
+    ns_image_convert_mono_to_color (img->pixmap, fg, bg);
 #else
   img->pixmap =
    (x_check_image_size (0, img->width, img->height)

=== modified file 'src/nsimage.m'
--- src/nsimage.m	2013-06-02 19:14:25 +0000
+++ src/nsimage.m	2013-07-27 19:59:19 +0000
@@ -151,6 +151,19 @@
   [(EmacsImage *)img setAlphaAtX: x Y: y to: a];
 }
 
+void
+ns_image_convert_mono_to_color (void *img, unsigned long fg_argb, unsigned long bg_argb)
+{
+  [(EmacsImage *)img setForegroundToRed: (fg_argb >> 16) & 0xFF
+				  green: (fg_argb >> 8) & 0xFF
+				   blue: fg_argb & 0xFF
+				  alpha: (fg_argb >> 24) & 0xFF
+		     andBackgroundToRed: (bg_argb >> 16) & 0xFF
+				  green: (bg_argb >> 8) & 0xFF
+				   blue: bg_argb & 0xFF
+				  alpha: (bg_argb >> 24) & 0xFF
+			 drawBackground: YES];
+}
 
 /* ==========================================================================
 
@@ -328,12 +341,8 @@
 }

-/* Set color for a bitmap image (see initFromSkipXBM).  Note that the alpha
-   is used as a mask, so we just memset the entire array. */
 - setXBMColor: (NSColor *)color
 {
-  NSSize s = [self size];
-  unsigned char *planes[5];
   EmacsCGFloat r, g, b, a;
   NSColor *rgbColor;
 
@@ -347,21 +356,12 @@
 
   [rgbColor getRed: &r green: &g blue: &b alpha: &a];
 
-  [bmRep getBitmapDataPlanes: planes];
-
-  /* we used to just do this, but Cocoa seems to have a bug when rendering
-     an alpha-masked image onto a dark background where it bloats the mask */
-   /* memset (planes[0..2], r, g, b*0xff, len); */
   {
-    int i, len = s.width*s.height;
     int rr = r * 0xff, gg = g * 0xff, bb = b * 0xff;
-    for (i =0; i<len; i++)
-      if (planes[3][i] != 0)
-        {
-          planes[0][i] = rr;
-          planes[1][i] = gg;
-          planes[2][i] = bb;
-        }
+
+    [self setForegroundToRed: rr green: gg blue: bb alpha: 0xff
+	  andBackgroundToRed: 0  green: 0  blue: 0  alpha: 0
+	      drawBackground: NO];
   }
 
   return self;
@@ -489,6 +489,42 @@
     }
 }
 
+/* Set color for a bitmap image (see initFromSkipXBM).  Note that the alpha
+   is used as a mask, and an alpha value of non zero
+   indicates a foreground pixel, an alpha value of zero is background. */
+
+- (void)setForegroundToRed: (unsigned char)f_r green: (unsigned char)f_g
+                      blue: (unsigned char)f_b alpha: (unsigned char)f_a
+	andBackgroundToRed: (unsigned char)b_r green: (unsigned char)b_g
+                      blue: (unsigned char)b_b alpha: (unsigned char)b_a
+	    drawBackground: (BOOL)draw_background;
+{
+  unsigned char *planes[5];
+
+  if (bmRep == nil)
+    return;
+
+  [bmRep getBitmapDataPlanes: planes];
+
+  {
+    int i, len = [bmRep pixelsHigh] * [bmRep pixelsWide];
+    for (int i=0; i<len; i++)
+      if (planes[3][i] != 0)
+	{
+	  planes[0][i] = f_r;
+	  planes[1][i] = f_g;
+	  planes[2][i] = f_b;
+	}
+      else if (draw_background)
+	{
+	  planes[0][i] = b_r;
+	  planes[1][i] = b_g;
+	  planes[2][i] = b_b;
+	  planes[3][i] = b_a;
+	}
+  }
+}
+
 /* returns a pattern color, which is cached here */
 - (NSColor *)stippleMask
 {

=== modified file 'src/nsterm.h'
--- src/nsterm.h	2013-07-06 17:58:41 +0000
+++ src/nsterm.h	2013-07-27 19:55:05 +0000
@@ -340,6 +340,12 @@
                green: (unsigned char)g blue: (unsigned char)b
               alpha:(unsigned char)a;
 - (void)setAlphaAtX: (int)x Y: (int)y to: (unsigned char)a;
+- (void)setForegroundToRed: (unsigned char)f_r green: (unsigned char)f_g
+                      blue: (unsigned char)f_b alpha: (unsigned char)f_a
+        andBackgroundToRed: (unsigned char)b_r green: (unsigned char)b_g
+                      blue: (unsigned char)b_b alpha: (unsigned char)b_a
+            drawBackground: (BOOL)draw_bakground;
+
 - (NSColor *)stippleMask;
 @end
 
@@ -860,6 +866,7 @@
 extern unsigned long ns_get_pixel (void *img, int x, int y);
 extern void ns_put_pixel (void *img, int x, int y, unsigned long argb);
 extern void ns_set_alpha (void *img, int x, int y, unsigned char a);
+extern void ns_image_convert_mono_to_color (void *img, unsigned long fg_argb, unsigned long bg_argb);
 
 extern int x_display_pixel_height (struct ns_display_info *);
 extern int x_display_pixel_width (struct ns_display_info *);