# HG changeset patch # User Eli Zaretskii # Date 1262293768 18000 # Node ID 0ff1b8888f6b34e9191464cfe88a5efb66633c58 # Parent cbca7f94b05721951d86f9bfea601f0f10b84d7f Retrospective commit from 2009-08-12. An (unsuccessful) attempt to solve the issue with row->start and row->end. xdisp.c (set_iterator_to_next, reseat, reseat_1) (reseat_at_next_visible_line_start): Accept additional argument force_logical_p; all callers changed. If force_logical_p is non-zero, force iteration in buffer's logical order even in bidi buffers. dispnew.c (direct_output_for_insert): Call set_iterator_to_next with additional argument zero. dispextern.h (set_iterator_to_next): Now accepts an additional argument. diff -r cbca7f94b057 -r 0ff1b8888f6b src/ChangeLog.bidi --- a/src/ChangeLog.bidi Thu Dec 31 15:56:07 2009 -0500 +++ b/src/ChangeLog.bidi Thu Dec 31 16:09:28 2009 -0500 @@ -1,3 +1,29 @@ +2009-09-12 Eli Zaretskii + + * dispnew.c (direct_output_for_insert): Give up if we are + reordering bidirectional text. + + * dispextern.h (IT_STACK_SIZE): Enlarge to 5. + + * xdisp.c (display_line): Set row->end and it->start for the next + row to the next character in logical order. If we are reordering + bidi text, push and pop the iterator before and after momentarily + iterating in logical order. + +2009-09-11 Eli Zaretskii + + * xdisp.c (set_iterator_to_next, reseat, reseat_1) + (reseat_at_next_visible_line_start): Accept additional argument + force_logical_p; all callers changed. If force_logical_p is + non-zero, force iteration in buffer's logical order even in bidi + buffers. + + * dispnew.c (direct_output_for_insert): Call set_iterator_to_next + with additional argument zero. + + * dispextern.h (set_iterator_to_next): Now accepts an additional + argument. + 2009-08-29 Eli Zaretskii * xdisp.c (set_cursor_from_row): Don't assume glyph->charpos diff -r cbca7f94b057 -r 0ff1b8888f6b src/dispextern.h --- a/src/dispextern.h Thu Dec 31 15:56:07 2009 -0500 +++ b/src/dispextern.h Thu Dec 31 16:09:28 2009 -0500 @@ -1927,7 +1927,7 @@ NUM_IT_METHODS }; -#define IT_STACK_SIZE 4 +#define IT_STACK_SIZE 5 /* Iterator for composition (both for static and automatic). */ struct composition_it @@ -2825,7 +2825,7 @@ void init_iterator_to_row_start P_ ((struct it *, struct window *, struct glyph_row *)); int get_next_display_element P_ ((struct it *)); -void set_iterator_to_next P_ ((struct it *, int)); +void set_iterator_to_next P_ ((struct it *, int, int)); void start_display P_ ((struct it *, struct window *, struct text_pos)); void move_it_to P_ ((struct it *, int, int, int, int, int)); void move_it_vertically P_ ((struct it *, int)); diff -r cbca7f94b057 -r 0ff1b8888f6b src/dispnew.c --- a/src/dispnew.c Thu Dec 31 15:56:07 2009 -0500 +++ b/src/dispnew.c Thu Dec 31 16:09:28 2009 -0500 @@ -1760,6 +1760,8 @@ /* Check that end position of `row' is equal to start position of next row. */ + /* WARNING: This assumption is blatantly incorrect when we are + reordering bdirectional text for display!! */ if (next->enabled_p && MATRIX_ROW_DISPLAYS_TEXT_P (next)) { xassert (MATRIX_ROW_END_CHARPOS (row) @@ -3500,6 +3502,8 @@ || !display_completed /* Give up if buffer appears in two places. */ || buffer_shared > 1 + /* Give up if we need to reorder bidirectional text. */ + || !NILP (current_buffer->bidi_display_reordering) /* Give up if currently displaying a message instead of the minibuffer contents. */ || (EQ (selected_window, minibuf_window) @@ -3608,7 +3612,7 @@ delta += 1; delta_bytes += it.len; - set_iterator_to_next (&it, 1); + set_iterator_to_next (&it, 1, 0); } /* Give up if we hit the right edge of the window. We would have @@ -3626,7 +3630,7 @@ { if (it2.c == '\t') return 0; - set_iterator_to_next (&it2, 1); + set_iterator_to_next (&it2, 1, 0); } /* Number of new glyphs produced. */ diff -r cbca7f94b057 -r 0ff1b8888f6b src/xdisp.c --- a/src/xdisp.c Thu Dec 31 15:56:07 2009 -0500 +++ b/src/xdisp.c Thu Dec 31 16:09:28 2009 -0500 @@ -963,11 +963,11 @@ static int get_overlay_strings P_ ((struct it *, int)); static int get_overlay_strings_1 P_ ((struct it *, int, int)); static void next_overlay_string P_ ((struct it *)); -static void reseat P_ ((struct it *, struct text_pos, int)); -static void reseat_1 P_ ((struct it *, struct text_pos, int)); +static void reseat P_ ((struct it *, struct text_pos, int, int)); +static void reseat_1 P_ ((struct it *, struct text_pos, int, int)); static void back_to_previous_visible_line_start P_ ((struct it *)); void reseat_at_previous_visible_line_start P_ ((struct it *)); -static void reseat_at_next_visible_line_start P_ ((struct it *, int)); +static void reseat_at_next_visible_line_start P_ ((struct it *, int, int)); static int next_element_from_ellipsis P_ ((struct it *)); static int next_element_from_display_vector P_ ((struct it *)); static int next_element_from_string P_ ((struct it *)); @@ -2823,7 +2823,7 @@ it->start = it->current; /* Compute faces etc. */ - reseat (it, it->current.pos, 1); + reseat (it, it->current.pos, 1, 0); } CHECK_IT (it); @@ -2883,7 +2883,7 @@ if (it->current.dpvec_index >= 0 || it->current.overlay_string_index >= 0) { - set_iterator_to_next (it, 1); + set_iterator_to_next (it, 1, 0); move_it_in_display_line_to (it, -1, -1, 0); } @@ -5242,7 +5242,7 @@ && it->c == '\n' && CHARPOS (it->position) == IT_CHARPOS (*it)) { - set_iterator_to_next (it, 0); + set_iterator_to_next (it, 0, 0); it->c = 0; return 1; } @@ -5263,7 +5263,7 @@ if (!get_next_display_element (it)) return 0; newline_found_p = it->what == IT_CHARACTER && it->c == '\n'; - set_iterator_to_next (it, 0); + set_iterator_to_next (it, 0, 0); } /* If we didn't find a newline near enough, see if we can use a @@ -5296,7 +5296,7 @@ && !newline_found_p) { newline_found_p = ITERATOR_AT_END_OF_LINE_P (it); - set_iterator_to_next (it, 0); + set_iterator_to_next (it, 0, 0); } } } @@ -5397,22 +5397,24 @@ struct it *it; { back_to_previous_visible_line_start (it); - reseat (it, it->current.pos, 1); + reseat (it, it->current.pos, 1, 0); CHECK_IT (it); } /* Reseat iterator IT on the next visible line start in the current buffer. ON_NEWLINE_P non-zero means position IT on the newline - preceding the line start. Skip over invisible text that is so - because of selective display. Compute faces, overlays etc at the - new position. Note that this function does not skip over text that - is invisible because of text properties. */ - -static void -reseat_at_next_visible_line_start (it, on_newline_p) - struct it *it; - int on_newline_p; + preceding the line start. FORCE_LOGICAL_P non-zero means force + iteration in logical order even if we are reordering bidirectional + text. Skip over invisible text that is so because of selective + display. Compute faces, overlays etc at the new position. Note + that this function does not skip over text that is invisible + because of text properties. */ + +static void +reseat_at_next_visible_line_start (it, on_newline_p, force_logical_p) + struct it *it; + int on_newline_p, force_logical_p; { int newline_found_p, skipped_p = 0; @@ -5443,13 +5445,16 @@ } else if (IT_CHARPOS (*it) > BEGV) { + if (on_newline_p + && !(force_logical_p || !it->bidi_p)) + abort (); --IT_CHARPOS (*it); --IT_BYTEPOS (*it); - reseat (it, it->current.pos, 0); + reseat (it, it->current.pos, 0, 1); } } else if (skipped_p) - reseat (it, it->current.pos, 0); + reseat (it, it->current.pos, 0, force_logical_p); CHECK_IT (it); } @@ -5463,17 +5468,19 @@ /* Change IT's current position to POS in current_buffer. If FORCE_P is non-zero, always check for text properties at the new position. Otherwise, text properties are only looked up if POS >= - IT->check_charpos of a property. */ - -static void -reseat (it, pos, force_p) + IT->check_charpos of a property. If FORCE_LOGICAL_P is non-zero, + force iteration in logical order even when reordering bidirectional + text. */ + +static void +reseat (it, pos, force_p, force_logical_p) struct it *it; struct text_pos pos; - int force_p; + int force_p, force_logical_p; { int original_pos = IT_CHARPOS (*it); - reseat_1 (it, pos, 0); + reseat_1 (it, pos, 0, force_logical_p); /* Determine where to check text properties. Avoid doing it where possible because text property lookup is very expensive. */ @@ -5487,13 +5494,15 @@ /* Change IT's buffer position to POS. SET_STOP_P non-zero means set - IT->stop_pos to POS, also. */ - -static void -reseat_1 (it, pos, set_stop_p) + IT->stop_pos to POS, also. FORCE_LOGICAL_P non-zero means force + iteration in logical order even when reordering bidirectional + text. */ + +static void +reseat_1 (it, pos, set_stop_p, force_logical_p) struct it *it; struct text_pos pos; - int set_stop_p; + int set_stop_p, force_logical_p; { /* Don't call this function when scanning a C string. */ xassert (it->s == NULL); @@ -5518,7 +5527,7 @@ it->string_from_display_prop_p = 0; it->face_before_selective_p = 0; - if (it->bidi_p) + if (it->bidi_p && !force_logical_p) { /* FIXME: L2R below is just for easyness of testing, as we currently support only left-to-right paragraphs. The value @@ -5729,7 +5738,7 @@ } else { - set_iterator_to_next (it, 0); + set_iterator_to_next (it, 0, 0); } goto get_next; } @@ -6051,6 +6060,9 @@ RESEAT_P non-zero means if called on a newline in buffer text, skip to the next visible line start. + FORCE_LOGICAL_P non-zero means force iteration in logical order + even when reordering bidirectional text. + Functions get_next_display_element and set_iterator_to_next are separate because I find this arrangement easier to handle than a get_next_display_element function that also increments IT's @@ -6062,9 +6074,9 @@ decrement position function which would not be easy to write. */ void -set_iterator_to_next (it, reseat_p) - struct it *it; - int reseat_p; +set_iterator_to_next (it, reseat_p, force_logical_p) + struct it *it; + int reseat_p, force_logical_p; { /* Reset flags indicating start and end of a sequence of characters with box. Reset them at the start of this function because @@ -6078,7 +6090,7 @@ current_buffer. Advance in the buffer, and maybe skip over invisible lines that are so because of selective display. */ if (ITERATOR_AT_END_OF_LINE_P (it) && reseat_p) - reseat_at_next_visible_line_start (it, 0); + reseat_at_next_visible_line_start (it, 0, force_logical_p); else if (it->cmp_it.id >= 0) { IT_CHARPOS (*it) += it->cmp_it.nchars; @@ -6097,7 +6109,7 @@ { xassert (it->len != 0); - if (! it->bidi_p) + if (!(it->bidi_p && !force_logical_p)) { IT_BYTEPOS (*it) += it->len; IT_CHARPOS (*it) += 1; @@ -6148,14 +6160,14 @@ /* Skip over characters which were displayed via IT->dpvec. */ if (it->dpvec_char_len < 0) - reseat_at_next_visible_line_start (it, 1); + reseat_at_next_visible_line_start (it, 1, 1); else if (it->dpvec_char_len > 0) { if (it->method == GET_FROM_STRING && it->n_overlay_strings > 0) it->ignore_overlay_strings_at_pos_p = 1; it->len = it->dpvec_char_len; - set_iterator_to_next (it, reseat_p); + set_iterator_to_next (it, reseat_p, 0); } /* Maybe recheck faces after display vector */ @@ -6461,7 +6473,7 @@ it->saved_face_id = it->face_id; it->method = GET_FROM_BUFFER; it->object = it->w->buffer; - reseat_at_next_visible_line_start (it, 1); + reseat_at_next_visible_line_start (it, 1, 1); it->face_before_selective_p = 1; } @@ -6849,7 +6861,7 @@ if (it->area != TEXT_AREA) { - set_iterator_to_next (it, 1); + set_iterator_to_next (it, 1, 0); continue; } @@ -6957,7 +6969,7 @@ } } - set_iterator_to_next (it, 1); + set_iterator_to_next (it, 1, 0); /* On graphical terminals, newlines may "overflow" into the fringe if overflow-newline-into-fringe is non-nil. @@ -7053,7 +7065,7 @@ /* The current display element has been consumed. Advance to the next. */ - set_iterator_to_next (it, 1); + set_iterator_to_next (it, 1, 0); /* Stop if lines are truncated and IT's current x-position is past the right edge of the window now. */ @@ -7298,13 +7310,13 @@ goto out; case MOVE_NEWLINE_OR_CR: - set_iterator_to_next (it, 1); + set_iterator_to_next (it, 1, 0); it->continuation_lines_width = 0; break; case MOVE_LINE_TRUNCATED: it->continuation_lines_width = 0; - reseat_at_next_visible_line_start (it, 0); + reseat_at_next_visible_line_start (it, 0, 0); if ((op & MOVE_TO_POS) != 0 && IT_CHARPOS (*it) > to_charpos) { @@ -7330,7 +7342,7 @@ { line_start_x = it->current_x + it->pixel_width - it->last_visible_x; - set_iterator_to_next (it, 0); + set_iterator_to_next (it, 0, 0); } } else @@ -7416,7 +7428,7 @@ reseat to skip forward over invisible text, set up the iterator to deliver from overlay strings at the new position etc. So, use reseat_1 here. */ - reseat_1 (it, it->current.pos, 1); + reseat_1 (it, it->current.pos, 1, 0); /* We are now surely at a line start. */ it->current_x = it->hpos = 0; @@ -7546,7 +7558,7 @@ rc = move_it_in_display_line_to (it, Z, 0, MOVE_TO_POS); if (rc == MOVE_NEWLINE_OR_CR) - set_iterator_to_next (it, 0); + set_iterator_to_next (it, 0, 0); } @@ -7575,7 +7587,7 @@ pos = *vmotion (IT_CHARPOS (*it), dvpos, it->w); SET_TEXT_POS (textpos, pos.bufpos, pos.bytepos); - reseat (it, textpos, 1); + reseat (it, textpos, 1, 0); it->vpos += pos.vpos; it->current_y += pos.vpos; } @@ -7611,7 +7623,7 @@ start_charpos = IT_CHARPOS (*it); for (i = -dvpos; i > 0 && IT_CHARPOS (*it) > BEGV; --i) back_to_previous_visible_line_start (it); - reseat (it, it->current.pos, 1); + reseat (it, it->current.pos, 1, 0); /* Move further back if we end up in a string or an image. */ while (!IT_POS_VALID_AFTER_MOVE_P (it)) @@ -7625,7 +7637,7 @@ /* If start of line is still in string or image, move further back. */ back_to_previous_visible_line_start (it); - reseat (it, it->current.pos, 1); + reseat (it, it->current.pos, 1, 0); dvpos--; } @@ -10226,7 +10238,7 @@ if (ITERATOR_AT_END_OF_LINE_P (it)) break; - set_iterator_to_next (it, 1); + set_iterator_to_next (it, 1, 0); } out:; @@ -16548,6 +16560,7 @@ int wrap_row_used = -1, wrap_row_ascent, wrap_row_height; int wrap_row_phys_ascent, wrap_row_phys_height; int wrap_row_extra_line_spacing; + struct display_pos row_end; /* We always start displaying at hpos zero even if hscrolled. */ xassert (it->hpos == 0 && it->current_x == 0); @@ -16636,6 +16649,7 @@ it->continuation_lines_width = 0; row->ends_at_zv_p = 1; + row_end = it->current; break; } @@ -16685,7 +16699,7 @@ it->max_phys_ascent + it->max_phys_descent); row->extra_line_spacing = max (row->extra_line_spacing, it->max_extra_line_spacing); - set_iterator_to_next (it, 1); + set_iterator_to_next (it, 1, 0); continue; } @@ -16764,7 +16778,7 @@ || IT_DISPLAYING_WHITESPACE (it))) goto back_to_wrap; - set_iterator_to_next (it, 1); + set_iterator_to_next (it, 1, 0); if (IT_OVERFLOW_NEWLINE_INTO_FRINGE (it)) { if (!get_next_display_element (it)) @@ -16870,6 +16884,7 @@ it->max_phys_descent = phys_descent; } + row_end = it->current; break; } else if (new_x > it->first_visible_x) @@ -16903,7 +16918,10 @@ /* End of this display line if row is continued. */ if (row->continued_p || row->ends_at_zv_p) - break; + { + row_end = it->current; + break; + } } at_end_of_line: @@ -16929,14 +16947,26 @@ row->glyphs[TEXT_AREA]->charpos = CHARPOS (it->position); /* Consume the line end. This skips over invisible lines. */ - set_iterator_to_next (it, 1); + if (it->bidi_p) + { + /* When we are reordering bidi text, we still need the + next character in logical order, to set row->end + correctly below. */ + push_it (it); + set_iterator_to_next (it, 1, 1); + row_end = it->current; + pop_it (it); + } + set_iterator_to_next (it, 1, 0); it->continuation_lines_width = 0; + if (!it->bidi_p) + row_end = it->current; break; } /* Proceed with next display element. Note that this skips over lines invisible because of selective display. */ - set_iterator_to_next (it, 1); + set_iterator_to_next (it, 1, 0); /* If we truncate lines, we are done when the last displayed glyphs reach past the right margin of the window. */ @@ -16968,6 +16998,7 @@ it->continuation_lines_width = 0; row->ends_at_zv_p = 1; row->exact_window_width_line_p = 1; + row_end = it->current; break; } if (ITERATOR_AT_END_OF_LINE_P (it)) @@ -16979,10 +17010,11 @@ row->truncated_on_right_p = 1; it->continuation_lines_width = 0; - reseat_at_next_visible_line_start (it, 0); + reseat_at_next_visible_line_start (it, 0, 0); row->ends_at_zv_p = FETCH_BYTE (IT_BYTEPOS (*it) - 1) != '\n'; it->hpos = hpos_before; it->current_x = x_before; + row_end = it->current; break; } } @@ -17043,7 +17075,7 @@ compute_line_metrics (it); /* Remember the position at which this line ends. */ - row->end = it->current; + row->end = row_end; /* Record whether this row ends inside an ellipsis. */ row->ends_in_ellipsis_p @@ -17080,7 +17112,7 @@ it->current_y += row->height; ++it->vpos; ++it->glyph_row; - it->start = it->current; + it->start = row_end; return row->displays_text_p; } @@ -19024,7 +19056,7 @@ break; } - set_iterator_to_next (it, 1); + set_iterator_to_next (it, 1, 0); /* Stop if truncating at the right edge. */ if (it->line_wrap == TRUNCATE