Re: [RFC] per-frame fonts_changed and cursor_type_changed flags

[Eli Zaretskii]

> Date: Fri, 16 Nov 2012 15:13:12 +0400
> From: Dmitry Antipov <address@hidden>
> 
> This patch tries to offload redisplay from some work, assuming that
> changing frame font and per-frame cursor type should invalidate only
> the affected frame. Comments are definitely welcome.

Some comments below.  But ultimately, the only practical way of
knowing if the patch is correct is to test it in variety of situations
and in different builds (with and without X, with and without
toolkits, with and without GTK, etc.).  The display engine is not
written to any formal specification, so its use of flags is anything
but regular and disciplined.

> @@ -446,7 +435,7 @@
>                                 || right != matrix->right_margin_glyphs);
>  
>        if (!marginal_areas_changed_p
> -       && !fonts_changed_p
> +       && !XFRAME (w->frame)->fonts_changed

Did that work for you?  You cannot safely dereference w here, because
it can be NULL (on TTY frames, see the commentary before the
function).  E.g., adjust_frame_glyphs_for_frame_redisplay calls this
function that way.

> @@ -13044,12 +13048,7 @@
>  
>    /* If new fonts have been loaded that make a glyph matrix adjustment
>       necessary, do it.  */
> -  if (fonts_changed_p)
> -    {
> -      adjust_glyphs (NULL);
> -      ++windows_or_buffers_changed;
> -      fonts_changed_p = 0;
> -    }
> +  frame_fonts_changed (fr);

'fr' is the frame of the selected window on entry to
redisplay_internal.  Why do you assume that only that frame's fonts
are involved here?  E.g., the call to select_frame_for_redisplay, just
above this hunk, could have changed the selected frame for redisplay.

> @@ -13176,7 +13177,7 @@
>        if (!display_last_displayed_message_p)
>       message_cleared_p = 0;
>  
> -      if (fonts_changed_p)
> +      if (fr->fonts_changed)
>       goto retry;

Similarly here: I think you need to test the fonts_changed flags of
all the frames, because not necessarily redisplaying frame 'fr'.

>     We can return without actually redisplaying the window if
> -   fonts_changed_p.  In that case, redisplay_internal will
> -   retry.  */
> +   fonts of it's frame was changed.  In that case, redisplay_internal
> +   will retry.  */

"its", not "it's" (the latter is a short for "it is").