bug#22383: 25.1.50; Wrong font height for some fonts

bug#22383: 25.1.50; Wrong font height for some fonts

[Fangwen Yu]

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

With DejaVu Sans Mono-11, text lines in Emacs are 1 pixel taller than in
gedit or gnome-terminal, also, bold faces are 1 pixel taller than
regular, so if the theme uses bold faces, texts will jump up and down
while editing. The line height problem also happens with Consolas font,
but the bold faces problem doesn't.

Since it clearly has something to do with font height, I digged into the
source and found the related code. Apply this patch, and the problem is
gone:

diff --git a/src/xftfont.c b/src/xftfont.c
index 956231e..d0f9a68 100644
--- a/src/xftfont.c
+++ b/src/xftfont.c
@@ -395,16 +395,16 @@ xftfont_open (struct frame *f, Lisp_Object entity,
int pixel_size)

   font->ascent = xftfont->ascent;
   font->descent = xftfont->descent;
-  if (pixel_size >= 5)
-    {
-      /* The above condition is a dirty workaround because
-     XftTextExtents8 behaves strangely for some fonts
-     (e.g. "Dejavu Sans Mono") when pixel_size is less than 5. */
-      if (font->ascent < extents.y)
-    font->ascent = extents.y;
-      if (font->descent < extents.height - extents.y)
-    font->descent = extents.height - extents.y;
-    }
+  /* if (pixel_size >= 5) */
+  /*   { */
+  /*     /\* The above condition is a dirty workaround because */
+  /*        XftTextExtents8 behaves strangely for some fonts */
+  /*        (e.g. "Dejavu Sans Mono") when pixel_size is less than 5. *\/
*/
+  /*     if (font->ascent < extents.y) */
+  /*       font->ascent = extents.y; */
+  /*     if (font->descent < extents.height - extents.y) */
+  /*       font->descent = extents.height - extents.y; */
+  /*   } */
   font->height = font->ascent + font->descent;

   if (XINT (AREF (entity, FONT_SIZE_INDEX)) == 0)


As the comment says, this piece of code is a dirty workaround, and
apparently it is causing some new problems.



In GNU Emacs 25.1.50.36 (x86_64-unknown-linux-gnu, GTK+ Version 3.16.7)
 of 2016-01-15 built on katana
Repository revision: 4580671f5e2a68885e0fca93eeaf9daaeebe82b3
Windowing system distributor 'The X.Org Foundation', version 11.0.11702000
System Description:    Ubuntu 15.10

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF GSETTINGS
NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11

Important settings:
  value of $LC_COLLATE: C
  value of $LC_CTYPE: zh_CN.UTF-8
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=fcitx
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Making completion list...

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message dired dired-loaddefs
format-spec rfc822 mml mml-sec mm-decode mm-bodies mm-encode mail-parse
rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045
ietf-drums mm-util help-fns help-mode easymenu cl-loaddefs pcase cl-lib
mail-prsvr mail-utils time-date mule-util china-util tooltip eldoc
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list newcomment elisp-mode lisp-mode prog-mode register page
menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core frame cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese charscript case-table epa-hook jka-cmpr-hook help
simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs button
faces cus-face macroexp files text-properties overlay sha1 md5 base64
format env code-pages mule custom widget hashtable-print-readable
backquote dbusbind inotify dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 80789 6761)
 (symbols 48 19157 0)
 (miscs 40 178 100)
 (strings 32 12817 4884)
 (string-bytes 1 387166)
 (vectors 16 11243)
 (vector-slots 8 468928 19771)
 (floats 8 142 27)
 (intervals 56 206 0)
 (buffers 976 12)
 (heap 1024 21736 1037))

[-- Attachment #2: Type: text/html, Size: 5189 bytes --]

bug#22383: 25.1.50; Wrong font height for some fonts

[Eli Zaretskii]

> Date: Sat, 16 Jan 2016 15:07:04 +0800
> From: Fangwen Yu <yynyygy@gmail.com>
> 
> With DejaVu Sans Mono-11, text lines in Emacs are 1 pixel taller than in
> gedit or gnome-terminal, also, bold faces are 1 pixel taller than
> regular, so if the theme uses bold faces, texts will jump up and down
> while editing. The line height problem also happens with Consolas font,
> but the bold faces problem doesn't.
> 
> Since it clearly has something to do with font height, I digged into the
> source and found the related code. Apply this patch, and the problem is
> gone:
> 
> diff --git a/src/xftfont.c b/src/xftfont.c
> index 956231e..d0f9a68 100644
> --- a/src/xftfont.c
> +++ b/src/xftfont.c
> @@ -395,16 +395,16 @@ xftfont_open (struct frame *f, Lisp_Object entity, int
> pixel_size)
> 
> font->ascent = xftfont->ascent;
> font->descent = xftfont->descent;
> - if (pixel_size >= 5)
> - {
> - /* The above condition is a dirty workaround because
> - XftTextExtents8 behaves strangely for some fonts
> - (e.g. "Dejavu Sans Mono") when pixel_size is less than 5. */
> - if (font->ascent < extents.y)
> - font->ascent = extents.y;
> - if (font->descent < extents.height - extents.y)
> - font->descent = extents.height - extents.y;
> - }
> + /* if (pixel_size >= 5) */
> + /* { */
> + /* /\* The above condition is a dirty workaround because */
> + /* XftTextExtents8 behaves strangely for some fonts */
> + /* (e.g. "Dejavu Sans Mono") when pixel_size is less than 5. *\/ */
> + /* if (font->ascent < extents.y) */
> + /* font->ascent = extents.y; */
> + /* if (font->descent < extents.height - extents.y) */
> + /* font->descent = extents.height - extents.y; */
> + /* } */
> font->height = font->ascent + font->descent;
> 
> if (XINT (AREF (entity, FONT_SIZE_INDEX)) == 0)
> 
> As the comment says, this piece of code is a dirty workaround, and
> apparently it is causing some new problems.

Thanks for the report and the patch.

I'm CC'ing Handa-san, who wrote that code, in the hope that he could
comment on this issue.

bug#22383: 25.1.50; Wrong font height for some fonts

[handa]

Sorry for the late response.

In article <834meeeyet.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:
> > Date: Sat, 16 Jan 2016 15:07:04 +0800
> > From: Fangwen Yu <yynyygy@gmail.com>
[...]
> > diff --git a/src/xftfont.c b/src/xftfont.c
> > index 956231e..d0f9a68 100644
> > --- a/src/xftfont.c
> > +++ b/src/xftfont.c
> > @@ -395,16 +395,16 @@ xftfont_open (struct frame *f, Lisp_Object entity, int
> > pixel_size)
> > 
> > font->ascent = xftfont->ascent;
> > font->descent = xftfont->descent;
> > - if (pixel_size >= 5)
> > - {
> > - /* The above condition is a dirty workaround because
> > - XftTextExtents8 behaves strangely for some fonts
> > - (e.g. "Dejavu Sans Mono") when pixel_size is less than 5. */
> > - if (font->ascent < extents.y)
> > - font->ascent = extents.y;
> > - if (font->descent < extents.height - extents.y)
> > - font->descent = extents.height - extents.y;
> > - }
> > + /* if (pixel_size >= 5) */
> > + /* { */
> > + /* /\* The above condition is a dirty workaround because */
> > + /* XftTextExtents8 behaves strangely for some fonts */
> > + /* (e.g. "Dejavu Sans Mono") when pixel_size is less than 5. *\/ */
> > + /* if (font->ascent < extents.y) */
> > + /* font->ascent = extents.y; */
> > + /* if (font->descent < extents.height - extents.y) */
> > + /* font->descent = extents.height - extents.y; */
> > + /* } */
> > font->height = font->ascent + font->descent;
> > 
> > if (XINT (AREF (entity, FONT_SIZE_INDEX)) == 0)
> > 
> > As the comment says, this piece of code is a dirty workaround, and
> > apparently it is causing some new problems.

I've just tried the latest Xft with the attached small program.  It
seems that there's no need to adjust ascent and descent of fonts
(including "dejavu sans mono"); i.e. we can trust xftfont->ascent and
xftfont->descent.  So, I think we can use your patch.

---
K. Handa
handa@gnu.org

#include <stdio.h>
#include <X11/Xlib.h>
#include <X11/Xft/Xft.h>

int
main (int argc, char **argv)
{
  Display *display = XOpenDisplay (NULL);
  FcPattern *pat = FcNameParse ((FcChar8 *) argv[1]);
  FcResult result;
  FcPattern *match = XftFontMatch (display, 0, pat, &result);
  XftFont *xftfont = XftFontOpenPattern (display, match);
  XGlyphInfo extents;
  XftTextExtents8 (display, xftfont, "ABCDEF", 6, &extents);
  printf ("%dx%d+%d+%d %d+%d\n",
	  extents.width, extents.height, extents.x, extents.y,
	  xftfont->ascent, xftfont->descent);
}

bug#22383: 25.1.50; Wrong font height for some fonts

[Eli Zaretskii]

> From: handa <handa@gnu.org>
> Cc: yynyygy@gmail.com, 22383@debbugs.gnu.org
> Date: Thu, 28 Jan 2016 22:00:43 +0900
> 
> I've just tried the latest Xft with the attached small program.  It
> seems that there's no need to adjust ascent and descent of fonts
> (including "dejavu sans mono"); i.e. we can trust xftfont->ascent and
> xftfont->descent.  So, I think we can use your patch.

Thanks, installed.