GNU bug report logs - #71159
[PATCH] Stipple support for MS Windows

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 71159 in the body.
You can then email your comments to 71159 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Elijah G." <eg642616 <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Stipple support for MS Windows
Date: Thu, 23 May 2024 19:27:19 -0600

[Message part 1 (text/plain, inline)]

This patch add stipple bitmap support to MS Windows.

I tested this patch with gnus.xbm file and other stipple pattern from
its bitmap data.

If anyone have a feedback i would gladly read it.
Thanks.

[0001-Stipple-support-for-MS-Windows.patch (text/x-patch, attachment)]

Message #8 received at 71159 <at> debbugs.gnu.org (full text, mbox):

From: "Elijah G." <eg642616 <at> gmail.com>
To: 71159 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#71159: [PATCH] Stipple support for MS Windows
Date: Wed, 29 May 2024 17:25:19 -0600

[Message part 1 (text/plain, inline)]

I'm sending here the fixed patch, now there is no problem with fg and
bg color.

[0001-Stipple-support-for-MS-Windows.patch (text/x-patch, attachment)]

Message #11 received at 71159 <at> debbugs.gnu.org (full text, mbox):

From: "Elijah G." <eg642616 <at> gmail.com>
To: 71159 <at> debbugs.gnu.org
Cc: Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#71159: [PATCH] Stipple support for MS Windows
Date: Thu, 30 May 2024 18:12:42 -0600

[Message part 1 (text/plain, inline)]

I've updated the patch for delete all the conflicts generated by
ae7d0e86b37eabc434c48f85f56df0a221e0e7c7 commit.

[0001-Stipple-support-for-MS-Windows.patch (text/x-patch, attachment)]

Message #14 received at 71159 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: "Elijah G." <eg642616 <at> gmail.com>
Cc: 71159 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#71159: [PATCH] Stipple support for MS Windows
Date: Sat, 01 Jun 2024 22:03:40 +0800

"Elijah G." <eg642616 <at> gmail.com> writes:

> I've updated the patch for delete all the conflicts generated by
> ae7d0e86b37eabc434c48f85f56df0a221e0e7c7 commit.

Thank you.  Please find several comments below:

> * src/w32term.c:
> (w32_fill_stipple_pattern): New function.
> (w32_draw_glyph_string_background, w32_draw_glyph_string_bg_rect)
> (w32_draw_stretch_glyph_string): Use new stipple function.

Please delete the redundant colon after the file name.

> * src/image.c:
> (image_create_bitmap_from_data): Create stipple bitmap.
> (image_create_bitmap_from_file): Add suuport for pixmap and create
> stipple bitmap.

Likewise.  This commit message is also excessively wide, and must be
filled to remain within 64 columns.

> +#ifdef HAVE_NTGUI
> +static HBITMAP w32_create_pixmap_from_bitmap_data (int width,
> +						   int height,
> +						   char *data);
> +
> +#endif

It should be aesthetically better if the parameter names were deleted
from this declaration's type list, and all of the types placed on one
line.

> +    int nbytes, i;
> +    ptrdiff_t sa_avail; /* Used only for SAFE_NALLOCA.  */

??? Please insert this boilerplate by means of the `USE_SAFE_ALLOCA'
macro.

> +    invertedBits = bits;
> +    nbytes = (width + CHAR_BIT - 1) / CHAR_BIT * height;
> +    sa_avail = MAX_ALLOCA
> +
> +    SAFE_NALLOCA (bits, 1, nbytes);

Shouldn't you subsequently call SAFE_FREE?

> +    for (i = 0; i < nbytes; i++)
> +      bits[i] = XBM_BIT_SHUFFLE(invertedBits[i]);

Absent space between macro name and parameters.

> +      data[i] = XBM_BIT_SHUFFLE(data[i]);

Here also.

> +  Emacs_Pixmap stipple;

Should not the object in this field be released in `free_bitmap_record'?

Message #17 received at 71159 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "Elijah G." <eg642616 <at> gmail.com>
Cc: 71159 <at> debbugs.gnu.org
Subject: Re: bug#71159: [PATCH] Stipple support for MS Windows
Date: Sat, 01 Jun 2024 17:28:03 +0300

> From: "Elijah G." <eg642616 <at> gmail.com>
> Cc: Eli Zaretskii <eliz <at> gnu.org>
> Date: Thu, 30 May 2024 18:12:42 -0600
> 
> I've updated the patch for delete all the conflicts generated by
> ae7d0e86b37eabc434c48f85f56df0a221e0e7c7 commit.

Thanks, but is this the right patch?  It includes a syntax error,
which GCC announces like this:

  image.c: In function 'image_create_bitmap_from_data':
  image.c:627:26: error: expected ';' before 'do'
    627 |     sa_avail = MAX_ALLOCA
	|                          ^
	|                          ;
  image.c:623:15: warning: variable 'sa_avail' set but not used [-Wunused-but-set-variable]
    623 |     ptrdiff_t sa_avail; /* Used only for SAFE_NALLOCA.  */
	|               ^~~~~~~~
  Makefile:457: recipe for target `image.o' failed
  make[2]: *** [image.o] Error 1

I'm guessing that you sent the wrong patch, because you couldn't have
built Emacs with this one.  So, while the fix is trivial, I haven't
installed this, out of fear that other parts might be incorrect.

Message #20 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Elijah G." <eg642616 <at> gmail.com>
To: Po Lu via "Bug reports for GNU Emacs, the Swiss army knife of text
 editors" <bug-gnu-emacs <at> gnu.org>
Cc: Po Lu <luangruo <at> yahoo.com>, 71159 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#71159: [PATCH] Stipple support for MS Windows
Date: Sat, 01 Jun 2024 12:54:22 -0600

[Message part 1 (text/plain, inline)]

Po Lu via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs <at> gnu.org> writes:

> "Elijah G." <eg642616 <at> gmail.com> writes:
>
>> I've updated the patch for delete all the conflicts generated by
>> ae7d0e86b37eabc434c48f85f56df0a221e0e7c7 commit.
>
> Thank you.  Please find several comments below:
>
>> * src/w32term.c:
>> (w32_fill_stipple_pattern): New function.
>> (w32_draw_glyph_string_background, w32_draw_glyph_string_bg_rect)
>> (w32_draw_stretch_glyph_string): Use new stipple function.
>
> Please delete the redundant colon after the file name.
>
>> * src/image.c:
>> (image_create_bitmap_from_data): Create stipple bitmap.
>> (image_create_bitmap_from_file): Add suuport for pixmap and create
>> stipple bitmap.
>
> Likewise.  This commit message is also excessively wide, and must be
> filled to remain within 64 columns.

It's now fixed, sorry about that i couldn't find how to insert
change-log skeleton or amend with vc.el, those where generated by
`diff-add-change-log-entries-other-window'.

>> +#ifdef HAVE_NTGUI
>> +static HBITMAP w32_create_pixmap_from_bitmap_data (int width,
>> +						   int height,
>> +						   char *data);
>> +
>> +#endif
>
> It should be aesthetically better if the parameter names were deleted
> from this declaration's type list, and all of the types placed on one
> line.

Done.

>> +    int nbytes, i;
>> +    ptrdiff_t sa_avail; /* Used only for SAFE_NALLOCA.  */
>
> ??? Please insert this boilerplate by means of the `USE_SAFE_ALLOCA'
> macro.
>
>> +    invertedBits = bits;
>> +    nbytes = (width + CHAR_BIT - 1) / CHAR_BIT * height;
>> +    sa_avail = MAX_ALLOCA
>> +
>> +    SAFE_NALLOCA (bits, 1, nbytes);
>
> Shouldn't you subsequently call SAFE_FREE?

Thank you, i forgot calling SAFE_FREE, i only defined sa_avail instead
using USE_SAFE_ALLOCA because i was getting an unused variable warning.

It's now fixed.

>> +    for (i = 0; i < nbytes; i++)
>> +      bits[i] = XBM_BIT_SHUFFLE(invertedBits[i]);
>
> Absent space between macro name and parameters.
>
>> +      data[i] = XBM_BIT_SHUFFLE(data[i]);
>
> Here also.

Done.

>> +  Emacs_Pixmap stipple;
>
> Should not the object in this field be released in `free_bitmap_record'?

Yes, i forgot about it, it's fixed.

[0001-Stipple-support-for-MS-Windows.patch (text/x-patch, attachment)]

Message #26 received at 71159 <at> debbugs.gnu.org (full text, mbox):

From: "Elijah G." <eg642616 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 71159 <at> debbugs.gnu.org
Subject: Re: bug#71159: [PATCH] Stipple support for MS Windows
Date: Sat, 01 Jun 2024 13:04:11 -0600

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: "Elijah G." <eg642616 <at> gmail.com>
>> Cc: Eli Zaretskii <eliz <at> gnu.org>
>> Date: Thu, 30 May 2024 18:12:42 -0600
>> 
>> I've updated the patch for delete all the conflicts generated by
>> ae7d0e86b37eabc434c48f85f56df0a221e0e7c7 commit.
>
> Thanks, but is this the right patch?  It includes a syntax error,
> which GCC announces like this:
>
>   image.c: In function 'image_create_bitmap_from_data':
>   image.c:627:26: error: expected ';' before 'do'
>     627 |     sa_avail = MAX_ALLOCA
> 	|                          ^
> 	|                          ;
>   image.c:623:15: warning: variable 'sa_avail' set but not used [-Wunused-but-set-variable]
>     623 |     ptrdiff_t sa_avail; /* Used only for SAFE_NALLOCA.  */
> 	|               ^~~~~~~~
>   Makefile:457: recipe for target `image.o' failed
>   make[2]: *** [image.o] Error 1
>
> I'm guessing that you sent the wrong patch, because you couldn't have
> built Emacs with this one.  So, while the fix is trivial, I haven't
> installed this, out of fear that other parts might be incorrect.

I didn't know about that, i think it was an error that i didn't notice
when i was generating the patch file, i was having troubles with my
local repo.

It should now be fixed in the new patch sent.

Message #29 received at 71159 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: "Elijah G." <eg642616 <at> gmail.com>
Cc: 71159 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#71159: [PATCH] Stipple support for MS Windows
Date: Sun, 02 Jun 2024 09:34:42 +0800

"Elijah G." <eg642616 <at> gmail.com> writes:

> Yes, i forgot about it, it's fixed.

Have you assigned copyright for this change to the FSF?

Message #32 received at 71159 <at> debbugs.gnu.org (full text, mbox):

From: "Elijah G." <eg642616 <at> gmail.com>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 71159 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#71159: [PATCH] Stipple support for MS Windows
Date: Sat, 01 Jun 2024 19:40:48 -0600

Po Lu <luangruo <at> yahoo.com> writes:

> "Elijah G." <eg642616 <at> gmail.com> writes:
>
>> Yes, i forgot about it, it's fixed.
>
> Have you assigned copyright for this change to the FSF?

Yes, I already have assigned the copyright for my future changes.

Message #35 received at 71159 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: "Elijah G." <eg642616 <at> gmail.com>
Cc: 71159 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#71159: [PATCH] Stipple support for MS Windows
Date: Sun, 02 Jun 2024 09:49:26 +0800

"Elijah G." <eg642616 <at> gmail.com> writes:

> Po Lu <luangruo <at> yahoo.com> writes:
>
>> "Elijah G." <eg642616 <at> gmail.com> writes:
>>
>>> Yes, i forgot about it, it's fixed.
>>
>> Have you assigned copyright for this change to the FSF?
>
> Yes, I already have assigned the copyright for my future changes.

Very well.  Eli, have you any further objections to this patch, or
should I install it now.

Message #38 received at 71159 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "Elijah G." <eg642616 <at> gmail.com>
Cc: luangruo <at> yahoo.com, 71159 <at> debbugs.gnu.org
Subject: Re: bug#71159: [PATCH] Stipple support for MS Windows
Date: Sun, 02 Jun 2024 07:54:34 +0300

> From: "Elijah G." <eg642616 <at> gmail.com>
> Cc: Po Lu <luangruo <at> yahoo.com>,  71159 <at> debbugs.gnu.org,  Eli Zaretskii
>  <eliz <at> gnu.org>
> Date: Sat, 01 Jun 2024 12:54:22 -0600
> 
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -2827,6 +2827,9 @@ thumbnail images and show them in the thumbnail buffer.  Unlike with
>  using 'convert', this fallback method is synchronous, so Emacs will wait
>  until all the thumbnails are created and displayed, before showing them.
>  
> +---
> +*** Emacs on MS-Windows now supports the :stipple face attribute

This should end with a period, and :stipple should be quoted as
':stipple'.

> +  Emacs_Pixmap stipple, bitmap;
>    bitmap = CreateBitmap (width, height,
> -			 FRAME_DISPLAY_INFO (XFRAME (frame))->n_planes,
> -			 FRAME_DISPLAY_INFO (XFRAME (frame))->n_cbits,
> +			 dpyinfo->n_planes,
> +			 dpyinfo->n_cbits,
>  			 bits);

Please rewrite this as follows:

  Emacs_Pixmap stipple;
  Emacs_Pixmap bitmap = CreateBitmap (width, height, dpyinfo->n_planes,
				      dpyinfo->n_cbits, bits);

> +  {
> +    char *invertedBits;
> +    int nbytes, i;
> +
> +    invertedBits = bits;
> +    nbytes = (width + CHAR_BIT - 1) / CHAR_BIT * height;

Also here, please initialize the local variables where appropriate:

  {
    char invertedBits = bits;
    int nbytes = (width + CHAR_BIT - 1) / CHAR_BIT * height, i;

Thanks.

Message #41 received at 71159 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 71159 <at> debbugs.gnu.org, eg642616 <at> gmail.com
Subject: Re: bug#71159: [PATCH] Stipple support for MS Windows
Date: Sun, 02 Jun 2024 07:54:57 +0300

> From: Po Lu <luangruo <at> yahoo.com>
> Cc: 71159 <at> debbugs.gnu.org,  Eli Zaretskii <eliz <at> gnu.org>
> Date: Sun, 02 Jun 2024 09:49:26 +0800
> 
> "Elijah G." <eg642616 <at> gmail.com> writes:
> 
> > Po Lu <luangruo <at> yahoo.com> writes:
> >
> >> "Elijah G." <eg642616 <at> gmail.com> writes:
> >>
> >>> Yes, i forgot about it, it's fixed.
> >>
> >> Have you assigned copyright for this change to the FSF?
> >
> > Yes, I already have assigned the copyright for my future changes.
> 
> Very well.  Eli, have you any further objections to this patch, or
> should I install it now.

I just sent a few more comments.

Message #44 received at 71159 <at> debbugs.gnu.org (full text, mbox):

From: "Elijah G." <eg642616 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: luangruo <at> yahoo.com, 71159 <at> debbugs.gnu.org
Subject: Re: bug#71159: [PATCH] Stipple support for MS Windows
Date: Sat, 01 Jun 2024 23:27:58 -0600

[Message part 1 (text/plain, inline)]

Thank you, i've fixed now the patch.

[0001-Stipple-support-for-MS-Windows.patch (text/x-patch, attachment)]

Message #49 received at 71159-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "Elijah G." <eg642616 <at> gmail.com>
Cc: luangruo <at> yahoo.com, 71159-done <at> debbugs.gnu.org
Subject: Re: bug#71159: [PATCH] Stipple support for MS Windows
Date: Sun, 02 Jun 2024 09:37:08 +0300

> From: "Elijah G." <eg642616 <at> gmail.com>
> Cc: luangruo <at> yahoo.com,  71159 <at> debbugs.gnu.org
> Date: Sat, 01 Jun 2024 23:27:58 -0600
> 
> Thank you, i've fixed now the patch.

Thanks, installed on the master branch, and closing the bug.

Message #52 received at 71159 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 71159 <at> debbugs.gnu.org, eg642616 <at> gmail.com
Subject: Re: bug#71159: [PATCH] Stipple support for MS Windows
Date: Sun, 02 Jun 2024 15:56:28 +0800

Eli Zaretskii <eliz <at> gnu.org> writes:

> I just sent a few more comments.

Thanks, everyone.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.