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

[Visuwesh <visuweshm@gmail.com>]

[சனி நவம்பர் 02, 2024] Eli Zaretskii wrote:

>> 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.

I am not sure if you wanted me to apply the above patch or not.  But
just loading a file that contains

    (set-face-attribute 'default nil
                :family "Cascadia Code"
                :height 110
                :weight 'bold)

    (load "/tmp/ligature.el")

    (ligature-set-ligatures 'prog-mode '("--" "---" "==" "===" "!=" "!==" "=!="
                                         "=:=" "=/=" "<=" ">=" "&&" "&&&" "&=" "++" "+++" "***" ";;" "!!"
                                         "??" "???" "?:" "?." "?=" "<:" ":<" ":>" ">:" "<:<" "<>" "<<<" ">>>"
                                         "<<" ">>" "||" "-|" "_|_" "|-" "||-" "|=" "||=" "##" "###" "####"
                                         "#{" "#[" "]#" "#(" "#?" "#_" "#_(" "#:" "#!" "#=" "^=" "<$>" "<$"
                                         "$>" "<+>" "<+" "+>" "<*>" "<*" "*>" "</" "</>" "/>" "<!--" "<#--"
                                         "-->" "->" "->>" "<<-" "<-" "<=<" "=<<" "<<=" "<==" "<=>" "<==>"
                                         "==>" "=>" "=>>" ">=>" ">>=" ">>-" ">-" "-<" "-<<" ">->" "<-<" "<-|"
                                         "<=|" "|=>" "|->" "<->" "<~~" "<~" "<~>" "~~" "~~>" "~>" "~-" "-~"
                                         "~@" "[||]" "|]" "[|" "|}" "{|" "[<" ">]" "|>" "<|" "||>" "<||"
                                         "|||>" "<|||" "<|>" "..." ".." ".=" "..<" ".?" "::" ":::" ":=" "::="
                                         ":?" ":?>" "//" "///" "/*" "*/" "/=" "//=" "/==" "@_" "__" "???"
                                         "<:<" ";;;"))
    (global-ligature-mode t)

was enough to trigger the breakpoint for me.  This is, of course, before
I got a chance to even reproduce the misalignment.

Here's what the gdb buffer looks like:

    (gdb) source .gdbinit
    SIGINT is used by the debugger.
    Are you sure you want to change it? (y or n) [answered Y; input not from terminal]
    DISPLAY = :0.0
    TERM = dumb
    Breakpoint 1 at 0x20b4b7: file emacs.c, line 432.
    Breakpoint 2 at 0x1cfa8a: file xterm.c, line 27102.
    (gdb) break hbfont.c:598 if xoff == 0 && yoff == 0 && wadjust == metrics.width && LGLYPH_ADJUSTMENT(lglyph) != Qnil
    Breakpoint 3 at 0x3c3ad7: file hbfont.c, line 598.
    (gdb) run -Q
    Starting program: /home/viz/lib/ports/emacs/src/emacs -Q
    [Thread debugging using libthread_db enabled]
    Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
    [New Thread 0x7ffff1b8f6c0 (LWP 198125)]
    [New Thread 0x7ffff118f6c0 (LWP 198126)]
    [New Thread 0x7fffebf8f6c0 (LWP 198127)]
    [New Thread 0x7fffeb58f6c0 (LWP 198128)]
    Error in testing condition for breakpoint 3:
    Attempt to take address of value not located in memory.

    Thread 1 "emacs" hit Breakpoint 3, hbfont_shape (lgstring=XIL(0x7ffff2a9c745), direction=XIL(0x33c0)) at hbfont.c:598
    598	      if (xoff || yoff || wadjust != metrics.width)