From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#73730: 31.0.50; Support for color fonts on MS-Windows Date: Thu, 10 Oct 2024 16:08:38 +0300 Message-ID: <86o73s14x5.fsf@gnu.org> References: <36a6b4d5-c719-44d6-957d-bcd7db5a854b@imayhem.com> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="16198"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 73730@debbugs.gnu.org To: Cecilio Pardo Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Thu Oct 10 15:10:10 2024 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1sysvh-00041A-Qk for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 10 Oct 2024 15:10:10 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sysvQ-00068P-AZ; Thu, 10 Oct 2024 09:09:52 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sysvP-00068H-3N for bug-gnu-emacs@gnu.org; Thu, 10 Oct 2024 09:09:51 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1sysvO-0007Ua-RA for bug-gnu-emacs@gnu.org; Thu, 10 Oct 2024 09:09:50 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=References:In-Reply-To:From:Date:To:Subject; bh=/tzh8UwW1yb1oy9AryFFO8qXqx5lSOJ0b0sRwkwXPk0=; b=OfMvlYZGpfTG7sIB/JfQhvbUsn+SBjJBGDO3HdcnH7ovCNeMVKhDei66ffg5MOUnvAX5ufQyUXp/6iZQosFUcKO0KHYb1HbQn9WFU+0o8YKj6h5Lp3Y4ckpoWefmu+Iiat3ABiOGhNdYU/L83xOFstSEAx0r2oyIS36UqQ3stUZE+1X4wbDNEoL0IHKi5x57TCnWUPaAF6YhY2q+Z3Yd5zhnl+DHQR2et9IhO3SreE/vCG2fA+Ei6aDEw40BLgPnIqhJIyyvZTKZD/RA2gPKIUEOKPFs/0tmHoLK67xRPRLdrmdTOJVttl5n9OH3oYSpkl8zNXltUW/YV0duhAw1MA==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1sysvZ-0000Zf-KR for bug-gnu-emacs@gnu.org; Thu, 10 Oct 2024 09:10:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 10 Oct 2024 13:10:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 73730 X-GNU-PR-Package: emacs Original-Received: via spool by 73730-submit@debbugs.gnu.org id=B73730.17285657432115 (code B ref 73730); Thu, 10 Oct 2024 13:10:01 +0000 Original-Received: (at 73730) by debbugs.gnu.org; 10 Oct 2024 13:09:03 +0000 Original-Received: from localhost ([127.0.0.1]:59011 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sysuc-0000Y1-8R for submit@debbugs.gnu.org; Thu, 10 Oct 2024 09:09:03 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:43290) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sysuZ-0000XW-4d for 73730@debbugs.gnu.org; Thu, 10 Oct 2024 09:09:00 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sysuH-0007Qy-SL; Thu, 10 Oct 2024 09:08:41 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=/tzh8UwW1yb1oy9AryFFO8qXqx5lSOJ0b0sRwkwXPk0=; b=jiWRJsaGcD4h lWWMA2PU50OymmZ0Yu4KAAkWZiMrh4YiIMC6urCiv5wbvhm++4uDMw0n8VkKLx5SI7HnM2LIV/N/K nIK2FeBMRCwPnrmWf6YXvNh2rtI64D9VlmujBaeri+peXnc/DTEqN4qdcuhyb5O+9IdC/by3Mg37o 3Gmv7l2SMrrpSFVjRgyZXwFa9F/qnbqjL7fSDtgGSX0NAX823Oxum04xGj2SQi0SVU4aFq5sGS/9R 7kyAmK36bwXfuJnlZNgypsvCI4ua1zK9fPcIEdKEExK57fYwj6WuHiTsIcAZkWVRbdJGaXIRoFgsd sx7cYOFa/fT5JP9l831PAA==; In-Reply-To: <36a6b4d5-c719-44d6-957d-bcd7db5a854b@imayhem.com> (message from Cecilio Pardo on Thu, 10 Oct 2024 13:16:23 +0200) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:293281 Archived-At: > Date: Thu, 10 Oct 2024 13:16:23 +0200 > From: Cecilio Pardo > > 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.