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

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

[Cecilio Pardo]

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


The attached patch is a preliminar implementation of a DirectWrite font
driver that allows for color fonts, tested only on Windows 11.

There is much to be refined about quality, performance (caching), OS
version conditionals, etc.

Before doing all that, I need to know that this is the right (or at
least good enough) way to do it.

The DirectWrite font driver mounts on top of a copy of the harfbuzz one,
and then replaces some of the functions, but some of the hb functions
include eassert to check that the driver for a font is harfbuzz. I don't
know how to deal with that.

I suppose that if we skip harfbuzz completely, then we would have to
reimplement a lot of stuff with DirectWrite?

Also uniscribe_open has been modified to change the driver assigned to
the font unconditionally to dwrite.

-- 
Cecilio Pardo

[-- Attachment #2: 0001-Font-driver-for-DirectWrite-MS-Windows-supporting-co.patch --]
[-- Type: text/plain, Size: 16794 bytes --]

From d004845fe6a9495af7c03ddd9d30d3ba71fc3e5d Mon Sep 17 00:00:00 2001
From: Cecilio Pardo <cpardo@imayhem.com>
Date: Wed, 9 Oct 2024 11:40:28 +0200
Subject: [PATCH] Font driver for DirectWrite (MS-Windows) supporting color
 fonts.

---
 configure.ac       |   4 +-
 src/font.h         |   1 +
 src/w32dwrite.c    | 470 +++++++++++++++++++++++++++++++++++++++++++++
 src/w32fns.c       |   7 +
 src/w32uniscribe.c |   2 +
 5 files changed, 482 insertions(+), 2 deletions(-)
 create mode 100644 src/w32dwrite.c

diff --git a/configure.ac b/configure.ac
index 8a5ba7db3d1..8a06d9d5027 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3172,7 +3172,7 @@ AC_DEFUN
   AC_CHECK_TOOL([WINDRES], [windres],
                 [AC_MSG_ERROR([No resource compiler found.])])
   W32_OBJ="w32fns.o w32menu.o w32reg.o w32font.o w32term.o"
-  W32_OBJ="$W32_OBJ w32xfns.o w32select.o w32uniscribe.o w32cygwinx.o"
+  W32_OBJ="$W32_OBJ w32xfns.o w32select.o w32uniscribe.o w32dwrite.o w32cygwinx.o"
   EMACSRES="emacs.res"
   case "$canonical" in
     x86_64-*-*) EMACS_MANIFEST="emacs-x64.manifest" ;;
@@ -3202,7 +3202,7 @@ AC_DEFUN
       W32_OBJ="$W32_OBJ w32image.o"
     fi
     W32_LIBS="$W32_LIBS -lwinmm -lgdi32 -lcomdlg32"
-    W32_LIBS="$W32_LIBS -lmpr -lwinspool -lole32 -lcomctl32"
+    W32_LIBS="$W32_LIBS -lmpr -lwinspool -lole32 -lcomctl32 -ldwrite"
     W32_RES_LINK="\$(EMACSRES)"
     CLIENTRES="emacsclient.res"
     CLIENTW="emacsclientw\$(EXEEXT)"
diff --git a/src/font.h b/src/font.h
index 8ee1940be0a..7e0e96d9bfa 100644
--- a/src/font.h
+++ b/src/font.h
@@ -978,6 +978,7 @@ valid_font_driver (struct font_driver const *d)
 extern struct font_driver uniscribe_font_driver;
 #ifdef HAVE_HARFBUZZ
 extern struct font_driver harfbuzz_font_driver;
+extern struct font_driver dwrite_font_driver;
 #endif
 extern void syms_of_w32font (void);
 #endif	/* HAVE_NTGUI */
diff --git a/src/w32dwrite.c b/src/w32dwrite.c
new file mode 100644
index 00000000000..32c9c179778
--- /dev/null
+++ b/src/w32dwrite.c
@@ -0,0 +1,470 @@
+#include <config.h>
+#include <math.h>
+#include <windows.h>
+
+#include "lisp.h"
+#include "coding.h"
+#include "w32term.h"
+#include "frame.h"
+#include "composite.h"
+#include "font.h"
+#include "w32font.h"
+#include "pdumper.h"
+#include "w32common.h"
+
+#include "initguid.h"
+#include <dwrite_3.h>
+
+void syms_of_w32dwrite (void);
+
+struct font_driver dwrite_font_driver;
+
+/* Initialized on syms_of_w32dwrite_for_pdumper  */
+IDWriteFactory *dwrite_factory = NULL;
+IDWriteFactory2 *dwrite_factory2 = NULL;
+IDWriteGdiInterop *gdi_interop = NULL;
+IDWriteRenderingParams *rendering_params = NULL;
+
+static void
+verify_hr (HRESULT hr, const char *msg)
+{
+  if (FAILED (hr))
+    {
+      printf ("Error: %s\n", msg);
+      abort ();
+    }
+}
+
+static float
+get_font_and_face (HFONT hfont, LOGFONTW *logfont_out,
+		   IDWriteFont **font, IDWriteFontFace **face)
+{
+  HRESULT hr;
+  LOGFONTW logfont;
+
+  GetObjectW (hfont, sizeof (LOGFONTW), &logfont);
+  hr = gdi_interop->lpVtbl->
+    CreateFontFromLOGFONT (gdi_interop, (const LOGFONTW *) &logfont, font);
+  verify_hr (hr, "Failed to create DWriteFont");
+  hr = (*font)->lpVtbl->
+    CreateFontFace (*font, face);
+  verify_hr (hr, "Failed to create DWriteFontFace");
+
+  if (logfont_out)
+    *logfont_out = logfont;
+
+  return abs (logfont.lfHeight);
+}
+
+static IDWriteBitmapRenderTarget *
+get_bitmap_render_target (HDC hdc, int width, int height)
+{
+  HRESULT hr;
+  IDWriteBitmapRenderTarget *brt;
+  hr = gdi_interop->lpVtbl->
+    CreateBitmapRenderTarget (gdi_interop, hdc, width, height, &brt);
+  verify_hr (hr, "Failed to create DWriteBitmapRenderTarget");
+
+  IDWriteBitmapRenderTarget1 *brt1;
+  hr = brt->lpVtbl->
+    QueryInterface (brt, &IID_IDWriteBitmapRenderTarget1, (void **)&brt1);
+  verify_hr (hr, "Failed to create DWriteBitmapRenderTarget1");
+  brt1->lpVtbl->
+    SetTextAntialiasMode ( brt1, DWRITE_TEXT_ANTIALIAS_MODE_CLEARTYPE );
+  brt1->lpVtbl->Release(brt1);
+
+  return brt;
+}
+
+
+/* This function does not fill in the ascent and descent fields of
+   metrics.  */
+static void text_extents_internal(IDWriteFont *dwrite_font,
+				  IDWriteFontFace *dwrite_font_face,
+				  float font_size,
+				  const unsigned *code,
+				  int nglyphs, struct font_metrics *metrics)
+{
+  HRESULT hr;
+
+  DWRITE_FONT_METRICS dwrite_font_metrics;
+  dwrite_font->lpVtbl->
+    GetMetrics (dwrite_font, &dwrite_font_metrics);
+
+  UINT16 *indices = alloca (nglyphs * sizeof (UINT16));
+  for (int i = 0; i < nglyphs; i++)
+    indices[i] = code[i];
+
+  DWRITE_GLYPH_METRICS* gmetrics =
+    alloca (nglyphs * sizeof (DWRITE_GLYPH_METRICS));
+
+  hr = dwrite_font_face->lpVtbl->
+    GetGdiCompatibleGlyphMetrics (dwrite_font_face, font_size, 1.0, NULL,
+				  TRUE, indices, nglyphs, gmetrics, false );
+  verify_hr (hr, "Failed to GetGdiCompatibleGlyphMetrics");
+
+  float width = 0;
+  int units_per_em = dwrite_font_metrics.designUnitsPerEm;
+
+  for (int i = 0; i < nglyphs; i++)
+    {
+      width += (float)gmetrics[i].advanceWidth * font_size / units_per_em;
+      float lbearing = round ((float) gmetrics[i].leftSideBearing * font_size
+			     / units_per_em);
+      float rbearing = round ((float) gmetrics[i].rightSideBearing * font_size
+			      / units_per_em);
+      if (i==0)
+	{
+	  metrics->lbearing = lbearing;
+	  metrics->rbearing = rbearing;
+	}
+      if (metrics->lbearing > lbearing)
+	metrics->lbearing = lbearing;
+      if (metrics->rbearing < rbearing)
+	metrics->rbearing = rbearing;
+    }
+  metrics->width = round(width);
+}
+
+static Lisp_Object
+dwrite_list (struct frame *f, Lisp_Object font_spec)
+{
+  Lisp_Object fonts = w32font_list_internal (f, font_spec, true);
+  for (Lisp_Object tail = fonts; CONSP (tail); tail = XCDR (tail))
+    ASET (XCAR (tail), FONT_TYPE_INDEX, Qdwrite);
+
+  FONT_ADD_LOG ("dwrite-list", font_spec, fonts);
+  return fonts;
+}
+
+static Lisp_Object
+dwrite_match (struct frame *f, Lisp_Object font_spec)
+{
+  Lisp_Object entity = w32font_match_internal (f, font_spec, true);
+  FONT_ADD_LOG ("dwrite-match", font_spec, entity);
+
+  if (! NILP (entity))
+    ASET (entity, FONT_TYPE_INDEX, Qdwrite);
+  return entity;
+}
+
+static unsigned
+dwrite_encode_char (struct font *font, int c)
+{
+  HRESULT hr;
+  IDWriteFont *dwrite_font;
+  IDWriteFontFace *dwrite_font_face;
+
+  get_font_and_face (((struct w32font_info *) font)->hfont,
+		     NULL, &dwrite_font, &dwrite_font_face );
+  UINT16 index;
+
+  hr = dwrite_font_face->lpVtbl->
+    GetGlyphIndices (dwrite_font_face, &c, 1, &index);
+  verify_hr (hr, "Failed to GetGlyphIndices");
+
+  dwrite_font->lpVtbl->Release (dwrite_font);
+  dwrite_font_face->lpVtbl->Release (dwrite_font_face);
+
+  if (index == 0)
+    return FONT_INVALID_CODE;
+
+  return index;
+}
+
+static void
+dwrite_text_extents (struct font *font, const unsigned *code,
+		     int nglyphs, struct font_metrics *metrics)
+{
+  IDWriteFont *dwrite_font;
+  IDWriteFontFace *dwrite_font_face;
+
+  HFONT hfont = ((struct w32font_info *) font)->hfont;
+  float font_size = get_font_and_face (hfont,
+				       NULL, &dwrite_font,
+				       &dwrite_font_face );
+
+  text_extents_internal (dwrite_font, dwrite_font_face, font_size, code,
+			 nglyphs, metrics);
+
+  dwrite_font->lpVtbl->Release (dwrite_font);
+  dwrite_font_face->lpVtbl->Release (dwrite_font_face);
+
+  metrics->ascent = font->ascent;
+  metrics->descent = font->descent;
+}
+
+static
+void dwrite_draw_internal (HDC hdc, int x, int y, unsigned *glyphs, int len,
+			   int baseline_y, COLORREF color, struct font *font )
+{
+  HRESULT hr;
+  LOGFONTW logfont;
+  IDWriteFont *dwrite_font;
+  IDWriteFontFace *dwrite_font_face;
+
+  float font_size = get_font_and_face (GetCurrentObject (hdc, OBJ_FONT),
+				       &logfont, &dwrite_font,
+				       &dwrite_font_face );
+
+  IDWriteTextFormat *text_format;
+  hr = dwrite_factory->lpVtbl->
+    CreateTextFormat (dwrite_factory,
+		      logfont.lfFaceName,
+		      NULL,
+		      DWRITE_FONT_WEIGHT_NORMAL,
+		      DWRITE_FONT_STYLE_NORMAL,
+		      DWRITE_FONT_STRETCH_NORMAL,
+		      font_size, L"",
+		      &text_format);
+  verify_hr (hr, "Failed to create DWriteTextFormat");
+
+  struct font_metrics metrics;
+  text_extents_internal (dwrite_font, dwrite_font_face,
+			 font_size, glyphs, len, &metrics);
+
+  int bitmap_width = metrics.width;
+  int bitmap_height = font->ascent + font->descent;
+
+  IDWriteBitmapRenderTarget *bitmap_render_target =
+    get_bitmap_render_target (hdc, bitmap_width, bitmap_height);
+
+  HDC text_dc = bitmap_render_target->lpVtbl->
+    GetMemoryDC (bitmap_render_target);
+
+  BitBlt (text_dc, 0, 0, bitmap_width, bitmap_height, hdc, x, y, SRCCOPY);
+
+  DWRITE_GLYPH_RUN glyph_run;
+
+  UINT16 *indices = alloca (len * sizeof (UINT16));
+
+  for (int i = 0; i < len; i++)
+    indices[i] = glyphs[i];
+
+  FLOAT *advances = alloca (len * sizeof (FLOAT));
+
+  for (int i = 0; i < len; i++)
+    {
+      text_extents_internal (dwrite_font, dwrite_font_face,
+			     font_size, glyphs + i, 1, &metrics);
+      advances[i] = metrics.width;
+    }
+
+  glyph_run.fontFace = dwrite_font_face;
+  glyph_run.fontEmSize = font_size;
+  glyph_run.glyphIndices = indices;
+  glyph_run.glyphCount = len;
+  glyph_run.isSideways = false;
+  glyph_run.bidiLevel = 0;
+  glyph_run.glyphOffsets = NULL;
+  glyph_run.glyphAdvances = advances;
+
+  IDWriteColorGlyphRunEnumerator *layers;
+
+  hr = dwrite_factory2->lpVtbl->
+    TranslateColorGlyphRun (dwrite_factory2,
+			    0, baseline_y,
+			    &glyph_run,
+			    NULL,
+			    DWRITE_MEASURING_MODE_GDI_CLASSIC,
+			    NULL,
+			    0,
+			    &layers );
+  if (hr == DWRITE_E_NOCOLOR)
+    {
+      bitmap_render_target->lpVtbl->
+	DrawGlyphRun (bitmap_render_target,
+		      0, baseline_y,
+		      DWRITE_MEASURING_MODE_GDI_CLASSIC,
+		      &glyph_run,
+		      rendering_params,
+		      color,
+		      NULL);
+    }
+  else
+    {
+      verify_hr (hr, "Failed at TranslateColorGlyphRun");
+      for (;;) {
+	HRESULT hr;
+	BOOL more_layers;
+	const DWRITE_COLOR_GLYPH_RUN *layer;
+
+	hr = layers->lpVtbl->MoveNext (layers, &more_layers);
+	verify_hr (hr, "Failed at MoveNext");
+	if (!more_layers)
+	  break;
+	hr = layers->lpVtbl->GetCurrentRun (layers, &layer);
+	verify_hr (hr, "Failed at GetCurrentRun");
+	bitmap_render_target->lpVtbl->
+	  DrawGlyphRun (bitmap_render_target,
+			layer->baselineOriginX, layer->baselineOriginY,
+			DWRITE_MEASURING_MODE_GDI_CLASSIC,
+			&layer->glyphRun,
+			rendering_params,
+			RGB( layer->runColor.r * 255,
+			     layer->runColor.g * 255,
+			     layer->runColor.b * 255 ),
+			NULL);
+      }
+
+      layers->lpVtbl->Release (layers);
+    }
+  BitBlt (hdc, x, y, bitmap_width, bitmap_height, text_dc, 0, 0, SRCCOPY);
+
+  text_format->lpVtbl->Release (text_format);
+  dwrite_font->lpVtbl->Release (dwrite_font);
+  bitmap_render_target->lpVtbl->Release (bitmap_render_target);
+  dwrite_font_face->lpVtbl->Release (dwrite_font_face);
+}
+
+static int
+dwrite_draw (struct glyph_string *s, int from, int to,
+	     int x, int y, bool with_background)
+{
+  HRGN orig_clip = NULL;
+  int len = to - from;
+
+  if (s->num_clips > 0)
+    {
+      HRGN new_clip = CreateRectRgnIndirect (s->clip);
+
+      /* Save clip region for later restoration.  */
+      orig_clip = CreateRectRgn (0, 0, 0, 0);
+      if (!GetClipRgn (s->hdc, orig_clip))
+	{
+	  DeleteObject (orig_clip);
+	  orig_clip = NULL;
+	}
+
+      if (s->num_clips > 1)
+        {
+          HRGN clip2 = CreateRectRgnIndirect (s->clip + 1);
+
+          CombineRgn (new_clip, new_clip, clip2, RGN_OR);
+          DeleteObject (clip2);
+        }
+
+      SelectClipRgn (s->hdc, new_clip);
+      DeleteObject (new_clip);
+    }
+
+  /* Using OPAQUE background mode can clear more background than expected
+     when Cleartype is used.  Draw the background manually to avoid this.  */
+  SetBkMode (s->hdc, TRANSPARENT);
+  if (with_background)
+    {
+      HBRUSH brush;
+      RECT rect;
+      struct font *font = s->font;
+      int ascent = font->ascent, descent = font->descent;
+
+      /* Font's global ascent and descent values might be
+	 preposterously large for some fonts.  We fix here the case
+	 when those fonts are used for display of glyphless
+	 characters, because drawing background with font dimensions
+	 in those cases makes the display illegible.  There's only one
+	 more call to the draw method with with_background set to
+	 true, and that's in w32_draw_glyph_string_foreground, when
+	 drawing the cursor, where we have no such heuristics
+	 available.  FIXME.  */
+      if (s->first_glyph->type == GLYPHLESS_GLYPH
+	  && (s->first_glyph->u.glyphless.method == GLYPHLESS_DISPLAY_HEX_CODE
+	      || s->first_glyph->u.glyphless.method == GLYPHLESS_DISPLAY_ACRONYM))
+	{
+	  ascent =
+	    s->first_glyph->slice.glyphless.lower_yoff
+	    - s->first_glyph->slice.glyphless.upper_yoff;
+	  descent = 0;
+	}
+      brush = CreateSolidBrush (s->gc->background);
+      rect.left = x;
+      rect.top = y - ascent;
+      rect.right = x + s->width;
+      rect.bottom = y + descent;
+      FillRect (s->hdc, &rect, brush);
+      DeleteObject (brush);
+    }
+
+  if (s->padding_p)
+    {
+      int i;
+      for (i = 0; i < len; i++)
+	  dwrite_draw_internal (s->hdc, x, y - s->font->ascent,
+				s->char2b + from, 1, s->font->ascent,
+				GetTextColor(s->hdc), s->font);
+    }
+  else
+    {
+      dwrite_draw_internal( s->hdc, x, y - s->font->ascent,
+			    s->char2b + from, len, s->font->ascent,
+			    GetTextColor(s->hdc), s->font );
+
+    }
+
+  /* Restore clip region.  */
+  if (s->num_clips > 0)
+    SelectClipRgn (s->hdc, orig_clip);
+
+  if (orig_clip)
+    DeleteObject (orig_clip);
+
+  return len;
+}
+
+static void
+syms_of_w32dwrite_for_pdumper (void)
+{
+  HRESULT hr;
+
+  if (!initialized)
+    return;
+
+  DEFSYM (Qdwrite, "dwrite");
+
+  dwrite_font_driver = harfbuzz_font_driver;
+  dwrite_font_driver.type = Qdwrite;
+  dwrite_font_driver.draw = dwrite_draw;
+  dwrite_font_driver.list = dwrite_list;
+  dwrite_font_driver.match = dwrite_match;
+  dwrite_font_driver.encode_char = dwrite_encode_char;
+  dwrite_font_driver.text_extents = dwrite_text_extents;
+  register_font_driver (&dwrite_font_driver, NULL);
+
+  hr = DWriteCreateFactory (DWRITE_FACTORY_TYPE_SHARED,
+			    &IID_IDWriteFactory,
+			    (IUnknown**)&dwrite_factory);
+  verify_hr (hr, "Failed to create DWriteFactory");
+
+  hr = dwrite_factory->lpVtbl->
+    QueryInterface (dwrite_factory, &IID_IDWriteFactory2,
+		    (void **)&dwrite_factory2);
+  verify_hr (hr, "Failed to create DWriteFactory2");
+
+  hr = dwrite_factory->lpVtbl->
+    GetGdiInterop (dwrite_factory, &gdi_interop);
+  verify_hr (hr, "Failed to get DWriteGgiInterop");
+
+  IDWriteRenderingParams *def;
+
+  hr = dwrite_factory->lpVtbl->
+    CreateRenderingParams (dwrite_factory, &def);
+  verify_hr (hr, "Failed to create DWriteRenderingParams");
+
+  hr = dwrite_factory->lpVtbl->
+    CreateCustomRenderingParams (dwrite_factory,
+				 def->lpVtbl->GetGamma(def),
+				 def->lpVtbl->GetEnhancedContrast(def),
+				 def->lpVtbl->GetClearTypeLevel(def),
+				 def->lpVtbl->GetPixelGeometry(def),
+				 DWRITE_RENDERING_MODE_GDI_CLASSIC,
+				 &rendering_params);
+  verify_hr (hr, "Failed to create DWriteRenderingParams");
+
+  def->lpVtbl->Release (def);
+}
+
+void
+syms_of_w32dwrite (void)
+{
+  pdumper_do_now_and_after_load (syms_of_w32dwrite_for_pdumper);
+}
diff --git a/src/w32fns.c b/src/w32fns.c
index 3ee13dcbbdd..aba91c1d41c 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -293,6 +293,10 @@ #define MENU_FREE_DELAY 1000
 extern int uniscribe_available;
 extern int harfbuzz_available;
 
+/* From w32dwrite.c  */
+extern void syms_of_w32dwrite (void);
+
+
 #ifdef WINDOWSNT
 /* From w32inevt.c */
 extern int faked_key;
@@ -6356,6 +6360,8 @@ DEFUN ("x-create-frame", Fx_create_frame, Sx_create_frame,
       specbind (Qx_resource_name, name);
     }
 
+  register_font_driver (&dwrite_font_driver, f);
+
 #ifdef HAVE_HARFBUZZ
   if (harfbuzz_available)
     register_font_driver (&harfbuzz_font_driver, f);
@@ -11811,6 +11817,7 @@ globals_of_w32fns (void)
   InitCommonControls ();
 
   syms_of_w32uniscribe ();
+  syms_of_w32dwrite ();
 }
 
 #ifdef WINDOWSNT
diff --git a/src/w32uniscribe.c b/src/w32uniscribe.c
index b77bf56b8cf..cbe57d8490a 100644
--- a/src/w32uniscribe.c
+++ b/src/w32uniscribe.c
@@ -211,6 +211,8 @@ uniscribe_open (struct frame *f, Lisp_Object font_entity, int pixel_size)
 #endif  /* HAVE_HARFBUZZ */
     uniscribe_font->w32_font.font.driver = &uniscribe_font_driver;
 
+  uniscribe_font->w32_font.font.driver = &dwrite_font_driver;
+
   return font_object;
 }
 
-- 
2.35.1.windows.2


[-- Attachment #3: emacs_3xjaQ7etE9.png --]
[-- Type: image/png, Size: 30997 bytes --]

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

[Eli Zaretskii]

> Date: Thu, 10 Oct 2024 13:16:23 +0200
> From: Cecilio Pardo <cpardo@imayhem.com>
> 
> The attached patch is a preliminar implementation of a DirectWrite font
> driver that allows for color fonts, tested only on Windows 11.

Thanks!

> There is much to be refined about quality, performance (caching), OS
> version conditionals, etc.
> 
> Before doing all that, I need to know that this is the right (or at
> least good enough) way to do it.
> 
> The DirectWrite font driver mounts on top of a copy of the harfbuzz one,
> and then replaces some of the functions, but some of the hb functions
> include eassert to check that the driver for a font is harfbuzz. I don't
> know how to deal with that.

If we use a (slightly modified) HarfBuzz font driver (see below), this
won't be a problem, right?

> I suppose that if we skip harfbuzz completely, then we would have to
> reimplement a lot of stuff with DirectWrite?

Yes, I don't want to skip HarfBuzz, since we want HarfBuzz to be our
primary text-shaping engine.  We don't want to rely on
Windows-specific shapers because MS has a habit of deprecating them at
will (which happened with Uniscribe).  The experts on text shaping are
hard to get on board, so we don't want to have to rewrite our display
routines when MS decide to come up with a shining new shaper.

> Also uniscribe_open has been modified to change the driver assigned to
> the font unconditionally to dwrite.

That's temporary, I presume?

Some comments about significant issues below.

> @@ -3202,7 +3202,7 @@ AC_DEFUN
>        W32_OBJ="$W32_OBJ w32image.o"
>      fi
>      W32_LIBS="$W32_LIBS -lwinmm -lgdi32 -lcomdlg32"
> -    W32_LIBS="$W32_LIBS -lmpr -lwinspool -lole32 -lcomctl32"
> +    W32_LIBS="$W32_LIBS -lmpr -lwinspool -lole32 -lcomctl32 -ldwrite"

This we cannot do, for two reasons: (1) mingw.org's MinGW doesn't have
the libdwrite.dll.a import library, and (2) linking against -ldwrite
will make Emacs refuse to start on systems without DWrite.dll.
Instead, we need to use LoadLibrary at startup time to load the DLL,
like we do with HarfBuzz, and if the load succeeds, use get_proc_addr
to assign the addresses of APIs we need to use to function pointers;
then we call the functions through those pointers.  See w32image.c for
how we do that with GdiPlus.dll and w32uniscribe.c for how we do that
with libharfbuzz-0.dll.

> --- a/src/font.h
> +++ b/src/font.h
> @@ -978,6 +978,7 @@ valid_font_driver (struct font_driver const *d)
>  extern struct font_driver uniscribe_font_driver;
>  #ifdef HAVE_HARFBUZZ
>  extern struct font_driver harfbuzz_font_driver;
> +extern struct font_driver dwrite_font_driver;

Do we really need a whole new font driver?  I hoped that we'd only
need to replace the functions we call to actually draw the font glyphs
in w32font_draw (ExtTextOutW etc.) with the appropriate replacements
from Direct2D.  You see, HarfBuzz already does the job of finding the
appropriate font glyphs when we need some non-trivial font processing
(like character compositions), so I thought all we needed was to be
able to actually draw the font glyphs of color fonts.

IOW, how about just modifying the few methods of the HarfBuzz font
driver when DirectWrite is available, and otherwise leaving the
HarfBuzz font driver be the one which supports this?  With Uniscribe
we had legacy support issues (Windows 9X etc.), but there's no such
problem with HarfBuzz vs DirectWrite, so adding yet another font
driver which needs to coexist with the others is a complexity I'd like
to avoid.  In my mental model, we just use DirectWrite for low-level
drawing of font glyphs, and otherwise we still keep the HarfBuzz font
driver.  Does that make sense?

> +/* Initialized on syms_of_w32dwrite_for_pdumper  */
> +IDWriteFactory *dwrite_factory = NULL;
> +IDWriteFactory2 *dwrite_factory2 = NULL;
> +IDWriteGdiInterop *gdi_interop = NULL;
> +IDWriteRenderingParams *rendering_params = NULL;

These should probably be static, if they are only used in this one
file.

> +static void
> +verify_hr (HRESULT hr, const char *msg)
> +{
> +  if (FAILED (hr))
> +    {
> +      printf ("Error: %s\n", msg);
> +      abort ();
> +    }
> +}

This will need to be replaced with something more suitable.

> +static Lisp_Object
> +dwrite_list (struct frame *f, Lisp_Object font_spec)
> +{
> +  Lisp_Object fonts = w32font_list_internal (f, font_spec, true);
> +  for (Lisp_Object tail = fonts; CONSP (tail); tail = XCDR (tail))
> +    ASET (XCAR (tail), FONT_TYPE_INDEX, Qdwrite);
> +
> +  FONT_ADD_LOG ("dwrite-list", font_spec, fonts);
> +  return fonts;
> +}
> +
> +static Lisp_Object
> +dwrite_match (struct frame *f, Lisp_Object font_spec)
> +{
> +  Lisp_Object entity = w32font_match_internal (f, font_spec, true);
> +  FONT_ADD_LOG ("dwrite-match", font_spec, entity);
> +
> +  if (! NILP (entity))
> +    ASET (entity, FONT_TYPE_INDEX, Qdwrite);
> +  return entity;
> +}

If we avoid defining a new font driver, the above two methods are not
needed, as they do the same as the HarfBuzz driver does.

> +static int
> +dwrite_draw (struct glyph_string *s, int from, int to,
> +	     int x, int y, bool with_background)
> +{
> +  HRGN orig_clip = NULL;
> +  int len = to - from;
> +
> +  if (s->num_clips > 0)
> +    {
> +      HRGN new_clip = CreateRectRgnIndirect (s->clip);
> +
> +      /* Save clip region for later restoration.  */
> +      orig_clip = CreateRectRgn (0, 0, 0, 0);
> +      if (!GetClipRgn (s->hdc, orig_clip))
> +	{
> +	  DeleteObject (orig_clip);
> +	  orig_clip = NULL;
> +	}
> +
> +      if (s->num_clips > 1)
> +        {
> +          HRGN clip2 = CreateRectRgnIndirect (s->clip + 1);
> +
> +          CombineRgn (new_clip, new_clip, clip2, RGN_OR);
> +          DeleteObject (clip2);
> +        }
> +
> +      SelectClipRgn (s->hdc, new_clip);
> +      DeleteObject (new_clip);
> +    }
> +
> +  /* Using OPAQUE background mode can clear more background than expected
> +     when Cleartype is used.  Draw the background manually to avoid this.  */
> +  SetBkMode (s->hdc, TRANSPARENT);
> +  if (with_background)
> +    {
> +      HBRUSH brush;
> +      RECT rect;
> +      struct font *font = s->font;
> +      int ascent = font->ascent, descent = font->descent;
> +
> +      /* Font's global ascent and descent values might be
> +	 preposterously large for some fonts.  We fix here the case
> +	 when those fonts are used for display of glyphless
> +	 characters, because drawing background with font dimensions
> +	 in those cases makes the display illegible.  There's only one
> +	 more call to the draw method with with_background set to
> +	 true, and that's in w32_draw_glyph_string_foreground, when
> +	 drawing the cursor, where we have no such heuristics
> +	 available.  FIXME.  */
> +      if (s->first_glyph->type == GLYPHLESS_GLYPH
> +	  && (s->first_glyph->u.glyphless.method == GLYPHLESS_DISPLAY_HEX_CODE
> +	      || s->first_glyph->u.glyphless.method == GLYPHLESS_DISPLAY_ACRONYM))
> +	{
> +	  ascent =
> +	    s->first_glyph->slice.glyphless.lower_yoff
> +	    - s->first_glyph->slice.glyphless.upper_yoff;
> +	  descent = 0;
> +	}
> +      brush = CreateSolidBrush (s->gc->background);
> +      rect.left = x;
> +      rect.top = y - ascent;
> +      rect.right = x + s->width;
> +      rect.bottom = y + descent;
> +      FillRect (s->hdc, &rect, brush);
> +      DeleteObject (brush);
> +    }
> +
> +  if (s->padding_p)
> +    {
> +      int i;
> +      for (i = 0; i < len; i++)
> +	  dwrite_draw_internal (s->hdc, x, y - s->font->ascent,
> +				s->char2b + from, 1, s->font->ascent,
> +				GetTextColor(s->hdc), s->font);
> +    }
> +  else
> +    {
> +      dwrite_draw_internal( s->hdc, x, y - s->font->ascent,
> +			    s->char2b + from, len, s->font->ascent,
> +			    GetTextColor(s->hdc), s->font );
> +
> +    }
> +
> +  /* Restore clip region.  */
> +  if (s->num_clips > 0)
> +    SelectClipRgn (s->hdc, orig_clip);
> +
> +  if (orig_clip)
> +    DeleteObject (orig_clip);
> +
> +  return len;
> +}

This method looks almost identical to w32font_draw, except that it
calls DirectWrite specific functions instead of ExtTextOutW.  It would
be better to reuse the code in w32font.c instead of copying it; a
single test for DirectWrite's availability should not be an issue, I
think (if you are worried about performance).

> +  hr = dwrite_factory->lpVtbl->
> +    CreateCustomRenderingParams (dwrite_factory,
> +				 def->lpVtbl->GetGamma(def),
> +				 def->lpVtbl->GetEnhancedContrast(def),
> +				 def->lpVtbl->GetClearTypeLevel(def),
> +				 def->lpVtbl->GetPixelGeometry(def),
> +				 DWRITE_RENDERING_MODE_GDI_CLASSIC,
> +				 &rendering_params);

Are there some options for the rendering here that we perhaps need to
discuss?  E.g., is DWRITE_RENDERING_MODE_GDI_CLASSIC the only
reasonable choice?

Thanks again for working on this.

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

[Cecilio Pardo]

On 10/10/2024 15:08, Eli Zaretskii wrote:

> IOW, how about just modifying the few methods of the HarfBuzz font
> driver when DirectWrite is available, and otherwise leaving the
> HarfBuzz font driver be the one which supports this?  With Uniscribe
> we had legacy support issues (Windows 9X etc.), but there's no such
> problem with HarfBuzz vs DirectWrite, so adding yet another font
> driver which needs to coexist with the others is a complexity I'd like
> to avoid.  In my mental model, we just use DirectWrite for low-level
> drawing of font glyphs, and otherwise we still keep the HarfBuzz font
> driver.  Does that make sense?

Yes. I'll rewrite it like that.

>> +  hr = dwrite_factory->lpVtbl->
>> +    CreateCustomRenderingParams (dwrite_factory,
>> +				 def->lpVtbl->GetGamma(def),
>> +				 def->lpVtbl->GetEnhancedContrast(def),
>> +				 def->lpVtbl->GetClearTypeLevel(def),
>> +				 def->lpVtbl->GetPixelGeometry(def),
>> +				 DWRITE_RENDERING_MODE_GDI_CLASSIC,
>> +				 &rendering_params);
> 
> Are there some options for the rendering here that we perhaps need to
> discuss?  E.g., is DWRITE_RENDERING_MODE_GDI_CLASSIC the only
> reasonable choice?

This gives poor quality, we will definitely use something else. But in 
my tests other methods gave fractional widths for glyphs, so I took this 
one for the first version.

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

[Eli Zaretskii]

> Date: Thu, 10 Oct 2024 17:14:34 +0200
> Cc: 73730@debbugs.gnu.org
> From: Cecilio Pardo <cpardo@imayhem.com>
> 
> On 10/10/2024 15:08, Eli Zaretskii wrote:
> 
> > IOW, how about just modifying the few methods of the HarfBuzz font
> > driver when DirectWrite is available, and otherwise leaving the
> > HarfBuzz font driver be the one which supports this?  With Uniscribe
> > we had legacy support issues (Windows 9X etc.), but there's no such
> > problem with HarfBuzz vs DirectWrite, so adding yet another font
> > driver which needs to coexist with the others is a complexity I'd like
> > to avoid.  In my mental model, we just use DirectWrite for low-level
> > drawing of font glyphs, and otherwise we still keep the HarfBuzz font
> > driver.  Does that make sense?
> 
> Yes. I'll rewrite it like that.

Thanks, SGTM.

> >> +  hr = dwrite_factory->lpVtbl->
> >> +    CreateCustomRenderingParams (dwrite_factory,
> >> +				 def->lpVtbl->GetGamma(def),
> >> +				 def->lpVtbl->GetEnhancedContrast(def),
> >> +				 def->lpVtbl->GetClearTypeLevel(def),
> >> +				 def->lpVtbl->GetPixelGeometry(def),
> >> +				 DWRITE_RENDERING_MODE_GDI_CLASSIC,
> >> +				 &rendering_params);
> > 
> > Are there some options for the rendering here that we perhaps need to
> > discuss?  E.g., is DWRITE_RENDERING_MODE_GDI_CLASSIC the only
> > reasonable choice?
> 
> This gives poor quality, we will definitely use something else. But in 
> my tests other methods gave fractional widths for glyphs, so I took this 
> one for the first version.

I'd guess DWRITE_RENDERING_MODE_DEFAULT should be the first choice?
And if it gets you corrupted glyphs on display, this is something that
needs debugging, I think.  Maybe the code which processes the glyphs,
which you basically copied from w32font.c, needs some adjustment for
DirectWrite or something?

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

[Cecilio Pardo]

> I'd guess DWRITE_RENDERING_MODE_DEFAULT should be the first choice?
> And if it gets you corrupted glyphs on display, this is something that
> needs debugging, I think.  Maybe the code which processes the glyphs,
> which you basically copied from w32font.c, needs some adjustment for
> DirectWrite or something?

It draws well, but when measuring character sizes it gives fractional 
values, not integers. Anyway, I will investigate it throroughly.

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

[Cecilio Pardo]

It looks like DirectWrite can't handle .FON files. When I build emacs, 
the fixed-pitch face, which uses the Monospace family, gets the 
"Courier" font which we can't use.

I'd say that this wasn't failing before today's windows update, but who 
knows...

Any pointers?

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

[Eli Zaretskii]

> Date: Thu, 10 Oct 2024 23:50:00 +0200
> From: Cecilio Pardo <cpardo@imayhem.com>
> 
> It looks like DirectWrite can't handle .FON files. When I build emacs, 
> the fixed-pitch face, which uses the Monospace family, gets the 
> "Courier" font which we can't use.
> 
> I'd say that this wasn't failing before today's windows update, but who 
> knows...
> 
> Any pointers?

What happens if you revert commit 7eba90c12227 ?

If this fixes the problem, please look at the rationale described in
the commit log message and see if you get back the problem described
there, and in particular what happens with the faces requested by
info.el.  It is possible that the problem described there only happens
on XP, where there's no DirectWrite, in which case we could condition
Qascii_0 so that it doesn't get in the way.

But it is somewhat bothersome that .fon fonts are not supported.  Are
you sure this is true?  What does Internet search bring?  There are
example programs on MSDN which use DirectWrite, can you try them with
Courier and see if they also fail?

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

[Eli Zaretskii]

> Cc: 73730@debbugs.gnu.org
> Date: Fri, 11 Oct 2024 06:36:55 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > Date: Thu, 10 Oct 2024 23:50:00 +0200
> > From: Cecilio Pardo <cpardo@imayhem.com>
> > 
> > It looks like DirectWrite can't handle .FON files. When I build emacs, 
> > the fixed-pitch face, which uses the Monospace family, gets the 
> > "Courier" font which we can't use.
> > 
> > I'd say that this wasn't failing before today's windows update, but who 
> > knows...
> > 
> > Any pointers?
> 
> What happens if you revert commit 7eba90c12227 ?
> 
> If this fixes the problem, please look at the rationale described in
> the commit log message and see if you get back the problem described
> there, and in particular what happens with the faces requested by
> info.el.  It is possible that the problem described there only happens
> on XP, where there's no DirectWrite, in which case we could condition
> Qascii_0 so that it doesn't get in the way.
> 
> But it is somewhat bothersome that .fon fonts are not supported.  Are
> you sure this is true?  What does Internet search bring?  There are
> example programs on MSDN which use DirectWrite, can you try them with
> Courier and see if they also fail?

After some more thinking: I think something is amiss in your code,
because AFAICT the existing code draws .fon fonts using the 'gdi' font
driver.  Here's how to see that:

  emacs -Q
  M-x font-lock-mode
  M-x load-library RET facemenu RET

Now mark some text in *scratch* and type:

  M-x facemenu-set-face RET fixed-pitch RET

Then go to some character displayed with fixed-pitch face and type
"C-u C-x =".  When I do that in the current Emacs, I see:

             position: 78 of 147 (52%), column: 6
            character: c (displayed as c) (codepoint 99, #o143, #x63)
              charset: ascii (ASCII (ISO646 IRV))
code point in charset: 0x63
               script: latin
               syntax: w 	which means: word
             category: .:Base, L:Strong L2R, a:ASCII, l:Latin, r:Roman
             to input: type "C-x 8 RET 63" or "C-x 8 RET LATIN SMALL LETTER C"
          buffer code: #x63
            file code: #x63 (encoded by coding system iso-latin-1-dos)
              display: by this font (glyph code):
    gdi:-raster-Courier-regular-normal-normal-mono-13-*-*-*-c-*-iso8859-1 (#x63)

Note the last line: we already detect that this font should use the
'gdi' font backend.  So either your new code somehow bypassed this
detection, or the fallback on old glyph-drawing APIs which are used on
the current master branch by the 'gdi' font driver is missing
something.  AFAIR (and my memory is not reliable in this matter, so
please check with the code), we decide that this font needs to be
drawn by the 'gdi' font driver when we look for fonts for the
"Monospace" family: I think the 'harfbuzz' font backend rejects it for
some reason (I don't remember which), whereas 'gdi' accepts it.

Bottom line: we should use the 'gdi' font backend for these fonts,
like we do with the current code.  The 'harfbuzz' font driver, with or
without the new DirectWrite methods, should not be used for them,
presumably for reasons similar to those which determined how the
current code works.

If the above doesn't help, please tell more details about how
DirectWrite "cannot handle .FON fonts", and let's take it from there.

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

[Cecilio Pardo]

On 11/10/2024 8:28, Eli Zaretskii wrote:

> Bottom line: we should use the 'gdi' font backend for these fonts,
> like we do with the current code.  The 'harfbuzz' font driver, with or
> without the new DirectWrite methods, should not be used for them,
> presumably for reasons similar to those which determined how the
> current code works.

Ok. Thanks a lot.