[Stephen.Berman@gmx.net: Re: redisplay]

[Stephen.Berman@gmx.net: Re: redisplay]

[Richard Stallman]

Would someone please debug this, then ack?

------- Start of forwarded message -------
X-Spam-Status: No, score=-0.0 required=5.0 tests=SPF_HELO_PASS,SPF_PASS,
	UNPARSEABLE_RELAY autolearn=failed version=3.1.0
To: emacs-pretest-bug@gnu.org
From: Stephen Berman <Stephen.Berman@gmx.net>
Date: Sun, 22 Apr 2007 14:17:14 +0200
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Subject: Re: redisplay

On Sat, 21 Apr 2007 20:47:19 -0400 Richard Stallman <rms@gnu.org> wrote:

>     (progn (new-frame)
> 	   (redisplay t)
> 	   (x-popup-dialog t '("Foo" ("Bar" . 1))))
>
> I tried this but it didn't fail -- the new frame was not empty.

In my Emacs (GNU Emacs 22.0.98.2 (i686-pc-linux-gnu, GTK+ Version
2.10.6) of 2007-04-20 on escher) I see two behaviors:

1. Evalling the above sexp in *scratch* with C-j or in any buffer with
M-: results in what the OP reported: "produces a blank frame while the
popup dialog is active, i.e., (redisplay t) fails to wait for
redisplay to finish."

2. Evalling the sexp with C-x C-e in any buffer results in what RMS
reported. 

Steve Berman



_______________________________________________
emacs-pretest-bug mailing list
emacs-pretest-bug@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
------- End of forwarded message -------

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

> Would someone please debug this, then ack?
>
> From: Stephen Berman <Stephen.Berman@gmx.net>
> Subject: Re: redisplay
> To: emacs-pretest-bug@gnu.org
>
> On Sat, 21 Apr 2007 20:47:19 -0400 Richard Stallman <rms@gnu.org> wrote:
>
>>     (progn (new-frame)
>> 	   (redisplay t)
>> 	   (x-popup-dialog t '("Foo" ("Bar" . 1))))
>>
>> I tried this but it didn't fail -- the new frame was not empty.
>
> In my Emacs (GNU Emacs 22.0.98.2 (i686-pc-linux-gnu, GTK+ Version
> 2.10.6) of 2007-04-20 on escher) I see two behaviors:
>
> 1. Evalling the above sexp in *scratch* with C-j or in any buffer with
> M-: results in what the OP reported: "produces a blank frame while the
> popup dialog is active, i.e., (redisplay t) fails to wait for
> redisplay to finish."
>
> 2. Evalling the sexp with C-x C-e in any buffer results in what RMS
> reported. 

I believe this is because x-popup-dialog is handled with a GTK dialog
when you build under GTK, instead of using the Emacs redisplay engine.
So the behaviors are slightly different.  I don't think this
"problem"---if it's really a problem---is serious, and it certainly
shouldn't block the Emacs 22 release.

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Jan Djärv]

Chong Yidong skrev:
> Richard Stallman <rms@gnu.org> writes:
> 
>> Would someone please debug this, then ack?
>>
>> From: Stephen Berman <Stephen.Berman@gmx.net>
>> Subject: Re: redisplay
>> To: emacs-pretest-bug@gnu.org
>>
>> On Sat, 21 Apr 2007 20:47:19 -0400 Richard Stallman <rms@gnu.org> wrote:
>>
>>>     (progn (new-frame)
>>> 	   (redisplay t)
>>> 	   (x-popup-dialog t '("Foo" ("Bar" . 1))))
>>>
>>> I tried this but it didn't fail -- the new frame was not empty.
>> In my Emacs (GNU Emacs 22.0.98.2 (i686-pc-linux-gnu, GTK+ Version
>> 2.10.6) of 2007-04-20 on escher) I see two behaviors:
>>
>> 1. Evalling the above sexp in *scratch* with C-j or in any buffer with
>> M-: results in what the OP reported: "produces a blank frame while the
>> popup dialog is active, i.e., (redisplay t) fails to wait for
>> redisplay to finish."
>>
>> 2. Evalling the sexp with C-x C-e in any buffer results in what RMS
>> reported. 
> 
> I believe this is because x-popup-dialog is handled with a GTK dialog
> when you build under GTK, instead of using the Emacs redisplay engine.

The redisplay engine don't handle any dialogs, Gtk+, Motif or otherwise.

> So the behaviors are slightly different.  I don't think this
> "problem"---if it's really a problem---is serious, and it certainly
> shouldn't block the Emacs 22 release.

I can't reproduce it with or without Gtk+, *scratch* or not.  I suspect it is 
a timing issue in X.  I don't think redisplay guarantees that all redisplay 
related events has been received and handeled by Emacs before it returns.  My 
guess is that it just makes the needed X calls and then returns.  For some 
reason or another, the dialog blocks the handling of other X events.  Can the 
OP try to move the dialog around a bit and see if the frame below it is 
redrawn at all?

	Jan D.

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Stephen Berman]

On Mon, 23 Apr 2007 21:14:35 +0200 Jan Djärv <jan.h.d@swipnet.se> wrote:

> I can't reproduce it with or without Gtk+, *scratch* or not.  I
> suspect it is a timing issue in X.  I don't think redisplay guarantees
> that all redisplay related events has been received and handeled by
> Emacs before it returns.  My guess is that it just makes the needed X
> calls and then returns.  For some reason or another, the dialog blocks
> the handling of other X events.  Can the OP try to move the dialog
> around a bit and see if the frame below it is redrawn at all?

I'm not the OP but I followed up with the observation of different
behaviors depending on how the sexp is evalled.  When I first tested,
I got consistent results, but after reading your post I tried it
again, both with -Q and with my initializations, and now found the
results to be inconsistent, i.e., sometimes I got a fully redrawn
frame and sometimes a blank frame (though when it was blank, moving
the dialog frame did not cause redrawing of the blank frame),
regardless of how I evalled the sexp.  I guess this supports your
suspicion about timing.

Steve Berman

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[YAMAMOTO Mitsuharu]

>>>>> On Tue, 24 Apr 2007 00:12:04 +0200, Stephen Berman <Stephen.Berman@gmx.net> said:

> When I first tested, I got consistent results, but after reading
> your post I tried it again, both with -Q and with my
> initializations, and now found the results to be inconsistent, i.e.,
> sometimes I got a fully redrawn frame and sometimes a blank frame
> (though when it was blank, moving the dialog frame did not cause
> redrawing of the blank frame), regardless of how I evalled the sexp.

Recently I observed a possibly related issue while experimenting with
"compositing windows" on Mac Carbon port.

Compositing window mode means all the drawing operations must be done
in response to "expose" (kEventControlDraw) events, and other drawing
operations are converted to making dirty the drawn area instead of
real drawings.

To make things work, expose_frame must correctly draw the contents
most of the time.  But with the experiment of compositing windows, I
observe that is not always the case even if !FRAME_GARBARGED_P(f) &&
updated_window != NULL.  It seems to fail to do so after the current
matrix is updated with try_window_reusing_current_matrix under a
certain condition, but I haven't figured it out yet.

I guess the problem with redrawing frames under dialog are related
with the above issue and only GTK+-build users notice this because it
clears the exposed area before calling expose_frame (compositing
window on Mac also does so automatically).

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

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[YAMAMOTO Mitsuharu]

>>>>> On Tue, 24 Apr 2007 07:50:17 +0900 (JST), YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> said:

> To make things work, expose_frame must correctly draw the contents
> most of the time.  But with the experiment of compositing windows, I
> observe that is not always the case even if !FRAME_GARBARGED_P(f) &&
> updated_window != NULL.

Sorry, I meant "if !FRAME_GARBAGED_P (f) && updated_window == NULL".

> I guess the problem with redrawing frames under dialog are related
> with the above issue and only GTK+-build users notice this because
> it clears the exposed area before calling expose_frame (compositing
> window on Mac also does so automatically).

Stephen, could try the following patch?  I don't think this is a right
fix, but at least we can check if we are sharing the same issue.

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

Index: src/xdisp.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/xdisp.c,v
retrieving revision 1.1148
diff -c -p -r1.1148 xdisp.c
*** src/xdisp.c	16 Apr 2007 15:12:24 -0000	1.1148
--- src/xdisp.c	24 Apr 2007 00:54:59 -0000
*************** expose_window (w, fr)
*** 23531,23542 ****
        /* Update lines intersecting rectangle R.  */
        first_overlapping_row = last_overlapping_row = NULL;
        for (row = w->current_matrix->rows;
! 	   row->enabled_p;
  	   ++row)
  	{
  	  int y0 = row->y;
  	  int y1 = MATRIX_ROW_BOTTOM_Y (row);
  
  	  if ((y0 >= r.y && y0 < r.y + r.height)
  	      || (y1 > r.y && y1 < r.y + r.height)
  	      || (r.y >= y0 && r.y < y1)
--- 23531,23545 ----
        /* Update lines intersecting rectangle R.  */
        first_overlapping_row = last_overlapping_row = NULL;
        for (row = w->current_matrix->rows;
! 	   row < w->current_matrix->rows + w->current_matrix->nrows;
  	   ++row)
  	{
  	  int y0 = row->y;
  	  int y1 = MATRIX_ROW_BOTTOM_Y (row);
  
+ 	  if (!row->enabled_p)
+ 	    continue;
+ 
  	  if ((y0 >= r.y && y0 < r.y + r.height)
  	      || (y1 > r.y && y1 < r.y + r.height)
  	      || (r.y >= y0 && r.y < y1)

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Stefan Monnier]

> I can't reproduce it with or without Gtk+, *scratch* or not.  I suspect it
> is a timing issue in X.  I don't think redisplay guarantees that all
> redisplay related events has been received and handeled by Emacs before it
> returns.

Even if it does an XSync?

In any case, I have regularly seen similar problems when leaving Emacs: if
I have processes running, Emacs displays a *processes* list, and because of my
pop-up-frames (and/or special-display-regexps) settings, it does it by first
popping up a new frame displaying this *processes* buffer, and then pops up
a dialog asking me whether I really want to quit.
The problem is that the *processes* frame is generally blank at that point.


        Stefan

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Jan Djärv]

Stefan Monnier skrev:
>> I can't reproduce it with or without Gtk+, *scratch* or not.  I suspect it
>> is a timing issue in X.  I don't think redisplay guarantees that all
>> redisplay related events has been received and handeled by Emacs before it
>> returns.
> 
> Even if it does an XSync?

XSync only guarantees that events have been sent to the server and replies 
have been received into Xlib.  There might be stuff Emacs have left to do 
still.  But I don't know redisplay that well to be sure.  XSync is our bet bet 
though.

> 
> In any case, I have regularly seen similar problems when leaving Emacs: if
> I have processes running, Emacs displays a *processes* list, and because of my
> pop-up-frames (and/or special-display-regexps) settings, it does it by first
> popping up a new frame displaying this *processes* buffer, and then pops up
> a dialog asking me whether I really want to quit.
> The problem is that the *processes* frame is generally blank at that point.

Hmm, I'll try to reproduce this some more.

	Jan D.

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Glenn Morris]

Stefan Monnier wrote:

> In any case, I have regularly seen similar problems when leaving
> Emacs: if I have processes running, Emacs displays a *processes*
> list, and because of my pop-up-frames (and/or
> special-display-regexps) settings, it does it by first popping up a
> new frame displaying this *processes* buffer, and then pops up a
> dialog asking me whether I really want to quit. The problem is that
> the *processes* frame is generally blank at that point.

I've seen something which sounds like this as well. I see it
reproducibly with:

emacs -Q
(setq special-display-regexps '(".*-diff\\*"))
(add-to-list 'minor-mode-map-alist
               `(vc-mode keymap
                         (menu-bar keymap
                                   (VC menu-item "VC" ,vc-menu-map))))

Then I visit a file under version control, make some changes, then
select VC->Revert to Base Version from the menu-bar. A frame with the
diffs gets popped up, as well as a dialogue box saying "Discard
Changes? Yes No". The diff frame is empty of mode-line and buffer
contents.

This is with:

x86_64-unknown-linux-gnu, X toolkit
Windowing system distributor `The X.Org Foundation', version 11.0.60802000
--with-x-toolkit=athena --without-toolkit-scroll-bars

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Jan Djärv]

Glenn Morris skrev:
> Stefan Monnier wrote:
> 
>> In any case, I have regularly seen similar problems when leaving
>> Emacs: if I have processes running, Emacs displays a *processes*
>> list, and because of my pop-up-frames (and/or
>> special-display-regexps) settings, it does it by first popping up a
>> new frame displaying this *processes* buffer, and then pops up a
>> dialog asking me whether I really want to quit. The problem is that
>> the *processes* frame is generally blank at that point.
> 
> I've seen something which sounds like this as well. I see it
> reproducibly with:
> 
> emacs -Q
> (setq special-display-regexps '(".*-diff\\*"))
> (add-to-list 'minor-mode-map-alist
>                `(vc-mode keymap
>                          (menu-bar keymap
>                                    (VC menu-item "VC" ,vc-menu-map))))
> 
> Then I visit a file under version control, make some changes, then
> select VC->Revert to Base Version from the menu-bar. A frame with the
> diffs gets popped up, as well as a dialogue box saying "Discard
> Changes? Yes No". The diff frame is empty of mode-line and buffer
> contents.
> 

Thanks, now I see it.  There is a race in here.  I can see that redisplay on 
the new frame have not happened before the dialog pops up.  When the dialog is 
up, redisplay is inhibited in xdisp.c, redisplay_internal.

Sometimes the frame will get redisplayed properly, only to be cleared almost 
at once by an Expose event.  This expose event happens in the middle of an 
ongoing redisplay, so a new redisplay is not started.  Directly when the 
ongoing redisplay has finished, the dialog is popped up, and no redisplay is made.

There are several possible solutions to this.

1.  SYNC_INPUT.  That way we won't get expose events in the middle of an 
ongoing redisplay.
2.  Restart redisplay if an redisplay-related event arrives in the middle of 
an ongoing redisplay.
3.  Use double buffering.  That would get rid of the clearing of the frame.

Unfortunately I don't see a quick fix for 22.1.

	Jan D.

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Stefan Monnier]

> 1.  SYNC_INPUT.  That way we won't get expose events in the middle of an
> ongoing redisplay.

Hmm... I've been running with SYNC_INPUT for a long time now, and I'm pretty
sure that I still see those problems.  So maybe SYNC_INPUT would allow to
write a solution, but it's not a solution in itself.


        Stefan

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Jan Djärv]

Stefan Monnier skrev:
>> 1.  SYNC_INPUT.  That way we won't get expose events in the middle of an
>> ongoing redisplay.
> 
> Hmm... I've been running with SYNC_INPUT for a long time now, and I'm pretty
> sure that I still see those problems.  So maybe SYNC_INPUT would allow to
> write a solution, but it's not a solution in itself.

Yes, you are right.  It would also require to run redisplay directly on the 
Expose event.

	Jan D.

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Jan Djärv]

Jan Djärv skrev:
> 
> 
> Stefan Monnier skrev:
>>> 1.  SYNC_INPUT.  That way we won't get expose events in the middle of an
>>> ongoing redisplay.
>>
>> Hmm... I've been running with SYNC_INPUT for a long time now, and I'm 
>> pretty
>> sure that I still see those problems.  So maybe SYNC_INPUT would allow to
>> write a solution, but it's not a solution in itself.
> 
> Yes, you are right.  It would also require to run redisplay directly on 
> the Expose event.
>

After some experimenting it seems that this patch fixes it.  Note that a call 
to redisplay() does not fix the bug.  I don't know why.  But a full redisplay 
before every dialog maybe is too much?

diff -w -c src/xmenu.c.~1.317.~ src/xmenu.c
Index: src/xmenu.c
*** src/xmenu.c.~1.317.~        2007-01-21 11:33:57.000000000 +0100
--- src/xmenu.c 2007-04-25 20:27:02.000000000 +0200
***************
*** 3333,3338 ****
--- 3333,3340 ----
     /* No selection has been chosen yet.  */
     menu_item_selection = 0;

+   Fredisplay (Qt);
+
     /* Actually create and show the dialog.  */
     create_and_show_dialog (f, first_wv);


	Jan D.

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Stefan Monnier]

> After some experimenting it seems that this patch fixes it.  Note that
> a call to redisplay() does not fix the bug.  I don't know why.  But a full
> redisplay before every dialog maybe is too much?

Am I wrong in thinking that Fredisplay(Qt) does not redraw every single
frame, but just does a normal redisplay (i.e. update the out-of-date
elements of the display), with the exception that it will complete the
redisplay even if there's input pending?
I.e. if the display is already up-to-date, Fredisplay(Qt) is a no-op?
If so, and it works, then your patch looks fine to me.


        Stefan


> diff -w -c src/xmenu.c.~1.317.~ src/xmenu.c
> Index: src/xmenu.c
> *** src/xmenu.c.~1.317.~        2007-01-21 11:33:57.000000000 +0100
> --- src/xmenu.c 2007-04-25 20:27:02.000000000 +0200
> ***************
> *** 3333,3338 ****
> --- 3333,3340 ----
>     /* No selection has been chosen yet.  */
>     menu_item_selection = 0;

> +   Fredisplay (Qt);
> +
>     /* Actually create and show the dialog.  */
>     create_and_show_dialog (f, first_wv);


> 	Jan D.



> _______________________________________________
> Emacs-devel mailing list
> Emacs-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[YAMAMOTO Mitsuharu]

>>>>> On Wed, 25 Apr 2007 20:30:08 +0200, Jan Djärv <jan.h.d@swipnet.se> said:

>> Yes, you are right.  It would also require to run redisplay
>> directly on the Expose event.

But Lisp evaluations might occur during redisplay (unless
inhibit-eval-during-redisplay is non-nil), and thus it's not safe to
do so in general.

> After some experimenting it seems that this patch fixes it.  Note
> that a call to redisplay() does not fix the bug.  I don't know why.

I think that's because redisplay is paused by a pending Expose event
if redisplay-dont-pause is nil (`Fredisplay (Qt)' temporarily binds it
to t).  I observed similar undrawn area with "compositing window mode"
mentioned in *1 when redisplay-dont-pause is nil and without the patch
in *2.

*1: http://lists.gnu.org/archive/html/emacs-devel/2007-04/msg01003.html
*2: http://lists.gnu.org/archive/html/emacs-devel/2007-04/msg01013.html

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

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Glenn Morris]

Jan Djärv wrote:

> After some experimenting it seems that this patch fixes it. Note
> that a call to redisplay() does not fix the bug. I don't know why.
> But a full redisplay before every dialog maybe is too much?

This seems to fix it for me [1]; but I was never particularly bothered
by this issue (which I've been seeing for some time), so personally
I'm happy for Emacs 22 to be released with this still extant, to give
you more time to test a solution.

[1] One time out of many tests I thought I saw the problem again, but
I could never repeat it and may well have been mistaken.

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Jan Djärv]

YAMAMOTO Mitsuharu skrev:
>>>>>> On Wed, 25 Apr 2007 20:30:08 +0200, Jan Djärv <jan.h.d@swipnet.se> said:
> 
>>> Yes, you are right.  It would also require to run redisplay
>>> directly on the Expose event.
> 
> But Lisp evaluations might occur during redisplay (unless
> inhibit-eval-during-redisplay is non-nil), and thus it's not safe to
> do so in general.

When SYNC_INPUT is on we are not handling X events in the signal handler 
anymore.  Isn't Lisp evaluations safe then?  Why not?  Reentrance-problems?

	Jan D.

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[YAMAMOTO Mitsuharu]

>>>>> On Thu, 26 Apr 2007 07:58:26 +0200, Jan Djärv <jan.h.d@swipnet.se> said:

>>>> Yes, you are right.  It would also require to run redisplay
>>>> directly on the Expose event.
>> 
>> But Lisp evaluations might occur during redisplay (unless
>> inhibit-eval-during-redisplay is non-nil), and thus it's not safe
>> to do so in general.

> When SYNC_INPUT is on we are not handling X events in the signal
> handler anymore.  Isn't Lisp evaluations safe then?  Why not?
> Reentrance-problems?

I was thinking about SYNC_INPUT and redisplay triggered by an Expose
event separately.  Actually, as I said in
http://lists.gnu.org/archive/html/emacs-devel/2005-04/msg00365.html,
I'd really appreciate it if evaluations during read_socket_hook become
possible.

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

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Stephen Berman]

On Tue, 24 Apr 2007 10:10:09 +0900 YAMAMOTO Mitsuharu
<mituharu@math.s.chiba-u.ac.jp> wrote: 

> Stephen, could try the following patch?  I don't think this is a right
> fix, but at least we can check if we are sharing the same issue.

I applied your patch and rebuilt Emacs, and I still see the blank
frame, though, as I mentioned in my previous followup, I don't see the
effect as consistently as I had thought and reported in my original
followup.  Now I'll try the patch that Jan Djärv posted.

Steve Berman

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Stephen Berman]

On Wed, 25 Apr 2007 20:30:08 +0200 Jan Djärv <jan.h.d@swipnet.se> wrote:

> After some experimenting it seems that this patch fixes it.  Note that a call 
> to redisplay() does not fix the bug.  I don't know why.  But a full redisplay 
> before every dialog maybe is too much?

I rebuilt with this patch and haven't seen the blank frame problem
yet, so it looks like it fixes the problem; thanks.  (Ediff failed to
apply the patch for some reason, so I just inserted the line by hand.)

Steve Berman

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Jan Djärv]

Stephen Berman skrev:
> On Wed, 25 Apr 2007 20:30:08 +0200 Jan Djärv <jan.h.d@swipnet.se> wrote:
> 
>> After some experimenting it seems that this patch fixes it.  Note that a call 
>> to redisplay() does not fix the bug.  I don't know why.  But a full redisplay 
>> before every dialog maybe is too much?
> 
> I rebuilt with this patch and haven't seen the blank frame problem
> yet, so it looks like it fixes the problem; thanks.  (Ediff failed to
> apply the patch for some reason, so I just inserted the line by hand.)
> 

Thunderbird sometimes makes a mess of whitespace in cut and pasted text.

	Jan D.

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[YAMAMOTO Mitsuharu]

>>>>> On Thu, 26 Apr 2007 22:58:30 +0200, Stephen Berman <Stephen.Berman@gmx.net> said:

>> Stephen, could try the following patch?  I don't think this is a
>> right fix, but at least we can check if we are sharing the same
>> issue.

> I applied your patch and rebuilt Emacs, and I still see the blank
> frame, though, as I mentioned in my previous followup, I don't see
> the effect as consistently as I had thought and reported in my
> original followup.

Thanks for testing.

>>>>> On Tue, 24 Apr 2007 07:50:17 +0900 (JST), YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> said:

> To make things work, expose_frame must correctly draw the contents
> most of the time.  But with the experiment of compositing windows, I
> observe that is not always the case even if !FRAME_GARBARGED_P(f) &&
> updated_window != NULL.  It seems to fail to do so after the current
> matrix is updated with try_window_reusing_current_matrix under a
> certain condition, but I haven't figured it out yet.

Though my issue above has turned out to be unrelated to the dialog
one, I think I figured out the problematic case for my issue.

In try_window_reusing_current_matrix (the scroll-down case), `it.vpos'
rows in the current matrix are disabled.  But the scroll-down height
in rows is stored in `nrows_scrolled' and they differ by 1 when the
header line is present.  As a result, the first row of the reused part
is erroneously disabled and the rows below it don't get redrawn by
Expose events.

The following patch seems to work for me.  Could someone check it and
comment on whether it should be installed before the release or not?

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


Index: src/xdisp.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/xdisp.c,v
retrieving revision 1.1149
diff -c -p -r1.1149 xdisp.c
*** src/xdisp.c	24 Apr 2007 01:32:41 -0000	1.1149
--- src/xdisp.c	27 Apr 2007 08:37:31 -0000
*************** try_window_reusing_current_matrix (w)
*** 13811,13817 ****
  			 nrows_scrolled);
  
  	  /* Disable lines that must be updated.  */
! 	  for (i = 0; i < it.vpos; ++i)
  	    (start_row + i)->enabled_p = 0;
  
  	  /* Re-compute Y positions.  */
--- 13811,13817 ----
  			 nrows_scrolled);
  
  	  /* Disable lines that must be updated.  */
! 	  for (i = 0; i < nrows_scrolled; ++i)
  	    (start_row + i)->enabled_p = 0;
  
  	  /* Re-compute Y positions.  */

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Richard Stallman]

Your fix is almost certainly an improvement.  Unless someone sees
a gross flaw in it, please install it tomorrow.

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[YAMAMOTO Mitsuharu]

>>>>> On Sat, 28 Apr 2007 00:06:50 -0400, Richard Stallman <rms@gnu.org> said:

> Your fix is almost certainly an improvement.  Unless someone sees a
> gross flaw in it, please install it tomorrow.

Done.

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

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[David Kastrup]

Richard Stallman <rms@gnu.org> writes:

> Your fix is almost certainly an improvement.  Unless someone sees
> a gross flaw in it, please install it tomorrow.

I see a gross flaw in installing "almost certain improvements" in a
central area of Emacs when we are trying to pin down a final release.

I have a really hard time believing this.  The way it looks, the only
chance we have for releasing 22.1 is to have yourself cut off from
Email.

I am unsubscribing from the list until the release is over.  I can't
stand this any longer.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Nick Roberts]

 > > Your fix is almost certainly an improvement.  Unless someone sees
 > > a gross flaw in it, please install it tomorrow.
 > 
 > I see a gross flaw in installing "almost certain improvements" in a
 > central area of Emacs when we are trying to pin down a final release.

I guess freedom doesn't mean being able to make someone do what they don't want
to.  It appears to me that the release process, having gone on for three years
now, is a more of a means to get bugs fixed than aimed at a release in itself.
But while Richard can control the release, Emacs is free software, so he can't
control the distribution and, indeed, some distros are starting to include
snapshots of Emacs 22.

The cat is, or soon will be, out of the bag.

 >...
 > I am unsubscribing from the list until the release is over.  I can't
 > stand this any longer.

Hello?  Are you still there?

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Richard Stallman]

Is this item fixed now?

** rgm@gnu.org, Apr 24: [Stephen.Berman@gmx.net: Re: redisplay]
  Gives test case for problem that a dialog box blocks redisplay.
http://lists.gnu.org/archive/html/emacs-devel/2007-04/msg00951.html

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Richard Stallman]

    I see a gross flaw in installing "almost certain improvements" in a
    central area of Emacs when we are trying to pin down a final release.

The improvement in question consists of at least partly fixing a bug.

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[David Kastrup]

Richard Stallman <rms@gnu.org> writes:

>     I see a gross flaw in installing "almost certain improvements"
>     in a central area of Emacs when we are trying to pin down a
>     final release.
>
> The improvement in question consists of at least partly fixing a bug.

You don't understand what prerelease testing is about.  The goal is
not to get a bugfree version of Emacs released.  Whoever thinks this
possible is lunatic.  The goal is to release a version of Emacs which
has not introduced severe regressions in a time frame where they could
have escaped noticing.

Every change, including bug fixes, in particular in general areas,
carries with it the danger of introducing _exactly_ those kind of
heavy regressions.  The ChangeLog files are _full_ of reverted fixes
which turned out to do something wrong.

If we go on fixing bugs that are not a direct consequence of very
recent changes, we will never get to the state where we can be sure
not to have release-critical bugs introduced without notice.

The bugs you found necessary fixing in the last few weeks are _much_
_much_ more harmless than having to use Emacs 21 instead of Emacs 22.

Emacs 22 will _never_ be in a bugfree state before the release.  Nor
will it become bugfree afterwards.  But at least afterwards we can
make overall progress.

In contrast to the problem of the delayed release, you are playing
with toy problems.  Problems that will pop up for years and years to
come.

It is not sane to delay the release because of that: a release is
appropriate when there is a definite improvement over the last release
without major regressions.  This has been the case for years,
actually.  There is no sense in instead of improvement demanding
perfection.  People experienced with software releases have told you
this time and again.  You chose to scorn them for their frustration
about you ignoring established procedures and conventional wisdom.

Do you really not see what you are doing?  For how long are you going
to sabotage the attempts of the developers to get Emacs 22.1 released?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[YAMAMOTO Mitsuharu]

>>>>> On Mon, 30 Apr 2007 00:14:12 +0200, David Kastrup <dak@gnu.org> said:

> Every change, including bug fixes, in particular in general areas,
> carries with it the danger of introducing _exactly_ those kind of
> heavy regressions.  The ChangeLog files are _full_ of reverted fixes
> which turned out to do something wrong.

If you wanted to say so, you shouldn't have brought it up against my
patch.  Its validity was too obvious to apply your principle above.

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

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[YAMAMOTO Mitsuharu]

>>>>> On Sun, 29 Apr 2007 17:41:07 -0400, Richard Stallman <rms@gnu.org> said:

> Is this item fixed now?
> ** rgm@gnu.org, Apr 24: [Stephen.Berman@gmx.net: Re: redisplay]
>   Gives test case for problem that a dialog box blocks redisplay.
> http://lists.gnu.org/archive/html/emacs-devel/2007-04/msg00951.html

According to the reports below, Jan's patch, which is already
installed to both trunk and branch, gave good results.

http://lists.gnu.org/archive/html/emacs-devel/2007-04/msg01264.html
http://lists.gnu.org/archive/html/emacs-devel/2007-04/msg01327.html

I've just made the corresponding changes to macmenu.c.

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

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Richard Stallman]

I deleted that item.  Thanks.

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[YAMAMOTO Mitsuharu]

[Revisiting an old change]

>>>>> On Wed, 25 Apr 2007 20:30:08 +0200, Jan Djärv <jan.h.d@swipnet.se> said:

> After some experimenting it seems that this patch fixes it.  Note
> that a call to redisplay() does not fix the bug.  I don't know why.
> But a full redisplay before every dialog maybe is too much?

> diff -w -c src/xmenu.c.~1.317.~ src/xmenu.c
> Index: src/xmenu.c
> *** src/xmenu.c.~1.317.~        2007-01-21 11:33:57.000000000 +0100
> --- src/xmenu.c 2007-04-25 20:27:02.000000000 +0200
> ***************
> *** 3333,3338 ****
> --- 3333,3340 ----
>      /* No selection has been chosen yet.  */
>      menu_item_selection = 0;

> +   Fredisplay (Qt);
> +
>      /* Actually create and show the dialog.  */
>      create_and_show_dialog (f, first_wv);

Fredisplay runs timers and may cause string data relocation by GC.
Just before the call to Fredisplay, a widget value is created and it
points to some string data, which are to be used as button labels
after the Fredisplay call.  So this may cause corruption of dialog
button labels.  I've never seen such corruption actually, but I think
that may happen in principle unless I'm missing something.

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

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[David Reitter]

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

On Mar 15, 2009, at 9:37 PM, YAMAMOTO Mitsuharu wrote:

> [Revisiting an old change]
>
>>>>>> On Wed, 25 Apr 2007 20:30:08 +0200, Jan Djärv  
>>>>>> <jan.h.d@swipnet.se> said:
>
>> After some experimenting it seems that this patch fixes it.  Note
>> that a call to redisplay() does not fix the bug.  I don't know why.
>> But a full redisplay before every dialog maybe is too much?
>
>> diff -w -c src/xmenu.c.~1.317.~ src/xmenu.c
>> Index: src/xmenu.c
>> *** src/xmenu.c.~1.317.~        2007-01-21 11:33:57.000000000 +0100
>> --- src/xmenu.c 2007-04-25 20:27:02.000000000 +0200
>> ***************
>> *** 3333,3338 ****
>> --- 3333,3340 ----
>>     /* No selection has been chosen yet.  */
>>     menu_item_selection = 0;
>
>> +   Fredisplay (Qt);
>> +
>>     /* Actually create and show the dialog.  */
>>     create_and_show_dialog (f, first_wv);
>
> Fredisplay runs timers and may cause string data relocation by GC.
> Just before the call to Fredisplay, a widget value is created and it
> points to some string data, which are to be used as button labels
> after the Fredisplay call.  So this may cause corruption of dialog
> button labels.  I've never seen such corruption actually, but I think
> that may happen in principle unless I'm missing something.


I absolutely see such corruption in the Carbon port, which creates the  
dialog in a similar fashion.  This was in connection with a header  
line that uses an :eval expression, and a modified map-y-or-n-p that  
switches buffers to show the right one.  As you say, the strings get  
destroyed, but that was relatively easy to fix by doing the redisplay  
a bit earlier.



*** src/macmenu.c	30 Aug 2008 00:10:18 -0400	1.56.2.7
--- src/macmenu.c	09 Feb 2009 13:21:20 -0500	
***************
*** 1882,1888 ****
         return Qnil;
       }

!   /* Create a tree of widget_value objects
        representing the text label and buttons.  */
     {
       Lisp_Object pane_name, prefix;
--- 1882,1893 ----
         return Qnil;
       }

!   /* Force a redisplay before showing the dialog.  If a frame is  
created
!      just before showing the dialog, its contents may not have been  
fully
!      drawn.  */
!   Fredisplay (Qt);
!
!  /* Create a tree of widget_value objects
        representing the text label and buttons.  */
     {
       Lisp_Object pane_name, prefix;
***************
*** 1985,1994 ****
     /* No selection has been chosen yet.  */
     menu_item_selection = 0;

-   /* Force a redisplay before showing the dialog.  If a frame is  
created
-      just before showing the dialog, its contents may not have been  
fully
-      drawn.  */
-   Fredisplay (Qt);

     /* Actually create the dialog.  */
   #if TARGET_API_MAC_CARBON
--- 1990,1995 ----


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Chong Yidong]

David Reitter <david.reitter@gmail.com> writes:

> I absolutely see such corruption in the Carbon port, which creates the
> dialog in a similar fashion.  This was in connection with a header
> line that uses an :eval expression, and a modified map-y-or-n-p that
> switches buffers to show the right one.  As you say, the strings get
> destroyed, but that was relatively easy to fix by doing the redisplay
> a bit earlier.

This does not work properly on X.  With that change, there are frame
exposure problems using the recipe from

http://lists.gnu.org/archive/html/emacs-devel/2007-04/msg01201.html

(i.e., when the dialog box is moved, the underlying frame is not drawn
properly.)  However, I have not had time to debug further.

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[YAMAMOTO Mitsuharu]

>>>>> On Mon, 16 Mar 2009 11:18:44 -0400, Chong Yidong <cyd@stupidchicken.com> said:

>> I absolutely see such corruption in the Carbon port, which creates
>> the dialog in a similar fashion.  This was in connection with a
>> header line that uses an :eval expression, and a modified
>> map-y-or-n-p that switches buffers to show the right one.  As you
>> say, the strings get destroyed, but that was relatively easy to fix
>> by doing the redisplay a bit earlier.

Ah, I didn't have to say about timers because redisplay can do Lisp
evaluations in the first place.  Actually I found this potentially
problematic case when I was reading the code related to timers.

> This does not work properly on X.  With that change, there are frame
> exposure problems using the recipe from

> http://lists.gnu.org/archive/html/emacs-devel/2007-04/msg01201.html

> (i.e., when the dialog box is moved, the underlying frame is not
> drawn properly.)  However, I have not had time to debug further.

Just to be sure, that doesn't happen if Fredisplay is called from the
original position?  Actually I also thought that it was sufficient to
move the Fredisplay call a bit earlier.

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

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Chong Yidong]

YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:

>> This does not work properly on X.  With that change, there are frame
>> exposure problems using the recipe from
>
>> http://lists.gnu.org/archive/html/emacs-devel/2007-04/msg01201.html
>
>> (i.e., when the dialog box is moved, the underlying frame is not
>> drawn properly.)  However, I have not had time to debug further.
>
> Just to be sure, that doesn't happen if Fredisplay is called from the
> original position?

Correct.  The current version works properly; moving the Fredisplay call
earlier leads to uncorrect display.

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[YAMAMOTO Mitsuharu]

>>>>> On Mon, 16 Mar 2009 23:59:40 -0400, Chong Yidong <cyd@stupidchicken.com> said:

>>> This does not work properly on X.  With that change, there are
>>> frame exposure problems using the recipe from
>> 
>>> http://lists.gnu.org/archive/html/emacs-devel/2007-04/msg01201.html
>> 
>>> (i.e., when the dialog box is moved, the underlying frame is not
>>> drawn properly.)  However, I have not had time to debug further.
>> 
>> Just to be sure, that doesn't happen if Fredisplay is called from
>> the original position?

> Correct.  The current version works properly; moving the Fredisplay
> call earlier leads to uncorrect display.

Perhaps `menu_items' or something like that is corrupted by the
Fredisplay call?  Could you try the following patch?

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

Index: src/xmenu.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/xmenu.c,v
retrieving revision 1.339
diff -c -p -r1.339 xmenu.c
*** src/xmenu.c	8 Jan 2009 03:16:12 -0000	1.339
--- src/xmenu.c	17 Mar 2009 04:10:10 -0000
*************** for instance using the window manager, t
*** 604,609 ****
--- 604,617 ----
    if (! FRAME_X_P (f) && ! FRAME_MSDOS_P (f))
      error ("Can not put X dialog on this terminal");
  
+   /* Force a redisplay before showing the dialog.  If a frame is created
+      just before showing the dialog, its contents may not have been fully
+      drawn, as this depends on timing of events from the X server.  Redisplay
+      is not done when a dialog is shown.  If redisplay could be done in the
+      X event loop (i.e. the X event loop does not run in a signal handler)
+      this would not be needed.  */
+   Fredisplay (Qt);
+ 
  #if ! defined (USE_X_TOOLKIT) && ! defined (USE_GTK)
    /* Display a menu with these alternatives
       in the middle of frame F.  */
*************** xdialog_show (f, keymaps, title, header,
*** 2448,2461 ****
    /* No selection has been chosen yet.  */
    menu_item_selection = 0;
  
-   /* Force a redisplay before showing the dialog.  If a frame is created
-      just before showing the dialog, its contents may not have been fully
-      drawn, as this depends on timing of events from the X server.  Redisplay
-      is not done when a dialog is shown.  If redisplay could be done in the
-      X event loop (i.e. the X event loop does not run in a signal handler)
-      this would not be needed.  */
-   Fredisplay (Qt);
- 
    /* Actually create and show the dialog.  */
    create_and_show_dialog (f, first_wv);
  
--- 2456,2461 ----

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[YAMAMOTO Mitsuharu]

>>>>> On Tue, 17 Mar 2009 13:13:43 +0900, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> said:

>>>> This does not work properly on X.  With that change, there are
>>>> frame exposure problems using the recipe from
>>> 
>>>> http://lists.gnu.org/archive/html/emacs-devel/2007-04/msg01201.html
>>> 
>>>> (i.e., when the dialog box is moved, the underlying frame is not
>>>> drawn properly.)  However, I have not had time to debug further.
>>> 
>>> Just to be sure, that doesn't happen if Fredisplay is called from
>>> the original position?

>> Correct.  The current version works properly; moving the Fredisplay
>> call earlier leads to uncorrect display.

> Perhaps `menu_items' or something like that is corrupted by the
> Fredisplay call?  Could you try the following patch?

I tried it myself with Ubuntu 8.10, GTK+ (as I could not reproduce the
redraw-failure with X11 on Mac OS X, Xaw).  It sometimes failed in
redrawing with each of three Fredisplay call positions, including the
original one.  So I think this is a separate issue.

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

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[Chong Yidong]

YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:

> I tried it myself with Ubuntu 8.10, GTK+ (as I could not reproduce the
> redraw-failure with X11 on Mac OS X, Xaw).  It sometimes failed in
> redrawing with each of three Fredisplay call positions, including the
> original one.  So I think this is a separate issue.

The failure seems to happen more often with the earlier Fredisplay call
position.  This is not surprising: the Fredisplay call is an attempt to
update the display of the frames, which depends on X server timings that
Emacs cannot affect directly.  Since the goal is to perform the update
just before opening the dialog, calling Fredisplay earlier makes this
hack less reliable.

Your analysis, however, makes it clear that this hack is problematic.
However, I don't see any good solution right now.  So, please go ahead
and move the Fredisplay call to an earlier, safe position.

Re: [Stephen.Berman@gmx.net: Re: redisplay]

[YAMAMOTO Mitsuharu]

>>>>> On Tue, 17 Mar 2009 11:13:02 -0400, Chong Yidong <cyd@stupidchicken.com> said:

> The failure seems to happen more often with the earlier Fredisplay
> call position.  This is not surprising: the Fredisplay call is an
> attempt to update the display of the frames, which depends on X
> server timings that Emacs cannot affect directly.  Since the goal is
> to perform the update just before opening the dialog, calling
> Fredisplay earlier makes this hack less reliable.

> Your analysis, however, makes it clear that this hack is
> problematic.  However, I don't see any good solution right now.  So,
> please go ahead and move the Fredisplay call to an earlier, safe
> position.

Done.

Also, I traced both good and bad cases and observed the following:

Good case (exposed area is redrawn)

  * For the "*vc-diff*" frame, xg_frame_resized is called twice before
    the Fredisplay call.
  * The height of the "*vc-diff*" window becomes 12.

Bad case (exposed are is not redrawn)

  * For the "*vc-diff*" frame, xg_frame_resized is called once before
    and once after the Fredisplay call.  (The second one does
    SET_FRAME_GARBAGED and that prevents subsequent expose_frame calls
    from redrawing the contents until the next redisplay.)
  * The height of the "*vc-diff*" window becomes 10.

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