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

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

[Willem Rein Oudshoorn]

[-- Attachment #1: Type: text/plain, Size: 1097 bytes --]

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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch for nextstep terminal to support color XBM images --]
[-- Type: text/x-patch, Size: 5530 bytes --]

=== 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  <woudshoo@xs4all.nl>
+
+	* 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  <eliz@gnu.org>
 
 	* 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 *);

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

[Jan D.]

Hello.

Can somebody say if this requires copyright assignment?

     Jan D.

Willem Rein Oudshoorn skrev 2013-07-27 22:12:
> 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.
>

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

[Stefan Monnier]

> Can somebody say if this requires copyright assignment?

I think it does, yes.


        Stefan

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

[Glenn Morris]

"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.

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

[Jan D.]

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.