Subject: bug#71159: [PATCH] Stipple support for MS Windows

From: Po Lu <[email-masked]>
Date: Sat, 01 Jun 2024 22:03:40 +0800

"Elijah G." <[email-masked]> 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'?