Old redisplay bug in xdisp.c, question about change

Old redisplay bug in xdisp.c, question about change

[Jan D.]

Hello.

I was looking for a redisplay bug I thought was in the GTK parts.  It turns
out that the bug is present in both the Motif and Lucid ports also.

To reproduce.
emacs -q --no-site-file
C-h i
Select a node with the mouse.

Sometimes the bug can be seen right here.  The tool bar arrows for up
and next are still disabled.  If they are disabled, do a Ctrl-l to fix.

Now press 'u' to go up.  The next and up tool bar arrows are now enabled,
they shall be disabled.  Again, Ctrl-l fixes it.

There seems to be a bit of race condition here, the bug does not always
show up.

I traced the origin to this change:

2002-03-26  Richard M. Stallman  <rms@gnu.org>

        * xdisp.c (update_menu_bar): Test only update_mode_lines;

The diff:

@@ -7435,8 +7438,14 @@
   window = FRAME_SELECTED_WINDOW (f);
   w = XWINDOW (window);
   
+#if 0 /* The if statement below this if statement used to include the
+         condition !NILP (w->update_mode_line), rather than using
+         update_mode_lines directly, and this if statement may have
+         been added to make that condition work.  Now the if
+         statement below matches its comment, this isn't needed.  */  
   if (update_mode_lines)
     w->update_mode_line = Qt;
+#endif
 
   if (FRAME_WINDOW_P (f)
       ?
@@ -7455,7 +7464,9 @@
         the rest of the redisplay algorithm is about the same as
         windows_or_buffers_changed anyway.  */
       if (windows_or_buffers_changed
-         || !NILP (w->update_mode_line)
+         /* This used to test w->update_mode_line, but we believe
+            there is no need to recompute the menu in that case.  */
+         || update_mode_lines
          || ((BUF_SAVE_MODIFF (XBUFFER (w->buffer))
               < BUF_MODIFF (XBUFFER (w->buffer)))
              != !NILP (w->last_had_star))

The comments indicate that the menu does not need to be updated, but
apparently the tool bar is not updated either, because w->update_mode_line
is not set to Qt.

There seems to be no ill effects from reverting this change, but since I
don't know why it was made in the first place, I can not be sure.
Does anybody remember the reason for this change?

Thanks,

	Jan D.

Re: Old redisplay bug in xdisp.c, question about change

[Richard Stallman]

    The comments indicate that the menu does not need to be updated, but
    apparently the tool bar is not updated either, because w->update_mode_line
    is not set to Qt.

    There seems to be no ill effects from reverting this change, but since I
    don't know why it was made in the first place, I can not be sure.
    Does anybody remember the reason for this change?

I think I made this change just as an optimization.  So it could be
reverted safely.

Another alternative is to execute, in that case,
just the parts of the if-statement that are really needed in that case.

Re: Old redisplay bug in xdisp.c, question about change

[Marshall, Simon]

	    The comments indicate that the menu does not need to be
updated, but
	    apparently the tool bar is not updated either, because
w->update_mode_line
	    is not set to Qt.

	    There seems to be no ill effects from reverting this change,
but since I
	    don't know why it was made in the first place, I can not be
sure.
	    Does anybody remember the reason for this change?

	I think I made this change just as an optimization.  So it could
be
	reverted safely.

	Another alternative is to execute, in that case,
	just the parts of the if-statement that are really needed in
that case.

This particular change was part of a group of changes made to prevent or
reduce multiple unnecessary runs of menu-bar-update-hook on things like
scrolls, tooltips, mouse-1 with mouse-sel-mode, C-s, C-x 2 or menu-bar
clicks.  (The performance impact is so bad that I use 21.1.90 + these
changes for all my editing, rather than 21.2 or 21.3 which do not have
them.)  The second part of this particular change made the code match
the comment; at the time Gerd said this code dated from at least 1994.

It would be a shame if this change was simply reverted as the particular
problem (multiple unnecessary runs of menu-bar-update-hook on scroll)
will presumably resurface.  Does the fact that you don't always see a
redisplay problem suggest that the bug lies elsewhere?

Richard, at the time you suggested a good general fix was to split the
functionality of windows_or_buffers_changed into different flags for
updating the mode line and updating menus, though I don't think anyone
volunteered to implement it.  Perhaps a flag for updating the tool bar
would also be useful.

Simon.

Re: Old redisplay bug in xdisp.c, question about change

[Jan D.]

>     The comments indicate that the menu does not need to be updated, but
>     apparently the tool bar is not updated either, because w->update_mode_line
>     is not set to Qt.
> 
>     There seems to be no ill effects from reverting this change, but since I
>     don't know why it was made in the first place, I can not be sure.
>     Does anybody remember the reason for this change?
> 
> I think I made this change just as an optimization.  So it could be
> reverted safely.
> 
> Another alternative is to execute, in that case,
> just the parts of the if-statement that are really needed in that case.

The if-statement is OK.  It is the setting of w->update_mode_line to Qt
that later makes update_tool_bar actually update the tool bar.

	Jan D.

Re: Old redisplay bug in xdisp.c, question about change

[Jan D.]

> This particular change was part of a group of changes made to prevent or
> reduce multiple unnecessary runs of menu-bar-update-hook on things like
> scrolls, tooltips, mouse-1 with mouse-sel-mode, C-s, C-x 2 or menu-bar
> clicks.  (The performance impact is so bad that I use 21.1.90 + these
> changes for all my editing, rather than 21.2 or 21.3 which do not have
> them.)  The second part of this particular change made the code match
> the comment; at the time Gerd said this code dated from at least 1994.
> 
> It would be a shame if this change was simply reverted as the particular
> problem (multiple unnecessary runs of menu-bar-update-hook on scroll)
> will presumably resurface.  Does the fact that you don't always see a
> redisplay problem suggest that the bug lies elsewhere?

It the if-statement in update_tool_bar that is entered if w->update_mode_line
is Qt:

      /* If the user has switched buffers or windows, we need to
	 recompute to reflect the new bindings.  But we'll
	 recompute when update_mode_lines is set too; that means
	 that people can use force-mode-line-update to request
	 that the menu bar be recomputed.  The adverse effect on
	 the rest of the redisplay algorithm is about the same as
	 windows_or_buffers_changed anyway.  */
      if (windows_or_buffers_changed
	  || !NILP (w->update_mode_line)
	  || ((BUF_SAVE_MODIFF (XBUFFER (w->buffer))
	       < BUF_MODIFF (XBUFFER (w->buffer)))
	      != !NILP (w->last_had_star))
	  || ((!NILP (Vtransient_mark_mode)
	       && !NILP (XBUFFER (w->buffer)->mark_active))
	      != !NILP (w->region_showing)))
	{

The other parts are false, but I really don't know what they all mean.
windows_or_buffers_changed is 0 when I switch Info nodes.  In effect,
the buffer or the window has not changed, but its contents, active
and inactive menu and tool bar items have.

	Jan D.

Re: Old redisplay bug in xdisp.c, question about change

[Richard Stallman]

The change you made completely eliminates the important optimization.
As I explained before, that is too drastic.  So I took it out,
and implemented a simple change that I think ought to cause
redisplay of the toolbar without the same drastic consequences.

I can't really test it; I don't seem to get a tool bar at all,
and I am not sure why, and I suspect it reflects something strange
about my machine's setup rather than a bug in Emacs.

Re: Old redisplay bug in xdisp.c, question about change

[Jan D.]

> I can't really test it; I don't seem to get a tool bar at all,
> and I am not sure why, and I suspect it reflects something strange
> about my machine's setup rather than a bug in Emacs.

It works OK.  If Emacs can't find any of the tool bar icon files
the tool bar will not show.

	Jan D.