* 'struct window' cleanup #4
@ 2012-07-02 10:45 Dmitry Antipov
2012-07-02 14:48 ` Stefan Monnier
2012-07-02 17:33 ` Eli Zaretskii
0 siblings, 2 replies; 6+ messages in thread
From: Dmitry Antipov @ 2012-07-02 10:45 UTC (permalink / raw)
To: Emacs development discussions
[-- Attachment #1: Type: text/plain, Size: 214 bytes --]
Do we have a consensus about type(s) of 'window_end_vpos', 'vscroll',
'hscroll' and 'min_hscroll' fields of 'struct window'? Now I'm just
assuming that ptrdiff_t is safer. Eli, please review if possible.
Dmitry
[-- Attachment #2: window_cleanup_4.patch --]
[-- Type: text/plain, Size: 30182 bytes --]
=== modified file 'src/dispnew.c'
--- src/dispnew.c 2012-06-28 07:50:50 +0000
+++ src/dispnew.c 2012-07-02 10:11:48 +0000
@@ -643,9 +643,8 @@
/* Window end is invalid, if inside of the rows that
are invalidated below. */
- if (INTEGERP (w->window_end_vpos)
- && XFASTINT (w->window_end_vpos) >= i)
- w->window_end_valid = Qnil;
+ if (w->window_end_vpos >= i)
+ w->window_end_valid = 0;
while (i < matrix->nrows)
matrix->rows[i++].enabled_p = 0;
@@ -902,7 +901,7 @@
else
{
clear_glyph_matrix (w->current_matrix);
- w->window_end_valid = Qnil;
+ w->window_end_valid = 0;
}
}
=== modified file 'src/window.c'
--- src/window.c 2012-06-30 09:13:54 +0000
+++ src/window.c 2012-07-02 10:11:48 +0000
@@ -1307,12 +1307,12 @@
The user can compute it with vertical-motion if he wants to.
It would be nicer to do it automatically,
but that's so slow that it would probably bother people. */
- if (NILP (w->window_end_valid))
+ if (!w->window_end_valid)
return Qnil;
#endif
if (! NILP (update)
- && ! (! NILP (w->window_end_valid)
+ && ! (w->window_end_valid
&& w->last_modified >= BUF_MODIFF (b)
&& w->last_overlay_modified >= BUF_OVERLAY_MODIFF (b))
&& !noninteractive)
@@ -1354,7 +1354,7 @@
set_buffer_internal (old_buffer);
}
else
- XSETINT (value, BUF_Z (b) - XFASTINT (w->window_end_pos));
+ XSETINT (value, BUF_Z (b) - w->window_end_pos);
return value;
}
@@ -1507,7 +1507,7 @@
b = XBUFFER (w->buffer);
/* Fail if current matrix is not up-to-date. */
- if (NILP (w->window_end_valid)
+ if (!w->window_end_valid
|| current_buffer->clip_changed
|| current_buffer->prevent_redisplay_optimizations_p
|| w->last_modified < BUF_MODIFF (b)
@@ -1841,9 +1841,9 @@
n->phys_cursor_width = -1;
n->must_be_updated_p = 0;
n->pseudo_window_p = 0;
- XSETFASTINT (n->window_end_vpos, 0);
- XSETFASTINT (n->window_end_pos, 0);
- n->window_end_valid = Qnil;
+ n->window_end_vpos = 0;
+ n->window_end_pos = 0;
+ n->window_end_valid = 0;
n->frozen_window_start_p = 0;
}
@@ -2773,7 +2773,7 @@
pos = *vmotion (startpos, -top, w);
set_marker_both (w->start, w->buffer, pos.bufpos, pos.bytepos);
- w->window_end_valid = Qnil;
+ w->window_end_valid = 0;
w->start_at_line_beg = (pos.bytepos == BEGV_BYTE
|| FETCH_BYTE (pos.bytepos - 1) == '\n');
/* We need to do this, so that the window-scroll-functions
@@ -2989,10 +2989,10 @@
XSETINT (BVAR (b, display_count), XINT (BVAR (b, display_count)) + 1);
BVAR (b, display_time) = Fcurrent_time ();
- XSETFASTINT (w->window_end_pos, 0);
- XSETFASTINT (w->window_end_vpos, 0);
+ w->window_end_pos = 0;
+ w->window_end_vpos = 0;
memset (&w->last_cursor, 0, sizeof w->last_cursor);
- w->window_end_valid = Qnil;
+ w->window_end_valid = 0;
if (!(keep_margins_p && samebuf))
{ /* If we're not actually changing the buffer, don't reset hscroll and
vscroll. This case happens for example when called from
@@ -3287,8 +3287,6 @@
w->start = Fmake_marker ();
w->pointm = Fmake_marker ();
w->vertical_scroll_bar_type = Qt;
- XSETFASTINT (w->window_end_pos, 0);
- XSETFASTINT (w->window_end_vpos, 0);
/* Initialize non-Lisp data. Note that allocate_window zeroes out all
non-Lisp data, so do it only for slots which should not be zero. */
@@ -3296,6 +3294,8 @@
w->phys_cursor_type = -1;
w->phys_cursor_width = -1;
w->sequence_number = ++sequence_number;
+ w->window_end_pos = -1;
+ w->window_end_vpos = -1;
/* Reset window_list. */
Vwindow_list = Qnil;
@@ -3762,7 +3762,7 @@
}
n->buffer = Qt;
- n->window_end_valid = Qnil;
+ n->window_end_valid = 0;
memset (&n->last_cursor, 0, sizeof n->last_cursor);
/* Get special geometry settings from reference window. */
@@ -5174,7 +5174,7 @@
/* Set the new window start. */
set_marker_both (w->start, w->buffer, charpos, bytepos);
- w->window_end_valid = Qnil;
+ w->window_end_valid = 0;
w->optional_new_start = 1;
@@ -6115,7 +6115,7 @@
adjust_window_margins (w);
clear_glyph_matrix (w->current_matrix);
- w->window_end_valid = Qnil;
+ w->window_end_valid = 0;
++windows_or_buffers_changed;
adjust_glyphs (XFRAME (WINDOW_FRAME (w)));
@@ -6184,7 +6184,7 @@
adjust_window_margins (w);
clear_glyph_matrix (w->current_matrix);
- w->window_end_valid = Qnil;
+ w->window_end_valid = 0;
++windows_or_buffers_changed;
adjust_glyphs (XFRAME (WINDOW_FRAME (w)));
=== modified file 'src/window.h'
--- src/window.h 2012-06-29 11:48:08 +0000
+++ src/window.h 2012-07-02 10:18:46 +0000
@@ -168,18 +168,6 @@
no scroll bar. A value of t means use frame value. */
Lisp_Object vertical_scroll_bar_type;
- /* Z - the buffer position of the last glyph in the current matrix
- of W. Only valid if WINDOW_END_VALID is not nil. */
- Lisp_Object window_end_pos;
- /* Glyph matrix row of the last glyph in the current matrix
- of W. Only valid if WINDOW_END_VALID is not nil. */
- Lisp_Object window_end_vpos;
- /* t if window_end_pos is truly valid.
- This is nil if nontrivial redisplay is preempted
- since in that case the frame image that window_end_pos
- did not get onto the frame. */
- Lisp_Object window_end_valid;
-
/* Display-table to use for displaying chars in this window.
Nil means use the buffer's own display-table. */
Lisp_Object display_table;
@@ -198,10 +186,6 @@
as long as the window shows that buffer. */
Lisp_Object base_line_pos;
- /* If we have highlighted the region (or any part of it),
- this is the mark position that we used, as an integer. */
- Lisp_Object region_showing;
-
/* The column number currently displayed in this window's mode line,
or nil if column numbers are not being displayed. */
Lisp_Object column_number_displayed;
@@ -256,6 +240,14 @@
it should be positive. */
ptrdiff_t last_point;
+ /* Z - the buffer position of the last glyph in the current matrix of W.
+ Should be nonnegative, and valid only if window_end_valid is not zero. */
+ ptrdiff_t window_end_pos;
+
+ /* Glyph matrix row of the last glyph in the current matrix of W.
+ Should be nonnegative, and valid only if window_end_valid is not zero. */
+ ptrdiff_t window_end_vpos;
+
/* Scaling factor for the glyph_matrix size calculation in this window.
Used if window contains many small images or uses proportional fonts,
as the normal may yield a matrix which is too small. */
@@ -289,6 +281,9 @@
was last updated. */
unsigned last_had_star : 1;
+ /* Non-zero if we have highlighted the region (or any part of it). */
+ unsigned region_showing : 1;
+
/* Non-zero means current value of `start'
was the beginning of a line when it was chosen. */
unsigned start_at_line_beg : 1;
@@ -331,9 +326,14 @@
Otherwise draw them between margin areas and text. */
unsigned fringes_outside_margins : 1;
+ /* Non-zero if window_end_pos is truly valid. This is zero if
+ nontrivial redisplay is preempted since in that case the
+ frame image that window_end_pos did not get onto the frame. */
+ unsigned window_end_valid : 1;
+
/* Amount by which lines of this window are scrolled in
y-direction (smooth scrolling). */
- int vscroll;
+ ptrdiff_t vscroll;
/* Z_BYTE - the buffer position of the last glyph in the current matrix of W.
Should be nonnegative, and only valid if window_end_valid is not nil. */
=== modified file 'src/xdisp.c'
--- src/xdisp.c 2012-06-29 18:52:54 +0000
+++ src/xdisp.c 2012-07-02 10:33:22 +0000
@@ -2504,11 +2504,11 @@
check_window_end (struct window *w)
{
if (!MINI_WINDOW_P (w)
- && !NILP (w->window_end_valid))
+ && w->window_end_valid)
{
struct glyph_row *row;
eassert ((row = MATRIX_ROW (w->current_matrix,
- XFASTINT (w->window_end_vpos)),
+ w->window_end_vpos),
!row->enabled_p
|| MATRIX_ROW_DISPLAYS_TEXT_P (row)
|| MATRIX_ROW_VPOS (row, w->current_matrix) == 0));
@@ -8850,7 +8850,7 @@
&& it->current_x == it->last_visible_x - 1
&& it->c != '\n'
&& it->c != '\t'
- && it->vpos < XFASTINT (it->w->window_end_vpos))
+ && it->vpos < it->w->window_end_vpos)
{
it->continuation_lines_width += it->current_x;
it->current_x = it->hpos = it->max_ascent = it->max_descent = 0;
@@ -11202,7 +11202,7 @@
!= w->last_had_star)
|| ((!NILP (Vtransient_mark_mode)
&& !NILP (BVAR (XBUFFER (w->buffer), mark_active)))
- != !NILP (w->region_showing)))
+ != w->region_showing))
{
struct buffer *prev = current_buffer;
ptrdiff_t count = SPECPDL_INDEX ();
@@ -11400,7 +11400,7 @@
!= w->last_had_star)
|| ((!NILP (Vtransient_mark_mode)
&& !NILP (BVAR (XBUFFER (w->buffer), mark_active)))
- != !NILP (w->region_showing)))
+ != w->region_showing))
{
struct buffer *prev = current_buffer;
ptrdiff_t count = SPECPDL_INDEX ();
@@ -12766,7 +12766,7 @@
reconsider_clip_changes (struct window *w, struct buffer *b)
{
if (b->clip_changed
- && !NILP (w->window_end_valid)
+ && w->window_end_valid
&& w->current_matrix->buffer == b
&& w->current_matrix->zv == BUF_ZV (b)
&& w->current_matrix->begv == BUF_BEGV (b))
@@ -12778,7 +12778,7 @@
b->clip_changed has already been set to 1, we can skip this
check. */
if (!b->clip_changed
- && BUFFERP (w->buffer) && !NILP (w->window_end_valid))
+ && BUFFERP (w->buffer) && w->window_end_valid)
{
ptrdiff_t pt;
@@ -13103,12 +13103,9 @@
/* If showing the region, and mark has changed, we must redisplay
the whole window. The assignment to this_line_start_pos prevents
the optimization directly below this if-statement. */
- if (((!NILP (Vtransient_mark_mode)
- && !NILP (BVAR (XBUFFER (w->buffer), mark_active)))
- != !NILP (w->region_showing))
- || (!NILP (w->region_showing)
- && !EQ (w->region_showing,
- Fmarker_position (BVAR (XBUFFER (w->buffer), mark)))))
+ if (w->region_showing
+ || (!NILP (Vtransient_mark_mode)
+ && !NILP (BVAR (XBUFFER (w->buffer), mark_active))))
CHARPOS (this_line_start_pos) = 0;
/* Optimize the case that only the line containing the cursor in the
@@ -13224,13 +13221,13 @@
adjusted. */
if ((it.glyph_row - 1)->displays_text_p)
{
- if (XFASTINT (w->window_end_vpos) < this_line_vpos)
- XSETINT (w->window_end_vpos, this_line_vpos);
+ if (w->window_end_vpos < this_line_vpos)
+ w->window_end_vpos = this_line_vpos;
}
- else if (XFASTINT (w->window_end_vpos) == this_line_vpos
+ else if (w->window_end_vpos == this_line_vpos
&& this_line_vpos > 0)
- XSETINT (w->window_end_vpos, this_line_vpos - 1);
- w->window_end_valid = Qnil;
+ w->window_end_vpos = this_line_vpos - 1;
+ w->window_end_valid = 0;
/* Update hint: No need to try to scroll in update_window. */
w->desired_matrix->no_scrolling_p = 1;
@@ -13276,7 +13273,7 @@
&& (EQ (selected_window,
BVAR (current_buffer, last_selected_window))
|| highlight_nonselected_windows)
- && NILP (w->region_showing)
+ && !w->region_showing
&& NILP (Vshow_trailing_whitespace)
&& !cursor_in_echo_area)
{
@@ -13695,7 +13692,7 @@
if (accurate_p)
{
- w->window_end_valid = w->buffer;
+ w->window_end_valid = 1;
w->update_mode_line = 0;
}
}
@@ -14935,6 +14932,10 @@
ptrdiff_t, thus zero means invalid position in a buffer. */
eassert (w->last_point > 0);
+ /* Just initialized windows should not pass here. */
+ eassert (w->window_end_pos >= 0);
+ eassert (w->window_end_vpos >= 0);
+
/* Handle case where text has not changed, only point, and it has
not moved off the frame. */
if (/* Point may be in this window. */
@@ -14953,7 +14954,7 @@
set the cursor. */
&& !(!NILP (Vtransient_mark_mode)
&& !NILP (BVAR (current_buffer, mark_active)))
- && NILP (w->region_showing)
+ && !w->region_showing
&& NILP (Vshow_trailing_whitespace)
/* This code is not used for mini-buffer for the sake of the case
of redisplaying to replace an echo area message; since in
@@ -14962,13 +14963,9 @@
since the handling of this_line_start_pos, etc., in redisplay
handles the same cases. */
&& !EQ (window, minibuf_window)
- /* When splitting windows or for new windows, it happens that
- redisplay is called with a nil window_end_vpos or one being
- larger than the window. This should really be fixed in
- window.c. I don't have this on my list, now, so we do
- approximately the same as the old redisplay code. --gerd. */
- && INTEGERP (w->window_end_vpos)
- && XFASTINT (w->window_end_vpos) < w->current_matrix->nrows
+ /* Previously, there was Gerd's comment suggesting
+ to fix window.c. I believe that it is done. */
+ && w->window_end_vpos < w->current_matrix->nrows
&& (FRAME_WINDOW_P (f)
|| !overlay_arrow_in_current_buffer_p ()))
{
@@ -15280,7 +15277,7 @@
start = marker_position (w->start) - BUF_BEGV (buf);
/* I don't think this is guaranteed to be right. For the
moment, we'll pretend it is. */
- end = BUF_Z (buf) - XFASTINT (w->window_end_pos) - BUF_BEGV (buf);
+ end = BUF_Z (buf) - w->window_end_pos - BUF_BEGV (buf);
if (end < start)
end = start;
@@ -15390,7 +15387,7 @@
set_buffer_internal_1 (XBUFFER (w->buffer));
current_matrix_up_to_date_p
- = (!NILP (w->window_end_valid)
+ = (w->window_end_valid
&& !current_buffer->clip_changed
&& !current_buffer->prevent_redisplay_optimizations_p
&& w->last_modified >= MODIFF
@@ -15414,7 +15411,7 @@
specbind (Qinhibit_point_motion_hooks, Qt);
buffer_unchanged_p
- = (!NILP (w->window_end_valid)
+ = (w->window_end_valid
&& !current_buffer->clip_changed
&& w->last_modified >= MODIFF
&& w->last_overlay_modified >= OVERLAY_MODIFF);
@@ -15428,7 +15425,7 @@
if (XMARKER (w->start)->buffer == current_buffer)
compute_window_start_on_continuation_line (w);
- w->window_end_valid = Qnil;
+ w->window_end_valid = 0;
}
/* Some sanity checks. */
@@ -15539,7 +15536,7 @@
w->force_start = 0;
w->vscroll = 0;
- w->window_end_valid = Qnil;
+ w->window_end_valid = 0;
/* Forget any recorded base line for line number display. */
if (!buffer_unchanged_p)
@@ -15962,8 +15959,8 @@
line.) */
if (w->cursor.vpos < 0)
{
- if (!NILP (w->window_end_valid)
- && PT >= Z - XFASTINT (w->window_end_pos))
+ if (w->window_end_valid
+ && PT >= Z - w->window_end_pos)
{
clear_glyph_matrix (w->desired_matrix);
move_it_by_lines (&it, 1);
@@ -16202,6 +16199,10 @@
struct glyph_row *last_text_row = NULL;
struct frame *f = XFRAME (w->frame);
+ /* Just initialized windows should not pass here. */
+ eassert (w->window_end_pos >= 0);
+ eassert (w->window_end_vpos >= 0);
+
/* Make POS the new window start. */
set_marker_both (w->start, Qnil, CHARPOS (pos), BYTEPOS (pos));
@@ -16252,7 +16253,7 @@
}
/* If bottom moved off end of frame, change mode line percentage. */
- if (XFASTINT (w->window_end_pos) <= 0
+ if (w->window_end_pos <= 0
&& Z != IT_CHARPOS (it))
w->update_mode_line = 1;
@@ -16264,22 +16265,22 @@
eassert (MATRIX_ROW_DISPLAYS_TEXT_P (last_text_row));
w->window_end_bytepos
= Z_BYTE - MATRIX_ROW_END_BYTEPOS (last_text_row);
- w->window_end_pos
- = make_number (Z - MATRIX_ROW_END_CHARPOS (last_text_row));
- w->window_end_vpos
- = make_number (MATRIX_ROW_VPOS (last_text_row, w->desired_matrix));
- eassert (MATRIX_ROW (w->desired_matrix, XFASTINT (w->window_end_vpos))
+ w->window_end_pos = Z - MATRIX_ROW_END_CHARPOS (last_text_row);
+ w->window_end_vpos = MATRIX_ROW_VPOS (last_text_row, w->desired_matrix);
+ eassert (w->window_end_pos >= 0);
+ eassert (w->window_end_vpos >= 0);
+ eassert (MATRIX_ROW (w->desired_matrix, w->window_end_vpos)
->displays_text_p);
}
else
{
w->window_end_bytepos = Z_BYTE - ZV_BYTE;
- w->window_end_pos = make_number (Z - ZV);
- w->window_end_vpos = make_number (0);
+ w->window_end_pos = Z - ZV;
+ w->window_end_vpos = 0;
}
/* But that is not valid info until redisplay finishes. */
- w->window_end_valid = Qnil;
+ w->window_end_valid = 0;
return 1;
}
@@ -16313,6 +16314,10 @@
return 0;
#endif
+ /* Just initialized windows should not pass here. */
+ eassert (w->window_end_pos >= 0);
+ eassert (w->window_end_vpos >= 0);
+
if (/* This function doesn't handle terminal frames. */
!FRAME_WINDOW_P (f)
/* Don't try to reuse the display if windows have been split
@@ -16324,7 +16329,7 @@
/* Can't do this if region may have changed. */
if ((!NILP (Vtransient_mark_mode)
&& !NILP (BVAR (current_buffer, mark_active)))
- || !NILP (w->region_showing)
+ || w->region_showing
|| !NILP (Vshow_trailing_whitespace))
return 0;
@@ -16503,29 +16508,28 @@
{
w->window_end_bytepos
= Z_BYTE - MATRIX_ROW_END_BYTEPOS (last_reused_text_row);
- w->window_end_pos
- = make_number (Z - MATRIX_ROW_END_CHARPOS (last_reused_text_row));
- w->window_end_vpos
- = make_number (MATRIX_ROW_VPOS (last_reused_text_row,
- w->current_matrix));
+ w->window_end_pos = Z - MATRIX_ROW_END_CHARPOS (last_reused_text_row);
+ w->window_end_vpos = MATRIX_ROW_VPOS (last_reused_text_row, w->current_matrix);
+ eassert (w->window_end_pos >= 0);
+ eassert (w->window_end_vpos >= 0);
}
else if (last_text_row)
{
w->window_end_bytepos
= Z_BYTE - MATRIX_ROW_END_BYTEPOS (last_text_row);
- w->window_end_pos
- = make_number (Z - MATRIX_ROW_END_CHARPOS (last_text_row));
- w->window_end_vpos
- = make_number (MATRIX_ROW_VPOS (last_text_row, w->desired_matrix));
+ w->window_end_pos = Z - MATRIX_ROW_END_CHARPOS (last_text_row);
+ w->window_end_vpos = MATRIX_ROW_VPOS (last_text_row, w->desired_matrix);
+ eassert (w->window_end_pos >= 0);
+ eassert (w->window_end_vpos >= 0);
}
else
{
/* This window must be completely empty. */
w->window_end_bytepos = Z_BYTE - ZV_BYTE;
- w->window_end_pos = make_number (Z - ZV);
- w->window_end_vpos = make_number (0);
+ w->window_end_pos = Z - ZV;
+ w->window_end_vpos = 0;
}
- w->window_end_valid = Qnil;
+ w->window_end_valid = 0;
/* Update hint: don't try scrolling again in update_window. */
w->desired_matrix->no_scrolling_p = 1;
@@ -16706,18 +16710,16 @@
{
w->window_end_bytepos
= Z_BYTE - MATRIX_ROW_END_BYTEPOS (last_text_row);
- w->window_end_pos
- = make_number (Z - MATRIX_ROW_END_CHARPOS (last_text_row));
- w->window_end_vpos
- = make_number (MATRIX_ROW_VPOS (last_text_row, w->desired_matrix));
+ w->window_end_pos = Z - MATRIX_ROW_END_CHARPOS (last_text_row);
+ w->window_end_vpos = MATRIX_ROW_VPOS (last_text_row, w->desired_matrix);
}
else
- {
- w->window_end_vpos
- = make_number (XFASTINT (w->window_end_vpos) - nrows_scrolled);
- }
-
- w->window_end_valid = Qnil;
+ w->window_end_vpos = w->window_end_vpos - nrows_scrolled;
+
+ eassert (w->window_end_pos >= 0);
+ eassert (w->window_end_vpos >= 0);
+
+ w->window_end_valid = 0;
w->desired_matrix->no_scrolling_p = 1;
#ifdef GLYPH_DEBUG
@@ -16850,16 +16852,16 @@
/* Display must not have been paused, otherwise the current matrix
is not up to date. */
- eassert (!NILP (w->window_end_valid));
+ eassert (w->window_end_valid);
/* A value of window_end_pos >= END_UNCHANGED means that the window
end is in the range of changed text. If so, there is no
unchanged row at the end of W's current matrix. */
- if (XFASTINT (w->window_end_pos) >= END_UNCHANGED)
+ if (w->window_end_pos >= END_UNCHANGED)
return NULL;
/* Set row to the last row in W's current matrix displaying text. */
- row = MATRIX_ROW (w->current_matrix, XFASTINT (w->window_end_vpos));
+ row = MATRIX_ROW (w->current_matrix, w->window_end_vpos);
/* If matrix is entirely empty, no unchanged row exists. */
if (MATRIX_ROW_DISPLAYS_TEXT_P (row))
@@ -16870,7 +16872,7 @@
buffer positions in the current matrix to current buffer
positions for characters not in changed text. */
ptrdiff_t Z_old =
- MATRIX_ROW_END_CHARPOS (row) + XFASTINT (w->window_end_pos);
+ MATRIX_ROW_END_CHARPOS (row) + w->window_end_pos;
ptrdiff_t Z_BYTE_old =
MATRIX_ROW_END_BYTEPOS (row) + w->window_end_bytepos;
ptrdiff_t last_unchanged_pos, last_unchanged_pos_old;
@@ -17104,6 +17106,10 @@
#define GIVE_UP(X) return 0
#endif
+ /* Just initialized windows should not pass here. */
+ eassert (w->window_end_pos >= 0);
+ eassert (w->window_end_vpos >= 0);
+
SET_TEXT_POS_FROM_MARKER (start, w->start);
/* Don't use this for mini-windows because these can show
@@ -17142,7 +17148,7 @@
GIVE_UP (7);
/* Verify that display wasn't paused. */
- if (NILP (w->window_end_valid))
+ if (!w->window_end_valid)
GIVE_UP (8);
/* Can't use this if highlighting a region because a cursor movement
@@ -17156,7 +17162,7 @@
GIVE_UP (11);
/* Likewise if showing a region. */
- if (!NILP (w->region_showing))
+ if (w->region_showing)
GIVE_UP (10);
/* Can't use this if overlay arrow position and/or string have
@@ -17206,7 +17212,7 @@
This case happens with stealth-fontification. Note that although
the display is unchanged, glyph positions in the matrix have to
be adjusted, of course. */
- row = MATRIX_ROW (w->current_matrix, XFASTINT (w->window_end_vpos));
+ row = MATRIX_ROW (w->current_matrix, w->window_end_vpos);
if (MATRIX_ROW_DISPLAYS_TEXT_P (row)
&& ((last_changed_charpos < CHARPOS (start)
&& CHARPOS (start) == BEGV)
@@ -17218,7 +17224,7 @@
/* Compute how many chars/bytes have been added to or removed
from the buffer. */
- Z_old = MATRIX_ROW_END_CHARPOS (row) + XFASTINT (w->window_end_pos);
+ Z_old = MATRIX_ROW_END_CHARPOS (row) + w->window_end_pos;
Z_BYTE_old = MATRIX_ROW_END_BYTEPOS (row) + w->window_end_bytepos;
Z_delta = Z - Z_old;
Z_delta_bytes = Z_BYTE - Z_BYTE_old;
@@ -17289,8 +17295,8 @@
{
/* We have to compute the window end anew since text
could have been added/removed after it. */
- w->window_end_pos
- = make_number (Z - MATRIX_ROW_END_CHARPOS (row));
+ w->window_end_pos = Z - MATRIX_ROW_END_CHARPOS (row);
+ eassert (w->window_end_pos >= 0);
w->window_end_bytepos
= Z_BYTE - MATRIX_ROW_END_BYTEPOS (row);
@@ -17325,7 +17331,7 @@
/* Give up if the window ends in strings. Overlay strings
at the end are difficult to handle, so don't try. */
- row = MATRIX_ROW (current_matrix, XFASTINT (w->window_end_vpos));
+ row = MATRIX_ROW (current_matrix, w->window_end_vpos);
if (MATRIX_ROW_START_CHARPOS (row) == MATRIX_ROW_END_CHARPOS (row))
GIVE_UP (20);
@@ -17668,7 +17674,7 @@
/* Set last_row to the glyph row in the current matrix where the
window end line is found. It has been moved up or down in
the matrix by dvpos. */
- int last_vpos = XFASTINT (w->window_end_vpos) + dvpos;
+ ptrdiff_t last_vpos = w->window_end_vpos + dvpos;
struct glyph_row *last_row = MATRIX_ROW (current_matrix, last_vpos);
/* If last_row is the window end line, it should display text. */
@@ -17724,21 +17730,22 @@
first_unchanged_at_end_row);
eassert (row && MATRIX_ROW_DISPLAYS_TEXT_P (row));
- w->window_end_pos = make_number (Z - MATRIX_ROW_END_CHARPOS (row));
+ w->window_end_pos = Z - MATRIX_ROW_END_CHARPOS (row);
w->window_end_bytepos = Z_BYTE - MATRIX_ROW_END_BYTEPOS (row);
- w->window_end_vpos
- = make_number (MATRIX_ROW_VPOS (row, w->current_matrix));
+ w->window_end_vpos = MATRIX_ROW_VPOS (row, w->current_matrix);
+ eassert (w->window_end_pos >= 0);
+ eassert (w->window_end_vpos >= 0);
eassert (w->window_end_bytepos >= 0);
IF_DEBUG (debug_method_add (w, "A"));
}
else if (last_text_row_at_end)
{
- w->window_end_pos
- = make_number (Z - MATRIX_ROW_END_CHARPOS (last_text_row_at_end));
+ w->window_end_pos = Z - MATRIX_ROW_END_CHARPOS (last_text_row_at_end);
w->window_end_bytepos
= Z_BYTE - MATRIX_ROW_END_BYTEPOS (last_text_row_at_end);
- w->window_end_vpos
- = make_number (MATRIX_ROW_VPOS (last_text_row_at_end, desired_matrix));
+ w->window_end_vpos = MATRIX_ROW_VPOS (last_text_row_at_end, desired_matrix);
+ eassert (w->window_end_pos >= 0);
+ eassert (w->window_end_vpos >= 0);
eassert (w->window_end_bytepos >= 0);
IF_DEBUG (debug_method_add (w, "B"));
}
@@ -17747,12 +17754,12 @@
/* We have displayed either to the end of the window or at the
end of the window, i.e. the last row with text is to be found
in the desired matrix. */
- w->window_end_pos
- = make_number (Z - MATRIX_ROW_END_CHARPOS (last_text_row));
+ w->window_end_pos = Z - MATRIX_ROW_END_CHARPOS (last_text_row);
w->window_end_bytepos
= Z_BYTE - MATRIX_ROW_END_BYTEPOS (last_text_row);
- w->window_end_vpos
- = make_number (MATRIX_ROW_VPOS (last_text_row, desired_matrix));
+ w->window_end_vpos = MATRIX_ROW_VPOS (last_text_row, desired_matrix);
+ eassert (w->window_end_pos >= 0);
+ eassert (w->window_end_vpos >= 0);
eassert (w->window_end_bytepos >= 0);
}
else if (first_unchanged_at_end_row == NULL
@@ -17762,7 +17769,7 @@
/* Displayed to end of window, but no line containing text was
displayed. Lines were deleted at the end of the window. */
int first_vpos = WINDOW_WANTS_HEADER_LINE_P (w) ? 1 : 0;
- int vpos = XFASTINT (w->window_end_vpos);
+ int vpos = w->window_end_vpos;
struct glyph_row *current_row = current_matrix->rows + vpos;
struct glyph_row *desired_row = desired_matrix->rows + vpos;
@@ -17780,20 +17787,22 @@
}
eassert (row != NULL);
- w->window_end_vpos = make_number (vpos + 1);
- w->window_end_pos = make_number (Z - MATRIX_ROW_END_CHARPOS (row));
+ w->window_end_vpos = vpos + 1;
+ w->window_end_pos = Z - MATRIX_ROW_END_CHARPOS (row);
w->window_end_bytepos = Z_BYTE - MATRIX_ROW_END_BYTEPOS (row);
+ eassert (w->window_end_pos >= 0);
+ eassert (w->window_end_vpos >= 0);
eassert (w->window_end_bytepos >= 0);
IF_DEBUG (debug_method_add (w, "C"));
}
else
abort ();
- IF_DEBUG (debug_end_pos = XFASTINT (w->window_end_pos);
- debug_end_vpos = XFASTINT (w->window_end_vpos));
+ IF_DEBUG (debug_end_pos = w->window_end_pos;
+ debug_end_vpos = w->window_end_vpos);
/* Record that display has not been completed. */
- w->window_end_valid = Qnil;
+ w->window_end_valid = 0;
w->desired_matrix->no_scrolling_p = 1;
return 3;
@@ -19134,7 +19143,7 @@
}
/* Is IT->w showing the region? */
- it->w->region_showing = it->region_beg_charpos > 0 ? Qt : Qnil;
+ it->w->region_showing = it->region_beg_charpos > 0;
/* Clear the result glyph row and enable it. */
prepare_desired_row (row);
@@ -21409,7 +21418,7 @@
ptrdiff_t pos = marker_position (w->start);
ptrdiff_t total = BUF_ZV (b) - BUF_BEGV (b);
- if (XFASTINT (w->window_end_pos) <= BUF_Z (b) - BUF_ZV (b))
+ if (w->window_end_pos <= BUF_Z (b) - BUF_ZV (b))
{
if (pos <= BUF_BEGV (b))
return "All";
@@ -21438,7 +21447,7 @@
case 'P':
{
ptrdiff_t toppos = marker_position (w->start);
- ptrdiff_t botpos = BUF_Z (b) - XFASTINT (w->window_end_pos);
+ ptrdiff_t botpos = BUF_Z (b) - w->window_end_pos;
ptrdiff_t total = BUF_ZV (b) - BUF_BEGV (b);
if (botpos >= BUF_ZV (b))
@@ -26312,7 +26321,7 @@
/* Find the rows corresponding to START_CHARPOS and END_CHARPOS. */
rows_from_pos_range (w, start_charpos, end_charpos, disp_string, &r1, &r2);
if (r1 == NULL)
- r1 = MATRIX_ROW (w->current_matrix, XFASTINT (w->window_end_vpos));
+ r1 = MATRIX_ROW (w->current_matrix, w->window_end_vpos);
/* If the before-string or display-string contains newlines,
rows_from_pos_range skips to its last row. Move back. */
if (!NILP (before_string) || !NILP (disp_string))
@@ -26334,7 +26343,7 @@
}
if (r2 == NULL)
{
- r2 = MATRIX_ROW (w->current_matrix, XFASTINT (w->window_end_vpos));
+ r2 = MATRIX_ROW (w->current_matrix, w->window_end_vpos);
hlinfo->mouse_face_past_end = 1;
}
else if (!NILP (after_string))
@@ -26342,7 +26351,7 @@
/* If the after-string has newlines, advance to its last row. */
struct glyph_row *next;
struct glyph_row *last
- = MATRIX_ROW (w->current_matrix, XFASTINT (w->window_end_vpos));
+ = MATRIX_ROW (w->current_matrix, w->window_end_vpos);
for (next = r2 + 1;
next <= last
@@ -27410,7 +27419,7 @@
And verify the buffer's text has not changed. */
b = XBUFFER (w->buffer);
if (part == ON_TEXT
- && EQ (w->window_end_valid, w->buffer)
+ && w->window_end_valid
&& w->last_modified == BUF_MODIFF (b)
&& w->last_overlay_modified == BUF_OVERLAY_MODIFF (b))
{
@@ -27649,7 +27658,7 @@
Lisp_Object lim2 =
NILP (BVAR (XBUFFER (buffer), bidi_display_reordering))
? make_number (BUF_Z (XBUFFER (buffer))
- - XFASTINT (w->window_end_pos))
+ - w->window_end_pos)
: Qnil;
if (NILP (overlay))
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: 'struct window' cleanup #4 2012-07-02 10:45 'struct window' cleanup #4 Dmitry Antipov @ 2012-07-02 14:48 ` Stefan Monnier 2012-07-02 17:07 ` Eli Zaretskii 2012-07-02 17:33 ` Eli Zaretskii 1 sibling, 1 reply; 6+ messages in thread From: Stefan Monnier @ 2012-07-02 14:48 UTC (permalink / raw) To: Dmitry Antipov; +Cc: Emacs development discussions > Do we have a consensus about type(s) of 'window_end_vpos', 'vscroll', > 'hscroll' and 'min_hscroll' fields of 'struct window'? Now I'm just > assuming that ptrdiff_t is safer. Eli, please review if possible. vscroll should be int, as should be the hscroll amounts. The vpos probably as well. `ptrdiff' is for things like buffer-positions (and it's only meaningful for the --with-wide-int compilation option which I tend to consider as a waste of time). Stefan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 'struct window' cleanup #4 2012-07-02 14:48 ` Stefan Monnier @ 2012-07-02 17:07 ` Eli Zaretskii 0 siblings, 0 replies; 6+ messages in thread From: Eli Zaretskii @ 2012-07-02 17:07 UTC (permalink / raw) To: Stefan Monnier; +Cc: dmantipov, emacs-devel > From: Stefan Monnier <monnier@iro.umontreal.ca> > Date: Mon, 02 Jul 2012 10:48:08 -0400 > Cc: Emacs development discussions <emacs-devel@gnu.org> > > vscroll should be int, as should be the hscroll amounts. > The vpos probably as well. I agree, on all 3 counts. Since the corresponding coordinates in pixel units (current_x and current_y from 'struct it'), which are typically about 10 times larger than the above 3, are ints, it doesn't make sense to have these declared as ptrdiff_t. Vertical and horizontal screen positions are generally the size of the Emacs window, i.e. bounded by the display size. (Some internal Emacs display routines simulate a larger window for the purposes of adjusting the displayed area, e.g., while scrolling. But they never go too far outside the display dimensions, because doing so makes the display sluggish, and people complain sooner or later.) Those values are a far cry from being anywhere near INT_MAX. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 'struct window' cleanup #4 2012-07-02 10:45 'struct window' cleanup #4 Dmitry Antipov 2012-07-02 14:48 ` Stefan Monnier @ 2012-07-02 17:33 ` Eli Zaretskii 2012-07-03 5:54 ` 'struct window' cleanup #5 [Re: 'struct window' cleanup #4] Dmitry Antipov 1 sibling, 1 reply; 6+ messages in thread From: Eli Zaretskii @ 2012-07-02 17:33 UTC (permalink / raw) To: Dmitry Antipov; +Cc: emacs-devel > Date: Mon, 02 Jul 2012 14:45:02 +0400 > From: Dmitry Antipov <dmantipov@yandex.ru> > > --- src/dispnew.c 2012-06-28 07:50:50 +0000 > +++ src/dispnew.c 2012-07-02 10:11:48 +0000 > @@ -643,9 +643,8 @@ > > /* Window end is invalid, if inside of the rows that > are invalidated below. */ > - if (INTEGERP (w->window_end_vpos) > - && XFASTINT (w->window_end_vpos) >= i) > - w->window_end_valid = Qnil; > + if (w->window_end_vpos >= i) > + w->window_end_valid = 0; Shouldn't we test w->window_end_valid for being non-zero, before we trust the value of w->window_end_vpos? Zero for a vertical position is perfectly valid, so if i is zero, you can err here. The comment says: /* Glyph matrix row of the last glyph in the current matrix of W. Should be nonnegative, and valid only if window_end_valid is not zero. */ ptrdiff_t window_end_vpos; So I think we should put our money where our mouth is, and do as we say ;-) > @@ -2989,10 +2989,10 @@ > XSETINT (BVAR (b, display_count), XINT (BVAR (b, display_count)) + 1); > BVAR (b, display_time) = Fcurrent_time (); > > - XSETFASTINT (w->window_end_pos, 0); > - XSETFASTINT (w->window_end_vpos, 0); > + w->window_end_pos = 0; > + w->window_end_vpos = 0; > memset (&w->last_cursor, 0, sizeof w->last_cursor); > - w->window_end_valid = Qnil; > + w->window_end_valid = 0; > if (!(keep_margins_p && samebuf)) > { /* If we're not actually changing the buffer, don't reset hscroll and > vscroll. This case happens for example when called from > @@ -3287,8 +3287,6 @@ > w->start = Fmake_marker (); > w->pointm = Fmake_marker (); > w->vertical_scroll_bar_type = Qt; > - XSETFASTINT (w->window_end_pos, 0); > - XSETFASTINT (w->window_end_vpos, 0); > > /* Initialize non-Lisp data. Note that allocate_window zeroes out all > non-Lisp data, so do it only for slots which should not be zero. */ > @@ -3296,6 +3294,8 @@ > w->phys_cursor_type = -1; > w->phys_cursor_width = -1; > w->sequence_number = ++sequence_number; > + w->window_end_pos = -1; > + w->window_end_vpos = -1; If the "invalid" value is going to be -1, then why did you initialize with zero above? > + /* Z - the buffer position of the last glyph in the current matrix of W. > + Should be nonnegative, and valid only if window_end_valid is not zero. */ > + ptrdiff_t window_end_pos; But since you initialize with -1, the "nonnegative" part is only true when window_end_valid is non-zero, right? > + /* Glyph matrix row of the last glyph in the current matrix of W. > + Should be nonnegative, and valid only if window_end_valid is not zero. */ > + ptrdiff_t window_end_vpos; Same here. > @@ -27410,7 +27419,7 @@ > And verify the buffer's text has not changed. */ > b = XBUFFER (w->buffer); > if (part == ON_TEXT > - && EQ (w->window_end_valid, w->buffer) > + && w->window_end_valid Bother: doesn't the comparison with w->buffer test for a specific buffer, rather than just non-nil value? The corresponding assignment, viz.: > if (accurate_p) > { > - w->window_end_valid = w->buffer; > + w->window_end_valid = 1; > w->update_mode_line = 0; > } seems to handle the case when the display is "accurate". Perhaps use a different value, like 2, for this situation? Other than that, looks okay, thanks. ^ permalink raw reply [flat|nested] 6+ messages in thread
* 'struct window' cleanup #5 [Re: 'struct window' cleanup #4] 2012-07-02 17:33 ` Eli Zaretskii @ 2012-07-03 5:54 ` Dmitry Antipov 2012-07-03 15:58 ` Eli Zaretskii 0 siblings, 1 reply; 6+ messages in thread From: Dmitry Antipov @ 2012-07-03 5:54 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel [-- Attachment #1: Type: text/plain, Size: 1531 bytes --] On 07/02/2012 09:33 PM, Eli Zaretskii wrote: > Shouldn't we test w->window_end_valid for being non-zero, before we > trust the value of w->window_end_vpos? Zero for a vertical position > is perfectly valid, so if i is zero, you can err here. The core idea is to have window_end_vpos (and window_end_pos) equal to -1 for just created window (i.e. pointer returned by make_window). For such a window, window_end_valid is always 0, and passing such a window to main redisplay routines is an error (so there are 4 debugging checks commented with "Just initialized windows should not pass here"). If the window is completely initialized, window_end_vpos may be 0 and window_end_valid may be 0 or 1. > If the "invalid" value is going to be -1, then why did you initialize > with zero above? See above. > Bother: doesn't the comparison with w->buffer test for a specific > buffer, rather than just non-nil value? The corresponding assignment, > viz.: > >> if (accurate_p) >> { >> - w->window_end_valid = w->buffer; >> + w->window_end_valid = 1; >> w->update_mode_line = 0; >> } > > seems to handle the case when the display is "accurate". Perhaps use > a different value, like 2, for this situation? I tried a lot, but never hits the situation where window buffer was changed on the way from mark_window_display_accurate_1 to note_mouse_highlight (buffer text may be changed, of course). So I suppose that w->window_end_valid = w->buffer is just a synonym for w->window_end_valid = Qt. Dmitry [-- Attachment #2: window_cleanup_5.patch --] [-- Type: text/plain, Size: 30962 bytes --] === modified file 'src/dispnew.c' --- src/dispnew.c 2012-06-28 07:50:50 +0000 +++ src/dispnew.c 2012-07-03 05:51:57 +0000 @@ -641,11 +641,12 @@ break; } - /* Window end is invalid, if inside of the rows that - are invalidated below. */ - if (INTEGERP (w->window_end_vpos) - && XFASTINT (w->window_end_vpos) >= i) - w->window_end_valid = Qnil; + /* Window end is invalid if the window is not completely + initialized or old glyph matrix row of the last glyph + is larger than the value computed above. */ + if (w->window_end_vpos == -1 + || (w->window_end_vpos > 0 && w->window_end_vpos >= i)) + w->window_end_valid = 0; while (i < matrix->nrows) matrix->rows[i++].enabled_p = 0; @@ -902,7 +903,7 @@ else { clear_glyph_matrix (w->current_matrix); - w->window_end_valid = Qnil; + w->window_end_valid = 0; } } === modified file 'src/window.c' --- src/window.c 2012-06-30 09:13:54 +0000 +++ src/window.c 2012-07-03 04:20:38 +0000 @@ -1307,12 +1307,12 @@ The user can compute it with vertical-motion if he wants to. It would be nicer to do it automatically, but that's so slow that it would probably bother people. */ - if (NILP (w->window_end_valid)) + if (!w->window_end_valid) return Qnil; #endif if (! NILP (update) - && ! (! NILP (w->window_end_valid) + && ! (w->window_end_valid && w->last_modified >= BUF_MODIFF (b) && w->last_overlay_modified >= BUF_OVERLAY_MODIFF (b)) && !noninteractive) @@ -1354,7 +1354,7 @@ set_buffer_internal (old_buffer); } else - XSETINT (value, BUF_Z (b) - XFASTINT (w->window_end_pos)); + XSETINT (value, BUF_Z (b) - w->window_end_pos); return value; } @@ -1507,7 +1507,7 @@ b = XBUFFER (w->buffer); /* Fail if current matrix is not up-to-date. */ - if (NILP (w->window_end_valid) + if (!w->window_end_valid || current_buffer->clip_changed || current_buffer->prevent_redisplay_optimizations_p || w->last_modified < BUF_MODIFF (b) @@ -1841,9 +1841,9 @@ n->phys_cursor_width = -1; n->must_be_updated_p = 0; n->pseudo_window_p = 0; - XSETFASTINT (n->window_end_vpos, 0); - XSETFASTINT (n->window_end_pos, 0); - n->window_end_valid = Qnil; + n->window_end_vpos = 0; + n->window_end_pos = 0; + n->window_end_valid = 0; n->frozen_window_start_p = 0; } @@ -2773,7 +2773,7 @@ pos = *vmotion (startpos, -top, w); set_marker_both (w->start, w->buffer, pos.bufpos, pos.bytepos); - w->window_end_valid = Qnil; + w->window_end_valid = 0; w->start_at_line_beg = (pos.bytepos == BEGV_BYTE || FETCH_BYTE (pos.bytepos - 1) == '\n'); /* We need to do this, so that the window-scroll-functions @@ -2989,10 +2989,10 @@ XSETINT (BVAR (b, display_count), XINT (BVAR (b, display_count)) + 1); BVAR (b, display_time) = Fcurrent_time (); - XSETFASTINT (w->window_end_pos, 0); - XSETFASTINT (w->window_end_vpos, 0); + w->window_end_pos = 0; + w->window_end_vpos = 0; memset (&w->last_cursor, 0, sizeof w->last_cursor); - w->window_end_valid = Qnil; + w->window_end_valid = 0; if (!(keep_margins_p && samebuf)) { /* If we're not actually changing the buffer, don't reset hscroll and vscroll. This case happens for example when called from @@ -3287,8 +3287,6 @@ w->start = Fmake_marker (); w->pointm = Fmake_marker (); w->vertical_scroll_bar_type = Qt; - XSETFASTINT (w->window_end_pos, 0); - XSETFASTINT (w->window_end_vpos, 0); /* Initialize non-Lisp data. Note that allocate_window zeroes out all non-Lisp data, so do it only for slots which should not be zero. */ @@ -3296,6 +3294,8 @@ w->phys_cursor_type = -1; w->phys_cursor_width = -1; w->sequence_number = ++sequence_number; + w->window_end_pos = -1; + w->window_end_vpos = -1; /* Reset window_list. */ Vwindow_list = Qnil; @@ -3762,7 +3762,7 @@ } n->buffer = Qt; - n->window_end_valid = Qnil; + n->window_end_valid = 0; memset (&n->last_cursor, 0, sizeof n->last_cursor); /* Get special geometry settings from reference window. */ @@ -5174,7 +5174,7 @@ /* Set the new window start. */ set_marker_both (w->start, w->buffer, charpos, bytepos); - w->window_end_valid = Qnil; + w->window_end_valid = 0; w->optional_new_start = 1; @@ -6115,7 +6115,7 @@ adjust_window_margins (w); clear_glyph_matrix (w->current_matrix); - w->window_end_valid = Qnil; + w->window_end_valid = 0; ++windows_or_buffers_changed; adjust_glyphs (XFRAME (WINDOW_FRAME (w))); @@ -6184,7 +6184,7 @@ adjust_window_margins (w); clear_glyph_matrix (w->current_matrix); - w->window_end_valid = Qnil; + w->window_end_valid = 0; ++windows_or_buffers_changed; adjust_glyphs (XFRAME (WINDOW_FRAME (w))); === modified file 'src/window.h' --- src/window.h 2012-06-29 11:48:08 +0000 +++ src/window.h 2012-07-03 04:21:21 +0000 @@ -168,18 +168,6 @@ no scroll bar. A value of t means use frame value. */ Lisp_Object vertical_scroll_bar_type; - /* Z - the buffer position of the last glyph in the current matrix - of W. Only valid if WINDOW_END_VALID is not nil. */ - Lisp_Object window_end_pos; - /* Glyph matrix row of the last glyph in the current matrix - of W. Only valid if WINDOW_END_VALID is not nil. */ - Lisp_Object window_end_vpos; - /* t if window_end_pos is truly valid. - This is nil if nontrivial redisplay is preempted - since in that case the frame image that window_end_pos - did not get onto the frame. */ - Lisp_Object window_end_valid; - /* Display-table to use for displaying chars in this window. Nil means use the buffer's own display-table. */ Lisp_Object display_table; @@ -198,10 +186,6 @@ as long as the window shows that buffer. */ Lisp_Object base_line_pos; - /* If we have highlighted the region (or any part of it), - this is the mark position that we used, as an integer. */ - Lisp_Object region_showing; - /* The column number currently displayed in this window's mode line, or nil if column numbers are not being displayed. */ Lisp_Object column_number_displayed; @@ -238,11 +222,11 @@ int sequence_number; /* Number of columns display within the window is scrolled to the left. */ - ptrdiff_t hscroll; + int hscroll; /* Minimum hscroll for automatic hscrolling. This is the value the user has set, by set-window-hscroll for example. */ - ptrdiff_t min_hscroll; + int min_hscroll; /* Displayed buffer's text modification events counter as of last time display completed. */ @@ -256,6 +240,14 @@ it should be positive. */ ptrdiff_t last_point; + /* Z - the buffer position of the last glyph in the current matrix of W. + Should be nonnegative, and valid only if window_end_valid is not zero. */ + ptrdiff_t window_end_pos; + + /* Glyph matrix row of the last glyph in the current matrix of W. + Should be nonnegative, and valid only if window_end_valid is not zero. */ + int window_end_vpos; + /* Scaling factor for the glyph_matrix size calculation in this window. Used if window contains many small images or uses proportional fonts, as the normal may yield a matrix which is too small. */ @@ -289,6 +281,9 @@ was last updated. */ unsigned last_had_star : 1; + /* Non-zero if we have highlighted the region (or any part of it). */ + unsigned region_showing : 1; + /* Non-zero means current value of `start' was the beginning of a line when it was chosen. */ unsigned start_at_line_beg : 1; @@ -331,6 +326,11 @@ Otherwise draw them between margin areas and text. */ unsigned fringes_outside_margins : 1; + /* Non-zero if window_end_pos is truly valid. This is zero if + nontrivial redisplay is preempted since in that case the + frame image that window_end_pos did not get onto the frame. */ + unsigned window_end_valid : 1; + /* Amount by which lines of this window are scrolled in y-direction (smooth scrolling). */ int vscroll; === modified file 'src/xdisp.c' --- src/xdisp.c 2012-06-29 18:52:54 +0000 +++ src/xdisp.c 2012-07-03 04:36:31 +0000 @@ -2504,11 +2504,11 @@ check_window_end (struct window *w) { if (!MINI_WINDOW_P (w) - && !NILP (w->window_end_valid)) + && w->window_end_valid) { struct glyph_row *row; eassert ((row = MATRIX_ROW (w->current_matrix, - XFASTINT (w->window_end_vpos)), + w->window_end_vpos), !row->enabled_p || MATRIX_ROW_DISPLAYS_TEXT_P (row) || MATRIX_ROW_VPOS (row, w->current_matrix) == 0)); @@ -8850,7 +8850,7 @@ && it->current_x == it->last_visible_x - 1 && it->c != '\n' && it->c != '\t' - && it->vpos < XFASTINT (it->w->window_end_vpos)) + && it->vpos < it->w->window_end_vpos) { it->continuation_lines_width += it->current_x; it->current_x = it->hpos = it->max_ascent = it->max_descent = 0; @@ -11202,7 +11202,7 @@ != w->last_had_star) || ((!NILP (Vtransient_mark_mode) && !NILP (BVAR (XBUFFER (w->buffer), mark_active))) - != !NILP (w->region_showing))) + != w->region_showing)) { struct buffer *prev = current_buffer; ptrdiff_t count = SPECPDL_INDEX (); @@ -11400,7 +11400,7 @@ != w->last_had_star) || ((!NILP (Vtransient_mark_mode) && !NILP (BVAR (XBUFFER (w->buffer), mark_active))) - != !NILP (w->region_showing))) + != w->region_showing)) { struct buffer *prev = current_buffer; ptrdiff_t count = SPECPDL_INDEX (); @@ -12331,7 +12331,7 @@ && (w->cursor.x >= text_area_width - h_margin)))))) { struct it it; - ptrdiff_t hscroll; + int hscroll; struct buffer *saved_current_buffer; ptrdiff_t pt; int wanted_x; @@ -12766,7 +12766,7 @@ reconsider_clip_changes (struct window *w, struct buffer *b) { if (b->clip_changed - && !NILP (w->window_end_valid) + && w->window_end_valid && w->current_matrix->buffer == b && w->current_matrix->zv == BUF_ZV (b) && w->current_matrix->begv == BUF_BEGV (b)) @@ -12778,7 +12778,7 @@ b->clip_changed has already been set to 1, we can skip this check. */ if (!b->clip_changed - && BUFFERP (w->buffer) && !NILP (w->window_end_valid)) + && BUFFERP (w->buffer) && w->window_end_valid) { ptrdiff_t pt; @@ -13103,12 +13103,9 @@ /* If showing the region, and mark has changed, we must redisplay the whole window. The assignment to this_line_start_pos prevents the optimization directly below this if-statement. */ - if (((!NILP (Vtransient_mark_mode) - && !NILP (BVAR (XBUFFER (w->buffer), mark_active))) - != !NILP (w->region_showing)) - || (!NILP (w->region_showing) - && !EQ (w->region_showing, - Fmarker_position (BVAR (XBUFFER (w->buffer), mark))))) + if (w->region_showing + || (!NILP (Vtransient_mark_mode) + && !NILP (BVAR (XBUFFER (w->buffer), mark_active)))) CHARPOS (this_line_start_pos) = 0; /* Optimize the case that only the line containing the cursor in the @@ -13224,13 +13221,13 @@ adjusted. */ if ((it.glyph_row - 1)->displays_text_p) { - if (XFASTINT (w->window_end_vpos) < this_line_vpos) - XSETINT (w->window_end_vpos, this_line_vpos); + if (w->window_end_vpos < this_line_vpos) + w->window_end_vpos = this_line_vpos; } - else if (XFASTINT (w->window_end_vpos) == this_line_vpos + else if (w->window_end_vpos == this_line_vpos && this_line_vpos > 0) - XSETINT (w->window_end_vpos, this_line_vpos - 1); - w->window_end_valid = Qnil; + w->window_end_vpos = this_line_vpos - 1; + w->window_end_valid = 0; /* Update hint: No need to try to scroll in update_window. */ w->desired_matrix->no_scrolling_p = 1; @@ -13276,7 +13273,7 @@ && (EQ (selected_window, BVAR (current_buffer, last_selected_window)) || highlight_nonselected_windows) - && NILP (w->region_showing) + && !w->region_showing && NILP (Vshow_trailing_whitespace) && !cursor_in_echo_area) { @@ -13695,7 +13692,7 @@ if (accurate_p) { - w->window_end_valid = w->buffer; + w->window_end_valid = 1; w->update_mode_line = 0; } } @@ -14935,6 +14932,10 @@ ptrdiff_t, thus zero means invalid position in a buffer. */ eassert (w->last_point > 0); + /* Just initialized windows should not pass here. */ + eassert (w->window_end_pos >= 0); + eassert (w->window_end_vpos >= 0); + /* Handle case where text has not changed, only point, and it has not moved off the frame. */ if (/* Point may be in this window. */ @@ -14953,7 +14954,7 @@ set the cursor. */ && !(!NILP (Vtransient_mark_mode) && !NILP (BVAR (current_buffer, mark_active))) - && NILP (w->region_showing) + && !w->region_showing && NILP (Vshow_trailing_whitespace) /* This code is not used for mini-buffer for the sake of the case of redisplaying to replace an echo area message; since in @@ -14962,13 +14963,9 @@ since the handling of this_line_start_pos, etc., in redisplay handles the same cases. */ && !EQ (window, minibuf_window) - /* When splitting windows or for new windows, it happens that - redisplay is called with a nil window_end_vpos or one being - larger than the window. This should really be fixed in - window.c. I don't have this on my list, now, so we do - approximately the same as the old redisplay code. --gerd. */ - && INTEGERP (w->window_end_vpos) - && XFASTINT (w->window_end_vpos) < w->current_matrix->nrows + /* Previously, there was Gerd's comment suggesting + to fix window.c. I believe that it is done. */ + && w->window_end_vpos < w->current_matrix->nrows && (FRAME_WINDOW_P (f) || !overlay_arrow_in_current_buffer_p ())) { @@ -15280,7 +15277,7 @@ start = marker_position (w->start) - BUF_BEGV (buf); /* I don't think this is guaranteed to be right. For the moment, we'll pretend it is. */ - end = BUF_Z (buf) - XFASTINT (w->window_end_pos) - BUF_BEGV (buf); + end = BUF_Z (buf) - w->window_end_pos - BUF_BEGV (buf); if (end < start) end = start; @@ -15390,7 +15387,7 @@ set_buffer_internal_1 (XBUFFER (w->buffer)); current_matrix_up_to_date_p - = (!NILP (w->window_end_valid) + = (w->window_end_valid && !current_buffer->clip_changed && !current_buffer->prevent_redisplay_optimizations_p && w->last_modified >= MODIFF @@ -15414,7 +15411,7 @@ specbind (Qinhibit_point_motion_hooks, Qt); buffer_unchanged_p - = (!NILP (w->window_end_valid) + = (w->window_end_valid && !current_buffer->clip_changed && w->last_modified >= MODIFF && w->last_overlay_modified >= OVERLAY_MODIFF); @@ -15428,7 +15425,7 @@ if (XMARKER (w->start)->buffer == current_buffer) compute_window_start_on_continuation_line (w); - w->window_end_valid = Qnil; + w->window_end_valid = 0; } /* Some sanity checks. */ @@ -15539,7 +15536,7 @@ w->force_start = 0; w->vscroll = 0; - w->window_end_valid = Qnil; + w->window_end_valid = 0; /* Forget any recorded base line for line number display. */ if (!buffer_unchanged_p) @@ -15962,8 +15959,8 @@ line.) */ if (w->cursor.vpos < 0) { - if (!NILP (w->window_end_valid) - && PT >= Z - XFASTINT (w->window_end_pos)) + if (w->window_end_valid + && PT >= Z - w->window_end_pos) { clear_glyph_matrix (w->desired_matrix); move_it_by_lines (&it, 1); @@ -16202,6 +16199,10 @@ struct glyph_row *last_text_row = NULL; struct frame *f = XFRAME (w->frame); + /* Just initialized windows should not pass here. */ + eassert (w->window_end_pos >= 0); + eassert (w->window_end_vpos >= 0); + /* Make POS the new window start. */ set_marker_both (w->start, Qnil, CHARPOS (pos), BYTEPOS (pos)); @@ -16252,7 +16253,7 @@ } /* If bottom moved off end of frame, change mode line percentage. */ - if (XFASTINT (w->window_end_pos) <= 0 + if (w->window_end_pos <= 0 && Z != IT_CHARPOS (it)) w->update_mode_line = 1; @@ -16264,22 +16265,22 @@ eassert (MATRIX_ROW_DISPLAYS_TEXT_P (last_text_row)); w->window_end_bytepos = Z_BYTE - MATRIX_ROW_END_BYTEPOS (last_text_row); - w->window_end_pos - = make_number (Z - MATRIX_ROW_END_CHARPOS (last_text_row)); - w->window_end_vpos - = make_number (MATRIX_ROW_VPOS (last_text_row, w->desired_matrix)); - eassert (MATRIX_ROW (w->desired_matrix, XFASTINT (w->window_end_vpos)) + w->window_end_pos = Z - MATRIX_ROW_END_CHARPOS (last_text_row); + w->window_end_vpos = MATRIX_ROW_VPOS (last_text_row, w->desired_matrix); + eassert (w->window_end_pos >= 0); + eassert (w->window_end_vpos >= 0); + eassert (MATRIX_ROW (w->desired_matrix, w->window_end_vpos) ->displays_text_p); } else { w->window_end_bytepos = Z_BYTE - ZV_BYTE; - w->window_end_pos = make_number (Z - ZV); - w->window_end_vpos = make_number (0); + w->window_end_pos = Z - ZV; + w->window_end_vpos = 0; } /* But that is not valid info until redisplay finishes. */ - w->window_end_valid = Qnil; + w->window_end_valid = 0; return 1; } @@ -16313,6 +16314,10 @@ return 0; #endif + /* Just initialized windows should not pass here. */ + eassert (w->window_end_pos >= 0); + eassert (w->window_end_vpos >= 0); + if (/* This function doesn't handle terminal frames. */ !FRAME_WINDOW_P (f) /* Don't try to reuse the display if windows have been split @@ -16324,7 +16329,7 @@ /* Can't do this if region may have changed. */ if ((!NILP (Vtransient_mark_mode) && !NILP (BVAR (current_buffer, mark_active))) - || !NILP (w->region_showing) + || w->region_showing || !NILP (Vshow_trailing_whitespace)) return 0; @@ -16503,29 +16508,28 @@ { w->window_end_bytepos = Z_BYTE - MATRIX_ROW_END_BYTEPOS (last_reused_text_row); - w->window_end_pos - = make_number (Z - MATRIX_ROW_END_CHARPOS (last_reused_text_row)); - w->window_end_vpos - = make_number (MATRIX_ROW_VPOS (last_reused_text_row, - w->current_matrix)); + w->window_end_pos = Z - MATRIX_ROW_END_CHARPOS (last_reused_text_row); + w->window_end_vpos = MATRIX_ROW_VPOS (last_reused_text_row, w->current_matrix); + eassert (w->window_end_pos >= 0); + eassert (w->window_end_vpos >= 0); } else if (last_text_row) { w->window_end_bytepos = Z_BYTE - MATRIX_ROW_END_BYTEPOS (last_text_row); - w->window_end_pos - = make_number (Z - MATRIX_ROW_END_CHARPOS (last_text_row)); - w->window_end_vpos - = make_number (MATRIX_ROW_VPOS (last_text_row, w->desired_matrix)); + w->window_end_pos = Z - MATRIX_ROW_END_CHARPOS (last_text_row); + w->window_end_vpos = MATRIX_ROW_VPOS (last_text_row, w->desired_matrix); + eassert (w->window_end_pos >= 0); + eassert (w->window_end_vpos >= 0); } else { /* This window must be completely empty. */ w->window_end_bytepos = Z_BYTE - ZV_BYTE; - w->window_end_pos = make_number (Z - ZV); - w->window_end_vpos = make_number (0); + w->window_end_pos = Z - ZV; + w->window_end_vpos = 0; } - w->window_end_valid = Qnil; + w->window_end_valid = 0; /* Update hint: don't try scrolling again in update_window. */ w->desired_matrix->no_scrolling_p = 1; @@ -16706,18 +16710,16 @@ { w->window_end_bytepos = Z_BYTE - MATRIX_ROW_END_BYTEPOS (last_text_row); - w->window_end_pos - = make_number (Z - MATRIX_ROW_END_CHARPOS (last_text_row)); - w->window_end_vpos - = make_number (MATRIX_ROW_VPOS (last_text_row, w->desired_matrix)); + w->window_end_pos = Z - MATRIX_ROW_END_CHARPOS (last_text_row); + w->window_end_vpos = MATRIX_ROW_VPOS (last_text_row, w->desired_matrix); } else - { - w->window_end_vpos - = make_number (XFASTINT (w->window_end_vpos) - nrows_scrolled); - } - - w->window_end_valid = Qnil; + w->window_end_vpos = w->window_end_vpos - nrows_scrolled; + + eassert (w->window_end_pos >= 0); + eassert (w->window_end_vpos >= 0); + + w->window_end_valid = 0; w->desired_matrix->no_scrolling_p = 1; #ifdef GLYPH_DEBUG @@ -16850,16 +16852,16 @@ /* Display must not have been paused, otherwise the current matrix is not up to date. */ - eassert (!NILP (w->window_end_valid)); + eassert (w->window_end_valid); /* A value of window_end_pos >= END_UNCHANGED means that the window end is in the range of changed text. If so, there is no unchanged row at the end of W's current matrix. */ - if (XFASTINT (w->window_end_pos) >= END_UNCHANGED) + if (w->window_end_pos >= END_UNCHANGED) return NULL; /* Set row to the last row in W's current matrix displaying text. */ - row = MATRIX_ROW (w->current_matrix, XFASTINT (w->window_end_vpos)); + row = MATRIX_ROW (w->current_matrix, w->window_end_vpos); /* If matrix is entirely empty, no unchanged row exists. */ if (MATRIX_ROW_DISPLAYS_TEXT_P (row)) @@ -16870,7 +16872,7 @@ buffer positions in the current matrix to current buffer positions for characters not in changed text. */ ptrdiff_t Z_old = - MATRIX_ROW_END_CHARPOS (row) + XFASTINT (w->window_end_pos); + MATRIX_ROW_END_CHARPOS (row) + w->window_end_pos; ptrdiff_t Z_BYTE_old = MATRIX_ROW_END_BYTEPOS (row) + w->window_end_bytepos; ptrdiff_t last_unchanged_pos, last_unchanged_pos_old; @@ -17104,6 +17106,10 @@ #define GIVE_UP(X) return 0 #endif + /* Just initialized windows should not pass here. */ + eassert (w->window_end_pos >= 0); + eassert (w->window_end_vpos >= 0); + SET_TEXT_POS_FROM_MARKER (start, w->start); /* Don't use this for mini-windows because these can show @@ -17142,7 +17148,7 @@ GIVE_UP (7); /* Verify that display wasn't paused. */ - if (NILP (w->window_end_valid)) + if (!w->window_end_valid) GIVE_UP (8); /* Can't use this if highlighting a region because a cursor movement @@ -17156,7 +17162,7 @@ GIVE_UP (11); /* Likewise if showing a region. */ - if (!NILP (w->region_showing)) + if (w->region_showing) GIVE_UP (10); /* Can't use this if overlay arrow position and/or string have @@ -17206,7 +17212,7 @@ This case happens with stealth-fontification. Note that although the display is unchanged, glyph positions in the matrix have to be adjusted, of course. */ - row = MATRIX_ROW (w->current_matrix, XFASTINT (w->window_end_vpos)); + row = MATRIX_ROW (w->current_matrix, w->window_end_vpos); if (MATRIX_ROW_DISPLAYS_TEXT_P (row) && ((last_changed_charpos < CHARPOS (start) && CHARPOS (start) == BEGV) @@ -17218,7 +17224,7 @@ /* Compute how many chars/bytes have been added to or removed from the buffer. */ - Z_old = MATRIX_ROW_END_CHARPOS (row) + XFASTINT (w->window_end_pos); + Z_old = MATRIX_ROW_END_CHARPOS (row) + w->window_end_pos; Z_BYTE_old = MATRIX_ROW_END_BYTEPOS (row) + w->window_end_bytepos; Z_delta = Z - Z_old; Z_delta_bytes = Z_BYTE - Z_BYTE_old; @@ -17289,8 +17295,8 @@ { /* We have to compute the window end anew since text could have been added/removed after it. */ - w->window_end_pos - = make_number (Z - MATRIX_ROW_END_CHARPOS (row)); + w->window_end_pos = Z - MATRIX_ROW_END_CHARPOS (row); + eassert (w->window_end_pos >= 0); w->window_end_bytepos = Z_BYTE - MATRIX_ROW_END_BYTEPOS (row); @@ -17325,7 +17331,7 @@ /* Give up if the window ends in strings. Overlay strings at the end are difficult to handle, so don't try. */ - row = MATRIX_ROW (current_matrix, XFASTINT (w->window_end_vpos)); + row = MATRIX_ROW (current_matrix, w->window_end_vpos); if (MATRIX_ROW_START_CHARPOS (row) == MATRIX_ROW_END_CHARPOS (row)) GIVE_UP (20); @@ -17668,7 +17674,7 @@ /* Set last_row to the glyph row in the current matrix where the window end line is found. It has been moved up or down in the matrix by dvpos. */ - int last_vpos = XFASTINT (w->window_end_vpos) + dvpos; + ptrdiff_t last_vpos = w->window_end_vpos + dvpos; struct glyph_row *last_row = MATRIX_ROW (current_matrix, last_vpos); /* If last_row is the window end line, it should display text. */ @@ -17724,21 +17730,22 @@ first_unchanged_at_end_row); eassert (row && MATRIX_ROW_DISPLAYS_TEXT_P (row)); - w->window_end_pos = make_number (Z - MATRIX_ROW_END_CHARPOS (row)); + w->window_end_pos = Z - MATRIX_ROW_END_CHARPOS (row); w->window_end_bytepos = Z_BYTE - MATRIX_ROW_END_BYTEPOS (row); - w->window_end_vpos - = make_number (MATRIX_ROW_VPOS (row, w->current_matrix)); + w->window_end_vpos = MATRIX_ROW_VPOS (row, w->current_matrix); + eassert (w->window_end_pos >= 0); + eassert (w->window_end_vpos >= 0); eassert (w->window_end_bytepos >= 0); IF_DEBUG (debug_method_add (w, "A")); } else if (last_text_row_at_end) { - w->window_end_pos - = make_number (Z - MATRIX_ROW_END_CHARPOS (last_text_row_at_end)); + w->window_end_pos = Z - MATRIX_ROW_END_CHARPOS (last_text_row_at_end); w->window_end_bytepos = Z_BYTE - MATRIX_ROW_END_BYTEPOS (last_text_row_at_end); - w->window_end_vpos - = make_number (MATRIX_ROW_VPOS (last_text_row_at_end, desired_matrix)); + w->window_end_vpos = MATRIX_ROW_VPOS (last_text_row_at_end, desired_matrix); + eassert (w->window_end_pos >= 0); + eassert (w->window_end_vpos >= 0); eassert (w->window_end_bytepos >= 0); IF_DEBUG (debug_method_add (w, "B")); } @@ -17747,12 +17754,12 @@ /* We have displayed either to the end of the window or at the end of the window, i.e. the last row with text is to be found in the desired matrix. */ - w->window_end_pos - = make_number (Z - MATRIX_ROW_END_CHARPOS (last_text_row)); + w->window_end_pos = Z - MATRIX_ROW_END_CHARPOS (last_text_row); w->window_end_bytepos = Z_BYTE - MATRIX_ROW_END_BYTEPOS (last_text_row); - w->window_end_vpos - = make_number (MATRIX_ROW_VPOS (last_text_row, desired_matrix)); + w->window_end_vpos = MATRIX_ROW_VPOS (last_text_row, desired_matrix); + eassert (w->window_end_pos >= 0); + eassert (w->window_end_vpos >= 0); eassert (w->window_end_bytepos >= 0); } else if (first_unchanged_at_end_row == NULL @@ -17762,7 +17769,7 @@ /* Displayed to end of window, but no line containing text was displayed. Lines were deleted at the end of the window. */ int first_vpos = WINDOW_WANTS_HEADER_LINE_P (w) ? 1 : 0; - int vpos = XFASTINT (w->window_end_vpos); + int vpos = w->window_end_vpos; struct glyph_row *current_row = current_matrix->rows + vpos; struct glyph_row *desired_row = desired_matrix->rows + vpos; @@ -17780,20 +17787,22 @@ } eassert (row != NULL); - w->window_end_vpos = make_number (vpos + 1); - w->window_end_pos = make_number (Z - MATRIX_ROW_END_CHARPOS (row)); + w->window_end_vpos = vpos + 1; + w->window_end_pos = Z - MATRIX_ROW_END_CHARPOS (row); w->window_end_bytepos = Z_BYTE - MATRIX_ROW_END_BYTEPOS (row); + eassert (w->window_end_pos >= 0); + eassert (w->window_end_vpos >= 0); eassert (w->window_end_bytepos >= 0); IF_DEBUG (debug_method_add (w, "C")); } else abort (); - IF_DEBUG (debug_end_pos = XFASTINT (w->window_end_pos); - debug_end_vpos = XFASTINT (w->window_end_vpos)); + IF_DEBUG (debug_end_pos = w->window_end_pos; + debug_end_vpos = w->window_end_vpos); /* Record that display has not been completed. */ - w->window_end_valid = Qnil; + w->window_end_valid = 0; w->desired_matrix->no_scrolling_p = 1; return 3; @@ -19134,7 +19143,7 @@ } /* Is IT->w showing the region? */ - it->w->region_showing = it->region_beg_charpos > 0 ? Qt : Qnil; + it->w->region_showing = it->region_beg_charpos > 0; /* Clear the result glyph row and enable it. */ prepare_desired_row (row); @@ -21409,7 +21418,7 @@ ptrdiff_t pos = marker_position (w->start); ptrdiff_t total = BUF_ZV (b) - BUF_BEGV (b); - if (XFASTINT (w->window_end_pos) <= BUF_Z (b) - BUF_ZV (b)) + if (w->window_end_pos <= BUF_Z (b) - BUF_ZV (b)) { if (pos <= BUF_BEGV (b)) return "All"; @@ -21438,7 +21447,7 @@ case 'P': { ptrdiff_t toppos = marker_position (w->start); - ptrdiff_t botpos = BUF_Z (b) - XFASTINT (w->window_end_pos); + ptrdiff_t botpos = BUF_Z (b) - w->window_end_pos; ptrdiff_t total = BUF_ZV (b) - BUF_BEGV (b); if (botpos >= BUF_ZV (b)) @@ -26312,7 +26321,7 @@ /* Find the rows corresponding to START_CHARPOS and END_CHARPOS. */ rows_from_pos_range (w, start_charpos, end_charpos, disp_string, &r1, &r2); if (r1 == NULL) - r1 = MATRIX_ROW (w->current_matrix, XFASTINT (w->window_end_vpos)); + r1 = MATRIX_ROW (w->current_matrix, w->window_end_vpos); /* If the before-string or display-string contains newlines, rows_from_pos_range skips to its last row. Move back. */ if (!NILP (before_string) || !NILP (disp_string)) @@ -26334,7 +26343,7 @@ } if (r2 == NULL) { - r2 = MATRIX_ROW (w->current_matrix, XFASTINT (w->window_end_vpos)); + r2 = MATRIX_ROW (w->current_matrix, w->window_end_vpos); hlinfo->mouse_face_past_end = 1; } else if (!NILP (after_string)) @@ -26342,7 +26351,7 @@ /* If the after-string has newlines, advance to its last row. */ struct glyph_row *next; struct glyph_row *last - = MATRIX_ROW (w->current_matrix, XFASTINT (w->window_end_vpos)); + = MATRIX_ROW (w->current_matrix, w->window_end_vpos); for (next = r2 + 1; next <= last @@ -27410,7 +27419,7 @@ And verify the buffer's text has not changed. */ b = XBUFFER (w->buffer); if (part == ON_TEXT - && EQ (w->window_end_valid, w->buffer) + && w->window_end_valid && w->last_modified == BUF_MODIFF (b) && w->last_overlay_modified == BUF_OVERLAY_MODIFF (b)) { @@ -27649,7 +27658,7 @@ Lisp_Object lim2 = NILP (BVAR (XBUFFER (buffer), bidi_display_reordering)) ? make_number (BUF_Z (XBUFFER (buffer)) - - XFASTINT (w->window_end_pos)) + - w->window_end_pos) : Qnil; if (NILP (overlay)) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 'struct window' cleanup #5 [Re: 'struct window' cleanup #4] 2012-07-03 5:54 ` 'struct window' cleanup #5 [Re: 'struct window' cleanup #4] Dmitry Antipov @ 2012-07-03 15:58 ` Eli Zaretskii 0 siblings, 0 replies; 6+ messages in thread From: Eli Zaretskii @ 2012-07-03 15:58 UTC (permalink / raw) To: Dmitry Antipov; +Cc: emacs-devel > Date: Tue, 03 Jul 2012 09:54:56 +0400 > From: Dmitry Antipov <dmantipov@yandex.ru> > CC: emacs-devel@gnu.org > > > Shouldn't we test w->window_end_valid for being non-zero, before we > > trust the value of w->window_end_vpos? Zero for a vertical position > > is perfectly valid, so if i is zero, you can err here. > > The core idea is to have window_end_vpos (and window_end_pos) equal to -1 > for just created window (i.e. pointer returned by make_window). For such > a window, window_end_valid is always 0, and passing such a window to > main redisplay routines is an error (so there are 4 debugging checks > commented with "Just initialized windows should not pass here"). If the > window is completely initialized, window_end_vpos may be 0 and > window_end_valid may be 0 or 1. AFAICT, window_end_valid and window_end_vpos have nothing to do with initializing windows. They are flags used by the display engine in order to know when certain redisplay optimizations are possible. So I don't see how a completely initialized window has anything to do with the fact that window_end_vpos is valid. Am I missing something? But if what you write is true, the commentary to window_end_vpos and window_end_pos should be modified, because right now the comments tell explicitly not to trust the values of these two if window_end_valid is zero. If not every use of those two must test window_end_valid first, then we should explain in the comments when such a test is needed. > > Bother: doesn't the comparison with w->buffer test for a specific > > buffer, rather than just non-nil value? The corresponding assignment, > > viz.: > > > >> if (accurate_p) > >> { > >> - w->window_end_valid = w->buffer; > >> + w->window_end_valid = 1; > >> w->update_mode_line = 0; > >> } > > > > seems to handle the case when the display is "accurate". Perhaps use > > a different value, like 2, for this situation? > > I tried a lot, but never hits the situation where window buffer was changed > on the way from mark_window_display_accurate_1 to note_mouse_highlight (buffer > text may be changed, of course). So I suppose that w->window_end_valid = w->buffer > is just a synonym for w->window_end_valid = Qt. I'm not convinced, sorry. note_mouse_highlight is usually invoked asynchronously (when the mouse hovers above some mouse-sensitive part of the display), so deliberately producing a situation could be tricky. The fact that you didn't hit that combination doesn't necessarily prove it is impossible. So I would still suggest to use a special value, because bugs in these areas are notoriously hard to track down. Thanks. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-07-03 15:58 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-07-02 10:45 'struct window' cleanup #4 Dmitry Antipov 2012-07-02 14:48 ` Stefan Monnier 2012-07-02 17:07 ` Eli Zaretskii 2012-07-02 17:33 ` Eli Zaretskii 2012-07-03 5:54 ` 'struct window' cleanup #5 [Re: 'struct window' cleanup #4] Dmitry Antipov 2012-07-03 15:58 ` 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).