Re: [PATCH] emoji on TUI emacs

Eli Zaretskii <[email protected]>
Newsgroups gmane.emacs.devel
Message-ID <[email protected]>
> From: Kai Ma <[email protected]>
> Cc: [email protected]
> Date: Sun, 16 Aug 2026 07:45:29 +0200
> 
> Kai Ma <[email protected]> writes:
>
>> Eli Zaretskii <[email protected]> writes:
>>
>>>> However, the column number change due to C-f, C-b, etc. is still the
>>>> total sum of CHARACTER_WIDTH (not necessarily 2).  Do we want to change
>>>> it as well?
>>>
>>> Yes, definitely.  current-column and move-to-column should both work
>>> correctly in these cases.
>>
>> Fixed.  Previously in composition_update_it, the width is computed by
>> summing character widths.  I'm not entirely sure whether it's correct to
>> sum glyph widths here.
>
> Apparently, this is incorrect, and it crashes for me occasionally
> because nglyphs != nchars in general.
>
> Here is a new patch that supersedes the wrong one.  It reuses
> 'composition_gstring_width' as cmp_it->width for automatic compositions.

Thanks, I have a couple of nits and one comment, and then there are a
few problems that still need to be fixed:

> --- a/src/composite.c
> +++ b/src/composite.c
> @@ -1535,12 +1535,12 @@ composition_update_it (struct composition_it *cmp_it, ptrdiff_t charpos, ptrdiff
>        glyph = LGSTRING_GLYPH (gstring, cmp_it->from);
>        cmp_it->nchars = LGLYPH_TO (glyph) + 1 - from;
>        cmp_it->nbytes = 0;
> -      cmp_it->width = 0;
> +      cmp_it->width = composition_gstring_width (gstring, cmp_it->from, cmp_it->to, NULL);
> +
>        for (i = cmp_it->nchars - 1; i >= 0; i--)
>  	{
>  	  c = XFIXNUM (LGSTRING_CHAR (gstring, from + i));
>  	  cmp_it->nbytes += CHAR_BYTES (c);
> -	  cmp_it->width += CHARACTER_WIDTH (c);
>  	}
>      }
>    return c;


Hmmm... is this guaranteed to produce correct results for both Emoji
and non-Emoji compositions?  Did you verify we are not breaking
anything here, for example with compositions like 'a' followed by an
accent?  It would help if you could talk me through the code and
explain how what composition_gstring_width does is equivalent to
summing CHARACTER_WIDTH for TTY frames, in cases other than Emoji.  We
need to convince ourselves that we don't introduce regressions here.
If we are not sure, perhaps special-casing Emoji would be safer.

> * admin/unidata/Makefile.in: Changed.

This should describe the change.

> * src/composite.c (composition_update_it): set the width of an automatic
> composition using composition_gstring_width.

The description of the change should begin with a capital letter
("Set", not "set").

I applied the patches and tried to use Emacs in a -nw session.  I got
assertion violation when scrolling through the HELLO file ("C-h h").
See the backtrace and some data from GDB below.  As result, I think
this code is incorrect:

  if (it->cmp_it.from + 1 >= it->cmp_it.to)
    return false;
  first = XFIXNUM (LGSTRING_CHAR (gstring, it->cmp_it.from));
  second = XFIXNUM (LGSTRING_CHAR (gstring, it->cmp_it.from + 1));

See the doc string of composition-get-gstring regarding the structure
of a gstring object.  My reading of that is that you always need to
use 0 and 1 to access the first and the second codepoint of a composed
sequence; FROM and TO are indices of the _glyphs_ in the composed
grapheme cluster, which is not what you want.

As you can see from the GDB session of an Emacs that hit assertion
violation, this gstring has only 2 composed characters, but FROM is 2
and TO is 4, so you are accessing a 3-component vector with indices
2+1 = 3 and 3+1 = 4, which is not right.

And I'm not sure you need the condition about FROM+1 >= TO, is that
perhaps some kind of defense against crashes you've seen?  If not, why
do you need this condition there?

I think the above fragment should be replaced with this:

  first = XFIXNUM (LGSTRING_CHAR (gstring, 0));
  if (LGSTRING_CHAR_LEN (gstring) <= 1)
    second = 0;
  else
    second = XFIXNUM (LGSTRING_CHAR (gstring, 1));

because I've seen cases that the function gets a composition with only
one codepoint (which is strange, but we should not crash or hit
assertions).

After fixing these two problems, I still see some incorrect
compositions in the various admin/unidata/emoji-*.txt files.  First,
cursor movement through Emoji sequences around line 3600 of
emoji-test.txt is wrong: it seems that the terminal thinks the "heart"
Emoji is two separate characters, or that its width is not 2.

The second problem is with the Keycap sequences around line 550 of
emoji-sequences: it seems Emacs is not composing these sequences for
some reason?  Do you see this on your terminal?

Apart of the above two issues, the display seems correct, which is a
nice improvement.

Here's the backtrace from the assertion violation:

  gdb) bt
  #0  0x7526ad23 in KERNELBASE!DebugBreak ()
     from C:\WINDOWS\SysWOW64\KernelBase.dll
  #1  0x00bbf015 in emacs_abort () at w32fns.c:12178
  #2  0x009b116f in terminate_due_to_signal (sig=22, backtrace_limit=2147483647)
      at emacs.c:480
  #3  0x00a73efd in die (
      msg=0xc96d08 <o_fwd+2184> "0 <= idx && idx < gc_asize (array)",
      file=0xc96c60 <o_fwd+2016> "lisp.h", line=1966) at alloc.c:7353
  #4  0x009885d2 in AREF (array=XIL(0xa00000000b3d9318), idx=3) at lisp.h:1966
  #5  0x0098d9f4 in composite_glyph_is_emoji_sequence (it=0x63fa710)
      at term.c:1874
  #6  0x0098dc1b in produce_composite_glyph (it=0x63fa710) at term.c:1925
  #7  0x0098ce29 in produce_glyphs (it=0x63fa710) at term.c:1657
  #8  0x008d3c10 in display_line (it=0x63fa710, cursor_vpos=40) at xdisp.c:26153
  #9  0x008c35f7 in try_window (window=XIL(0xa00000000a72b480), pos=..., flags=0)
      at xdisp.c:21699
  #10 0x008be8bd in redisplay_window (window=XIL(0xa00000000a72b480),
      just_this_one_p=true) at xdisp.c:20785
  #11 0x008b5a67 in redisplay_window_1 (window=XIL(0xa00000000a72b480))
      at xdisp.c:18547
  #12 0x00ab2666 in internal_condition_case_1 (
      bfun=0x8b5a0f <redisplay_window_1>, arg=XIL(0xa00000000a72b480),
      handlers=XIL(0xc000000009e0787c), hfun=0x8b57e7 <redisplay_window_error>)
      at eval.c:1736
  #13 0x008b474c in redisplay_internal () at xdisp.c:18053
  #14 0x008b1a98 in redisplay () at xdisp.c:17071
  #15 0x009bed8e in read_char (commandflag=1, map=XIL(0xc00000000b4f4580),
      prev_event=XIL(0), used_mouse_menu=0x63fecff, end_time=0x0)
      at keyboard.c:2711
  #16 0x009d8be6 in read_key_sequence (keybuf=0x63fefd8, prompt=XIL(0),
      dont_downcase_last=false, can_return_switch_frame=true,
      fix_current_buffer=true, prevent_redisplay=false,
      disable_text_conversion_p=false) at keyboard.c:11224
  #17 0x009ba6ca in command_loop_1 () at keyboard.c:1424
  #18 0x00ab257c in internal_condition_case (bfun=0x9ba06b <command_loop_1>,
      handlers=XIL(0x90), hfun=0x9b90c7 <cmd_error>) at eval.c:1712
  #19 0x009b9ad3 in command_loop_2 (handlers=XIL(0x90)) at keyboard.c:1163
  #20 0x00ab1721 in internal_catch (tag=XIL(0x12ff0),
      func=0x9b9a9c <command_loop_2>, arg=XIL(0x90)) at eval.c:1392
  #21 0x009b9a3e in command_loop () at keyboard.c:1141
  #22 0x009b8b15 in recursive_edit_1 () at keyboard.c:749
  #23 0x009b8dc5 in Frecursive_edit () at keyboard.c:832
  #24 0x009b3dbf in main (argc=3, argv=0x8172350) at emacs.c:2635

  Lisp Backtrace:
  "redisplay_internal (C function)" (0x0)
  (gdb) fr 5
  #5  0x0098d9f4 in composite_glyph_is_emoji_sequence (it=0x63fa710)
      at term.c:1874
  1874      first = XFIXNUM (LGSTRING_CHAR (gstring, it->cmp_it.from));
  (gdb) p it->cmp_it.from
  $1 = 2
  (gdb) p gstring
  $2 = XIL(0xa00000000b3f6078)
  (gdb) pp gstring
  [[cp65001 43438 43446] 133 [0 0 32 43438 1 0 0 1 0 nil] [0 0 43438 43438 0 0 0 1 0 nil] [1 1 32 43446 1 0 0 1 0 nil] [1 1 43446 43446 0 0 0 1 0 nil]]
  (gdb) pp AREF(gstring,0)
  [cp65001 43438 43446]
  (gdb) p/x 43438
  $3 = 0xa9ae
  (gdb) p it->cmp_it
  $4 = {
    stop_pos = 2399,
    id = 133,
    ch = 43446,
    rule_idx = 0,
    lookback = 1,
    nglyphs = 4,
    reversed_p = false,
    parent_it = 0x63fa710,
    charpos = 2400,
    nchars = 1,
    nbytes = 3,
    from = 2,
    to = 4,
    width = 1
  }
  (gdb)
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.