bug#10903: 24.0.93; redisplay glitch with before-string and display overlays

bug#10903: 24.0.93; redisplay glitch with before-string and display overlays

[Stephen Berman]

1. emacs -Q

2. Type `C-x b a RET' to get a fresh buffer in Fundamental mode, and  at
(point-min) type "012".

3. Type `M-<' so that the cursor is over (i.e. just before) `0'.

4. Type `M-: (overlay-put (make-overlay (point) (point)) 'before-string
"*")'.  Now you see this:

*012

5. Type `M-: (overlay-put (make-overlay (point) (1+ (point))) 'display
"")'.  Now you see this:

*12

6. Type `M-: (remove-overlays)' to restore "012".

7. Type SPC to advance the cursor and the start of "012" to column one.

8. Repeat step 4.  Now you see this:

 *012

9. Repeat step 5.  Now you see this:

 *112

According to my tests, this redisplay glitch happens when the overlays
begin anywhere other than (point-min).  It also only happens when the
before-string overlay is added prior to the display string overlay, and
only when the value of the display string is "".  I don't know when this
problem appeared, but it does not happen in Emacs 23.3.


In GNU Emacs 24.0.93.1 (i686-suse-linux-gnu, GTK+ Version 2.24.7)
 of 2012-02-26 on escher
Windowing system distributor `The X.Org Foundation', version 11.0.11004000
Configured using:
 `configure '--without-toolkit-scroll-bars' 'CFLAGS=-g''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=local
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

bug#10903: 24.0.93; redisplay glitch with before-string and display overlays

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Date: Mon, 27 Feb 2012 23:04:03 +0100
> 
> 1. emacs -Q
> 
> 2. Type `C-x b a RET' to get a fresh buffer in Fundamental mode, and  at
> (point-min) type "012".
> 
> 3. Type `M-<' so that the cursor is over (i.e. just before) `0'.
> 
> 4. Type `M-: (overlay-put (make-overlay (point) (point)) 'before-string
> "*")'.  Now you see this:
> 
> *012
> 
> 5. Type `M-: (overlay-put (make-overlay (point) (1+ (point))) 'display
> "")'.  Now you see this:
> 
> *12
> 
> 6. Type `M-: (remove-overlays)' to restore "012".
> 
> 7. Type SPC to advance the cursor and the start of "012" to column one.
> 
> 8. Repeat step 4.  Now you see this:
> 
>  *012
> 
> 9. Repeat step 5.  Now you see this:
> 
>  *112

We were hitting on an old redisplay optimization that didn't play well
with the bidi-aware display engine.

I think I fixed this.  Thanks for such a clear-cut test case.

Since Savannah is on the move, I cannot commit the fix, but perhaps
you could test it locally and see if there are any leftovers.  Here's
the patch:

=== modified file 'src/xdisp.c'
--- src/xdisp.c	2012-02-12 09:46:33 +0000
+++ src/xdisp.c	2012-02-28 20:35:41 +0000
@@ -5156,6 +5156,10 @@ next_overlay_string (struct it *it)
       it->current.overlay_string_index = -1;
       it->n_overlay_strings = 0;
       it->overlay_strings_charpos = -1;
+      /* If there's an empty display string on the stack, pop the
+	 stack, to resync the bidi iterator with IT's position.  */
+      if (it->sp > 0 && STRINGP (it->string) && !SCHARS (it->string))
+	pop_it (it);
 
       /* If we're at the end of the buffer, record that we have
 	 processed the overlay strings there already, so that
@@ -5453,8 +5457,15 @@ get_overlay_strings_1 (struct it *it, EM
       xassert (!compute_stop_p || it->sp == 0);
 
       /* When called from handle_stop, there might be an empty display
-         string loaded.  In that case, don't bother saving it.  */
-      if (!STRINGP (it->string) || SCHARS (it->string))
+         string loaded.  In that case, don't bother saving it.  But
+         don't use this optimization with the bidi iterator, since we
+         need the corresponding pop_it call to resync the bidi
+         iterator's position with IT's position, after we are done
+         with the overlay strings.  (The corresponding call to pop_it
+         in case of an empty display string is in
+         next_overlay_string.)  */
+      if (!(!it->bidi_p
+	    && STRINGP (it->string) && !SCHARS (it->string)))
 	push_it (it, NULL);
 
       /* Set up IT to deliver display elements from the first overlay

bug#10903: 24.0.93; redisplay glitch with before-string and display overlays

[Stephen Berman]

On Tue, 28 Feb 2012 22:40:17 +0200 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Date: Mon, 27 Feb 2012 23:04:03 +0100
>> 
>> 1. emacs -Q
>> 
>> 2. Type `C-x b a RET' to get a fresh buffer in Fundamental mode, and  at
>> (point-min) type "012".
>> 
>> 3. Type `M-<' so that the cursor is over (i.e. just before) `0'.
>> 
>> 4. Type `M-: (overlay-put (make-overlay (point) (point)) 'before-string
>> "*")'.  Now you see this:
>> 
>> *012
>> 
>> 5. Type `M-: (overlay-put (make-overlay (point) (1+ (point))) 'display
>> "")'.  Now you see this:
>> 
>> *12
>> 
>> 6. Type `M-: (remove-overlays)' to restore "012".
>> 
>> 7. Type SPC to advance the cursor and the start of "012" to column one.
>> 
>> 8. Repeat step 4.  Now you see this:
>> 
>>  *012
>> 
>> 9. Repeat step 5.  Now you see this:
>> 
>>  *112
>
> We were hitting on an old redisplay optimization that didn't play well
> with the bidi-aware display engine.
>
> I think I fixed this.  Thanks for such a clear-cut test case.
>
> Since Savannah is on the move, I cannot commit the fix, but perhaps
> you could test it locally and see if there are any leftovers.  Here's
> the patch:

I applied the patch and rebuilt, and the redisplay glitches I observed
are gone; thanks very much for the quick fix!  I consider this bug
resolved, but it should probably be closed after the fix is committed to
the trunk.  Thanks again.

Steve Berman

bug#10903: 24.0.93; redisplay glitch with before-string and display overlays

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: 10903@debbugs.gnu.org
> Date: Tue, 28 Feb 2012 23:25:13 +0100
> 
> > Since Savannah is on the move, I cannot commit the fix, but perhaps
> > you could test it locally and see if there are any leftovers.  Here's
> > the patch:
> 
> I applied the patch and rebuilt, and the redisplay glitches I observed
> are gone; thanks very much for the quick fix!  I consider this bug
> resolved, but it should probably be closed after the fix is committed to
> the trunk.  Thanks again.

Thanks for testing.  I committed the fix (revision 107465 on the
trunk) and closed the bug.