bug#8703: truncated unicode glyphs in X11

bug#8703: truncated unicode glyphs in X11

[Bertram Felgenhauer]

The behaviour is probably best demonstrated with screenshots,

http://cl-informatik.uibk.ac.at/users/bf3/emacs/

To summarize, Proof General (which is an emacs based frontend for
automatic theorem provers) can display parts of formulas using unicode
characters. In recent versions of emacs, these get truncated (i.e.
the corresponding character cells are too narrow to contain the
character.) This bug affects emacs 23.3 and the current development
version. Version 23.2 is fine.

I have tracked down the regression using git-bisect, and found it
was introduced by

http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=5d747e944fd5a15bb36f865efc214024803c5fcf

Undoing a single change restores the correct behaviour for me:

diff --git a/src/xdisp.c b/src/xdisp.c
index 3c9d385..20365ff 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -5926,8 +5926,7 @@ get_next_display_element (struct it *it)
                           : STRINGP (it->string) ? IT_STRING_CHARPOS (*it)
                           : IT_CHARPOS (*it));
 
-         it->face_id = FACE_FOR_CHAR (it->f, face, it->char_to_display, pos,
-                                      it->string);
+         it->face_id = FACE_FOR_CHAR (it->f, face, it->c, pos, it->string);
        }
     }
 #endif

I do not know the code in question, so this fix may be wrong.

Best regards,

Bertram Felgenhauer

bug#8703: truncated unicode glyphs in X11

[Eli Zaretskii]

> Date: Thu, 19 May 2011 22:17:02 +0200
> From: Bertram Felgenhauer <bertram.felgenhauer@googlemail.com>
> 
> I have tracked down the regression using git-bisect, and found it
> was introduced by
> 
> http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=5d747e944fd5a15bb36f865efc214024803c5fcf

For those using bzr, this is revision 100011 on the emacs-23 branch or
revision 99634.13.1 on the trunk.  Perhaps Handa-san could take a look
at this, as he made that change.

> Undoing a single change restores the correct behaviour for me:
> 
> diff --git a/src/xdisp.c b/src/xdisp.c
> index 3c9d385..20365ff 100644
> --- a/src/xdisp.c
> +++ b/src/xdisp.c
> @@ -5926,8 +5926,7 @@ get_next_display_element (struct it *it)
>                            : STRINGP (it->string) ? IT_STRING_CHARPOS (*it)
>                            : IT_CHARPOS (*it));
>  
> -         it->face_id = FACE_FOR_CHAR (it->f, face, it->char_to_display, pos,
> -                                      it->string);
> +         it->face_id = FACE_FOR_CHAR (it->f, face, it->c, pos, it->string);

Can you tell what are the values of it->c and it->char_to_display at
this point, for one of the characters hose display is truncated?

bug#8703: truncated unicode glyphs in X11

[Bertram Felgenhauer]

Dear Eli,

Eli Zaretskii wrote:
> > Date: Thu, 19 May 2011 22:17:02 +0200
> > From: Bertram Felgenhauer <bertram.felgenhauer@googlemail.com>
> > 
> > I have tracked down the regression using git-bisect, and found it
> > was introduced by
> > 
> > http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=5d747e944fd5a15bb36f865efc214024803c5fcf
> 
> For those using bzr, this is revision 100011 on the emacs-23 branch or
> revision 99634.13.1 on the trunk.  Perhaps Handa-san could take a look
> at this, as he made that change.
> 
> > Undoing a single change restores the correct behaviour for me:
> > 
> > diff --git a/src/xdisp.c b/src/xdisp.c
> > index 3c9d385..20365ff 100644
> > --- a/src/xdisp.c
> > +++ b/src/xdisp.c
> > @@ -5926,8 +5926,7 @@ get_next_display_element (struct it *it)
> >                            : STRINGP (it->string) ? IT_STRING_CHARPOS (*it)
> >                            : IT_CHARPOS (*it));
> >  
> > -         it->face_id = FACE_FOR_CHAR (it->f, face, it->char_to_display, pos,
> > -                                      it->string);
> > +         it->face_id = FACE_FOR_CHAR (it->f, face, it->c, pos, it->string);
> 
> Can you tell what are the values of it->c and it->char_to_display at
> this point, for one of the characters hose display is truncated?

For the example screenshot, I found the following cases where
it->c and it->char_to_display differ:

it->c                                  it->char_to_display
 8896 (N-ARY LOGICAL AND)              34 (QUOTATION MARK)
 8801 (IDENTICAL TO)                   32 (SPACE)
10233 (LONG RIGHTWARDS DOUBLE ARROW)   32 (SPACE)

it->char_to_display is the previous character in the string each time,
and indeed there's a possibility that this path is taken but
it->char_to_display was not set by get_next_display_element: when
it->what == IT_COMPOSITION and it->cmp_it.ch < 0.

The following patch also works for me:

diff --git a/src/xdisp.c b/src/xdisp.c
index 3c9d385..c9848fd 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -5902,6 +5902,8 @@ get_next_display_element (struct it *it)
          it->char_to_display = it->c;
        }
     }
+  else if (it->what == IT_COMPOSITION && it->cmp_it.ch < 0)
+    it->char_to_display = it->c;
 
 #ifdef HAVE_WINDOW_SYSTEM
   /* Adjust face id for a multibyte character.  There are no multibyte


For completeness: The behaviour seems to be independent of the font
being used. I tried with the 6x13 system font (which by its fixed
width nature displays fine, btw) and with "DejaVu Sans Mono" which
I used for the screenshots (sorry for not mentioning that in the
original report.)

Thanks,

Bertram

bug#8703: truncated unicode glyphs in X11

[Eli Zaretskii]

> Date: Fri, 20 May 2011 13:56:04 +0200
> From: Bertram Felgenhauer <bertram.felgenhauer@googlemail.com>
> Cc: Kenichi Handa <handa@m17n.org>, 8703@debbugs.gnu.org
> 
> For the example screenshot, I found the following cases where
> it->c and it->char_to_display differ:
> 
> it->c                                  it->char_to_display
>  8896 (N-ARY LOGICAL AND)              34 (QUOTATION MARK)
>  8801 (IDENTICAL TO)                   32 (SPACE)
> 10233 (LONG RIGHTWARDS DOUBLE ARROW)   32 (SPACE)
> 
> it->char_to_display is the previous character in the string each time,
> and indeed there's a possibility that this path is taken but
> it->char_to_display was not set by get_next_display_element: when
> it->what == IT_COMPOSITION and it->cmp_it.ch < 0.

This certainly sounds like the root cause, thanks.  I'll let Handa-san
judge.

bug#8703: truncated unicode glyphs in X11

[Kenichi Handa]

In article <83tycpfz78.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:

> > Date: Thu, 19 May 2011 22:17:02 +0200
> > From: Bertram Felgenhauer <bertram.felgenhauer@googlemail.com>
> > 
> > I have tracked down the regression using git-bisect, and found it
> > was introduced by
> > 
> > http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=5d747e944fd5a15bb36f865efc214024803c5fcf

> For those using bzr, this is revision 100011 on the emacs-23 branch or
> revision 99634.13.1 on the trunk.  Perhaps Handa-san could take a look
> at this, as he made that change.

Thank you for the report.  I've just installed a fix
(attached) to emacs-23 branch.  Could you please try it?

---
Kenichi Handa
handa@m17n.org

=== modified file 'src/xdisp.c'
--- src/xdisp.c	2011-05-09 09:59:23 +0000
+++ src/xdisp.c	2011-05-25 01:18:58 +0000
@@ -5922,9 +5922,21 @@
 	  int pos = (it->s ? -1
 		     : STRINGP (it->string) ? IT_STRING_CHARPOS (*it)
 		     : IT_CHARPOS (*it));
+	  int c;
+
+	  if (it->what == IT_CHARACTER)
+	    c = it->char_to_display;
+	  else
+	    {
+	      struct composition *cmp = composition_table[it->cmp_it.id];
+	      int i;
 
-	  it->face_id = FACE_FOR_CHAR (it->f, face, it->char_to_display, pos,
-				       it->string);
+	      c = ' ';
+	      for (i = 0; i < cmp->glyph_len; i++)
+		if ((c = COMPOSITION_GLYPH (cmp, i)) != '\t')
+		  break;
+	    }
+	  it->face_id = FACE_FOR_CHAR (it->f, face, c, pos, it->string);
 	}
     }
 #endif

bug#8703: truncated unicode glyphs in X11

[Bertram Felgenhauer]

Kenichi Handa wrote:
> Thank you for the report.  I've just installed a fix
> (attached) to emacs-23 branch.  Could you please try it?

I tried it, and it works great.

Thanks a lot!

Bertram