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

Previous Next

Full log

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)]

Previous Next

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