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

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.]

> 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.

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

[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

[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.