googlefonts / Inconsolata

Development repo of Inconsolata Fonts by Raph Levien
http://levien.com/type/myfonts/inconsolata.html
SIL Open Font License 1.1
1.39k stars 64 forks source link

Inconsolata v3.000 letter-spacing is too wide in programs that use Xft #42

Open agreselin opened 4 years ago

agreselin commented 4 years ago

Hello, after upgrading to Inconsolata v3.000 on Fedora 31, letter-spacing has become too wide in Emacs, as shown in the attached screenshot; it shows the Emacs window I get on running emacs -Q. I'm running Emacs 26.3.

Screenshot from 2019-12-23 10-03-52

Note that the font works correctly in other applications such as Gedit and LibreOffice.

mjmartineau commented 4 years ago

I see this problem on Fedora 30. It happens with both emacs 26.2 and 26.3.

aterweele commented 4 years ago

I can also repro on Emacs 27.0.50.

2020-01-10-220558_629x737_scrot

raphlinus commented 4 years ago

This is very likely a bug in Emacs and not the font, though I'm happy to apply a workaround. I recommend filing a bug against Emacs and see if they can analyze it.

There are a couple of culprits which seem plausible. One is the ligatures (though if that were the case it would probably have triggered with an earlier version). Perhaps a more likely explanation is that when it assembles a family it gets confused and thinks that the expanded widths are the reference for width.

agreselin commented 4 years ago

I recommend filing a bug against Emacs and see if they can analyze it.

Done, here it is: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=39082

Eli-Zaretskii commented 4 years ago

I can also repro on Emacs 27.0.50

27.0.50 is an old development version of an unknown date. Current Emacs master is at 28.0.50, and the emacs-27 release branch is at 27.0.60.

Please make sure you are using the latest Emacs codebase, and make sure your build is with HarfBuzz, which is our default font-backend starting from Emacs 27.

FWIW, I see no problems with this font in Emacs 27, but I'm on MS-Windows, not on Fedora. Maybe it's a problem with font software on Fedora.

Eli-Zaretskii commented 4 years ago

Perhaps a more likely explanation is that when it assembles a family it gets confused and thinks that the expanded widths are the reference for width.

Could you please elaborate on this? What do you mean by "assemble a family", and what are the "expanded widths" and where do they come from? And how is this different from the older versions of Inconsolata?

I'm not an expert on fonts, but with enough information I could try looking at the relevant Emacs code to see whether it gets confused by this font.

TIA

Eli-Zaretskii commented 4 years ago

Also: are the problems with OTF or TTF version of the font?

Eli-Zaretskii commented 4 years ago

And one more question: what does the below produce in an Emacs session with the problematic display?

    M-x set-variable RET eval-expression-print-length RET nil RET
    M-: (font-info (font-at 1)) RET

Please show the full output (take it from the *Messages* buffer if needed.)

agreselin commented 4 years ago

Also: are the problems with OTF or TTF version of the font?

I use the fonts from Fedora's levien-inconsolata-fonts package, which are TTF's.

And one more question: what does the below produce in an Emacs session with the problematic display?

I get ["-CYRE-Inconsolata-normal-normal-normal-*-21-*-*-*-m-0-iso10646-1" "Inconsolata:pixelsize=21:foundry=CYRE:weight=normal:slant=normal:width=normal:spacing=100:scalable=true" 21 23 0 0 0 32 19 4 32 32 "/usr/share/fonts/levien-inconsolata/Inconsolata-Regular.ttf" (opentype ((DFLT ...) (latn ... ... ... ... ... ... ... ... ...)) (DFLT (nil mark mkmk)) (latn (nil mark mkmk) (AZE\ mark mkmk) (CAT\ mark mkmk) (CRT\ mark mkmk) (KAZ\ mark mkmk) (MOL\ mark mkmk) (ROM\ mark mkmk) (TAT\ mark mkmk) (TRK\ mark mkmk)))]. Note that this is in Emacs 26.3 (the current version from Fedora's repos).

Eli-Zaretskii commented 4 years ago

And if you invoke Emacs as emacs -Q -fn Inconsolata-12, what happens then with the output of the above command and with the spacing on display?

Eli-Zaretskii commented 4 years ago

Also, can you try the OTF font?

agreselin commented 4 years ago

And if you invoke Emacs as emacs -Q -fn Inconsolata-12, what happens then with the output of the above command and with the spacing on display?

["-CYRE-Inconsolata-normal-normal-normal-*-19-*-*-*-m-0-iso10646-1" "Inconsolata:pixelsize=19:foundry=CYRE:weight=normal:slant=normal:width=normal:spacing=100:scalable=true" 19 21 0 0 0 29 17 4 29 29 "/usr/share/fonts/levien-inconsolata/Inconsolata-Regular.ttf" (opentype ((DFLT ...) (latn ... ... ... ... ... ... ... ... ...)) (DFLT (nil mark mkmk)) (latn (nil mark mkmk) (AZE\ mark mkmk) (CAT\ mark mkmk) (CRT\ mark mkmk) (KAZ\ mark mkmk) (MOL\ mark mkmk) (ROM\ mark mkmk) (TAT\ mark mkmk) (TRK\ mark mkmk)))]

Screenshot from 2020-01-12 15-13-50

Also, can you try the OTF font?

The OTF font I've downloaded from dl.fedoraproject.org is also afflicted by this bug. (font-info (font-at 1)) output after running emacs -Q -fn Inconsolata-12 is ["-CYRE-Inconsolata-normal-normal-normal-*-19-*-*-*-m-0-iso10646-1" "Inconsolata:pixelsize=19:foundry=CYRE:weight=normal:slant=normal:width=normal:spacing=100:scalable=true" 19 21 0 0 0 29 17 4 29 29 "/home/andrea/.fonts/Inconsolata-3.000/otf/Inconsolata-Regular.otf" (opentype ((DFLT ...) (latn ... ... ... ... ... ... ... ... ...)) (DFLT (nil mark mkmk)) (latn (nil mark mkmk) (AZE\ mark mkmk) (CAT\ mark mkmk) (CRT\ mark mkmk) (KAZ\ mark mkmk) (MOL\ mark mkmk) (ROM\ mark mkmk) (TAT\ mark mkmk) (TRK\ mark mkmk)))]

Eli-Zaretskii commented 4 years ago

Using the related Emacs APIs, we've established that Emacs treats the glyphs of this font as if they had width of 29 pixels, instead of the expected 10 or thereabouts. The way Emacs accesses the glyph metrics is as follows:

static void xftfont_text_extents (struct font font, const unsigned int code, int nglyphs, struct font_metrics metrics) { struct font_info xftfont_info = (struct font_info *) font; XGlyphInfo extents;

block_input (); XftGlyphExtents (xftfont_info->display, xftfont_info->xftfont, code, nglyphs, &extents); unblock_input ();

metrics->lbearing = - extents.x; metrics->rbearing = - extents.x + extents.width; metrics->width = extents.xOff; metrics->ascent = extents.y; metrics->descent = extents.height - extents.y; }

(where CODE is the index of the glyph for the character in this font).

Can the font experts here tell whether this is incorrect, at least with this font, and if so, how to fix it?

Eli-Zaretskii commented 4 years ago

According to https://lists.gnu.org/archive/html/bug-gnu-emacs/2020-01/msg00509.html, this seems to be a bug in Xft, and the workaround is to use the Cairo build of Emacs, which doesn't have this problem. Xft seems to be unmaintained, and Emacs will probably deprecate its use in some future version. So I guess a better way forward is to switch to the Cairo build.

raphlinus commented 4 years ago

@Eli-Zaretskii Thanks for the update, that's useful. Do you know if there's a workaround that can be done in the font in the meantime?

Eli-Zaretskii commented 4 years ago

Sorry, I have no idea. Someone will have to debug Xft and find out where they fail, and then perhaps there could be a workaround in the font. At this point, I don't even know which change in the font triggers this problem in Xft, I just know that both Cairo's cairo_scaled_font_glyph_extents API and MS-Windows GetGlyphOutlineW API do return correct advance data for this font, while Xft returns a value that is about 3 times larger than what's expected given the font size.

rpluim commented 4 years ago

Fixing this in Xft is unlikely. Is it possible to identify which version of Inconsolata does not have this issue? If so, it might be possible to compare with the problematic version and fix it in the font.

rpluim commented 4 years ago

Answering my own question: git bisect blames the following commit:

59a0b4209de24c4b396298ba08bebb96d109062e is the first bad commit
commit 59a0b4209de24c4b396298ba08bebb96d109062e
Author: Brenton Simpson <appsforartists@google.com>
Date:   Wed Mar 7 18:22:53 2018 -0800

    Added ligatures for !==, ===, =>, <=, and >=.
Eli-Zaretskii commented 4 years ago

Thanks for identifying the culprit. However, it is strange that providing ligatures for these somehow affected ASCII letters. Does the actual change explain why that happened?

rpluim commented 4 years ago

I don't know, it's a huge change and I don't know the format at all

D4N commented 4 years ago

According to https://lists.gnu.org/archive/html/bug-gnu-emacs/2020-01/msg00509.html, this seems to be a bug in Xft, and the workaround is to use the Cairo build of Emacs, which doesn't have this problem. Xft seems to be unmaintained, and Emacs will probably deprecate its use in some future version. So I guess a better way forward is to switch to the Cairo build.

I can confirm that a Cairo build of emacs doesn't the font spacing issue.

sassmann commented 4 years ago

xterm is suffering from the same issue, making Inconsolata 2.012 and following basically unusable in xterm. When comparing the width reported by xfd -fa Inconsolata For version 2.000 it reports a width of 8 while starting with 2.012 width is reported as 24.

That matches the output from xterm debug output (compiled with --enable-trace) version 2.000:

xtermComputeFontInfo font 0: norm(face Inconsolata:style:regular, size 12.0)
Using Xft 20303
Using FontConfig 21392
[...]
SetFontWidth  8
SetFontHeight 17

version 2.012

xtermComputeFontInfo font 0: norm(face Inconsolata:style:regular, size 12.0)
Using Xft 20303
Using FontConfig 21392
[...]
SetFontWidth  24
SetFontHeight 17
tejohnso commented 4 years ago

Seems Inconsolata 3 broke st as well.

prosoitos commented 4 years ago

Arch Linux:

And thanks for the Xft pointer. That explains why there is no issue in urxvt or Emacs outside X11...

dumblob commented 4 years ago

See https://github.com/googlefonts/Inconsolata/issues/35#issuecomment-615951622 for an idea to allow building Inconsolata easily without ligatures which could actually completely avoid this problem (and if @raphlinus by default also provided an already built version of Inconsolata without ligatures, then it'd be super easy to just select this variant of Inconsolata in your app settings and all such Xft apps would be good to go).

agreselin commented 4 years ago

It looks allright now on Emacs 27. Is it ok if I close the issue?

dumblob commented 4 years ago

@agreselin could you maybe leave it open and rename the issue to a more generic name to account for xterm and other Xft-only apps (instead of just Emacs)?

JasonBrownDeveloper commented 3 years ago

For monospace fonts, Xft uses the max_advance_width global metric to move the pen after each glyph. For Inconsolata, that's coming out at 20, but the advance.x for the individual glyphs seem to be 7.

mokulus commented 3 years ago

I've checked in st, and xOff value returned in XGlyphInfo is 3 times as high as it should be. As mentioned before it seems to be related to added ligatures (biggest ligature glyph, eg <=> is thrice as big as normal letter). In st apply this patch:

diff --git a/x.c b/x.c
index 36dd8bc..68781cf 100644
--- a/x.c
+++ b/x.c
@@ -953,7 +953,7 @@ xloadfont(Font *f, FcPattern *pattern)
    f->rbearing = f->match->max_advance_width;

    f->height = f->ascent + f->descent;
-   f->width = DIVCEIL(extents.xOff, strlen(ascii_printable));
+   f->width = DIVCEIL(extents.xOff > 18 ? extents.xOff / 3 : extents.xOff, strlen(ascii_printable));

    return 0;
 }

Increase the font with Ctrl+Shift+PageUp. The font looks normal.

JBlocklove commented 3 years ago

I get the same font spacing issue with dmenu. I can confirm that the above patch fixed the font spacing issue in st for me, so tried to apply a similar patch to dmenu but with no luck. Shrinking the xOff value in dmenu's drw.c file seems to shrink the width of the window as a whole but has no affect on the text. Any ideas on what might help fix that?

prosoitos commented 3 years ago

As an alternative to @mokulus's patch, using Inconsolata Regular:family=mono instead of inconsolata does the trick as well.

I tested this with success in urxvt:

URxvt.font:     xft:Inconsolata Regular:family=mono:size=28

and in st:

static char *font = "Inconsolata Regular:family=mono:size=26:antialias=true:autohint=true";

Maybe you can try that with dmenu?

As a side note, if that doesn't work, I use rofi instead of dmenu (a replacement with added functionality) with inconsolata and I don't have any problem.

JBlocklove commented 3 years ago

No dice, still weird spacing. Thanks, though. And I used to use rofi, but I prefer dmenu.

vith commented 3 years ago

This is also affecting tk apps such as gitk. I don't know how the tk toolkit is choosing the default fonts but I had to uninstall inconsolata-ttf from my system to avoid the problem.

It also affects the tk example apps, so it's not gitk specific. Running /usr/lib/tk8.6/demos/widget and launching the 1. Basic editable text demo from the Text category, I see the same problem.

On Arch Linux.

kronikpillow commented 3 years ago

Inconsolata is my favorite monospaced font, and i can't live without it, but I use suckless dwm/dmenu/st and i can't live without those either, and this problem exists in all 3 of them, anyone found a viable solution for this yet cuz i'm stuck to using the pre 2.012

JasonBrownDeveloper commented 3 years ago

I'm sure this is not the correct fix for Xft, but this has been working for me for about two months now.

diff --git a/src/xftglyphs.c b/src/xftglyphs.c
index 4b5fb82..baeb1d9 100644
--- a/src/xftglyphs.c
+++ b/src/xftglyphs.c
@@ -551,11 +551,11 @@ XftFontLoadGlyphs (Display        *dpy,
        if (font->info.load_flags & FT_LOAD_VERTICAL_LAYOUT)
        {
            xftg->metrics.xOff = 0;
-           xftg->metrics.yOff = -font->public.max_advance_width;
+           xftg->metrics.yOff = -TRUNC(ROUND(glyphslot->advance.x));
        }
        else
        {
-           xftg->metrics.xOff = font->public.max_advance_width;
+           xftg->metrics.xOff = TRUNC(ROUND(glyphslot->advance.x));
            xftg->metrics.yOff = 0;
        }
        }
0xc0392b commented 3 years ago

Inconsolata is my favorite monospaced font, and i can't live without it, but I use suckless dwm/dmenu/st and i can't live without those either, and this problem exists in all 3 of them, anyone found a viable solution for this yet cuz i'm stuck to using the pre 2.012

I'm on 2.24.32 using Inconsolata 3.001.

I compiled from source using ./configure --with-cairo and (set-frame-font "Inconsolata") in my .emacs.

I believe using Cairo over Xft is still experimental but I have been using my current build for three months and have had no issues.

kronikpillow commented 3 years ago

wait what do you mean built from source using ./configure --with-cairo ? build the font from source with cairo? I have no clue what you just told me @0xc0392b

smemsh commented 2 years ago

@mokulus curious where the value 18 came from in your st patch? Your fix seems to work in st, but not with older versions of Inconsolata prior to these ligatures. The xOff values returned by XftTextExtentsUtf8() in &extents are much larger than 18 for old vs new version of Inconsolata (on my system):

 Inconsolata2: width = 2471, height = 45, x = 1, y = 35, xOff = 2470, yOff = 0
 Insonsolata3: width = 7358, height = 49, x = 1, y = 39, xOff = 7410, yOff = 0

I do not understand where these large values are coming from though, they seem wildly wrong. According to https://bugzilla.mozilla.org/show_bug.cgi?id=128153#c25 :

Width/height are the size of the glyph image in pixels. x,y is the offset from the origin of the glyph image to the rendering origin. xOff, yOff is the normal spacing to the next glyph

So if they are pixels and it's a relative offset, not sure how value can be in the thousands? (these are as seen in gdb and also error prints with %hu) The height, x and y values do seem to be pixels but width and xOff values are super high. Not sure if something changed in Xft since your patch? I'm on libxft v2.3.4; it does look like libxft has semi-recent commits but max_advance_width is still used to create xOff.

I tried with several fonts with and without ligatures, xOff seems to always have a larger gap from width with old vs new fonts. Zooming increases both xOff and width, they seem to keep the same absolute delta. If I use your value from original patch 18 for that comparison as the threshold, eg:

--- a/x.c
+++ b/x.c
@@ -1057,7 +1057,10 @@ xloadfont(Font *f, FcPattern *pattern)
        f->rbearing = f->match->max_advance_width;

        f->height = f->ascent + f->descent;
-       f->width = DIVCEIL(extents.xOff, strlen(ascii_printable));
+       unsigned short xoff = extents.xOff;
+       signed xdelta = xoff - extents.width;
+       _Bool isnewfont = abs(xdelta) > 18;
+       f->width = DIVCEIL(isnewfont ? xoff / 3 : xoff, strlen(ascii_printable));

        return 0;
 }

then st works with both old and new fonts. I have kept only the latest Inconsolata-VariableFont_wdth,wght.ttf from Google, renamed it with a 3 and then renamed the font itself to Inconsolata3 using this font rename tool, which causes fc-match to find the old file for Inconsolata but the new one for Inconsolata3 . This way I can use the latter for st with the patch and former for programs confused by the Xft output.

Also note, xoff / 3 may not be exactly the right math, but does "look" right, could be subtly off. Probably breaks on fonts where there are more or less than 3 character max ligature width...

dumblob commented 2 years ago

it does look like libxft has semi-recent commits but max_advance_width is still used to create xOff.

Wow, I thought xft upstream is long dead. If not completely, then it would make sense to submit a patch for this behavior there.

smemsh commented 2 years ago

it would make sense to submit a patch for this behavior [to libxft upstream]

https://gitlab.freedesktop.org/xorg/lib/libxft/-/issues/15

dumblob commented 2 years ago

@smemsh awesome, thanks a lot! I have now subscribed to that issue in their GitLab.

JasonBrownDeveloper commented 2 years ago

I've been meaning to write a more in-depth analysis of this issue with examples, but I just haven't found the time. Essentially there is no "bug" here on the part either side of libxft or Inconsolata, but a clashing of ideology. Ultimately whose responsibility is it to maintain the monospacing of a font? libxft makes the assumption that if a font says that it's monospace then advance width should be the same for every glyph. This would smooth out fonts that have glyphs that have similar but not the same advances, but would have problems in fonts that have glyphs that represent multiple characters.

I think the best solution is for the fonts themselves to maintain their advances for each glyph and for libxft to not handle monospace fonts specially. I've thought about the kinds of heuristics libxft would need to make "bad" monospace fonts look good, while not breaking "good" monospace fonts and it's a bit untenable.

The very very simple heuristic in use now, using the max advance obviously doesn't work for fonts with multi-character glyphs. So a further step would be to take the mode advance and always advance in increments of that. That would work for a "good" font, say single character glyphs advance 7 units so the mode would be 7, and the triple glyph would advance 21 units. Let's think of a "bad" font, imagine half the glyphs have an advance of 7 and half -1 have an advance of 8, The mode would be 7 and half -1 of the glyphs would have an advance of 14 because they're 1 unit bigger than the mode.

Now you need to take a standard deviation from the mode to account for glyphs that are similar but not the same, but advance issues become more apparent the more you multiply them out. A triple glyph designed on the smaller 7 unit size would have an advance of 24 and would present a noticeable gap at then end of it. And on and on and on with the heuristics of trying to get "bad" fonts to look good.

I've been using the patch I posted above since I posted it and haven't noticed any unintended side-effects, but I'm only one user and that doesn't make a good sample size. Also the patch doesn't remove the monospace special case, it only changes the advance in the special case. The proper patch would be to remove the monospace handling completely and just treat a monospace font like other fonts.

dumblob commented 2 years ago

@JasonBrownDeveloper thanks for your input and important observations. I am 100% convinced we must be only pragmatic in this case.

If your patch solves the problem (and IMHO it does), then let us spread the patch as much as we can to make it work for the next 10-20 years before Wayland(-like) systems will fully take over the reign of X11.

Finally a little nice thing would be to let other unrelated but relevant projects know about this and advice them how to avoid such issues.

smemsh commented 2 years ago

Xft project has now merged @JasonBrownDeveloper 's libxft fix upstream, so this issue may be complete. See aforementioned gitlab issue. Perhaps not the most correct fix, but it can help a lot of folks for now, so thanks.