bug#73752: 29.4; Ligatures are randomly rendered with extra spaces

[Eli Zaretskii]

> From: Tim Ruffing <dev@real-or-random.org>
> Cc: 73752@debbugs.gnu.org, xuan@xlk.me
> Date: Sat, 02 Nov 2024 04:34:17 +0100
> 
> Neither the critical line in composition_gstring_adjust_zero_width nor
> the calls to lglyph-set-adjustment in composite.el are triggered. But
> this hack in hbfont_shape means I can't reproduce the bug anymore using
> Yixuan's init file:
> 
> diff --git a/src/hbfont.c b/src/hbfont.c
> index 37ed4132492..6fae513069a 100644
> --- a/src/hbfont.c
> +++ b/src/hbfont.c
> @@ -592,11 +592,12 @@ hbfont_shape (Lisp_Object lgstring, Lisp_Object
> direction)
>        LGLYPH_SET_ASCENT (lglyph, metrics.ascent);
>        LGLYPH_SET_DESCENT (lglyph, metrics.descent);
>  
>        xoff = lround (pos[i].x_offset * position_unit);
>        yoff = - lround (pos[i].y_offset * position_unit);
> -      wadjust = lround (pos[i].x_advance * position_unit);
> +      /* wadjust = lround (pos[i].x_advance * position_unit); */
> +      wadjust = metrics.width;
>        if (xoff || yoff || wadjust != metrics.width)
>       LGLYPH_SET_ADJUSTMENT (lglyph, CALLN (Fvector,
>                                            make_fixnum (xoff),
>                                            make_fixnum (yoff),
>                                            make_fixnum (wadjust)));

Which leads me to the original suggestion in bug#50951, to do this:

diff --git a/src/hbfont.c b/src/hbfont.c
index 37ed413..040658a 100644
--- a/src/hbfont.c
+++ b/src/hbfont.c
@@ -600,6 +600,8 @@ hbfont_shape (Lisp_Object lgstring, Lisp_Object direction)
                                              make_fixnum (xoff),
                                              make_fixnum (yoff),
                                              make_fixnum (wadjust)));
+      else
+       LGLYPH_SET_ADJUSTMENT (lglyph, Qnil);
     }
 
   return make_fixnum (glyph_len);

In bug#50951, an alternative change in fill_gstring_body was proposed
(see https://debbugs.gnu.org/cgi/bugreport.cgi?bug=50951#97), which
was eventually installed, but I guess that might not be enough?  Can
you and Visuwesh please confirm that we sometimes get an lgstring
argument to hbfont_shape whose glyphs already have a non-nil
LGLYPH_ADJUSTMENT component, and we leave it unmodified because the
condition to use LGLYPH_SET_ADJUSTMENT isn't satisfied?  Like this:

  (gdb) break hbfont.c:598 if xoff == 0 && yoff == 0 && wadjust == 
metrics.width && LGLYPH_ADJUSTMENT(lglyph) != Qnil

In my testing, a breakpoint with this condition never breaks.  But
that could be because I use a different font (Cascadia Code, instead
of JetBrains Mono, which I don't have), or because my version of
HarfBuzz is much older than yours, or for some other reason.

> Anyway, I tried adding a printf inside the critical "if" in hbfont.c.
> This is what I see when the bug triggers:
> 
> xoff: 0, yoff: 0, wadjust: 18, metrics.width: 9, pos[i].x_advance * 
> position_unit: 18.000000
> xoff: 0, yoff: 0, wadjust: 18, metrics.width: 9, pos[i].x_advance * 
> position_unit: 18.000000
> xoff: 0, yoff: 0, wadjust: 18, metrics.width: 9, pos[i].x_advance * 
> position_unit: 18.000000
> xoff: 0, yoff: 0, wadjust: 18, metrics.width: 9, pos[i].x_advance * 
> position_unit: 18.000000
> xoff: 0, yoff: 0, wadjust: 18, metrics.width: 9, pos[i].x_advance * 
> position_unit: 18.000000
> xoff: 0, yoff: 0, wadjust: 18, metrics.width: 9, pos[i].x_advance * 
> position_unit: 18.000000
> xoff: 0, yoff: 0, wadjust: 18, metrics.width: 9, pos[i].x_advance * 
> position_unit: 18.000000

And what do you see for LGLYPH_ADJUSTMENT in those cases? are they nil
or some non-nil vector?  If they are always nil, there's some other
factor at work here.

> Much more interesting:
> I can reproduce this only when "enough" (or I guess, the right)
> ligatures are visible in the buffer. With Yixuan's init file, when I
> not only load but also visit this init file -- just for testing the
> rendering, because it contains relevant ligatures -- then I can trigger
> the bug. But I can't trigger it with a reduced version (that notably
> doesn't contain arrow ligatures)!

Alas, this explains nothing, except that there's some mysterious
factor that rears its ugly head only under some unknown conditions.
Which is something we already knew.

> Eli, perhaps this is precise enough already so that you can reproduce
> it? See the attached archive: Use it as init-dir. It contains the init
> script (but I removed the automatic repeat). Visit bad.el. Press C=# to
> initialize the font randomization, and then keep pressing C-RET until
> you hopefully hit the bug.

It never happens here, no matter how long I hold C-RET when displaying
bad.el.

> So it seems that harfbuzz gives different results for some composition
> depending on the "context", i.e., the other characters (and ligatures?)
> that appear in the buffer passed to harfbuzz. Is this a bug in
> harfbuzz?

We did not yet capture in GDB even a single case when such "bad"
grapheme clusters are produced by HarfBuzz, at the moment of their
production.  So we don't have a C backtrace for those cases, and
cannot go up the call-stack to understand the conditions under which
this happens.  Until we do, we cannot claim this to be a bug in
HarfBuzz.  (The code in question was written by a HarfBuzz developer,
btw, so it presumably is correct.)  It could be due to how we call
HarfBuzz, or what we pass to it (which is determined, among other
things, by the code in ligature.el).

> If you still can't reproduce, what's a good way to pretty print the
> lgstring passed to hbfont_shape?

Use the "pp" command defined on our src/.gdbinit, of course.  If you
start Emacs from GDB, the result of "pp" will be shown in the GDB
interaction window.  If you attach GDB to an already running Emacs, be
sure to start Emacs from a shell prompt, and don't close the window
where the shell runs -- in which case the result of "pp" will show in
that window, because we tell Emacs to write the result to its stderr.