Subject: bug#73730: 31.0.50; Support for color fonts on MS-Windows

From: Eli Zaretskii <[email-masked]>
Date: Wed, 23 Oct 2024 13:03:02 +0300

> Date: Wed, 23 Oct 2024 00:14:12 +0200
> Cc: [email-masked], [email-masked]
> From: Cecilio Pardo <[email-masked]>
> 
> Here is a new version with pending issues addressed.

Thanks.

> - DirectWrite is available since Windows 7, but color glyphs are
>    available only since Windows 8.1.  We only enable DirectWrite since
>    Windows 8.1. This is stated on the NEWS entry.

Sounds okay, but I don't see the Windows version tested anywhere in
the code which initializes DWrite.  What did I miss?

> - The default value for the Gamma render parameter in DirectWrite
>    gives very 'light' characters.  I changed it to 1.4, which matches
>    GDI and looks better. But that's just my opinion.

Does the best value depend on whether the theme is light or dark?  In
any case, this sounds like a good candidate for a variable that people
could modify if they want.  Maybe we should also expose to Lisp the
values of enhanced contrast and clear-type level?  Assuming that
setting these from .emacs (or maybe in early-init?) at all could take
effect, that is.

> I couldn't find any hebrew font with color glyphs to test RTL
> display. I found a latin+arabic one, Cairo Play:
> 
> https://github.com/Gue3bara/Cairo
> https://fonts.google.com/specimen/Cairo+Play
> 
> It displays diacritics in different colors on both scripts.
> 
> It seems to work fine in emacs. I attach captures with and without
> DirectWrite, and compared to Courier New.

Looks okay to me, thanks.

> +** Emacs on MS-Windows now supports color fonts.
> +On Windows 8.1 and later versions Emacs now uses DirectWrite to draw
> +text, which supports color fonts.  This can be disabled by setting the
> +variable w32-inhibit-dwrite to t.

Names of variables and functions in NEWS should be quoted 'like this'.

> +  /* We can get fonts with a size of 0.  GDI handles this by using a default
> +     size.  We do the same.  */
> +  if (font_size <= 0.0f)
> +      font_size = FRAME_LINE_HEIGHT (XFRAME (selected_frame));
                                                ^^^^^^^^^^^^^^
Maybe use SLECTED_FRAME() here.

> +  if (!verify_hr (hr, "Failed to CreateBitmapRenderTarget"))
> +      return NULL;
     ^^^^^^^^^^^^^^^
The indentation here is incorrect: should be 2 columns, not 4.

> +  direct_write_available = true;
> +
> +  w32_inhibit_dwrite = false;

Does it work to set w32-inhibit-dwrite in the .emacs init file?

> +  UINT16 *indices = alloca (len * sizeof (UINT16));
> +
> +  for (int i = 0; i < len; i++)
> +    indices[i] = glyphs[i];
> +
> +  FLOAT *advances = alloca (len * sizeof (FLOAT));

How large can 'len' be?  Maybe we should use USE_SAFE_ALLOCA and
friends here, to avoid the possibility of overflowing the C stack if
alloca is called with a too-large argument?

> +      for (;;) {

That's not how we format for-loops: the brace should be on the next
line.

Thanks again for working on this.