bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[David Fussner via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 4948 bytes --]

After emacs -Q, open a unicode text file and type a few letters, then
the Combining Grapheme Joiner (C-x 8 RET 34f), then a few more
letters. Press and hold the left arrow key to go back across the CGJ and
here it leaves behind a second box cursor while the actual cursor
continues to move across the text beyond. If you input multiple CGJs
you'll get multiple box cursors scattered wherever you put them,
assuming you don't change to the right arrow and hold that down, which
removes the artifacts as it goes along. (See attached image, with 3
artifacts in a line and the actual cursor off to the left.)

I came upon this while typing Biblical Hebrew in xeLaTeX, where the CGJ
allows the placement of metheg before the vowel. In a RTL paragraph it's
still the left arrow that makes the artifact appear, while the right
arrow still removes it as it passes. Switching to another buffer in the
same frame then back to the text file also removes the extra cursors.

By default, my emacs uses the Tibetan Machine Uni font to display the
CGJ, so I blacklisted that face and it now uses the same DejaVu Sans
face as the rest of the text, with the same results. The state of
blink-cursor-mode doesn't affect it, though when it's on and you time
the use of the left arrow key just right you can get past the CGJ
without leaving a visible artifact.

I tested it on GNU Emacs 26.3, and the result was the same, so it's not
a regression, even assuming it's a bug at all, and not a feature to show
where this invisible character is present (?)

Many thanks,

David.

In GNU Emacs 27.0.91 (build 2, i686-pc-linux-gnu, GTK+ Version 3.18.9)
 of 2020-06-01 built on newfont
Repository revision: 44c0e074f7cb84481785cb49515a4bd7235a074b
Repository branch: emacs-27
Windowing system distributor 'The X.Org Foundation', version 11.0.11803000
System Description: Slackware 14.2

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Found ‘set-fontset-font’ in Command Index.  (Only match)
Mark set [2 times]
("Tibetan Machine Uni" "Noto Color Emoji")
Type C-x 1 to delete the help window.
Char: ͏ (847, #o1517, #x34f, file ...) point=34 of 74 (45%) column=7

Configured using:
 'configure
 PKG_CONFIG_PATH=/usr/local/lib/pkgconfig:/usr/local/share/pkgconfig:/usr/lib/pkgconfig:/usr/share/pkgconfig'

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND GPM DBUS GSETTINGS GLIB NOTIFY INOTIFY
ACL GNUTLS LIBXML2 FREETYPE HARFBUZZ XFT ZLIB TOOLKIT_SCROLL_BARS GTK3
X11 XDBE XIM MODULES THREADS PDUMPER LCMS2 GMP

Important settings:
  value of $LC_COLLATE: C
  value of $LANG: en_US.ISO8859-1
  locale-coding-system: iso-latin-1-unix

Major mode: Text

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
format-spec rfc822 mml mml-sec password-cache epa derived epg epg-config
gnus-util rmail rmail-loaddefs text-property-search time-date subr-x seq
byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils wid-edit descr-text
help-mode cl-loaddefs cl-lib mule-util info easymenu tooltip eldoc
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win
x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote threads dbusbind inotify lcms2 dynamic-setting
system-font-setting font-render-setting move-toolbar gtk x-toolkit x
multi-tty make-network-process emacs)

Memory information:
((conses 8 176167 18366)
 (symbols 24 19078 2)
 (strings 16 84935 2635)
 (string-bytes 1 1983984)
 (vectors 8 17687)
 (vector-slots 4 1268552 55708)
 (floats 8 30 62)
 (intervals 28 37417 199)
 (buffers 568 14)
 (heap 1024 18077 2648))

[-- Attachment #2: snapshot3.png --]
[-- Type: image/png, Size: 2760 bytes --]

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Eli Zaretskii]

> Date: Mon, 1 Jun 2020 14:45:15 +0100
> From: David Fussner via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> After emacs -Q, open a unicode text file and type a few letters, then
> the Combining Grapheme Joiner (C-x 8 RET 34f), then a few more
> letters. Press and hold the left arrow key to go back across the CGJ and
> here it leaves behind a second box cursor while the actual cursor
> continues to move across the text beyond. If you input multiple CGJs
> you'll get multiple box cursors scattered wherever you put them,
> assuming you don't change to the right arrow and hold that down, which
> removes the artifacts as it goes along. (See attached image, with 3
> artifacts in a line and the actual cursor off to the left.)

I cannot reproduce this on my system, FWIW.  Very strange.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Pip Cet]

[-- Attachment #1: Type: text/plain, Size: 946 bytes --]

On Mon, Jun 1, 2020 at 3:31 PM Eli Zaretskii <eliz@gnu.org> wrote:
> > After emacs -Q, open a unicode text file and type a few letters, then
> > the Combining Grapheme Joiner (C-x 8 RET 34f), then a few more
> > letters. Press and hold the left arrow key to go back across the CGJ and
> > here it leaves behind a second box cursor while the actual cursor
> > continues to move across the text beyond. If you input multiple CGJs
> > you'll get multiple box cursors scattered wherever you put them,
> > assuming you don't change to the right arrow and hold that down, which
> > removes the artifacts as it goes along. (See attached image, with 3
> > artifacts in a line and the actual cursor off to the left.)
>
> I cannot reproduce this on my system, FWIW.  Very strange.

I can reproduce this, but it depends on which font it uses. Is it
possible it's bug 41454? I'm attaching that patch in case it does
anything and people feel like testing it.

[-- Attachment #2: 0001-Don-t-get-confused-by-mid-gstring-face-changes-bug-4.patch --]
[-- Type: text/x-patch, Size: 789 bytes --]

From c1d4778001567f7dfc7825b69193089ec69897bb Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@gmail.com>
Date: Sun, 31 May 2020 19:55:48 +0000
Subject: [PATCH] Don't get confused by mid-gstring face changes (bug#41454)

* src/xdisp.c (fill_gstring_glyph_string): Don't extend the glyph
string past face changes.
---
 src/xdisp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/xdisp.c b/src/xdisp.c
index db0ec68315..989958fa11 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -27698,6 +27698,7 @@ fill_gstring_glyph_string (struct glyph_string *s, int face_id,
   while (glyph < last
 	 && glyph->u.cmp.automatic
 	 && glyph->u.cmp.id == s->cmp_id
+	 && glyph->face_id == face_id
 	 && s->cmp_to == glyph->slice.cmp.from)
     {
       s->width += glyph->pixel_width;
-- 
2.27.0.rc0

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[David Fussner via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Is the patch for master? (Forgive my confusion).

David.

On Mon, 1 Jun 2020 at 16:50, Pip Cet <pipcet@gmail.com> wrote:
>
> On Mon, Jun 1, 2020 at 3:31 PM Eli Zaretskii <eliz@gnu.org> wrote:
> > > After emacs -Q, open a unicode text file and type a few letters, then
> > > the Combining Grapheme Joiner (C-x 8 RET 34f), then a few more
> > > letters. Press and hold the left arrow key to go back across the CGJ and
> > > here it leaves behind a second box cursor while the actual cursor
> > > continues to move across the text beyond. If you input multiple CGJs
> > > you'll get multiple box cursors scattered wherever you put them,
> > > assuming you don't change to the right arrow and hold that down, which
> > > removes the artifacts as it goes along. (See attached image, with 3
> > > artifacts in a line and the actual cursor off to the left.)
> >
> > I cannot reproduce this on my system, FWIW.  Very strange.
>
> I can reproduce this, but it depends on which font it uses. Is it
> possible it's bug 41454? I'm attaching that patch in case it does
> anything and people feel like testing it.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Eli Zaretskii]

> From: Pip Cet <pipcet@gmail.com>
> Date: Mon, 1 Jun 2020 15:50:16 +0000
> Cc: David Fussner <dfussner@googlemail.com>, 41645@debbugs.gnu.org
> 
> > I cannot reproduce this on my system, FWIW.  Very strange.
> 
> I can reproduce this, but it depends on which font it uses.

I have only 2 fonts that cover that codepoint, and both produce
correct display.  When I try a font that doesn't support it, I see a
box with hex code, which, of course, causes no problems.

When you do reproduce the problem, do the characters around CGJ use
the same font as the CGJ itself, or do they use a different font?  If
the latter, the composition won't work, and maybe that is when the
problem happens?

> Is it possible it's bug 41454? I'm attaching that patch in case it
> does anything and people feel like testing it.

I'm guessing you already tested, and it doesn't fix the problem for
you (when you use the fonts which do reproduce the problem)?

I think that bug is unlikely to be the culprit: symptoms such as those
described here usually mean that Emacs becomes confused about the
metrics of the character's glyph on display.  So to find the root
cause, one should step through the code which draws the character and
then the code which draws the cursor on that character, and see what's
going on there with the metrics.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Eli Zaretskii]

> From: David Fussner <dfussner@googlemail.com>
> Date: Mon, 1 Jun 2020 17:06:33 +0100
> Cc: Eli Zaretskii <eliz@gnu.org>, 41645@debbugs.gnu.org
> 
> Is the patch for master? (Forgive my confusion).

Yes (and I think it won't work well on other branches).

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[David Fussner via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Thanks. First I'll try some other fonts and see if I can reproduce Pip
Cet's observations.

On Mon, 1 Jun 2020 at 17:36, Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: David Fussner <dfussner@googlemail.com>
> > Date: Mon, 1 Jun 2020 17:06:33 +0100
> > Cc: Eli Zaretskii <eliz@gnu.org>, 41645@debbugs.gnu.org
> >
> > Is the patch for master? (Forgive my confusion).
>
> Yes (and I think it won't work well on other branches).

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Pip Cet]

[-- Attachment #1: Type: text/plain, Size: 2039 bytes --]

On Mon, Jun 1, 2020 at 4:14 PM Eli Zaretskii <eliz@gnu.org> wrote:
> > From: Pip Cet <pipcet@gmail.com>
> > Date: Mon, 1 Jun 2020 15:50:16 +0000
> > Cc: David Fussner <dfussner@googlemail.com>, 41645@debbugs.gnu.org
> >
> > > I cannot reproduce this on my system, FWIW.  Very strange.
> >
> > I can reproduce this, but it depends on which font it uses.
>
> I have only 2 fonts that cover that codepoint, and both produce
> correct display.  When I try a font that doesn't support it, I see a
> box with hex code, which, of course, causes no problems.

The problem is a zero-width glyph in a gstring. Fixing the artifact is
easy (see attached patch), but that still means when the cursor is on
the CGJ it's invisible. My impression is most code paths avoid
zero-width glyphs for that reason.

> When you do reproduce the problem, do the characters around CGJ use
> the same font as the CGJ itself, or do they use a different font?

The same font.

> If
> the latter, the composition won't work, and maybe that is when the
> problem happens?

Indeed, the composition gstring is a single zero-width glyph.

> > Is it possible it's bug 41454? I'm attaching that patch in case it
> > does anything and people feel like testing it.
>
> I'm guessing you already tested, and it doesn't fix the problem for
> you (when you use the fonts which do reproduce the problem)?

I tested, thought it had fixed the problem, had to run off, returned,
tested again, and it turned out not to fix the problem. Sorry about
that.

> I think that bug is unlikely to be the culprit: symptoms such as those
> described here usually mean that Emacs becomes confused about the
> metrics of the character's glyph on display.  So to find the root
> cause, one should step through the code which draws the character and
> then the code which draws the cursor on that character, and see what's
> going on there with the metrics.

How about this, as a first stop-gap until we figure out how to
properly prevent automatic compositions resulting in zero-width struct
glyphs?

[-- Attachment #2: 0001-Avoid-cursor-display-artifacts-for-zero-width-glyphs.patch --]
[-- Type: text/x-patch, Size: 1123 bytes --]

From 8e218339e164da57c22c6c72179d2499403b3ddb Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@gmail.com>
Date: Mon, 1 Jun 2020 18:07:51 +0000
Subject: [PATCH] Avoid cursor display artifacts for zero-width glyphs
 (Bug#41645)

---
 src/xdisp.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/xdisp.c b/src/xdisp.c
index 2a059c7c60..b2c2ff09c6 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -31450,13 +31450,12 @@ display_and_set_cursor (struct window *w, bool on,
       && (!on
 	  || w->phys_cursor.x != x
 	  || w->phys_cursor.y != y
+	  || w->phys_cursor_width != new_cursor_width
 	  /* HPOS can be negative in R2L rows whose
 	     exact_window_width_line_p flag is set (i.e. their newline
 	     would "overflow into the fringe").  */
 	  || hpos < 0
-	  || new_cursor_type != w->phys_cursor_type
-	  || ((new_cursor_type == BAR_CURSOR || new_cursor_type == HBAR_CURSOR)
-	      && new_cursor_width != w->phys_cursor_width)))
+	  || new_cursor_type != w->phys_cursor_type))
     erase_phys_cursor (w);
 
   /* Don't check phys_cursor_on_p here because that flag is only set
-- 
2.27.0.rc0

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Pip Cet]

On Mon, Jun 1, 2020 at 4:09 PM David Fussner <dfussner@googlemail.com> wrote:
> Is the patch for master? (Forgive my confusion).

Sorry; I should have sent the right patch for the branch you clearly
specified in the bug report.

The first patch is for master, the second patch applies to master or emacs-27.

The behavior I'm observing with the second patch, on master, is that
there's a zero-width cursor position in between the non-combined
characters: the cursor disappears completely in emacs -Q, except when
the mouse pointer leaves the window, as long as point is on the
character in question.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Eli Zaretskii]

> From: Pip Cet <pipcet@gmail.com>
> Date: Mon, 1 Jun 2020 18:09:27 +0000
> Cc: dfussner@googlemail.com, 41645@debbugs.gnu.org
> 
> The problem is a zero-width glyph in a gstring. Fixing the artifact is
> easy (see attached patch), but that still means when the cursor is on
> the CGJ it's invisible. My impression is most code paths avoid
> zero-width glyphs for that reason.

Sorry, I don't follow.  The composition-function-table entry for CGJ
is this:

  (aref composition-function-table #x34f)
   => (["\\c.\\c^+" 1 compose-gstring-for-graphic]
       [nil 0 compose-gstring-for-graphic])

So CGJ is supposed to be composed with the previous character,
similarly to diacritics.  When the composition happens, what glyphs
are returned from the shaper?  On my system, with a font that supports
that character, if I type "a b c d CGJ x y z" (without the spaces), I
see just "abcdxyz" with nothing between d and x, not even a thin
space.  And "C-u C-x =" on d then reports:

	       position: 149 of 153 (97%), column: 3
	      character: d (displayed as d) (codepoint 100, #o144, #x64)
		charset: ascii (ASCII (ISO646 IRV))
  code point in charset: 0x64
		 script: latin
		 syntax: w 	which means: word
	       category: .:Base, L:Left-to-right (strong), a:ASCII, l:Latin, r:Roman
	       to input: type "C-x 8 RET 64" or "C-x 8 RET LATIN SMALL LETTER D"
	    buffer code: #x64
	      file code: #x64 (encoded by coding system iso-latin-1-dos)
		display: composed to form "d͏" (see below)

  Composed with the following character(s) "͏" using this font:
    harfbuzz:-outline-Code2000-normal-normal-normal-*-16-*-*-*-p-*-iso8859-1
  by these glyphs:
    [0 1 100 71 10 0 9 11 0 nil]
    [0 1 847 3 6 0 1 0 1 [0 0 0]]

Is what you see on your system similar?  If so, which glyph is
zero-width, and why does it cause trouble on your system, but not on
mine?

> Indeed, the composition gstring is a single zero-width glyph.

See the composition information above: my interpretation of it is that
the composed glyph is not zero-width.

> How about this, as a first stop-gap until we figure out how to
> properly prevent automatic compositions resulting in zero-width struct
> glyphs?

Not sure yet.  I need to understand the problem better before I have
an opinion about the proposed change.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Eli Zaretskii]

> Date: Mon, 01 Jun 2020 21:37:38 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: dfussner@googlemail.com, 41645@debbugs.gnu.org
> 
>   Composed with the following character(s) "͏" using this font:
>     harfbuzz:-outline-Code2000-normal-normal-normal-*-16-*-*-*-p-*-iso8859-1
>   by these glyphs:
>     [0 1 100 71 10 0 9 11 0 nil]
>     [0 1 847 3 6 0 1 0 1 [0 0 0]]
> 
> Is what you see on your system similar?  If so, which glyph is
> zero-width, and why does it cause trouble on your system, but not on
> mine?

And another question: in the cases where you see artifacts, does the
call to font-shape-gstring inside compose-gstring-for-graphic return
nil or non-nil?  IOW, is the shaper+font producing the composed glyph,
or is that the fallback Lisp code in compose-gstring-for-graphic,
because font-shape-gstring returns nil?  Perhaps that's the reason for
the differences between what you see and what I see.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Pip Cet]

On Mon, Jun 1, 2020 at 6:37 PM Eli Zaretskii <eliz@gnu.org> wrote:
> > From: Pip Cet <pipcet@gmail.com>
> > Date: Mon, 1 Jun 2020 18:09:27 +0000
> > Cc: dfussner@googlemail.com, 41645@debbugs.gnu.org
> >
> > The problem is a zero-width glyph in a gstring. Fixing the artifact is
> > easy (see attached patch), but that still means when the cursor is on
> > the CGJ it's invisible. My impression is most code paths avoid
> > zero-width glyphs for that reason.
>
> Sorry, I don't follow.  The composition-function-table entry for CGJ
> is this:
>
>   (aref composition-function-table #x34f)
>    => (["\\c.\\c^+" 1 compose-gstring-for-graphic]
>        [nil 0 compose-gstring-for-graphic])

> So CGJ is supposed to be composed with the previous character,
> similarly to diacritics.

(BTW, isn't that regexp wrong? a base character can be followed by two
diacritics, then a CGJ, then another diacritic...)

> When the composition happens, what glyphs
> are returned from the shaper?

921          lgstring = safe_call (7, Vauto_composition_function, AREF
(rule, 2),
(gdb) n
925      return unbind_to (count, lgstring);
(gdb) pp lgstring
[[#<font-object "-PfEd-DejaVu
Sans-normal-normal-normal-*-22-*-*-*-*-0-iso10646-1"> 847] nil [0 0
847 768 0 0 0 0 0 [0 0 0]] nil nil nil nil nil nil nil]
(gdb) pp rule
[nil 0 compose-gstring-for-graphic]
(gdb) c
Continuing.

> On my system, with a font that supports
> that character, if I type "a b c d CGJ x y z" (without the spaces), I
> see just "abcdxyz" with nothing between d and x, not even a thin
> space.  And "C-u C-x =" on d then reports:
>
>                position: 149 of 153 (97%), column: 3
>               character: d (displayed as d) (codepoint 100, #o144, #x64)
>                 charset: ascii (ASCII (ISO646 IRV))
>   code point in charset: 0x64
>                  script: latin
>                  syntax: w      which means: word
>                category: .:Base, L:Left-to-right (strong), a:ASCII, l:Latin, r:Roman
>                to input: type "C-x 8 RET 64" or "C-x 8 RET LATIN SMALL LETTER D"
>             buffer code: #x64
>               file code: #x64 (encoded by coding system iso-latin-1-dos)
>                 display: composed to form "d͏" (see below)
>
>   Composed with the following character(s) "͏" using this font:
>     harfbuzz:-outline-Code2000-normal-normal-normal-*-16-*-*-*-p-*-iso8859-1
>   by these glyphs:
>     [0 1 100 71 10 0 9 11 0 nil]
>     [0 1 847 3 6 0 1 0 1 [0 0 0]]
>
> Is what you see on your system similar?

Not at all.

> If so, which glyph is
> zero-width, and why does it cause trouble on your system, but not on
> mine?

> > Indeed, the composition gstring is a single zero-width glyph.
> See the composition information above: my interpretation of it is that
> the composed glyph is not zero-width.

... something is odd here, I agree.

> > How about this, as a first stop-gap until we figure out how to
> > properly prevent automatic compositions resulting in zero-width struct
> > glyphs?
>
> Not sure yet.  I need to understand the problem better before I have
> an opinion about the proposed change.

As I said, it's a stop-gap at best.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Pip Cet]

On Mon, Jun 1, 2020 at 7:35 PM Eli Zaretskii <eliz@gnu.org> wrote:

> And another question: in the cases where you see artifacts, does the
> call to font-shape-gstring inside compose-gstring-for-graphic return
> nil or non-nil?

Neither, it's never reached. The first rule fails because font_range
restricts the composition range to a single character, so the second
rule applies. In that rule, nchars is 1 in
compose-gstring-for-graphic, so all it does is set the adjustment on
the lglyph:

[[#<font-object "-PfEd-DejaVu
Sans-normal-normal-normal-*-22-*-*-*-*-0-iso10646-1"> 847] nil [0 0
847 768 0 0 0 0 0 nil] nil nil nil nil nil nil nil]

turns into

[[#<font-object "-PfEd-DejaVu
Sans-normal-normal-normal-*-22-*-*-*-*-0-iso10646-1"> 847] nil [0 0
847 768 0 0 0 0 0 [0 0 0]] nil nil nil nil nil nil nil]

> IOW, is the shaper+font producing the composed glyph,
> or is that the fallback Lisp code in compose-gstring-for-graphic,
> because font-shape-gstring returns nil?  Perhaps that's the reason for
> the differences between what you see and what I see.

I think we're just failing to deal with a zero-width autocomposition
glyph, because we're dealing fine with the same glyph when
autocomposition is off.

xdisp.c:
30008          if (get_char_glyph_code (it->char_to_display, font, &char2b))
30009            {
30010              pcm = get_per_char_metric (font, &char2b);
30011              if (pcm->width == 0
30012              && pcm->rbearing == 0 && pcm->lbearing == 0)
30013            pcm = NULL;
30014            }

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Pip Cet]

On Mon, Jun 1, 2020 at 7:48 PM Pip Cet <pipcet@gmail.com> wrote:
> > > Indeed, the composition gstring is a single zero-width glyph.
> > See the composition information above: my interpretation of it is that
> > the composed glyph is not zero-width.
>
> ... something is odd here, I agree.

I think it's a very odd combination of things:
1. a font which defines an isolated CGJ to have zero width
2. an isolated CGJ appearing in the first place (in this case, because
another font does not support CGJ)
3. the fall-back [nil 0 compose-gstring-for-graphic] rule defined for
codepoint #x34f
4. compose-gstring-for-graphic attempting to salvage non-spacing
characters not following base characters, and producing zero-width
lgstrings from zero-width lglyphs

Avoiding any of the four will avoid the problem. (1) is something we
cannot fix directly. (2) is also something that a user may want. (3)
could be dropped, and (4) could be expanded to take care of the
zero-width case.

However, as long as zero-width gstrings can somehow slip through, I
suggest we also apply the patch I sent, assuming it fixes the problem.

We might consider simply prohibiting zero-width zero-lbearing
zero-rbearing gstrings, the way we prohibit zero-width zero-lbearing
zero-rbearing characters in the code I posted.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[David Fussner via Bug reports for GNU Emacs, the Swiss army knife of text editors]

A couple of data points, in case they're helpful:

On 27.0.91 _unpatched_, I see the artifact whenever the font of the
CGJ is different from that of the glyph before it, no matter which
script I'm using. When the font of the CGJ and the previous glyph are
the same, I don't see the artifact, except in Hebrew, where it's still
present. C-u C-x = displays the CGJ on its own, as a separate glyph,
whenever it's used in Hebrew and also whenever its font doesn't match
that of the glyph before it. When the font does match, in Latin or
Greek script, the cursor doesn't stop on the CGJ, and C-u C-x = shows
it as composed with the previous character.

With Pip Cet's second patch, 27.0.91 shows exactly the same behavior
with C-u C-x =, but the visual artifact never appears, at least in my
testing, neither in Hebrew nor in the LTR scripts.

Hope this helps.

On Mon, 1 Jun 2020 at 23:37, Pip Cet <pipcet@gmail.com> wrote:
>
> On Mon, Jun 1, 2020 at 7:48 PM Pip Cet <pipcet@gmail.com> wrote:
> > > > Indeed, the composition gstring is a single zero-width glyph.
> > > See the composition information above: my interpretation of it is that
> > > the composed glyph is not zero-width.
> >
> > ... something is odd here, I agree.
>
> I think it's a very odd combination of things:
> 1. a font which defines an isolated CGJ to have zero width
> 2. an isolated CGJ appearing in the first place (in this case, because
> another font does not support CGJ)
> 3. the fall-back [nil 0 compose-gstring-for-graphic] rule defined for
> codepoint #x34f
> 4. compose-gstring-for-graphic attempting to salvage non-spacing
> characters not following base characters, and producing zero-width
> lgstrings from zero-width lglyphs
>
> Avoiding any of the four will avoid the problem. (1) is something we
> cannot fix directly. (2) is also something that a user may want. (3)
> could be dropped, and (4) could be expanded to take care of the
> zero-width case.
>
> However, as long as zero-width gstrings can somehow slip through, I
> suggest we also apply the patch I sent, assuming it fixes the problem.
>
> We might consider simply prohibiting zero-width zero-lbearing
> zero-rbearing gstrings, the way we prohibit zero-width zero-lbearing
> zero-rbearing characters in the code I posted.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[David Fussner via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Addendum to my previous email: my apologies for the inaccuracy in my
initial bug report -- I believe I failed to notice in the Latin script
that the letter glyphs were from DejaVu Sans Mono and the CGJ from
DejaVu Sans, hence the artifacts there. When both are DejaVu Sans, the
issue doesn't appear, except in the Hebrew examples.

On Tue, 2 Jun 2020 at 14:45, David Fussner <dfussner@googlemail.com> wrote:
>
> A couple of data points, in case they're helpful:
>
> On 27.0.91 _unpatched_, I see the artifact whenever the font of the
> CGJ is different from that of the glyph before it, no matter which
> script I'm using. When the font of the CGJ and the previous glyph are
> the same, I don't see the artifact, except in Hebrew, where it's still
> present. C-u C-x = displays the CGJ on its own, as a separate glyph,
> whenever it's used in Hebrew and also whenever its font doesn't match
> that of the glyph before it. When the font does match, in Latin or
> Greek script, the cursor doesn't stop on the CGJ, and C-u C-x = shows
> it as composed with the previous character.
>
> With Pip Cet's second patch, 27.0.91 shows exactly the same behavior
> with C-u C-x =, but the visual artifact never appears, at least in my
> testing, neither in Hebrew nor in the LTR scripts.
>
> Hope this helps.
>
> On Mon, 1 Jun 2020 at 23:37, Pip Cet <pipcet@gmail.com> wrote:
> >
> > On Mon, Jun 1, 2020 at 7:48 PM Pip Cet <pipcet@gmail.com> wrote:
> > > > > Indeed, the composition gstring is a single zero-width glyph.
> > > > See the composition information above: my interpretation of it is that
> > > > the composed glyph is not zero-width.
> > >
> > > ... something is odd here, I agree.
> >
> > I think it's a very odd combination of things:
> > 1. a font which defines an isolated CGJ to have zero width
> > 2. an isolated CGJ appearing in the first place (in this case, because
> > another font does not support CGJ)
> > 3. the fall-back [nil 0 compose-gstring-for-graphic] rule defined for
> > codepoint #x34f
> > 4. compose-gstring-for-graphic attempting to salvage non-spacing
> > characters not following base characters, and producing zero-width
> > lgstrings from zero-width lglyphs
> >
> > Avoiding any of the four will avoid the problem. (1) is something we
> > cannot fix directly. (2) is also something that a user may want. (3)
> > could be dropped, and (4) could be expanded to take care of the
> > zero-width case.
> >
> > However, as long as zero-width gstrings can somehow slip through, I
> > suggest we also apply the patch I sent, assuming it fixes the problem.
> >
> > We might consider simply prohibiting zero-width zero-lbearing
> > zero-rbearing gstrings, the way we prohibit zero-width zero-lbearing
> > zero-rbearing characters in the code I posted.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Pip Cet]

On Tue, Jun 2, 2020 at 2:00 PM David Fussner <dfussner@googlemail.com> wrote:
> Addendum to my previous email: my apologies for the inaccuracy in my
> initial bug report -- I believe I failed to notice in the Latin script
> that the letter glyphs were from DejaVu Sans Mono and the CGJ from
> DejaVu Sans

I was confused about the same point, actually!

> , hence the artifacts there. When both are DejaVu Sans, the
> issue doesn't appear, except in the Hebrew examples.

Not even when you try entering a CGJ as the first character in a
buffer? I still saw the issue in that case, because
compose-gstring-for-graphic was called with a single glyph, for which
it returned the questionable lgstring.

Thanks for testing my patch :-)

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[David Fussner via Bug reports for GNU Emacs, the Swiss army knife of text editors]

You're quite right! At beginning of buffer, or indeed beginning of
line, I see the artifact and the CGJ as a separate glyph after C-u C-x
=, even if the fonts match. With your patch, C-u C-x = is the same,
but no artifact.

On Tue, 2 Jun 2020 at 15:07, Pip Cet <pipcet@gmail.com> wrote:
>
> On Tue, Jun 2, 2020 at 2:00 PM David Fussner <dfussner@googlemail.com> wrote:
> > Addendum to my previous email: my apologies for the inaccuracy in my
> > initial bug report -- I believe I failed to notice in the Latin script
> > that the letter glyphs were from DejaVu Sans Mono and the CGJ from
> > DejaVu Sans
>
> I was confused about the same point, actually!
>
> > , hence the artifacts there. When both are DejaVu Sans, the
> > issue doesn't appear, except in the Hebrew examples.
>
> Not even when you try entering a CGJ as the first character in a
> buffer? I still saw the issue in that case, because
> compose-gstring-for-graphic was called with a single glyph, for which
> it returned the questionable lgstring.
>
> Thanks for testing my patch :-)

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Pip Cet]

David Fussner <dfussner@googlemail.com> writes:
> A couple of data points, in case they're helpful:

Thanks again for testing.

> On 27.0.91 _unpatched_, I see the artifact whenever the font of the
> CGJ is different from that of the glyph before it, no matter which
> script I'm using. When the font of the CGJ and the previous glyph are
> the same, I don't see the artifact, except in Hebrew, where it's still
> present.

Which font are you using for Hebrew text?

> C-u C-x = displays the CGJ on its own, as a separate glyph,
> whenever it's used in Hebrew and also whenever its font doesn't match
> that of the glyph before it. When the font does match, in Latin or
> Greek script, the cursor doesn't stop on the CGJ, and C-u C-x = shows
> it as composed with the previous character.


That sounds as it should be. I'm not sure I understand what you're
seeing in Hebrew text, though: you said you saw the artifact there, but
also that the CGJ is displayed as a separate glyph. Is that corrcet?

> With Pip Cet's second patch, 27.0.91 shows exactly the same behavior
> with C-u C-x =, but the visual artifact never appears, at least in my
> testing, neither in Hebrew nor in the LTR scripts.

So that sounds like an improvement.

While I think we definitely want the patch I sent , it doesn't solve the
real issue: zero-width glyph strings. If we want to allow those, a lot
of the display code has to be changed (we're going to have to figure out
how to show the cursor, for starters); if we don't, that's a change to
the composition-function interface, albeit a minor one.

> Hope this helps.
>
> On Mon, 1 Jun 2020 at 23:37, Pip Cet <pipcet@gmail.com> wrote:
>>
>> On Mon, Jun 1, 2020 at 7:48 PM Pip Cet <pipcet@gmail.com> wrote:
>> > > > Indeed, the composition gstring is a single zero-width glyph.
>> > > See the composition information above: my interpretation of it is that
>> > > the composed glyph is not zero-width.
>> >
>> > ... something is odd here, I agree.
>>
>> I think it's a very odd combination of things:
>> 1. a font which defines an isolated CGJ to have zero width
>> 2. an isolated CGJ appearing in the first place (in this case, because
>> another font does not support CGJ)
>> 3. the fall-back [nil 0 compose-gstring-for-graphic] rule defined for
>> codepoint #x34f
>> 4. compose-gstring-for-graphic attempting to salvage non-spacing
>> characters not following base characters, and producing zero-width
>> lgstrings from zero-width lglyphs
>>
>> Avoiding any of the four will avoid the problem. (1) is something we
>> cannot fix directly. (2) is also something that a user may want. (3)
>> could be dropped, and (4) could be expanded to take care of the
>> zero-width case.
>>
>> However, as long as zero-width gstrings can somehow slip through, I
>> suggest we also apply the patch I sent, assuming it fixes the problem.
>>
>> We might consider simply prohibiting zero-width zero-lbearing
>> zero-rbearing gstrings, the way we prohibit zero-width zero-lbearing
>> zero-rbearing characters in the code I posted.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[David Fussner via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On Tue, 2 Jun 2020 at 15:35, Pip Cet <pipcet@gmail.com> wrote:
>
> David Fussner <dfussner@googlemail.com> writes:
> > A couple of data points, in case they're helpful:
>
> Thanks again for testing.
>
> > On 27.0.91 _unpatched_, I see the artifact whenever the font of the
> > CGJ is different from that of the glyph before it, no matter which
> > script I'm using. When the font of the CGJ and the previous glyph are
> > the same, I don't see the artifact, except in Hebrew, where it's still
> > present.
>
> Which font are you using for Hebrew text?

DejaVu Sans, in this instance, which at least has its own CGJ.

>
> > C-u C-x = displays the CGJ on its own, as a separate glyph,
> > whenever it's used in Hebrew and also whenever its font doesn't match
> > that of the glyph before it. When the font does match, in Latin or
> > Greek script, the cursor doesn't stop on the CGJ, and C-u C-x = shows
> > it as composed with the previous character.
>
>
> That sounds as it should be. I'm not sure I understand what you're
> seeing in Hebrew text, though: you said you saw the artifact there, but
> also that the CGJ is displayed as a separate glyph. Is that corrcet?

Yes, C-u C-x = doesn't present the CGJ as having been composed with
anything else in Hebrew text.

>
> > With Pip Cet's second patch, 27.0.91 shows exactly the same behavior
> > with C-u C-x =, but the visual artifact never appears, at least in my
> > testing, neither in Hebrew nor in the LTR scripts.
>
> So that sounds like an improvement.
>
> While I think we definitely want the patch I sent , it doesn't solve the
> real issue: zero-width glyph strings. If we want to allow those, a lot
> of the display code has to be changed (we're going to have to figure out
> how to show the cursor, for starters); if we don't, that's a change to
> the composition-function interface, albeit a minor one.
>
> > Hope this helps.
> >
> > On Mon, 1 Jun 2020 at 23:37, Pip Cet <pipcet@gmail.com> wrote:
> >>
> >> On Mon, Jun 1, 2020 at 7:48 PM Pip Cet <pipcet@gmail.com> wrote:
> >> > > > Indeed, the composition gstring is a single zero-width glyph.
> >> > > See the composition information above: my interpretation of it is that
> >> > > the composed glyph is not zero-width.
> >> >
> >> > ... something is odd here, I agree.
> >>
> >> I think it's a very odd combination of things:
> >> 1. a font which defines an isolated CGJ to have zero width
> >> 2. an isolated CGJ appearing in the first place (in this case, because
> >> another font does not support CGJ)
> >> 3. the fall-back [nil 0 compose-gstring-for-graphic] rule defined for
> >> codepoint #x34f
> >> 4. compose-gstring-for-graphic attempting to salvage non-spacing
> >> characters not following base characters, and producing zero-width
> >> lgstrings from zero-width lglyphs
> >>
> >> Avoiding any of the four will avoid the problem. (1) is something we
> >> cannot fix directly. (2) is also something that a user may want. (3)
> >> could be dropped, and (4) could be expanded to take care of the
> >> zero-width case.
> >>
> >> However, as long as zero-width gstrings can somehow slip through, I
> >> suggest we also apply the patch I sent, assuming it fixes the problem.
> >>
> >> We might consider simply prohibiting zero-width zero-lbearing
> >> zero-rbearing gstrings, the way we prohibit zero-width zero-lbearing
> >> zero-rbearing characters in the code I posted.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Eli Zaretskii]

> From: David Fussner <dfussner@googlemail.com>
> Date: Tue, 2 Jun 2020 14:45:50 +0100
> Cc: Eli Zaretskii <eliz@gnu.org>, 41645@debbugs.gnu.org
> 
> A couple of data points, in case they're helpful:
> 
> On 27.0.91 _unpatched_, I see the artifact whenever the font of the
> CGJ is different from that of the glyph before it, no matter which
> script I'm using. When the font of the CGJ and the previous glyph are
> the same, I don't see the artifact, except in Hebrew, where it's still
> present. C-u C-x = displays the CGJ on its own, as a separate glyph,
> whenever it's used in Hebrew and also whenever its font doesn't match
> that of the glyph before it. When the font does match, in Latin or
> Greek script, the cursor doesn't stop on the CGJ, and C-u C-x = shows
> it as composed with the previous character.

The visual artifact is due to the font you have that is used to
display CGJ, a font that is different from the one used for displaying
the surrounding text.  More about this in a separate message.

Regardless, the effect of CGJ on Hebrew points that you wanted to
have---the details can be seen in this URL:

  https://en.wikipedia.org/wiki/Combining_Grapheme_Joiner

did not work in Emacs, because Hebrew has its own composition rules,
and those rules didn't allow for the CGJ among the points.  I've now
added that, so if you rebuild the master branch, and use a font that
can support both Hebrew characters and CGJ, you should see the effect
of that on reordering the Hebrew points.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Eli Zaretskii]

> From: Pip Cet <pipcet@gmail.com>
> Date: Mon, 1 Jun 2020 18:09:27 +0000
> Cc: dfussner@googlemail.com, 41645@debbugs.gnu.org
> 
> How about this, as a first stop-gap until we figure out how to
> properly prevent automatic compositions resulting in zero-width struct
> glyphs?

I now think that your other proposal is much better, as it will fix
not only the cursor display, but also the display of the CGJ character
itself.  See my other message.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[David Fussner via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Thank you. I shall build the updated master and report back.

On Tue, 2 Jun 2020 at 16:51, Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: David Fussner <dfussner@googlemail.com>
> > Date: Tue, 2 Jun 2020 14:45:50 +0100
> > Cc: Eli Zaretskii <eliz@gnu.org>, 41645@debbugs.gnu.org
> >
> > A couple of data points, in case they're helpful:
> >
> > On 27.0.91 _unpatched_, I see the artifact whenever the font of the
> > CGJ is different from that of the glyph before it, no matter which
> > script I'm using. When the font of the CGJ and the previous glyph are
> > the same, I don't see the artifact, except in Hebrew, where it's still
> > present. C-u C-x = displays the CGJ on its own, as a separate glyph,
> > whenever it's used in Hebrew and also whenever its font doesn't match
> > that of the glyph before it. When the font does match, in Latin or
> > Greek script, the cursor doesn't stop on the CGJ, and C-u C-x = shows
> > it as composed with the previous character.
>
> The visual artifact is due to the font you have that is used to
> display CGJ, a font that is different from the one used for displaying
> the surrounding text.  More about this in a separate message.
>
> Regardless, the effect of CGJ on Hebrew points that you wanted to
> have---the details can be seen in this URL:
>
>   https://en.wikipedia.org/wiki/Combining_Grapheme_Joiner
>
> did not work in Emacs, because Hebrew has its own composition rules,
> and those rules didn't allow for the CGJ among the points.  I've now
> added that, so if you rebuild the master branch, and use a font that
> can support both Hebrew characters and CGJ, you should see the effect
> of that on reordering the Hebrew points.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Eli Zaretskii]

> From: Pip Cet <pipcet@gmail.com>
> Date: Mon, 1 Jun 2020 19:48:15 +0000
> Cc: dfussner@googlemail.com, 41645@debbugs.gnu.org
> 
> >   (aref composition-function-table #x34f)
> >    => (["\\c.\\c^+" 1 compose-gstring-for-graphic]
> >        [nil 0 compose-gstring-for-graphic])
> 
> > So CGJ is supposed to be composed with the previous character,
> > similarly to diacritics.
> 
> (BTW, isn't that regexp wrong? a base character can be followed by two
> diacritics, then a CGJ, then another diacritic...)

No, I don't think the regexp is wrong.  Every character whose Unicode
general-category property is Mn is given the '^' ("combining")
category, see characters.el.  The CGJ is one of them, but all the
accents and diacritics are also of that class.  So the above matches
any sequence of Mn characters in any order and permutation -- which is
of course more than is needed, but we rely on the shaper to combine
only those that should be.

For example, if I type the following 4 characters:

  LATIN SMALL LETTER U (U+0075) + COMBINING GRAVE ACCENT (U+0300) +
  CGJ (U+034F) + COMBINING DIAERESIS (U+0308)

I see the composition working correctly, provided that I use a font
that supports all the 4 codepoints.

> > And another question: in the cases where you see artifacts, does the
> > call to font-shape-gstring inside compose-gstring-for-graphic return
> > nil or non-nil?
> 
> Neither, it's never reached. The first rule fails because font_range
> restricts the composition range to a single character, so the second
> rule applies.

OK, that's the crucial fact: it means that the font used for CGJ is
not the same one as used for the surrounding text.  This is a
situation I never saw on my systems.  In addition, the font used for
the CGJ has a zero-width glyph for it, which is another thing I never
saw (I meanwhile tried almost 20 different fonts supporting the CGJ,
and all of them either produce a 1-pixel thin space or the funny box
with a circle inside).

So I think now it's clear what is going when this particular font is
present, and we are left...

> I think we're just failing to deal with a zero-width autocomposition
> glyph, because we're dealing fine with the same glyph when
> autocomposition is off.
> 
> xdisp.c:
> 30008          if (get_char_glyph_code (it->char_to_display, font, &char2b))
> 30009            {
> 30010              pcm = get_per_char_metric (font, &char2b);
> 30011              if (pcm->width == 0
> 30012              && pcm->rbearing == 0 && pcm->lbearing == 0)
> 30013            pcm = NULL;
> 30014            }
> 

...with this.  I think you are right, and we should do the same with
zero-width LGLYPH_STRING, forcing it->glyph_not_available_p to
non-zero, and then doing something like this in
fill_gstring_glyph_string:

  if (s->font == NULL || glyph_not_available_p)
    {
      s->font_not_found_p = true;
      s->font = FRAME_FONT (s->f);
    }

similar to what fill_glyph_string does.  WDYT?

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Pip Cet]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Pip Cet <pipcet@gmail.com>
>> Date: Mon, 1 Jun 2020 19:48:15 +0000
>> Cc: dfussner@googlemail.com, 41645@debbugs.gnu.org
>> 
>> >   (aref composition-function-table #x34f)
>> >    => (["\\c.\\c^+" 1 compose-gstring-for-graphic]
>> >        [nil 0 compose-gstring-for-graphic])
>> 
>> > So CGJ is supposed to be composed with the previous character,
>> > similarly to diacritics.
>> 
>> (BTW, isn't that regexp wrong? a base character can be followed by two
>> diacritics, then a CGJ, then another diacritic...)
>
> No, I don't think the regexp is wrong.  Every character whose Unicode
> general-category property is Mn is given the '^' ("combining")
> category, see characters.el.  The CGJ is one of them, but all the
> accents and diacritics are also of that class.  So the above matches
> any sequence of Mn characters in any order and permutation -- which is
> of course more than is needed, but we rely on the shaper to combine
> only those that should be.

You're right, thanks for the explanation.

>> > And another question: in the cases where you see artifacts, does the
>> > call to font-shape-gstring inside compose-gstring-for-graphic return
>> > nil or non-nil?
>> 
>> Neither, it's never reached. The first rule fails because font_range
>> restricts the composition range to a single character, so the second
>> rule applies.
>
> OK, that's the crucial fact: it means that the font used for CGJ is
> not the same one as used for the surrounding text.  This is a
> situation I never saw on my systems.  In addition, the font used for
> the CGJ has a zero-width glyph for it, which is another thing I never
> saw (I meanwhile tried almost 20 different fonts supporting the CGJ,
> and all of them either produce a 1-pixel thin space or the funny box
> with a circle inside).
>
> So I think now it's clear what is going when this particular font is
> present, and we are left...
>
>> I think we're just failing to deal with a zero-width autocomposition
>> glyph, because we're dealing fine with the same glyph when
>> autocomposition is off.
>> 
>> xdisp.c:
>> 30008          if (get_char_glyph_code (it->char_to_display, font, &char2b))
>> 30009            {
>> 30010              pcm = get_per_char_metric (font, &char2b);
>> 30011              if (pcm->width == 0
>> 30012              && pcm->rbearing == 0 && pcm->lbearing == 0)
>> 30013            pcm = NULL;
>> 30014            }
>> 
>
> ...with this.  I think you are right, and we should do the same with
> zero-width LGLYPH_STRING, forcing it->glyph_not_available_p to
> non-zero, and then doing something like this in
> fill_gstring_glyph_string:
>
>   if (s->font == NULL || glyph_not_available_p)
>     {
>       s->font_not_found_p = true;
>       s->font = FRAME_FONT (s->f);
>     }
>
> similar to what fill_glyph_string does.  WDYT?

I agree; the more I think about it, the more dangerous zero-sized
characters seem to me.

And almost all of my concerns apply to characters with zero x advance,
no matter whether they have lbearing or rbearing > 0.

Maybe, for master, we should reject those as well? I was going to say
"or force their width to be at least a single pixel", but I'm not even
sure that's sufficient on hidpi screens...

So, in summary, I'd like to do the following:

1. abort if we ever find ourselves drawing a zero-width cursor
2. reject lgstrings of zero width
3. reject all glyphs of zero width outside of compositions
4. allow users to specify a minimum width, perhaps relative to the font
size, so they can always see their cursor

...which would be quite a different patch.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[David Fussner via Bug reports for GNU Emacs, the Swiss army knife of text editors]

I've tried Eli's fix on master, and it looks exactly right in both
Hebrew and the LTR scripts. The only place where I can still get the
CGJ as an independent glyph to turn up in C-u C-x =, assuming the same
font is used for it and for the surrounding glyphs, is at the
beginning of a line, both LTR and RTL. I'm struggling to think of a
use case for this (?)

I'll test other fixes that come along for the artifacts -- thanks again.

On Tue, 2 Jun 2020 at 20:21, Pip Cet <pipcet@gmail.com> wrote:
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> >> From: Pip Cet <pipcet@gmail.com>
> >> Date: Mon, 1 Jun 2020 19:48:15 +0000
> >> Cc: dfussner@googlemail.com, 41645@debbugs.gnu.org
> >>
> >> >   (aref composition-function-table #x34f)
> >> >    => (["\\c.\\c^+" 1 compose-gstring-for-graphic]
> >> >        [nil 0 compose-gstring-for-graphic])
> >>
> >> > So CGJ is supposed to be composed with the previous character,
> >> > similarly to diacritics.
> >>
> >> (BTW, isn't that regexp wrong? a base character can be followed by two
> >> diacritics, then a CGJ, then another diacritic...)
> >
> > No, I don't think the regexp is wrong.  Every character whose Unicode
> > general-category property is Mn is given the '^' ("combining")
> > category, see characters.el.  The CGJ is one of them, but all the
> > accents and diacritics are also of that class.  So the above matches
> > any sequence of Mn characters in any order and permutation -- which is
> > of course more than is needed, but we rely on the shaper to combine
> > only those that should be.
>
> You're right, thanks for the explanation.
>
> >> > And another question: in the cases where you see artifacts, does the
> >> > call to font-shape-gstring inside compose-gstring-for-graphic return
> >> > nil or non-nil?
> >>
> >> Neither, it's never reached. The first rule fails because font_range
> >> restricts the composition range to a single character, so the second
> >> rule applies.
> >
> > OK, that's the crucial fact: it means that the font used for CGJ is
> > not the same one as used for the surrounding text.  This is a
> > situation I never saw on my systems.  In addition, the font used for
> > the CGJ has a zero-width glyph for it, which is another thing I never
> > saw (I meanwhile tried almost 20 different fonts supporting the CGJ,
> > and all of them either produce a 1-pixel thin space or the funny box
> > with a circle inside).
> >
> > So I think now it's clear what is going when this particular font is
> > present, and we are left...
> >
> >> I think we're just failing to deal with a zero-width autocomposition
> >> glyph, because we're dealing fine with the same glyph when
> >> autocomposition is off.
> >>
> >> xdisp.c:
> >> 30008          if (get_char_glyph_code (it->char_to_display, font, &char2b))
> >> 30009            {
> >> 30010              pcm = get_per_char_metric (font, &char2b);
> >> 30011              if (pcm->width == 0
> >> 30012              && pcm->rbearing == 0 && pcm->lbearing == 0)
> >> 30013            pcm = NULL;
> >> 30014            }
> >>
> >
> > ...with this.  I think you are right, and we should do the same with
> > zero-width LGLYPH_STRING, forcing it->glyph_not_available_p to
> > non-zero, and then doing something like this in
> > fill_gstring_glyph_string:
> >
> >   if (s->font == NULL || glyph_not_available_p)
> >     {
> >       s->font_not_found_p = true;
> >       s->font = FRAME_FONT (s->f);
> >     }
> >
> > similar to what fill_glyph_string does.  WDYT?
>
> I agree; the more I think about it, the more dangerous zero-sized
> characters seem to me.
>
> And almost all of my concerns apply to characters with zero x advance,
> no matter whether they have lbearing or rbearing > 0.
>
> Maybe, for master, we should reject those as well? I was going to say
> "or force their width to be at least a single pixel", but I'm not even
> sure that's sufficient on hidpi screens...
>
> So, in summary, I'd like to do the following:
>
> 1. abort if we ever find ourselves drawing a zero-width cursor
> 2. reject lgstrings of zero width
> 3. reject all glyphs of zero width outside of compositions
> 4. allow users to specify a minimum width, perhaps relative to the font
> size, so they can always see their cursor
>
> ...which would be quite a different patch.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Eli Zaretskii]

> From: Pip Cet <pipcet@gmail.com>
> Cc: dfussner@googlemail.com,  41645@debbugs.gnu.org
> Date: Tue, 02 Jun 2020 19:21:31 +0000
> 
> >> xdisp.c:
> >> 30008          if (get_char_glyph_code (it->char_to_display, font, &char2b))
> >> 30009            {
> >> 30010              pcm = get_per_char_metric (font, &char2b);
> >> 30011              if (pcm->width == 0
> >> 30012              && pcm->rbearing == 0 && pcm->lbearing == 0)
> >> 30013            pcm = NULL;
> >> 30014            }
> >> 
> >
> > ...with this.  I think you are right, and we should do the same with
> > zero-width LGLYPH_STRING, forcing it->glyph_not_available_p to
> > non-zero, and then doing something like this in
> > fill_gstring_glyph_string:
> >
> >   if (s->font == NULL || glyph_not_available_p)
> >     {
> >       s->font_not_found_p = true;
> >       s->font = FRAME_FONT (s->f);
> >     }
> >
> > similar to what fill_glyph_string does.  WDYT?
> 
> I agree; the more I think about it, the more dangerous zero-sized
> characters seem to me.
> 
> And almost all of my concerns apply to characters with zero x advance,
> no matter whether they have lbearing or rbearing > 0.

AFAIU, the advance metric is what we call "pixel width", and if so,
the above snippet from xdisp.c already tests it.  Right?

> Maybe, for master, we should reject those as well? I was going to say
> "or force their width to be at least a single pixel", but I'm not even
> sure that's sufficient on hidpi screens...
> 
> So, in summary, I'd like to do the following:
> 
> 1. abort if we ever find ourselves drawing a zero-width cursor
> 2. reject lgstrings of zero width
> 3. reject all glyphs of zero width outside of compositions
> 4. allow users to specify a minimum width, perhaps relative to the font
> size, so they can always see their cursor
> 
> ...which would be quite a different patch.

I'm not sure I follow.  What do you mean by "reject"?  I thought the
code which ignores the metric and sets the font_not_found_p flag when
we get a zero-width glyph is a kind of "rejection".  I like the idea
of doing that in the case of compositions because that is consistent
with what we do when we extract the metrics directly from the font.

Are you saying that what we do with simple characters in this case is
not good enough?  If you disable auto-composition-mode, and use the
fonts which shows CGJ as zero-width glyph, do you still see display
artifacts?  If not, what do you see and why is this kind of
'rejection" not enough?

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Pip Cet]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Pip Cet <pipcet@gmail.com>
>> Cc: dfussner@googlemail.com,  41645@debbugs.gnu.org
>> Date: Tue, 02 Jun 2020 19:21:31 +0000
>> 
>> >> xdisp.c:
>> >> 30008 if (get_char_glyph_code (it->char_to_display, font,
>> >> &char2b))
>> >> 30009            {
>> >> 30010              pcm = get_per_char_metric (font, &char2b);
>> >> 30011              if (pcm->width == 0
>> >> 30012              && pcm->rbearing == 0 && pcm->lbearing == 0)
>> >> 30013            pcm = NULL;
>> >> 30014            }
>> >> 
>> >
>> > ...with this.  I think you are right, and we should do the same with
>> > zero-width LGLYPH_STRING, forcing it->glyph_not_available_p to
>> > non-zero, and then doing something like this in
>> > fill_gstring_glyph_string:
>> >
>> >   if (s->font == NULL || glyph_not_available_p)
>> >     {
>> >       s->font_not_found_p = true;
>> >       s->font = FRAME_FONT (s->f);
>> >     }
>> >
>> > similar to what fill_glyph_string does.  WDYT?
>> 
>> I agree; the more I think about it, the more dangerous zero-sized
>> characters seem to me.
>> 
>> And almost all of my concerns apply to characters with zero x advance,
>> no matter whether they have lbearing or rbearing > 0.
>
> AFAIU, the advance metric is what we call "pixel width", and if so,
> the above snippet from xdisp.c already tests it.  Right?

Some code after it does, yes.

>> Maybe, for master, we should reject those as well? I was going to say
>> "or force their width to be at least a single pixel", but I'm not even
>> sure that's sufficient on hidpi screens...
>> 
>> So, in summary, I'd like to do the following:
>> 
>> 1. abort if we ever find ourselves drawing a zero-width cursor
>> 2. reject lgstrings of zero width
>> 3. reject all glyphs of zero width outside of compositions
>> 4. allow users to specify a minimum width, perhaps relative to the font
>> size, so they can always see their cursor
>> 
>> ...which would be quite a different patch.
>
> I'm not sure I follow.  What do you mean by "reject"?  I thought the
> code which ignores the metric and sets the font_not_found_p flag when
> we get a zero-width glyph is a kind of "rejection".

I mean "treat the glyph as non-existent". Currently, for (3), glyphs
with lbearing but no pixel width are treated as valid and expanded to
cover a single pixel, which is all but invisible on my screen.

> I like the idea
> of doing that in the case of compositions because that is consistent
> with what we do when we extract the metrics directly from the font.

I do too.

> Are you saying that what we do with simple characters in this case is
> not good enough?

I'm saying it's not good enough for non-spacing characters that actually
do print something: they're displayed as single-pixel-wide glyphs, which
I think is insufficient to let users be aware of them.

> If you disable auto-composition-mode, and use the
> fonts which shows CGJ as zero-width glyph, do you still see display
> artifacts?

No.

> If not, what do you see and why is this kind of
> 'rejection" not enough?

CGJ is displayed as a black box as wide as a space, which is perfectly
fine. It's U+301 that's not.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Eli Zaretskii]

> From: Pip Cet <pipcet@gmail.com>
> Cc: dfussner@googlemail.com,  41645@debbugs.gnu.org
> Date: Wed, 03 Jun 2020 14:58:07 +0000
> 
> > I'm not sure I follow.  What do you mean by "reject"?  I thought the
> > code which ignores the metric and sets the font_not_found_p flag when
> > we get a zero-width glyph is a kind of "rejection".
> 
> I mean "treat the glyph as non-existent". Currently, for (3), glyphs
> with lbearing but no pixel width are treated as valid and expanded to
> cover a single pixel, which is all but invisible on my screen.

They are hard to spot, but if one looks close enough, IME they are
visible.

We could perhaps introduce a feature whereby such thin-space glyphs
are somehow made to stand out more, but that would be a separate
feature, because right now we have these 1-pixel thin spaces with many
control characters.

> > If you disable auto-composition-mode, and use the
> > fonts which shows CGJ as zero-width glyph, do you still see display
> > artifacts?
> 
> No.

Then I think your suggestion to handle such lgstrings as we do with
simple characters is sufficient to fix situations such as this one.

> > If not, what do you see and why is this kind of
> > 'rejection" not enough?
> 
> CGJ is displayed as a black box as wide as a space, which is perfectly
> fine. It's U+301 that's not.

Hmm... how does U+301 enter this picture?  What problems do you see
with its display?

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Pip Cet]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Pip Cet <pipcet@gmail.com>
>> Cc: dfussner@googlemail.com,  41645@debbugs.gnu.org
>> Date: Wed, 03 Jun 2020 14:58:07 +0000
>> 
>> > I'm not sure I follow.  What do you mean by "reject"?  I thought the
>> > code which ignores the metric and sets the font_not_found_p flag when
>> > we get a zero-width glyph is a kind of "rejection".
>> 
>> I mean "treat the glyph as non-existent". Currently, for (3), glyphs
>> with lbearing but no pixel width are treated as valid and expanded to
>> cover a single pixel, which is all but invisible on my screen.
>
> They are hard to spot, but if one looks close enough, IME they are
> visible.
>
> We could perhaps introduce a feature whereby such thin-space glyphs
> are somehow made to stand out more, but that would be a separate
> feature, because right now we have these 1-pixel thin spaces with many
> control characters.

Yes, you're right.

>
>> > If you disable auto-composition-mode, and use the
>> > fonts which shows CGJ as zero-width glyph, do you still see display
>> > artifacts?
>> 
>> No.
>
> Then I think your suggestion to handle such lgstrings as we do with
> simple characters is sufficient to fix situations such as this one.

Okay.  How's this patch?

From 1b8683cb24ba364475c864f708da7b32319fdd2e Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@gmail.com>
Date: Wed, 3 Jun 2020 20:10:51 +0000
Subject: [PATCH] Avoid zero-width glyphs and the resulting cursor artifacts.

* src/xdisp.c (gui_produce_glyphs): Widen zero-width compositions to
one pixel.  (Bug#41645)
---
 src/xdisp.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/xdisp.c b/src/xdisp.c
index 0f06a38d40..414dc8809b 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -30592,6 +30592,12 @@ gui_produce_glyphs (struct it *it)
 	it->glyph_row->contains_overlapping_glyphs_p = true;
 
       it->pixel_width = cmp->pixel_width;
+      if (it->pixel_width == 0)
+	{
+	  /* We assure that all visible glyphs have at least 1-pixel
+	     width.  */
+	  it->pixel_width = 1;
+	}
       it->ascent = it->phys_ascent = cmp->ascent;
       it->descent = it->phys_descent = cmp->descent;
       IT_APPLY_FACE_BOX(it, face);
@@ -30623,6 +30629,12 @@ gui_produce_glyphs (struct it *it)
       it->pixel_width
 	= composition_gstring_width (gstring, it->cmp_it.from, it->cmp_it.to,
 				     &metrics);
+      if (it->pixel_width == 0)
+	{
+	  /* We assure that all visible glyphs have at least 1-pixel
+	     width.  */
+	  it->pixel_width = 1;
+	}
       if (it->glyph_row
 	  && (metrics.lbearing < 0 || metrics.rbearing > metrics.width))
 	it->glyph_row->contains_overlapping_glyphs_p = true;
-- 
2.27.0.rc0

>> > If not, what do you see and why is this kind of
>> > 'rejection" not enough?
>> 
>> CGJ is displayed as a black box as wide as a space, which is perfectly
>> fine. It's U+301 that's not.
>
> Hmm... how does U+301 enter this picture?  What problems do you see
> with its display?

It's a single pixel in width and leaves the accent slightly off-center
as a result. Nothing major.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Eli Zaretskii]

> From: Pip Cet <pipcet@gmail.com>
> Cc: dfussner@googlemail.com,  41645@debbugs.gnu.org
> Date: Wed, 03 Jun 2020 20:23:47 +0000
> 
> diff --git a/src/xdisp.c b/src/xdisp.c
> index 0f06a38d40..414dc8809b 100644
> --- a/src/xdisp.c
> +++ b/src/xdisp.c
> @@ -30592,6 +30592,12 @@ gui_produce_glyphs (struct it *it)
>  	it->glyph_row->contains_overlapping_glyphs_p = true;
>  
>        it->pixel_width = cmp->pixel_width;
> +      if (it->pixel_width == 0)
> +	{
> +	  /* We assure that all visible glyphs have at least 1-pixel
> +	     width.  */
> +	  it->pixel_width = 1;
> +	}
>        it->ascent = it->phys_ascent = cmp->ascent;
>        it->descent = it->phys_descent = cmp->descent;
>        IT_APPLY_FACE_BOX(it, face);
> @@ -30623,6 +30629,12 @@ gui_produce_glyphs (struct it *it)
>        it->pixel_width
>  	= composition_gstring_width (gstring, it->cmp_it.from, it->cmp_it.to,
>  				     &metrics);
> +      if (it->pixel_width == 0)
> +	{
> +	  /* We assure that all visible glyphs have at least 1-pixel
> +	     width.  */
> +	  it->pixel_width = 1;
> +	}
>        if (it->glyph_row
>  	  && (metrics.lbearing < 0 || metrics.rbearing > metrics.width))
>  	it->glyph_row->contains_overlapping_glyphs_p = true;

I like this less than your original proposal.  Artificially "fixing"
the pixel width of a glyph without changing anything else might get us
in trouble, since the font glyph is still zero-width and its other
metrics are incompatible with this "fixed" value.

Why did you prefer this to the original proposal, which was to set the
font_not_found_p flag?

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Pip Cet]

[-- Attachment #1: Type: text/plain, Size: 1875 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Pip Cet <pipcet@gmail.com>
>> Cc: dfussner@googlemail.com,  41645@debbugs.gnu.org
>> Date: Wed, 03 Jun 2020 20:23:47 +0000
>> 
>> diff --git a/src/xdisp.c b/src/xdisp.c
>> index 0f06a38d40..414dc8809b 100644
>> --- a/src/xdisp.c
>> +++ b/src/xdisp.c
>> @@ -30592,6 +30592,12 @@ gui_produce_glyphs (struct it *it)
>>  	it->glyph_row->contains_overlapping_glyphs_p = true;
>>  
>>        it->pixel_width = cmp->pixel_width;
>> +      if (it->pixel_width == 0)
>> +	{
>> +	  /* We assure that all visible glyphs have at least 1-pixel
>> +	     width.  */
>> +	  it->pixel_width = 1;
>> +	}
>>        it->ascent = it->phys_ascent = cmp->ascent;
>>        it->descent = it->phys_descent = cmp->descent;
>>        IT_APPLY_FACE_BOX(it, face);
>> @@ -30623,6 +30629,12 @@ gui_produce_glyphs (struct it *it)
>>        it->pixel_width
>>  	= composition_gstring_width (gstring, it->cmp_it.from, it->cmp_it.to,
>>  				     &metrics);
>> +      if (it->pixel_width == 0)
>> +	{
>> +	  /* We assure that all visible glyphs have at least 1-pixel
>> +	     width.  */
>> +	  it->pixel_width = 1;
>> +	}
>>        if (it->glyph_row
>>  	  && (metrics.lbearing < 0 || metrics.rbearing > metrics.width))
>>  	it->glyph_row->contains_overlapping_glyphs_p = true;
>
> I like this less than your original proposal.

Okay, so do I. I'd misunderstoond your previous comment, then.

> Artificially "fixing"
> the pixel width of a glyph without changing anything else might get us
> in trouble, since the font glyph is still zero-width and its other
> metrics are incompatible with this "fixed" value.
>
> Why did you prefer this to the original proposal, which was to set the
> font_not_found_p flag?

Is this any better? I'm not sure what to do about static compositions,
to be honest, so maybe we shouldn't touch those at all. What do you think?



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Avoid-zero-width-glyphs-and-the-resulting-cursor-art.patch --]
[-- Type: text/x-diff, Size: 3285 bytes --]

From 6efd912c82be9c071f5a02eb67ffcc0d6b6ebc2e Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@gmail.com>
Date: Thu, 4 Jun 2020 06:55:57 +0000
Subject: [PATCH] Avoid zero-width glyphs and the resulting cursor artifacts

* src/xdisp.c (fill_gstring_glyph_string): Handle unavailable glyphs.
(append_composite_glyph): Mark unavailable glyphs.
(gui_produce_glyphs): Make glyphs unavailable for zero-width
compositions.  (Bug#41645)
---
 src/xdisp.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/src/xdisp.c b/src/xdisp.c
index 327e8a183b..5fb278c420 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -27689,10 +27689,12 @@ fill_gstring_glyph_string (struct glyph_string *s, int face_id,
   struct glyph *glyph, *last;
   Lisp_Object lgstring;
   int i;
+  bool glyph_not_available_p;
 
   s->for_overlaps = overlaps;
   glyph = s->row->glyphs[s->area] + start;
   last = s->row->glyphs[s->area] + end;
+  glyph_not_available_p = glyph->glyph_not_available_p;
   s->cmp_id = glyph->u.cmp.id;
   s->cmp_from = glyph->slice.cmp.from;
   s->cmp_to = glyph->slice.cmp.to + 1;
@@ -27707,7 +27709,8 @@ fill_gstring_glyph_string (struct glyph_string *s, int face_id,
 	 && glyph->u.cmp.automatic
 	 && glyph->u.cmp.id == s->cmp_id
 	 && glyph->face_id == face_id
-	 && s->cmp_to == glyph->slice.cmp.from)
+	 && s->cmp_to == glyph->slice.cmp.from
+	 && glyph->glyph_not_available_p == glyph_not_available_p)
     {
       s->width += glyph->pixel_width;
       s->cmp_to = (glyph++)->slice.cmp.to + 1;
@@ -27722,6 +27725,14 @@ fill_gstring_glyph_string (struct glyph_string *s, int face_id,
       s->char2b[i] = code & 0xFFFF;
     }
 
+  /* If the specified font could not be loaded, record that fact in
+     S->font_not_found_p so that we can draw rectangles for the
+     characters of the glyph string.  */
+  if (glyph_not_available_p)
+    {
+      s->font_not_found_p = true;
+    }
+
   return glyph - s->row->glyphs[s->area];
 }
 
@@ -28918,7 +28929,7 @@ append_composite_glyph (struct it *it)
       glyph->overlaps_vertically_p = (it->phys_ascent > it->ascent
 				      || it->phys_descent > it->descent);
       glyph->padding_p = false;
-      glyph->glyph_not_available_p = false;
+      glyph->glyph_not_available_p = it->glyph_not_available_p;
       glyph->face_id = it->face_id;
       glyph->font_type = FONT_TYPE_UNKNOWN;
       if (it->bidi_p)
@@ -30595,6 +30606,11 @@ gui_produce_glyphs (struct it *it)
 	it->glyph_row->contains_overlapping_glyphs_p = true;
 
       it->pixel_width = cmp->pixel_width;
+      if (it->pixel_width == 0)
+	{
+	  it->glyph_not_available_p = true;
+	  it->pixel_width = font->space_width;
+	}
       it->ascent = it->phys_ascent = cmp->ascent;
       it->descent = it->phys_descent = cmp->descent;
       IT_APPLY_FACE_BOX(it, face);
@@ -30626,6 +30642,11 @@ gui_produce_glyphs (struct it *it)
       it->pixel_width
 	= composition_gstring_width (gstring, it->cmp_it.from, it->cmp_it.to,
 				     &metrics);
+      if (it->pixel_width == 0)
+	{
+	  it->glyph_not_available_p = true;
+	  it->pixel_width = face->font->space_width;
+	}
       if (it->glyph_row
 	  && (metrics.lbearing < 0 || metrics.rbearing > metrics.width))
 	it->glyph_row->contains_overlapping_glyphs_p = true;
-- 
2.27.0.rc0

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Eli Zaretskii]

> From: Pip Cet <pipcet@gmail.com>
> Cc: dfussner@googlemail.com,  41645@debbugs.gnu.org
> Date: Thu, 04 Jun 2020 06:58:47 +0000
> 
> > I like this less than your original proposal.
> 
> Okay, so do I. I'd misunderstoond your previous comment, then.

Apologies if I wrote something which caused a misunderstanding.

> Is this any better?

Yes, thanks.  That's what I had in mind.

> I'm not sure what to do about static compositions,
> to be honest, so maybe we shouldn't touch those at all. What do you think?

Yes, let's not alter the static compositions code until we have a
valid reason.

> @@ -30626,6 +30642,11 @@ gui_produce_glyphs (struct it *it)
>        it->pixel_width
>  	= composition_gstring_width (gstring, it->cmp_it.from, it->cmp_it.to,
>  				     &metrics);
> +      if (it->pixel_width == 0)
> +	{
> +	  it->glyph_not_available_p = true;
> +	  it->pixel_width = face->font->space_width;
> +	}
>        if (it->glyph_row
>  	  && (metrics.lbearing < 0 || metrics.rbearing > metrics.width))
>  	it->glyph_row->contains_overlapping_glyphs_p = true;

The IT_CHARACTER case also does this when the pixel width comes out as
zero:

	      it->phys_ascent = it->ascent;
	      it->phys_descent = it->descent;

That is, it doesn't trust the ascent/descent values from metrics.
Should we do the same for compositions, or did you see any reasons to
believe the metrics in this case?

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Eli Zaretskii]

> From: Pip Cet <pipcet@gmail.com>
> Cc: dfussner@googlemail.com,  41645@debbugs.gnu.org
> Date: Thu, 04 Jun 2020 22:48:19 +0000
> 
> > The IT_CHARACTER case also does this when the pixel width comes out as
> > zero:
> >
> > 	      it->phys_ascent = it->ascent;
> > 	      it->phys_descent = it->descent;
> 
> > That is, it doesn't trust the ascent/descent values from metrics.
> > Should we do the same for compositions, or did you see any reasons to
> > believe the metrics in this case?
> 
> The former, I believe. I still think fonts with such glyphs are broken,
> and we should rely as little as possible on what they say about them.
> 
> So I've done that in this new version.

Thanks.  Please push to the master branch, with one nit:

> +  if (glyph_not_available_p)
> +    {
> +      s->font_not_found_p = true;
> +    }

Our style convention is not to use braces when the block entails only
one line.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Pip Cet]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Pip Cet <pipcet@gmail.com>
>> Cc: dfussner@googlemail.com,  41645@debbugs.gnu.org
>> Date: Thu, 04 Jun 2020 22:48:19 +0000
>> 
>> > The IT_CHARACTER case also does this when the pixel width comes out as
>> > zero:
>> >
>> > 	      it->phys_ascent = it->ascent;
>> > 	      it->phys_descent = it->descent;
>> 
>> > That is, it doesn't trust the ascent/descent values from metrics.
>> > Should we do the same for compositions, or did you see any reasons to
>> > believe the metrics in this case?
>> 
>> The former, I believe. I still think fonts with such glyphs are broken,
>> and we should rely as little as possible on what they say about them.
>> 
>> So I've done that in this new version.
>
> Thanks.  Please push to the master branch, with one nit:
>
>> +  if (glyph_not_available_p)
>> +    {
>> +      s->font_not_found_p = true;
>> +    }
>
> Our style convention is not to use braces when the block entails only
> one line.

Thanks! I'd changed that code and missed that.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[David Fussner via Bug reports for GNU Emacs, the Swiss army knife of text editors]

It looks fine on master here -- many thanks to you both. Should the
bug be closed?

On Fri, 5 Jun 2020 at 09:31, Pip Cet <pipcet@gmail.com> wrote:
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> >> From: Pip Cet <pipcet@gmail.com>
> >> Cc: dfussner@googlemail.com,  41645@debbugs.gnu.org
> >> Date: Thu, 04 Jun 2020 22:48:19 +0000
> >>
> >> > The IT_CHARACTER case also does this when the pixel width comes out as
> >> > zero:
> >> >
> >> >          it->phys_ascent = it->ascent;
> >> >          it->phys_descent = it->descent;
> >>
> >> > That is, it doesn't trust the ascent/descent values from metrics.
> >> > Should we do the same for compositions, or did you see any reasons to
> >> > believe the metrics in this case?
> >>
> >> The former, I believe. I still think fonts with such glyphs are broken,
> >> and we should rely as little as possible on what they say about them.
> >>
> >> So I've done that in this new version.
> >
> > Thanks.  Please push to the master branch, with one nit:
> >
> >> +  if (glyph_not_available_p)
> >> +    {
> >> +      s->font_not_found_p = true;
> >> +    }
> >
> > Our style convention is not to use braces when the block entails only
> > one line.
>
> Thanks! I'd changed that code and missed that.

bug#41645: 27.0.91; Combining Grapheme Joiner (#x34f) gui artifacts

[Eli Zaretskii]

> From: David Fussner <dfussner@googlemail.com>
> Date: Fri, 5 Jun 2020 16:54:14 +0100
> Cc: Eli Zaretskii <eliz@gnu.org>, 41645@debbugs.gnu.org
> 
> It looks fine on master here -- many thanks to you both. Should the
> bug be closed?

Yes, done.