bug#6918: Colors of xbm image are swapped on Windows

bug#6918: Colors of xbm image are swapped on Windows

[Kazuhiro Ito]

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

When I evaluate below expression with Emacs on Windows, colors of the
second image are swapped.  With Emacs on other environment
(e.g. Debian), second image is identical to first image.


(let ((image "#define default_width 8
#define default_height 16
static char default_bits[] = {
0x81, 0x81, 0x42, 0x42, 0x24, 0x24, 0x18, 0x18, 0x18,
 0x18, 0x24, 0x24, 0x42, 0x42, 0x81, 0x81};")
      (face 'default)
      (string "_"))
  (set-text-properties 0 (length string) `(face ,face) string)
  (with-temp-buffer
    (switch-to-buffer (current-buffer))
    (insert-image
     (create-image image 'xbm t) string)
    (sit-for 1)
    (insert-image
     (create-image image 'xbm t
		   :foreground (face-foreground face)
		   :background (face-background face)
		   ) string)
    (sit-for 3)))


Attached patch would fix problem.  But I don't know this patch is
correct, because this swapping seems intended.

> 2007-06-21  Jason Rumney  <jasonr@gnu.org>
> 	* image.c (convert_mono_to_color_image): Swap fore and background.

I guess this swapping has become unnecessary anymore by later change.

> 2007-12-05  Jason Rumney  <jasonr@gnu.org>
> 	* image.c (xbm_load) [WINDOWSNT]: Shuffle the bits of directly
> 	specified XBMs.

-- 
Kazuhiro Ito

[-- Attachment #2: image.c.diff --]
[-- Type: application/octet-stream, Size: 539 bytes --]

=== modified file 'src/image.c'
--- src/image.c	2010-06-19 19:43:47 +0000
+++ src/image.c	2010-08-26 07:35:54 +0000
@@ -2643,8 +2643,8 @@
   new_prev = SelectObject (new_img_dc, new_pixmap);
   /* Windows convention for mono bitmaps is black = background,
      white = foreground.  */
-  SetTextColor (new_img_dc, background);
-  SetBkColor (new_img_dc, foreground);
+  SetTextColor (new_img_dc, foreground);
+  SetBkColor (new_img_dc, background);
 
   BitBlt (new_img_dc, 0, 0, img->width, img->height, old_img_dc,
 	  0, 0, SRCCOPY);

bug#6918: Colors of xbm image are swapped on Windows

[Lars Ingebrigtsen]

Kazuhiro Ito <kzhr@d1.dion.ne.jp> writes:

> When I evaluate below expression with Emacs on Windows, colors of the
> second image are swapped.  With Emacs on other environment
> (e.g. Debian), second image is identical to first image.

[...]

> Attached patch would fix problem.  But I don't know this patch is
> correct, because this swapping seems intended.
>
>> 2007-06-21  Jason Rumney  <jasonr@gnu.org>
>> 	* image.c (convert_mono_to_color_image): Swap fore and background.
>
> I guess this swapping has become unnecessary anymore by later change.
>
>> 2007-12-05  Jason Rumney  <jasonr@gnu.org>
>> 	* image.c (xbm_load) [WINDOWSNT]: Shuffle the bits of directly
>> 	specified XBMs.

The code in question seems odd, indeed:

  /* Windows convention for mono bitmaps is black = background,
     white = foreground.  */
  SetTextColor (new_img_dc, background);
  SetBkColor (new_img_dc, foreground);

Can anybody who works on Windows say whether this is the case?

In any case, it seems very odd that the test code (included below again
for easy C-x C-e) should produce image with different colours, since
they should just be two ways of specifying the same image.  I think?

(let ((image "#define default_width 8
#define default_height 16
static char default_bits[] = {
0x81, 0x81, 0x42, 0x42, 0x24, 0x24, 0x18, 0x18, 0x18,
 0x18, 0x24, 0x24, 0x42, 0x42, 0x81, 0x81};")
      (face 'default)
      (string "_"))
  (set-text-properties 0 (length string) `(face ,face) string)
  (with-temp-buffer
    (switch-to-buffer (current-buffer))
    (insert-image
     (create-image image 'xbm t) string)
    (sit-for 1)
    (insert-image
     (create-image image 'xbm t
		   :foreground (face-foreground face)
		   :background (face-background face)
		   ) string)
    (sit-for 3)))


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#6918: Colors of xbm image are swapped on Windows

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Thu, 26 Sep 2019 19:59:44 +0200
> Cc: 6918@debbugs.gnu.org
> 
> Kazuhiro Ito <kzhr@d1.dion.ne.jp> writes:
> 
> > When I evaluate below expression with Emacs on Windows, colors of the
> > second image are swapped.  With Emacs on other environment
> > (e.g. Debian), second image is identical to first image.
> 
> [...]
> 
> > Attached patch would fix problem.  But I don't know this patch is
> > correct, because this swapping seems intended.
> >
> >> 2007-06-21  Jason Rumney  <jasonr@gnu.org>
> >> 	* image.c (convert_mono_to_color_image): Swap fore and background.
> >
> > I guess this swapping has become unnecessary anymore by later change.
> >
> >> 2007-12-05  Jason Rumney  <jasonr@gnu.org>
> >> 	* image.c (xbm_load) [WINDOWSNT]: Shuffle the bits of directly
> >> 	specified XBMs.
> 
> The code in question seems odd, indeed:
> 
>   /* Windows convention for mono bitmaps is black = background,
>      white = foreground.  */
>   SetTextColor (new_img_dc, background);
>   SetBkColor (new_img_dc, foreground);
> 
> Can anybody who works on Windows say whether this is the case?

Say whether WHAT is the case?

I can confirm that the second image has its colors inverted.  But we
have no experts on image display on Windows on board; Jason was the
last one we had, so his word will remain the last one until we have
someone to replace his expertise.  I'm adding Jason to the addresses
in the hope that he will respond.  Failing that, I'm sorry, but I'm
not going to agree to a change like that due to some obscure use case,
which succeeded to lay low for the last 9 years without popping
anywhere else.

bug#6918: Colors of xbm image are swapped on Windows

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> The code in question seems odd, indeed:
>> 
>>   /* Windows convention for mono bitmaps is black = background,
>>      white = foreground.  */
>>   SetTextColor (new_img_dc, background);
>>   SetBkColor (new_img_dc, foreground);
>> 
>> Can anybody who works on Windows say whether this is the case?
>
> Say whether WHAT is the case?

That the convention in Windows is as described in the comment above.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#6918: Colors of xbm image are swapped on Windows

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: jasonr@gnu.org,  jasonrumney@gmail.com,  kzhr@d1.dion.ne.jp,
>   6918@debbugs.gnu.org
> Date: Thu, 26 Sep 2019 20:34:15 +0200
> 
> >>   /* Windows convention for mono bitmaps is black = background,
> >>      white = foreground.  */
> >>   SetTextColor (new_img_dc, background);
> >>   SetBkColor (new_img_dc, foreground);
> >> 
> >> Can anybody who works on Windows say whether this is the case?
> >
> > Say whether WHAT is the case?
> 
> That the convention in Windows is as described in the comment above.

No clue, sorry.

bug#6918: Colors of xbm image are swapped on Windows

[Jason Rumney]

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

I wouldn't call myself an expert on images on Windows. I basically got it
working through trial and error, and from the looks of that history there
may have been bugs fixed later that invalidated some early decisions. Most
of the monchrome Emacs icons were PBM IIRC, so XBM may not have had a lot
of testing on Windows over the past 12 years.


On Fri, 27 Sep 2019, 4:44 AM Eli Zaretskii, <eliz@gnu.org> wrote:

> > From: Lars Ingebrigtsen <larsi@gnus.org>
> > Cc: jasonr@gnu.org,  jasonrumney@gmail.com,  kzhr@d1.dion.ne.jp,
> >   6918@debbugs.gnu.org
> > Date: Thu, 26 Sep 2019 20:34:15 +0200
> >
> > >>   /* Windows convention for mono bitmaps is black = background,
> > >>      white = foreground.  */
> > >>   SetTextColor (new_img_dc, background);
> > >>   SetBkColor (new_img_dc, foreground);
> > >>
> > >> Can anybody who works on Windows say whether this is the case?
> > >
> > > Say whether WHAT is the case?
> >
> > That the convention in Windows is as described in the comment above.
>
> No clue, sorry.
>

[-- Attachment #2: Type: text/html, Size: 1823 bytes --]

bug#6918: Colors of xbm image are swapped on Windows

[Eli Zaretskii]

> From: Jason Rumney <jasonrumney@gmail.com>
> Date: Fri, 27 Sep 2019 05:38:54 +0800
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, kzhr@d1.dion.ne.jp, 6918@debbugs.gnu.org
> 
> I wouldn't call myself an expert on images on Windows. I basically got it working through trial and error, and
> from the looks of that history there may have been bugs fixed later that invalidated some early decisions. Most
> of the monchrome Emacs icons were PBM IIRC, so XBM may not have had a lot of testing on Windows over
> the past 12 years.

How do you suggest to test the proposed change, to make sure it
doesn't break something else?  Any pointers to those monochrome PBM
icons used in the past, for example, or any other files?

Or would you suggest to install the proposed change?

Thanks.

bug#6918: Colors of xbm image are swapped on Windows

[Jason Rumney]

On Fri, 27 Sep 2019 at 17:30, Eli Zaretskii <eliz@gnu.org> wrote:
>
> How do you suggest to test the proposed change, to make sure it
> doesn't break something else?  Any pointers to those monochrome PBM
> icons used in the past, for example, or any other files?

Are the icons not still under etc/images?  It looks like they are to me.
IIRC, it was gnus that was missing xpm versions, so those were the
icons that were actually used in practice, but it was many years
(decades even) ago, and you might have to dig through the git history
to find the exact icons.

-- 
Jason

bug#6918: Colors of xbm image are swapped on Windows

[Eli Zaretskii]

> From: Jason Rumney <jasonrumney@gmail.com>
> Date: Thu, 10 Oct 2019 19:59:46 +1300
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, kzhr@d1.dion.ne.jp, 6918@debbugs.gnu.org
> 
> On Fri, 27 Sep 2019 at 17:30, Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > How do you suggest to test the proposed change, to make sure it
> > doesn't break something else?  Any pointers to those monochrome PBM
> > icons used in the past, for example, or any other files?
> 
> Are the icons not still under etc/images?  It looks like they are to me.

There are *.pbm icons in etc/images/, and then there are *.ico files
in nt/icons/.  Which ones did you have in mind?

> IIRC, it was gnus that was missing xpm versions, so those were the
> icons that were actually used in practice, but it was many years
> (decades even) ago, and you might have to dig through the git history
> to find the exact icons.

So you are saying that if, after the change, the PBM icons we have in
the repository display fine, the change is OK?

As for digging through VCS history: I'm not sure I understand what
should I be looking for there.  Can you elaborate a bit?

Thanks.

bug#6918: Colors of xbm image are swapped on Windows

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Jason Rumney <jasonrumney@gmail.com>
>> Date: Thu, 10 Oct 2019 19:59:46 +1300
>> Cc: Lars Ingebrigtsen <larsi@gnus.org>, kzhr@d1.dion.ne.jp, 6918@debbugs.gnu.org
>>
>> On Fri, 27 Sep 2019 at 17:30, Eli Zaretskii <eliz@gnu.org> wrote:
>> >
>> > How do you suggest to test the proposed change, to make sure it
>> > doesn't break something else?  Any pointers to those monochrome PBM
>> > icons used in the past, for example, or any other files?
>>
>> Are the icons not still under etc/images?  It looks like they are to me.
>
> There are *.pbm icons in etc/images/, and then there are *.ico files
> in nt/icons/.  Which ones did you have in mind?

(That was in 2019, but Jason never replied back.)

My guess is the *.pbm icons - as far as I can tell, Emacs doesn't have
support for the .ico ones without ImageMagick.  The patch is also about
the pbm code, AFAICT.  So I would check them.

>> IIRC, it was gnus that was missing xpm versions, so those were the
>> icons that were actually used in practice, but it was many years
>> (decades even) ago, and you might have to dig through the git history
>> to find the exact icons.
>
> So you are saying that if, after the change, the PBM icons we have in
> the repository display fine, the change is OK?

I understand him to be saying that.

> As for digging through VCS history: I'm not sure I understand what
> should I be looking for there.  Can you elaborate a bit?

Again, I can only interpret what is written, but I think he means here
that the icons may or may not still exist in the git repository.
However, AFAICT they do still exist, under etc/images.  We haven't
removed any relevant images from there as can be seen with this command:

    git --no-pager log --diff-filter=D --summary etc/images

bug#6918: Colors of xbm image are swapped on Windows

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Fri, 5 Nov 2021 06:26:01 -0700
> Cc: Jason Rumney <jasonrumney@gmail.com>, larsi@gnus.org, kzhr@d1.dion.ne.jp, 
> 	6918@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Jason Rumney <jasonrumney@gmail.com>
> >> Date: Thu, 10 Oct 2019 19:59:46 +1300
> >> Cc: Lars Ingebrigtsen <larsi@gnus.org>, kzhr@d1.dion.ne.jp, 6918@debbugs.gnu.org
> >>
> >> On Fri, 27 Sep 2019 at 17:30, Eli Zaretskii <eliz@gnu.org> wrote:
> >> >
> >> > How do you suggest to test the proposed change, to make sure it
> >> > doesn't break something else?  Any pointers to those monochrome PBM
> >> > icons used in the past, for example, or any other files?
> >>
> >> Are the icons not still under etc/images?  It looks like they are to me.
> >
> > There are *.pbm icons in etc/images/, and then there are *.ico files
> > in nt/icons/.  Which ones did you have in mind?
> 
> (That was in 2019, but Jason never replied back.)
> 
> My guess is the *.pbm icons - as far as I can tell, Emacs doesn't have
> support for the .ico ones without ImageMagick.  The patch is also about
> the pbm code, AFAICT.  So I would check them.

I checked them after the change, and they still display correctly.  So
I've now installed the change on master, and I'm closing this bug.

Thanks.