Re: [Emacs-diffs] master 5c9304e: Disable some display optimizations when frames need redisplay

[Stefan Monnier <monnier@iro.umontreal.ca>]

>     Disable some display optimizations when frames need redisplay
>     These optimizations were previously disabled by the
>     windows_or_buffers_changed flag, which now is not set
>     when only some frames need to be redrawn.

Hmm... I only replaced things like

   windows_or_buffers_changed = 45;

with

   fset_redisplay (f);

which internally will set windows_or_buffers_changed to 2, so as long as
windows_or_buffers_changed is treated as a boolean, the code should not
be affected.

> +  /* True means we need to redraw frames whose 'redisplay' bit is set.  */
> +  bool consider_some_frames_p = false;

Why not use "windows_or_buffers_changed == REDISPLAY_SOME" as an
equivalent condition?

>    /* Build desired matrices, and update the display.  If
> -     consider_all_windows_p, do it for all windows on all frames.
> -     Otherwise do it for selected_window, only.  */
> +     consider_all_windows_p, do it for all windows on all frames.  If
> +     a frame's 'redisplay' flag is set, do it for all windows on each
> +     such frame.  Otherwise do it for selected_window, only.  */
> 
> -  if (consider_all_windows_p)
> +  if (!consider_all_windows_p)
>      {
>        FOR_EACH_FRAME (tail, frame)
> -	XFRAME (frame)->updated_p = false;
> +	{
> +	  if (XFRAME (frame)->redisplay && XFRAME (frame) != sf)
> +	    {
> +	      consider_some_frames_p = true;
> +	      break;
> +	    }
> +	}
> +    }
> +
> +  if (consider_all_windows_p || consider_some_frames_p)
> +    {
> +      FOR_EACH_FRAME (tail, frame)
> +	{
> +	  if (XFRAME (frame)->redisplay || consider_all_windows_p)
> +	    XFRAME (frame)->updated_p = false;
> +	}

I don't understand in which kind of scenario this could make
a difference.

>        && !windows_or_buffers_changed
>        && !f->cursor_type_changed
> +      && !f->redisplay

Same here: IIUC this change should make no difference, since
setting f->redisplay also sets windows_or_buffers_changed to
a non-nil value.

>        || windows_or_buffers_changed
> +      || f->redisplay

And same again here.

> +  if (windows_or_buffers_changed || f->cursor_type_changed || f->redisplay)

And here.

So, what am I missing?


        Stefan