After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 793391 - Do not cache ligatured glyphs (e.g. for "<=>")
Do not cache ligatured glyphs (e.g. for "<=>")
Status: RESOLVED OBSOLETE
Product: vte
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: vte-0-52
Assigned To: VTE Maintainers
VTE Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-02-12 15:33 UTC by grmat
Modified: 2021-06-10 15:29 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
wrong ligature display with gnome-terminal (3.69 KB, image/png)
2018-02-12 15:33 UTC, grmat
  Details
correct ligature replacement in Konsole (4.11 KB, image/png)
2018-02-12 15:33 UTC, grmat
  Details
Easy workaround (799 bytes, patch)
2018-02-12 22:42 UTC, Egmont Koblinger
committed Details | Review

Description grmat 2018-02-12 15:33:12 UTC
Created attachment 368264 [details]
wrong ligature display with gnome-terminal

There are monospace fonts designed for coding, improving appeal and readability, e.g. Monoid, Fira Code or Hasklig. Those fonts use ligatures for common character sequences like >> >= != etc. All VTE-based terminals seem to have problems with those. E.g. when entering

echo test > /tmp/test

they would replace the '>' char with the ligature for '=>'.

Please see attached screenshots with this input in gnome-terminal and konsole for reference. For demonstration purposes, I've also appended a useless '=>' to the string.

Tested with gnome-terminal and termite (also see https://github.com/thestinger/termite/issues/315)
Comment 1 grmat 2018-02-12 15:33:44 UTC
Created attachment 368265 [details]
correct ligature replacement in Konsole
Comment 2 Christian Persch 2018-02-12 15:51:15 UTC
What exactly do you consider wrong in your screenshots? The arrow being too long in the first one means that the font is not monospace (or it's substituting a different font); that's not supported.

In any case, vte does not support ligatures, at all.

*** This bug has been marked as a duplicate of bug 584160 ***
Comment 3 Egmont Koblinger 2018-02-12 19:27:52 UTC
See also bug 762832 comment 1.

On the other hand, even though I don't know how ligatures are implemented in fonts, a simple ">" showing up as "=>" indeed looks pretty bad. I wonder how this could happen, and whether it's a bug in VTE or in the font. I'm pretty sure that if a non-ligature-aware app uses such a font, it should at least properly show up without ligatures.
Comment 4 Egmont Koblinger 2018-02-12 20:23:06 UTC
Let me undup.

VTE not properly supporting ligatures is one thing. But not being able to pick the ligature-free basic glyphs is a more serious problem.

I can reproduce the reported problem with Fira Code Regular, plus some other symbols (e.g. '<' and "=") don't show up at all (i.e. look like spaces).

The same thing shows up properly in e.g.
  pango-view --font 'Fira Code Regular 10' --text '<'

Since VTE renders its glyphs one by one, I'm wondering why it doesn't pick the very same glyph as pango-view with a single-char text.

IMO this really should be fixed much sooner than we get to the actual ligature/harfbuzz story.
Comment 5 Egmont Koblinger 2018-02-12 22:05:26 UTC
As a pretty blind (but successful) guess, I've commented out the line
  uinfo->coverage = COVERAGE_USE_CAIRO_GLYPH;
from vtedraw.cc font_info_cache_ascii(), and it seems to fix the issue.

According to the long comments at the top of the file, I got rid of "the fastest way to draw text as it bypasses Pango completely" and I guess it falls through to one of the slower ones using Pango.
Comment 6 Egmont Koblinger 2018-02-12 22:27:41 UTC
font_info_measure_font() sets info->layout to a string containing the ASCII codepoints 32..126 in this order, i.e. "[…]:;<=>?@[…]".

Then it calls font_info_cache_ascii() on this layout to measure/cache its contents.

That is, Pango is asked to render the substring "<=>" in a single step, hence it's substituted by the corresponding ligature. Then it's queried about the glyphs it rendered (in a bit more clever way than visually cutting up the result; so somehow the codepoints for '<' and '=' receive an empty glyph, while '>' receives the ligatured rendering of '<=>'), and our cache is built up from this.

Assuming that we don't want to ditch caching, we should at least build up the cache by rendering the characters one by one (or e.g. adding a space in between). (Or is there a flag to Pango to disable ligatures?)
Comment 7 Egmont Koblinger 2018-02-12 22:42:07 UTC
Created attachment 368280 [details] [review]
Easy workaround

Since the code doesn't make any assumption about the contents of VTE_DRAW_SINGLE_WIDE_CHARACTERS (it just uses this string for measuring the font and caching the glyphs), simply putting a space here in between potentially ligatured (or rather: all) codepoints fixes the issue.
Comment 8 Egmont Koblinger 2018-02-12 22:54:10 UTC
+cc Behdad,

Do you happen to have any comments, insights, recommendation for the proper solution here?
Comment 9 Behdad Esfahbod 2018-02-12 23:00:20 UTC
It's supposed to skip caching any ligatures formed:

↦       ↦       /* Only cache simple clusters */ 
↦       ↦       if (iter.start_char +1 != iter.end_char  || 
↦       ↦           iter.start_index+1 != iter.end_index || 
↦       ↦           iter.start_glyph+1 != iter.end_glyph) 
↦       ↦       ↦       continue; 

I suggest we remove the caching-ASCII thing completely. Or debug why that is not working as expected.
Comment 10 Christian Persch 2018-03-03 12:28:43 UTC
Comment on attachment 368280 [details] [review]
Easy workaround

I can repro the problem, and in the code in comment 9, iter.start + 1 == iter.end, etc, so we do end up caching them.

Let's go with the workaround patch for now, but keep this open.
Comment 11 Egmont Koblinger 2018-03-03 16:46:26 UTC
Submitted the workaround for the forthcoming 0.52 release.

Sure, let's keep it open for a more proper solution!
Comment 12 GNOME Infrastructure Team 2021-06-10 15:29:38 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/vte/-/issues/2477.