* [RFC] per-frame fonts_changed and cursor_type_changed flags
@ 2012-11-16 11:13 Dmitry Antipov
2012-11-16 13:30 ` Eli Zaretskii
0 siblings, 1 reply; 2+ messages in thread
From: Dmitry Antipov @ 2012-11-16 11:13 UTC (permalink / raw)
To: Emacs development discussions
[-- Attachment #1: Type: text/plain, Size: 198 bytes --]
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.
Dmitry
[-- Attachment #2: redisplay_fonts_and_cursor.patch --]
[-- Type: text/plain, Size: 16217 bytes --]
=== modified file 'src/dispextern.h'
--- src/dispextern.h 2012-10-07 22:31:58 +0000
+++ src/dispextern.h 2012-11-16 09:09:06 +0000
@@ -1126,12 +1126,6 @@
((ROW)->phys_height - (ROW)->phys_ascent \
> (ROW)->height - (ROW)->ascent)
-/* True means that fonts have been loaded since the last glyph
- matrix adjustments. The function redisplay_internal adjusts glyph
- matrices when this flag is true. */
-
-extern bool fonts_changed_p;
-
/* A glyph for a space. */
extern struct glyph space_glyph;
=== modified file 'src/dispnew.c'
--- src/dispnew.c 2012-11-13 06:11:40 +0000
+++ src/dispnew.c 2012-11-16 09:10:16 +0000
@@ -78,7 +78,6 @@
static void update_frame_line (struct frame *, int);
static int required_matrix_height (struct window *);
static int required_matrix_width (struct window *);
-static void adjust_frame_glyphs (struct frame *);
static void change_frame_size_1 (struct frame *, int, int, bool, bool, bool);
static void increment_row_positions (struct glyph_row *, ptrdiff_t, ptrdiff_t);
static void fill_up_frame_row_with_spaces (struct glyph_row *, int);
@@ -165,16 +164,6 @@
static struct frame *frame_matrix_frame;
-/* True means that fonts have been loaded since the last glyph
- matrix adjustments. Redisplay must stop, and glyph matrices must
- be adjusted when this flag becomes true during display. The
- reason fonts can be loaded so late is that fonts of fontsets are
- loaded on demand. Another reason is that a line contains many
- characters displayed by zero width or very narrow glyphs of
- variable-width fonts. */
-
-bool fonts_changed_p;
-
/* Convert vpos and hpos from frame to window and vice versa.
This may only be used for terminal frames. */
@@ -446,7 +435,7 @@
|| right != matrix->right_margin_glyphs);
if (!marginal_areas_changed_p
- && !fonts_changed_p
+ && !XFRAME (w->frame)->fonts_changed
&& !header_line_changed_p
&& matrix->window_left_col == WINDOW_LEFT_EDGE_COL (w)
&& matrix->window_top_line == WINDOW_TOP_EDGE_LINE (w)
@@ -1821,10 +1810,9 @@
}
-/* Re-allocate/ re-compute glyph matrices on frame F. If F is null,
- do it for all frames; otherwise do it just for the given frame.
- This function must be called when a new frame is created, its size
- changes, or its window configuration changes. */
+/* Re-allocate/ re-compute glyph matrices on frame F. This function must
+ be called when a new frame is created, its size changes, or its window
+ configuration changes. */
void
adjust_glyphs (struct frame *f)
@@ -1833,24 +1821,6 @@
glyph matrices are not processed while we are changing them. */
block_input ();
- if (f)
- adjust_frame_glyphs (f);
- else
- {
- Lisp_Object tail, lisp_frame;
-
- FOR_EACH_FRAME (tail, lisp_frame)
- adjust_frame_glyphs (XFRAME (lisp_frame));
- }
-
- unblock_input ();
-}
-
-/* Allocate/reallocate glyph matrices of a single frame F. */
-
-static void
-adjust_frame_glyphs (struct frame *f)
-{
if (FRAME_WINDOW_P (f))
adjust_frame_glyphs_for_window_redisplay (f);
else
@@ -1862,6 +1832,8 @@
adjust_decode_mode_spec_buffer (f);
f->glyphs_initialized_p = 1;
+
+ unblock_input ();
}
/* Return true if any window in the tree has nonzero window margins. See
=== modified file 'src/font.c'
--- src/font.c 2012-11-06 13:26:20 +0000
+++ src/font.c 2012-11-16 09:09:06 +0000
@@ -2864,14 +2864,14 @@
{
FRAME_SMALLEST_CHAR_WIDTH (f) = min_width;
FRAME_SMALLEST_FONT_HEIGHT (f) = height;
- fonts_changed_p = 1;
+ f->fonts_changed = 1;
}
else
{
if (FRAME_SMALLEST_CHAR_WIDTH (f) > min_width)
- FRAME_SMALLEST_CHAR_WIDTH (f) = min_width, fonts_changed_p = 1;
+ FRAME_SMALLEST_CHAR_WIDTH (f) = min_width, f->fonts_changed = 1;
if (FRAME_SMALLEST_FONT_HEIGHT (f) > height)
- FRAME_SMALLEST_FONT_HEIGHT (f) = height, fonts_changed_p = 1;
+ FRAME_SMALLEST_FONT_HEIGHT (f) = height, f->fonts_changed = 1;
}
#endif
=== modified file 'src/frame.h'
--- src/frame.h 2012-11-12 16:02:46 +0000
+++ src/frame.h 2012-11-16 09:48:11 +0000
@@ -224,6 +224,11 @@
Used to see the difference between a font change and face change. */
unsigned default_face_done_p : 1;
+ /* Nonzero means that fonts have been loaded for this frame since the last
+ glyph matrix adjustments. The function redisplay_internal adjusts glyph
+ matrices when this flag is true. */
+ unsigned fonts_changed : 1;
+
/* Set to non-zero if this frame has already been hscrolled during
current redisplay. */
unsigned already_hscrolled_p : 1;
@@ -434,6 +439,9 @@
/* Nonzero means that the pointer is invisible. */
unsigned pointer_invisible :1;
+ /* Nonzero means a frame's cursor type has been changed. */
+ unsigned cursor_type_changed : 1;
+
/* Nonzero if we should actually display the scroll bars on this frame. */
enum vertical_scroll_bar_type vertical_scroll_bar_type;
=== modified file 'src/nsfns.m'
--- src/nsfns.m 2012-11-03 05:59:17 +0000
+++ src/nsfns.m 2012-11-16 09:47:59 +0000
@@ -900,9 +900,6 @@
x_set_cursor_type (FRAME_PTR f, Lisp_Object arg, Lisp_Object oldval)
{
set_frame_cursor_types (f, arg);
-
- /* Make sure the cursor gets redrawn. */
- cursor_type_changed = 1;
}
\f
=== modified file 'src/w32fns.c'
--- src/w32fns.c 2012-11-12 04:00:55 +0000
+++ src/w32fns.c 2012-11-16 09:46:48 +0000
@@ -1474,9 +1474,6 @@
x_set_cursor_type (FRAME_PTR f, Lisp_Object arg, Lisp_Object oldval)
{
set_frame_cursor_types (f, arg);
-
- /* Make sure the cursor gets redrawn. */
- cursor_type_changed = 1;
}
\f
void
=== modified file 'src/window.h'
--- src/window.h 2012-11-02 10:34:26 +0000
+++ src/window.h 2012-11-16 09:46:54 +0000
@@ -943,10 +943,6 @@
extern int windows_or_buffers_changed;
-/* Nonzero means a frame's cursor type has been changed. */
-
-extern int cursor_type_changed;
-
/* Number of windows displaying the selected buffer. Normally this is
1, but it can be more. */
=== modified file 'src/xdisp.c'
--- src/xdisp.c 2012-11-14 11:13:33 +0000
+++ src/xdisp.c 2012-11-16 10:57:39 +0000
@@ -552,10 +552,6 @@
int windows_or_buffers_changed;
-/* Nonzero means a frame's cursor type has been changed. */
-
-int cursor_type_changed;
-
/* Nonzero after display_mode_line if %l was used and it displayed a
line number. */
@@ -11998,7 +11994,7 @@
if (WINDOW_TOTAL_LINES (w) != old_height)
{
clear_glyph_matrix (w->desired_matrix);
- fonts_changed_p = 1;
+ f->fonts_changed = 1;
return 1;
}
}
@@ -12100,7 +12096,7 @@
{
clear_glyph_matrix (w->desired_matrix);
f->n_tool_bar_rows = nrows;
- fonts_changed_p = 1;
+ f->fonts_changed = 1;
return 1;
}
}
@@ -12935,6 +12931,14 @@
} while (!EQ (frame, old) && (frame = old, 1));
}
+/* Recompute F's glyph matrices due to fonts change. Return
+ 1 if glyphs was adjusted, and 0 if it isn't necessary. */
+
+static int
+frame_fonts_changed (struct frame *f)
+{
+ return f->fonts_changed ? (adjust_glyphs (f), f->fonts_changed = 0, 1) : 0;
+}
#define STOP_POLLING \
do { if (! polling_stopped_here) stop_polling (); \
@@ -12960,7 +12964,7 @@
int number_of_visible_frames;
ptrdiff_t count, count1;
struct frame *sf;
- int polling_stopped_here = 0;
+ int cursor_type_changed = 0, polling_stopped_here = 0;
Lisp_Object old_frame = selected_frame;
struct backtrace backtrace;
@@ -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);
/* If face_change_count is non-zero, init_iterator will free all
realized faces, which includes the faces referenced from current
@@ -13088,6 +13087,8 @@
if (FRAME_VISIBLE_P (f))
++number_of_visible_frames;
clear_desired_matrices (f);
+ /* Also check whether a cursor type was changed somewhere. */
+ cursor_type_changed += f->cursor_type_changed;
}
}
@@ -13176,7 +13177,7 @@
if (!display_last_displayed_message_p)
message_cleared_p = 0;
- if (fonts_changed_p)
+ if (fr->fonts_changed)
goto retry;
else if (window_height_changed_p)
{
@@ -13471,6 +13472,8 @@
variables. */
select_frame_for_redisplay (frame);
+ retry_frame:
+
/* Mark all the scroll bars to be removed; we'll redeem
the ones we want when we redisplay their windows. */
if (FRAME_TERMINAL (f)->condemn_scroll_bars_hook)
@@ -13489,10 +13492,8 @@
FRAME_TERMINAL (f)->judge_scroll_bars_hook (f);
/* If fonts changed, display again. */
- /* ??? rms: I suspect it is a mistake to jump all the way
- back to retry here. It should just retry this frame. */
- if (fonts_changed_p)
- goto retry;
+ if (frame_fonts_changed (f))
+ goto retry_frame;
if (FRAME_VISIBLE_P (f) && !FRAME_OBSCURED_P (f))
{
@@ -13501,7 +13502,7 @@
{
f->already_hscrolled_p = 1;
if (hscroll_windows (f->root_window))
- goto retry;
+ goto retry_frame;
}
/* Prevent various kinds of signals during display
@@ -13566,7 +13567,7 @@
update:
/* If fonts changed, display again. */
- if (fonts_changed_p)
+ if (sf->fonts_changed)
goto retry;
/* Prevent various kinds of signals during display update.
@@ -13637,7 +13638,6 @@
update_mode_lines = 0;
windows_or_buffers_changed = 0;
- cursor_type_changed = 0;
}
/* Start SIGIO interrupts coming again. Having them off during the
@@ -15064,7 +15064,7 @@
cases. */
&& !update_mode_lines
&& !windows_or_buffers_changed
- && !cursor_type_changed
+ && !f->cursor_type_changed
/* Can't use this case if highlighting a region. When a
region exists, cursor movement has to do more than just
set the cursor. */
@@ -15418,8 +15418,8 @@
selected_window is redisplayed.
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. */
static void
redisplay_window (Lisp_Object window, int just_this_one_p)
@@ -15789,7 +15789,7 @@
debug_method_add (w, "try_window_id %d", tem);
#endif
- if (fonts_changed_p)
+ if (f->fonts_changed)
goto need_larger_matrices;
if (tem > 0)
goto done;
@@ -15860,12 +15860,12 @@
IF_DEBUG (debug_method_add (w, "1"));
if (try_window (window, startp, TRY_WINDOW_CHECK_MARGINS) < 0)
/* -1 means we need to scroll.
- 0 means we need new matrices, but fonts_changed_p
+ 0 means we need new matrices, but f->fonts_changed
is set in that case, so we will detect it below. */
goto try_to_scroll;
}
- if (fonts_changed_p)
+ if (f->fonts_changed)
goto need_larger_matrices;
if (w->cursor.vpos >= 0)
@@ -16056,7 +16056,7 @@
/* Redisplay the window. */
if (!current_matrix_up_to_date_p
|| windows_or_buffers_changed
- || cursor_type_changed
+ || f->cursor_type_changed
/* Don't use try_window_reusing_current_matrix in this case
because it can have changed the buffer. */
|| !NILP (Vwindow_scroll_functions)
@@ -16069,7 +16069,7 @@
/* If new fonts have been loaded (due to fontsets), give up. We
have to start a new redisplay since we need to re-adjust glyph
matrices. */
- if (fonts_changed_p)
+ if (f->fonts_changed)
goto need_larger_matrices;
/* If cursor did not appear assume that the middle of the window is
@@ -16181,7 +16181,7 @@
if (WINDOW_WANTS_MODELINE_P (w)
&& CURRENT_MODE_LINE_HEIGHT (w) != DESIRED_MODE_LINE_HEIGHT (w))
{
- fonts_changed_p = 1;
+ f->fonts_changed = 1;
MATRIX_MODE_LINE_ROW (w->current_matrix)->height
= DESIRED_MODE_LINE_HEIGHT (w);
}
@@ -16191,12 +16191,12 @@
if (WINDOW_WANTS_HEADER_LINE_P (w)
&& CURRENT_HEADER_LINE_HEIGHT (w) != DESIRED_HEADER_LINE_HEIGHT (w))
{
- fonts_changed_p = 1;
+ f->fonts_changed = 1;
MATRIX_HEADER_LINE_ROW (w->current_matrix)->height
= DESIRED_HEADER_LINE_HEIGHT (w);
}
- if (fonts_changed_p)
+ if (f->fonts_changed)
goto need_larger_matrices;
}
@@ -16262,7 +16262,7 @@
}
#endif /* HAVE_WINDOW_SYSTEM */
- /* We go to this label, with fonts_changed_p set,
+ /* We go to this label, with W's frame fonts_changed set,
if it is necessary to try again using larger glyph matrices.
We have to redeem the scroll bar even in this case,
because the loop in redisplay_internal expects that. */
@@ -16334,7 +16334,7 @@
{
if (display_line (&it))
last_text_row = it.glyph_row - 1;
- if (fonts_changed_p && !(flags & TRY_WINDOW_IGNORE_FONTS_CHANGE))
+ if (f->fonts_changed && !(flags & TRY_WINDOW_IGNORE_FONTS_CHANGE))
return 0;
}
@@ -16436,7 +16436,7 @@
/* Don't try to reuse the display if windows have been split
or such. */
|| windows_or_buffers_changed
- || cursor_type_changed)
+ || f->cursor_type_changed)
return 0;
/* Can't do this if region may have changed. */
@@ -16486,7 +16486,7 @@
last_text_row = last_reused_text_row = NULL;
while (it.current_y < it.last_visible_y
- && !fonts_changed_p)
+ && !f->fonts_changed)
{
/* If we have reached into the characters in the START row,
that means the line boundaries have changed. So we
@@ -16714,7 +16714,7 @@
if (pt_row == NULL)
w->cursor.vpos = -1;
last_text_row = NULL;
- while (it.current_y < it.last_visible_y && !fonts_changed_p)
+ while (it.current_y < it.last_visible_y && !f->fonts_changed)
if (display_line (&it))
last_text_row = it.glyph_row - 1;
@@ -17238,7 +17238,7 @@
GIVE_UP (1);
/* This flag is used to prevent redisplay optimizations. */
- if (windows_or_buffers_changed || cursor_type_changed)
+ if (windows_or_buffers_changed || f->cursor_type_changed)
GIVE_UP (2);
/* Verify that narrowing has not changed.
@@ -17572,7 +17572,7 @@
last_text_row = NULL;
overlay_arrow_seen = 0;
while (it.current_y < it.last_visible_y
- && !fonts_changed_p
+ && !f->fonts_changed
&& (first_unchanged_at_end_row == NULL
|| IT_CHARPOS (it) < stop_pos))
{
@@ -17580,7 +17580,7 @@
last_text_row = it.glyph_row - 1;
}
- if (fonts_changed_p)
+ if (f->fonts_changed)
return -1;
@@ -17825,7 +17825,7 @@
/* Display the rest of the lines at the window end. */
it.glyph_row = MATRIX_ROW (desired_matrix, it.vpos);
while (it.current_y < it.last_visible_y
- && !fonts_changed_p)
+ && !f->fonts_changed)
{
/* Is it always sure that the display agrees with lines in
the current matrix? I don't think so, so we mark rows
@@ -19328,7 +19328,7 @@
>= it->w->desired_matrix->nrows)
{
it->w->nrows_scale_factor++;
- fonts_changed_p = 1;
+ it->f->fonts_changed = 1;
return 0;
}
@@ -23679,12 +23679,12 @@
#define IT_EXPAND_MATRIX_WIDTH(it, area) \
{ \
- if (!fonts_changed_p \
+ if (!it->f->fonts_changed \
&& (it->glyph_row->glyphs[area] \
< it->glyph_row->glyphs[area + 1])) \
{ \
it->w->ncols_scale_factor++; \
- fonts_changed_p = 1; \
+ it->f->fonts_changed = 1; \
} \
}
@@ -25605,6 +25605,9 @@
}
else
FRAME_BLINK_OFF_CURSOR (f) = DEFAULT_CURSOR;
+
+ /* Make sure the cursor gets redrawn for this frame. */
+ f->cursor_type_changed = 1;
}
=== modified file 'src/xfns.c'
--- src/xfns.c 2012-11-12 04:00:55 +0000
+++ src/xfns.c 2012-11-16 09:47:51 +0000
@@ -1082,9 +1082,6 @@
x_set_cursor_type (FRAME_PTR f, Lisp_Object arg, Lisp_Object oldval)
{
set_frame_cursor_types (f, arg);
-
- /* Make sure the cursor gets redrawn. */
- cursor_type_changed = 1;
}
\f
static void
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [RFC] per-frame fonts_changed and cursor_type_changed flags
2012-11-16 11:13 [RFC] per-frame fonts_changed and cursor_type_changed flags Dmitry Antipov
@ 2012-11-16 13:30 ` Eli Zaretskii
0 siblings, 0 replies; 2+ messages in thread
From: Eli Zaretskii @ 2012-11-16 13:30 UTC (permalink / raw)
To: Dmitry Antipov; +Cc: emacs-devel
> Date: Fri, 16 Nov 2012 15:13:12 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
>
> 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").
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-11-16 13:30 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-16 11:13 [RFC] per-frame fonts_changed and cursor_type_changed flags Dmitry Antipov
2012-11-16 13:30 ` Eli Zaretskii
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).