bug#20410: 24.5.50; cursor not updated after yank of non-ASCII string from the clipboard

bug#20410: 24.5.50; cursor not updated after yank of non-ASCII string from the clipboard

[YAMAMOTO Mitsuharu]

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

Steps to reproduce:

1. Copy some non-ASCII string (say, "あ") to the clipboard.
2. $ emacs -Q -D &
3. C-x C-b
4. C-y
5. C-p

Expected Result:

The cursor moves upward by Step 5.

Actual Result:

The display is not changed between Step 4 and 5.

Moreover, if you make the Emacs frame expose by obscuring and
revealing it with another window after you turn off your X11
compositing manager, then you'll see the parts other than the upper
mode-line and the echo area are not redrawn (see the attachment).

				      YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

In GNU Emacs 24.5.50.1 (i686-pc-linux-gnu, GTK+ Version 2.10.4)
 of 2015-04-23 on localhost.localdomain
Windowing system distributor `The X.Org Foundation', version 11.0.70101000
System Description:	CentOS release 5.11 (Final)

Important settings:
  value of $LANG: ja_JP.UTF-8
  value of $XMODIFIERS: @im=SCIM
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-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


[-- Attachment #2: tmp.png --]
[-- Type: image/png, Size: 18168 bytes --]

bug#20410: 24.5.50; cursor not updated after yank of non-ASCII string from the clipboard

[Eli Zaretskii]

> Date: Thu, 23 Apr 2015 18:59:45 +0900
> From: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> 
> 1. Copy some non-ASCII string (say, "あ") to the clipboard.
> 2. $ emacs -Q -D &
> 3. C-x C-b
> 4. C-y
> 5. C-p
> 
> Expected Result:
> 
> The cursor moves upward by Step 5.
> 
> Actual Result:
> 
> The display is not changed between Step 4 and 5.

What does "C-x =" report before and after step 5?

Anyway, this seems to be X-specific: I cannot reproduce it on
MS-Windows.

bug#20410: 24.5.50; cursor not updated after yank of non-ASCII string from the clipboard

[YAMAMOTO Mitsuharu]

>>>>> On Thu, 23 Apr 2015 13:42:40 +0300, Eli Zaretskii <eliz@gnu.org> said:

>> Date: Thu, 23 Apr 2015 18:59:45 +0900
>> From: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
>> 
>> 1. Copy some non-ASCII string (say, "あ") to the clipboard.
>> 2. $ emacs -Q -D &
>> 3. C-x C-b
>> 4. C-y
>> 5. C-p
>> 
>> Expected Result:
>> 
>> The cursor moves upward by Step 5.
>> 
>> Actual Result:
>> 
>> The display is not changed between Step 4 and 5.

> What does "C-x =" report before and after step 5?

It says

  Char: C-j (10, #o12, #xa) point=191 of 192 (99%) column=0

So, the cursor is actually moved internally, but display is not
updated accordingly.

> Anyway, this seems to be X-specific: I cannot reproduce it on
> MS-Windows.

It turned out Step 3 above was not necessary.

I also tried git bisect, and it shows:

bd0443bb8799b908cefd0b11c86db6115180850d is the first bad commit
commit bd0443bb8799b908cefd0b11c86db6115180850d
Author: Dmitry Antipov <dmantipov@yandex.ru>
Date:   Fri Sep 6 11:00:29 2013 +0400

    Attempt to make redisplay more selective when changing fonts.
    * frame.h (struct frame): New bitfield fonts_changed.
    * dispextern.h (fonts_changed_p, adjust_glyphs): Remove declaration.
    (adjust_frame_glyphs): Add prototype.
    * dispnew.c (fonts_changed_p): Remove.
    (adjust_glyphs): Remove becase we do not
    adjust matrices on all frames at once any more.
    (adjust_frame_glyphs): Block and unblock input here.
    (adjust_glyph_matrix): Use fonts_changed.
    (change_frame_size_1): Use adjust_frame_glyphs.
    * font.c (font_open_entity): Use fonts_changed.
    * frame.c (set_menu_bar_lines, Fmake_terminal_frame):
    * w32fns.c (x_set_menu_bar_lines, x_set_tool_bar_lines, Fx_show_tip):
    * window.c (Fdelete_other_windows_internal, Fwindow_resize_apply)
    (Fsplit_window_internal, Fdelete_window_internal, grow_mini_window)
    (shrink_mini_window, Fresize_mini_window_internal)
    (window_scroll_pixel_based, Fset_window_configuration)
    (apply_window_adjustment, Fset_window_vscroll):
    * xfns.c (x_set_menu_bar_lines, x_set_tool_bar_lines, Fx_show_tip):
    Use adjust_frame_glyphs.
    * xdisp.c (redisplay_tool_bar, redisplay_window, try_window)
    (try_window_reusing_current_matrix, try_window_id, display_line)
    (IT_EXPAND_MATRIX_WIDTH): Use fonts_changed.
    (redisplay_internal): Consider fonts_changed and adjust frame
    matrices for each frame only if the frame is visible.  If font
    has been changed on some frame during full redisplay, retry
    only visible frames where the font has been actually changed.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

bug#20410: 24.5.50; cursor not updated after yank of non-ASCII string from the clipboard

[Eli Zaretskii]

> Date: Fri, 24 Apr 2015 13:09:29 +0900
> From: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> Cc: 20410@debbugs.gnu.org
> 
> >>>>> On Thu, 23 Apr 2015 13:42:40 +0300, Eli Zaretskii <eliz@gnu.org> said:
> 
> >> Date: Thu, 23 Apr 2015 18:59:45 +0900
> >> From: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> >> 
> >> 1. Copy some non-ASCII string (say, "あ") to the clipboard.
> >> 2. $ emacs -Q -D &
> >> 3. C-x C-b
> >> 4. C-y
> >> 5. C-p
> >> 
> >> Expected Result:
> >> 
> >> The cursor moves upward by Step 5.
> >> 
> >> Actual Result:
> >> 
> >> The display is not changed between Step 4 and 5.
> 
> > What does "C-x =" report before and after step 5?
> 
> It says
> 
>   Char: C-j (10, #o12, #xa) point=191 of 192 (99%) column=0
> 
> So, the cursor is actually moved internally, but display is not
> updated accordingly.

You show only one result of "C-x =", so it's hard to interpret it,
especially since you didn't show what was displayed after the paste.

Does the cursor move to the correct place if you type C-l or invoke
redraw-display, after performing the recipe?

In any case, cursor motion and display are unrelated to pasting from
the clipboard, so I wonder whether you could reproduce this without
the paste, just by typing that character.  I also wonder what does all
this have to do with expose events, as mentioned in your original
report.  Anything that I could reproduce here and debug it would be
helpful.

Failing that, cursor positioning is done in set_cursor_from_row, so
perhaps you could step through that function and see what's going
wrong there, and why.  That function is called once for every window,
so to be sure you look at the correct invocation, make sure the value
of w->contents at function entry is the *scratch* buffer.

The output of the GDB command "pgrow", defined on src/.gdbinit, when
inside set_cursor_from_row, is also important.

> I also tried git bisect, and it shows:
> 
> bd0443bb8799b908cefd0b11c86db6115180850d is the first bad commit
> commit bd0443bb8799b908cefd0b11c86db6115180850d
> Author: Dmitry Antipov <dmantipov@yandex.ru>
> Date:   Fri Sep 6 11:00:29 2013 +0400
> 
>     Attempt to make redisplay more selective when changing fonts.

Thanks, but such jumbo changes rarely help in debugging problems,
especially since they don't seem to touch the cursor setting function,
at least not directly.  So I think we need to debug this problem.

bug#20410: 24.5.50; cursor not updated after yank of non-ASCII string from the clipboard

[YAMAMOTO Mitsuharu]

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

>>>>> On Fri, 24 Apr 2015 09:58:28 +0300, Eli Zaretskii <eliz@gnu.org> said:

>> > What does "C-x =" report before and after step 5?
>> 
>> It says
>> 
>> Char: C-j (10, #o12, #xa) point=191 of 192 (99%) column=0
>> 
>> So, the cursor is actually moved internally, but display is not
>> updated accordingly.

> You show only one result of "C-x =", so it's hard to interpret it,
> especially since you didn't show what was displayed after the paste.

I thought it was clear from the screenshot attached in the first mail.
The point value in the output of "C-x =" indicates that the cursor is
between the initial message of the *scratch* buffer and the yanked
text.

(length ";; This buffer is for notes you don't want to save, and for Lisp evaluation.
;; If you want to create a file, visit that file with C-x C-f,
;; then enter the text in that file's own buffer.
")
=> 190

> Does the cursor move to the correct place if you type C-l or invoke
> redraw-display, after performing the recipe?

The cursor appears.  But the cursor at the old position is not
cleared.  See the attached screenshot.

> In any case, cursor motion and display are unrelated to pasting from
> the clipboard, so I wonder whether you could reproduce this without
> the paste, just by typing that character.  I also wonder what does all
> this have to do with expose events, as mentioned in your original
> report.  Anything that I could reproduce here and debug it would be
> helpful.

I could reproduce it without yank.

1. Create a file named a.el with the following contents:

  (global-set-key "a" (string #x3042))	; HIRAGANA LETTER A

2. $ emacs -Q -D -l a.el &
3. Type "a" to insert a Japanese character.
4. Type "C-p" to move the cursor upward.

Display is updated correctly until Step 3.  But Step 4 does not change
the display from the previous one.

As for expose events, I suspect the current matrices get corrupted,
because the result of git bisect implies that it has something to do
with adjustment of glyph matrices triggered by font loading.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

[-- Attachment #2: after-C-l.png --]
[-- Type: image/png, Size: 13739 bytes --]

bug#20410: 24.5.50; cursor not updated after yank of non-ASCII string from the clipboard

[Eli Zaretskii]

> Date: Sat, 25 Apr 2015 18:49:56 +0900
> From: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> Cc: 20410@debbugs.gnu.org
> 
> >> > What does "C-x =" report before and after step 5?
> >> 
> >> It says
> >> 
> >> Char: C-j (10, #o12, #xa) point=191 of 192 (99%) column=0
> >> 
> >> So, the cursor is actually moved internally, but display is not
> >> updated accordingly.
> 
> > You show only one result of "C-x =", so it's hard to interpret it,
> > especially since you didn't show what was displayed after the paste.
> 
> I thought it was clear from the screenshot attached in the first mail.

Since the issue is with incorrect display of cursor, I generally don't
believe my eyes, since the place where the cursor is drawn might not
be the place where Emacs thinks it is.

> > Does the cursor move to the correct place if you type C-l or invoke
> > redraw-display, after performing the recipe?
> 
> The cursor appears.  But the cursor at the old position is not
> cleared.  See the attached screenshot.

Thanks, this is important information.

> I could reproduce it without yank.
> 
> 1. Create a file named a.el with the following contents:
> 
>   (global-set-key "a" (string #x3042))	; HIRAGANA LETTER A
> 
> 2. $ emacs -Q -D -l a.el &
> 3. Type "a" to insert a Japanese character.
> 4. Type "C-p" to move the cursor upward.
> 
> Display is updated correctly until Step 3.  But Step 4 does not change
> the display from the previous one.

I succeeded in reproducing this, after changing the default fontset to
use a specific font for the u+3042 character.  Thanks, I will look
into this.

> As for expose events, I suspect the current matrices get corrupted,
> because the result of git bisect implies that it has something to do
> with adjustment of glyph matrices triggered by font loading.

Sounds plausible, thanks.

bug#20410: 24.5.50; cursor not updated after yank of non-ASCII string from the clipboard

[Eli Zaretskii]

> Date: Sat, 25 Apr 2015 13:09:07 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 20410@debbugs.gnu.org
> 
> > 1. Create a file named a.el with the following contents:
> > 
> >   (global-set-key "a" (string #x3042))	; HIRAGANA LETTER A
> > 
> > 2. $ emacs -Q -D -l a.el &
> > 3. Type "a" to insert a Japanese character.
> > 4. Type "C-p" to move the cursor upward.
> > 
> > Display is updated correctly until Step 3.  But Step 4 does not change
> > the display from the previous one.
> 
> I succeeded in reproducing this, after changing the default fontset to
> use a specific font for the u+3042 character.  Thanks, I will look
> into this.
> 
> > As for expose events, I suspect the current matrices get corrupted,
> > because the result of git bisect implies that it has something to do
> > with adjustment of glyph matrices triggered by font loading.
> 
> Sounds plausible, thanks.

Adjusting frame glyphs makes all glyph rows disabled, so various
redisplay optimizations should be disabled.  In particular, new cursor
position was correctly computed, but because it was on a disabled
glyph row, display_and_set_cursor would not obey that new position,
and exposing a window would not redraw the disabled glyph rows.

Does the patch below fix the problems you see?  (It might look a bit
drastic, but then loading new fonts is not supposed to be a frequent
operation, I think.)

diff --git a/src/dispnew.c b/src/dispnew.c
index 693dd49..c0fd679 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -1795,6 +1795,12 @@ struct glyph_row *
 
   f->glyphs_initialized_p = 1;
 
+  /* Disable all redisplay optimizations for this frame.  This is
+     because adjusting frame glyphs makes all glyph rows disabled, so
+     many optimizations will fail anyway, and some might not test this
+     flag and do bogus things.  */
+  SET_FRAME_GARBAGED (f);
+
   unblock_input ();
 }
 

bug#20410: 24.5.50; cursor not updated after yank of non-ASCII string from the clipboard

[YAMAMOTO Mitsuharu]

>>>>> On Sat, 25 Apr 2015 15:32:22 +0300, Eli Zaretskii <eliz@gnu.org> said:

> Adjusting frame glyphs makes all glyph rows disabled, so various
> redisplay optimizations should be disabled.  In particular, new cursor
> position was correctly computed, but because it was on a disabled
> glyph row, display_and_set_cursor would not obey that new position,
> and exposing a window would not redraw the disabled glyph rows.

> Does the patch below fix the problems you see?  (It might look a bit
> drastic, but then loading new fonts is not supposed to be a frequent
> operation, I think.)

Yes, it solves the problems as far as I tested.  But because
adjust_frame_glyphs are called from many places other than font
changes, I wonder if it might disable some cases where some
optimizations were applied successfully otherwise (sorry, I don't have
any ideas about concrete examples).  Some calls to adjust_frame_glyphs
are already accompanied by SET_FRAME_GARBAGED, but not always.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

bug#20410: 24.5.50; cursor not updated after yank of non-ASCII string from the clipboard

[Eli Zaretskii]

> Date: Sun, 26 Apr 2015 19:02:05 +0900
> From: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> Cc: 20410@debbugs.gnu.org
> 
> > Does the patch below fix the problems you see?  (It might look a bit
> > drastic, but then loading new fonts is not supposed to be a frequent
> > operation, I think.)
> 
> Yes, it solves the problems as far as I tested.  But because
> adjust_frame_glyphs are called from many places other than font
> changes, I wonder if it might disable some cases where some
> optimizations were applied successfully otherwise (sorry, I don't have
> any ideas about concrete examples).  Some calls to adjust_frame_glyphs
> are already accompanied by SET_FRAME_GARBAGED, but not always.

Hmm... you are right, we could be more selective.  Does the
alternative patch below work for you?

diff --git a/src/xdisp.c b/src/xdisp.c
index 5a27adc..cbe46eb 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -13359,6 +13359,13 @@ static void debug_method_add (struct window *, char const *, ...)
 	  if (f->fonts_changed)
 	    {
 	      adjust_frame_glyphs (f);
+	      /* Disable all redisplay optimizations for this frame.
+		 This is because adjust_frame_glyphs resets the
+		 enabled_p flag for all glyph rows of all windows, so
+		 many optimizations will fail anyway, and some might
+		 fail to test that flag and do bogus things as
+		 result.  */
+	      SET_FRAME_GARBAGED (f);
 	      f->fonts_changed = false;
 	    }
 	  /* If cursor type has been changed on the frame
@@ -13753,6 +13760,10 @@ static void debug_method_add (struct window *, char const *, ...)
 		  if (f->fonts_changed)
 		    {
 		      adjust_frame_glyphs (f);
+		      /* Disable all redisplay optimizations for this
+			 frame.  See the comment for the previous call
+			 to adjust_frame_glyphs for the reasons.  */
+		      SET_FRAME_GARBAGED (f);
 		      f->fonts_changed = false;
 		      goto retry_frame;
 		    }

bug#20410: 24.5.50; cursor not updated after yank of non-ASCII string from the clipboard

[YAMAMOTO Mitsuharu]

>>>>> On Sun, 26 Apr 2015 18:04:56 +0300, Eli Zaretskii <eliz@gnu.org> said:

>> Yes, it solves the problems as far as I tested.  But because
>> adjust_frame_glyphs are called from many places other than font
>> changes, I wonder if it might disable some cases where some
>> optimizations were applied successfully otherwise (sorry, I don't have
>> any ideas about concrete examples).  Some calls to adjust_frame_glyphs
>> are already accompanied by SET_FRAME_GARBAGED, but not always.

> Hmm... you are right, we could be more selective.  Does the
> alternative patch below work for you?

Yes, this works fine.  Thanks a lot.

The previous one (for src/dispnew.c) caused redraw on another window
on the same frame when I scrolled one window using set-window-vscroll,
which may call adjust_frame_glyphs.  But the current one (for
src/xdisp.c) doesn't have such a problem.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

bug#20410: 24.5.50; cursor not updated after yank of non-ASCII string from the clipboard

[Eli Zaretskii]

> Date: Mon, 27 Apr 2015 12:28:58 +0900
> From: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> Cc: 20410@debbugs.gnu.org
> 
> >>>>> On Sun, 26 Apr 2015 18:04:56 +0300, Eli Zaretskii <eliz@gnu.org> said:
> 
> >> Yes, it solves the problems as far as I tested.  But because
> >> adjust_frame_glyphs are called from many places other than font
> >> changes, I wonder if it might disable some cases where some
> >> optimizations were applied successfully otherwise (sorry, I don't have
> >> any ideas about concrete examples).  Some calls to adjust_frame_glyphs
> >> are already accompanied by SET_FRAME_GARBAGED, but not always.
> 
> > Hmm... you are right, we could be more selective.  Does the
> > alternative patch below work for you?
> 
> Yes, this works fine.  Thanks a lot.
> 
> The previous one (for src/dispnew.c) caused redraw on another window
> on the same frame when I scrolled one window using set-window-vscroll,
> which may call adjust_frame_glyphs.  But the current one (for
> src/xdisp.c) doesn't have such a problem.

Thanks, pushed as commit d89687b.