From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Po Lu via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#54564: 29.0.50; [PATCH] Use gsettings font rendering entries for pgtk builds Date: Sat, 26 Mar 2022 09:16:03 +0800 Message-ID: <87o81t5x9o.fsf@yahoo.com> References: <165c1dab82dbf3233ed5f9f481a008eb724aff31.camel@teloden.nl> Reply-To: Po Lu Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="21951"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.91 (gnu/linux) Cc: 54564@debbugs.gnu.org To: Pieter van Prooijen Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Mar 26 02:17:14 2022 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 1nXv3I-0005VJ-9f for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 26 Mar 2022 02:17:12 +0100 Original-Received: from localhost ([::1]:41998 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nXv3G-0000ES-Hi for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 25 Mar 2022 21:17:10 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:33620) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nXv38-0000CU-Hh for bug-gnu-emacs@gnu.org; Fri, 25 Mar 2022 21:17:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:58892) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1nXv38-0004nC-99 for bug-gnu-emacs@gnu.org; Fri, 25 Mar 2022 21:17:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1nXv38-0001Me-5C for bug-gnu-emacs@gnu.org; Fri, 25 Mar 2022 21:17:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Po Lu Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 26 Mar 2022 01:17:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 54564 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 54564-submit@debbugs.gnu.org id=B54564.16482573855178 (code B ref 54564); Sat, 26 Mar 2022 01:17:02 +0000 Original-Received: (at 54564) by debbugs.gnu.org; 26 Mar 2022 01:16:25 +0000 Original-Received: from localhost ([127.0.0.1]:52784 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nXv2W-0001LS-HH for submit@debbugs.gnu.org; Fri, 25 Mar 2022 21:16:25 -0400 Original-Received: from sonic303-22.consmr.mail.ne1.yahoo.com ([66.163.188.148]:38171) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nXv2T-0001LC-SG for 54564@debbugs.gnu.org; Fri, 25 Mar 2022 21:16:23 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1648257375; bh=6vHW+W3yZekilwKZsmuyuXiif+Bulotyxfbuu8BBhfs=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From:Subject:Reply-To; b=apABF9E2adFqzh9n688v6DvPPzI+Z9tBTTz/mjCwQwt+fAWxjy+mAlw9Ps/nkMYn+Rhp4sXgFKC+DSWAbuOMdkCxM5bfONMYFSE7KUVzyzUHEUAr5IEQO7DEfYfKVW2iWi+DNXoM7VnKZibVNu2TkF0YNNcQ1UkotOW0V3cVS+Vz61+oDThd1p5XdObV+EYEVcpJ5PY3Ds6Xva0Q9DGSAmLQY4+SDEViwRRRYNCkVb2V0XGHoUKLUByjSoEwm9FPveyjF7fhYYHGMNgIF737nYFnucD5FcMO20cm87kzXwiEw0UPxaAxHKajBFl10EdnNHGozMyuyTz0Vzv888T+uw== X-SONIC-DKIM-SIGN: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1648257375; bh=bx1F90PaboPJigRcXx3kTHni8JCu+k0zEtu1TeEx/LR=; h=X-Sonic-MF:From:To:Subject:Date:From:Subject; b=uJQrIbj/eCNbY2CZE/6S6VyMWGq5/rVe4DvqpKAnvozJ20s3U6aJ9g7D8HGXibyIaRpw/jnv9h4hpEQdkJ4wVw6obUtZ6aUBEXwCojFNCEb6xYqGQpXin5W3mMlhXNX6tD+x9p0V2MF7zCxOLia1Shui6K80jK6qoz0dy2DthfcIEM+QAkF471sVTsBgW26DEPeXIL79Ae+BXlml+OvpinNqXhvjVLuD/Dv7l4VLPqaAbwRM5T96FTlIKXPtZmf9Soz4UW1UkbP9ujowvKeZKz/Dy3KxGf4ZwXFtb5qKC3Ua+dP5pz3fuCGE7fM0QWpQSr36lfgjPFQ24NtgvwjEgA== X-YMail-OSG: 6GrtUWQVM1nh8iaftx3YdzHYMJ7rENqfNDU.BUrs4y.7Gr7Gp.KjRrP06OQNNWA WytEZ0tafF3mwLfhmDOAU5GZG0z38yBeouSYmO4oEKCheJing_IXgGPwg6AqEZi2OTuQ2qwK1ilS EWJouo4UPW8X6oRJTmZ7i_nOQ1OLrSqhVRlkDWz4lBt.I1qUWE1FZfHKPRZ.9yZ4zUklON4aHZXB yaGPsg56MplytC3PbTR2SyuDDEqAs7IAXjBuRAFYzaTE2wTDpvkIrViKipXD4k01RrFuG3nlhr16 K8dcqHyZbXwCt82O70xX1I5lD.Lt1yvXH489PdUUkIjCMUmeWHXUiiYTKJFIl70F_U0nqYTtd3Dv ZXzoWAPDbb7jKcnH6lCQdKExICEOM.7NakjiTqM.MuvL95ih1b8u7VrrhaJvxigz6GnOG69ONuIX CYJ0KqNO3I.wFcpQz8J27kAwq3qG39pQKLBr8twWpILcWVsHNa2xlrUB4vmNM3IhdJhWk.Od5uu1 brucULATV3Rd2HXya8plZDKyR61_wZo6FbMRh8M8184cMFqjZR0WwXOE1oYqgVG086bLt19GLgld zGnm3QNAvzsDaJbTnJJb5v.XY9ZLXxHDp2b3h0wW70B_eVp9wc2QOz.1y3yoAxYkcQePDAf9CM9Z tUATCyN1QhG8op67wDSCFpTFAnUKahmLhWgyi3yPqsHzQH7dpu4.dhlid6OZexQNrS1RAqsQ4KjI Y_w8vxGajfB4D_bwIeoily_K6uUo1jUn50DIzx6WylzsQhvKHxDjJh8S50eWcxp.lvur4VOiVGEn eUa6pCagYmgir8V9vAJUxoTLmBK0uom.NUoGHFg2RE X-Sonic-MF: Original-Received: from sonic.gate.mail.ne1.yahoo.com by sonic303.consmr.mail.ne1.yahoo.com with HTTP; Sat, 26 Mar 2022 01:16:15 +0000 Original-Received: by kubenode517.mail-prod1.omega.sg3.yahoo.com (VZM Hermes SMTP Server) with ESMTPA ID 8e6f6728c38b0321039853bbec3714f9; Sat, 26 Mar 2022 01:16:08 +0000 (UTC) In-Reply-To: <165c1dab82dbf3233ed5f9f481a008eb724aff31.camel@teloden.nl> (Pieter van Prooijen's message of "Fri, 25 Mar 2022 12:23:10 +0100") X-Mailer: WebService/1.1.19987 mail.backend.jedi.jws.acl:role.jedi.acl.token.atz.jws.hermes.yahoo 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" Xref: news.gmane.io gmane.emacs.bugs:228961 Archived-At: Pieter van Prooijen writes: > When using the pgtk build of emacs under Wayland/Ubuntu, I noticed that > the font hinting was different from the regular X (Ubuntu supplied) > version of emacs. > > I have the gsettings config for font hinting set to "full" on my > system, using the fonts tab in the gnome-tweaks tool. > > It looks like Cairo defaults to the "slight" hinting setting when using > the Wayland backend for display and will only use the gsettings config > when rendering to an X backend, when it reads the current font settings > from xrdb. > > I've attached a patch (against > 380f0443b288c68df3762ee20d78719f08dd92ff) which applies the font > entries from gsettings (if present) when creating a font in Cairo. > > Note that this patch won't dynamically re-render when emacs is already > running (like it does when changing the system font). I made an > attempt to hook into the gsettings change callback, but could not > force a re-creation of the font with the changed parameters,for > instance using a 'font-render' config changed event. Any pointers on > how to achieve this? I'm not sure, but perhaps someone else around here knows. > Do you think if this patch is a good approach to get the pgtk build to > use the system preferences for font hinting and anti-aliasing? The basic approach is good, thanks for working on this. However, I have some comments on your code below. > Kind Regards, > > Pieter van Prooijen > > From 6fe888e7c14ef5aad124c0d68fcd071cd5b65e81 Mon Sep 17 00:00:00 2001 > From: Pieter van Prooijen > Date: Fri, 25 Mar 2022 11:47:39 +0100 > Subject: [PATCH] Use gsettings font rendering entries for pgtk builds > > If present, apply the gsettings font hinting and antialiasing entries > when creating a font in cairo. > * src/xsettings.c, src/xsettings.h: read and apply gsetting entries. > * src/ftcrfont.c: use the font_options from xsettings.c when creating a font. Please capitalize the first character of each line in the changelog message after the files. Also, try to write an entry for each function that was changed, instead of lumping all the changes together in one file. > +#include "xsettings.h" This must be conditional on HAVE_X_WINDOWS || HAVE_PGTK. > - cairo_font_options_t *options = cairo_font_options_create (); > + > #ifdef USE_BE_CAIRO > + cairo_font_options_t *options = cairo_font_options_create(); > if (be_use_subpixel_antialiasing ()) > cairo_font_options_set_antialias (options, CAIRO_ANTIALIAS_SUBPIXEL); > +#else > + cairo_font_options_t *options = xsettings_get_font_options(); > #endif This else statement should be conditional on HAVE_PGTK, correct? The Cairo font backend currently supports 3 platforms: X Windows, where Cairo will detect the font options by itself, Haiku (where it is configured manually), and PGTK. > +#ifdef USE_CAIRO > +static cairo_font_options_t *font_options; > +#endif This is conditional on USE_CAIRO, but most of the code below is conditional on HAVE_PGTK. Is that really correct? > +/* For hinting and antialias no dynamic re-display is (yet) possible, > + because cairo applies these options when creating the font, not when > + rendering it. > +*/ This is not how we write comments. Please keep the closing part on the same line, with only two spaces separating it from the last line, like this: /* Changes in hinting and antialiasing preferences cannot currently be applied to existing fonts, since the Cairo font backend does not support changing font options after a font object is created. */ > +#ifdef HAVE_PGTK > +/* Apply changes in the hinting system setting */ > +static void > +apply_gsettings_font_hinting(GSettings *settings) > +{ > + GVariant *val = g_settings_get_value (settings, GSETTINGS_FONT_HINTING); > + if (val) > + { > + g_variant_ref_sink (val); > + if (g_variant_is_of_type (val, G_VARIANT_TYPE_STRING)) > + { > + const char *hinting = g_variant_get_string (val, NULL); > + > + if (strcmp (hinting, "full") == 0) > + cairo_font_options_set_hint_style(font_options, CAIRO_HINT_STYLE_FULL); > + else if (strcmp (hinting, "medium") == 0) > + cairo_font_options_set_hint_style(font_options, CAIRO_HINT_STYLE_MEDIUM); > + else if (strcmp (hinting, "slight") == 0) > + cairo_font_options_set_hint_style(font_options, CAIRO_HINT_STYLE_SLIGHT); > + else if (strcmp (hinting, "none") == 0) > + cairo_font_options_set_hint_style(font_options, CAIRO_HINT_STYLE_NONE); > + } > + g_variant_unref (val); > + } > +} This will need some stylistic changes as well: we put a space between the function name and its parameter list, like such: static void apply_gsettings_font_hinting (GSettings *settings) And inside function calls as well: cairo_font_options_set_hint_style (font_options, CAIRO_HINT_STYLE_FULL); Also, the idomatic style is to write !strcmp (hinting, "full") instead of saying strcmp (hinting, "full") == 0. > +/* Apply changes in the antialiasing system setting */ Please also put a period after the sentences in each comment, followed by two spaces. > +#ifdef HAVE_PGTK > + font_options = cairo_font_options_create (); > + apply_gsettings_font_antialias(gsettings_client); > + apply_gsettings_font_hinting(gsettings_client); > + apply_gsettings_font_rgba_order(gsettings_client); > +#endif /* HAVE_PGTK */ Similar stylistic fixes are also needed to the function calls here. > +#ifdef USE_CAIRO > +/* Return the cairo font options, possibly initialized in > + init_gsettings() */ > +cairo_font_options_t * > +xsettings_get_font_options (void) { > + if (font_options == NULL) > + return cairo_font_options_create(); > + else > + return cairo_font_options_copy(font_options); > +} > +#endif Please don't say "init_gsettings()" inside a comment to mean that `init_gsettings' is a function. That makes it look like a function call with no arguments. I would write the comment like this: /* Return the cairo font options based on the user's font preferences, possibly initialized in `init_gsettings'. */ Our style is also to put the opening brace of a function definition on a new line, so that tools which look for defuns by looking for opening parens in column 0 can operate correctly. This should probably be conditional on HAVE_PGTK instead of USE_CAIRO as well. > +#ifdef USE_CAIRO > + font_options = NULL; > + PDUMPER_IGNORE (font_options); > +#endif Likewise. > +#ifdef USE_CAIRO > +struct cairo_font_options_t; > +extern cairo_font_options_t *xsettings_get_font_options (void); > +#endif This too. Also, just include `cairo.h' instead of declaring `struct cairo_font_options_t'. Thanks again for working on Emacs.