From a85cde3870d61cb9cc6662e5ec6245f474146f7e Mon Sep 17 00:00:00 2001 From: Eli Zaretskii Date: Fri, 28 May 2010 15:00:47 +0300 Subject: [PATCH 1/2] Prevent unnecessary scrolling in bidi-reordered buffers. xdisp.c (try_cursor_movement): back up to non-continuation line only after finding point's row.. --- src/ChangeLog | 5 +++++ src/xdisp.c | 58 +++++++++++++++++++++++++++------------------------ 2 files changed, 36 insertions(+), 27 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index eeee6540e68..a530e844bf9 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,8 @@ +2010-05-28 Eli Zaretskii + + * xdisp.c (try_cursor_movement): Prevent unnecessary scrolling in + bidi-reordered buffers. + 2010-05-28 Kenichi Handa * font.c (font_delete_unmatched): Check Vface_ignored_fonts. diff --git a/src/xdisp.c b/src/xdisp.c index 31fa5f39d5f..a94049a780b 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -13763,32 +13763,6 @@ try_cursor_movement (window, startp, scroll_step) ++row; if (!row->enabled_p) rc = CURSOR_MOVEMENT_MUST_SCROLL; - /* If rows are bidi-reordered, back up until we find a row - that does not belong to a continuation line. This is - because we must consider all rows of a continued line as - candidates for cursor positioning, since row start and - end positions change non-linearly with vertical position - in such rows. */ - /* FIXME: Revisit this when glyph ``spilling'' in - continuation lines' rows is implemented for - bidi-reordered rows. */ - if (!NILP (XBUFFER (w->buffer)->bidi_display_reordering)) - { - while (MATRIX_ROW_CONTINUATION_LINE_P (row)) - { - xassert (row->enabled_p); - --row; - /* If we hit the beginning of the displayed portion - without finding the first row of a continued - line, give up. */ - if (row <= w->current_matrix->rows) - { - rc = CURSOR_MOVEMENT_MUST_SCROLL; - break; - } - - } - } } if (rc == CURSOR_MOVEMENT_CANNOT_BE_USED) @@ -13888,7 +13862,37 @@ try_cursor_movement (window, startp, scroll_step) rc = CURSOR_MOVEMENT_MUST_SCROLL; } else if (rc != CURSOR_MOVEMENT_SUCCESS - && MATRIX_ROW_PARTIALLY_VISIBLE_P (w, row) + && !NILP (XBUFFER (w->buffer)->bidi_display_reordering)) + { + /* If rows are bidi-reordered and point moved, back up + until we find a row that does not belong to a + continuation line. This is because we must consider + all rows of a continued line as candidates for the + new cursor positioning, since row start and end + positions change non-linearly with vertical position + in such rows. */ + /* FIXME: Revisit this when glyph ``spilling'' in + continuation lines' rows is implemented for + bidi-reordered rows. */ + while (MATRIX_ROW_CONTINUATION_LINE_P (row)) + { + xassert (row->enabled_p); + --row; + /* If we hit the beginning of the displayed portion + without finding the first row of a continued + line, give up. */ + if (row <= w->current_matrix->rows) + { + rc = CURSOR_MOVEMENT_CANNOT_BE_USED; + break; + } + + } + } + if (rc == CURSOR_MOVEMENT_SUCCESS + || rc == CURSOR_MOVEMENT_CANNOT_BE_USED) + ; + else if (MATRIX_ROW_PARTIALLY_VISIBLE_P (w, row) && make_cursor_line_fully_visible_p) { if (PT == MATRIX_ROW_END_CHARPOS (row) From 6d26bbb23ef6751cfcd7066c4d4006ec18408449 Mon Sep 17 00:00:00 2001 From: Eli Zaretskii Date: Sat, 29 May 2010 13:49:21 +0300 Subject: [PATCH 2/2] Finish debugging of cursor motion in bidi-reordered lines. xdisp.c (try_cursor_movement): Fix the logic. Rewrite the loop over continuation lines in bidi-reordered buffers. Return CURSOR_MOVEMENT_MUST_SCROLL upon failure to find a suitable row, rather than CURSOR_MOVEMENT_CANNOT_BE_USED. --- src/ChangeLog | 11 +++++++++-- src/xdisp.c | 39 +++++++++++++++++++++------------------ 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index a530e844bf9..814b30bbdad 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,7 +1,14 @@ +2010-05-29 Eli Zaretskii + + * xdisp.c (try_cursor_movement): Fix the logic. Rewrite the loop + over continuation lines in bidi-reordered buffers. Return + CURSOR_MOVEMENT_MUST_SCROLL upon failure to find a suitable row, + rather than CURSOR_MOVEMENT_CANNOT_BE_USED. + 2010-05-28 Eli Zaretskii - * xdisp.c (try_cursor_movement): Prevent unnecessary scrolling in - bidi-reordered buffers. + * xdisp.c (try_cursor_movement): Backup to non-continuation line + only after finding point's row. 2010-05-28 Kenichi Handa diff --git a/src/xdisp.c b/src/xdisp.c index a94049a780b..c8043308ec8 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -13767,7 +13767,7 @@ try_cursor_movement (window, startp, scroll_step) if (rc == CURSOR_MOVEMENT_CANNOT_BE_USED) { - int scroll_p = 0; + int scroll_p = 0, must_scroll = 0; int last_y = window_text_bottom_y (w) - this_scroll_margin; if (PT > XFASTINT (w->last_point)) @@ -13860,6 +13860,7 @@ try_cursor_movement (window, startp, scroll_step) { /* if PT is not in the glyph row, give up. */ rc = CURSOR_MOVEMENT_MUST_SCROLL; + must_scroll = 1; } else if (rc != CURSOR_MOVEMENT_SUCCESS && !NILP (XBUFFER (w->buffer)->bidi_display_reordering)) @@ -13883,17 +13884,17 @@ try_cursor_movement (window, startp, scroll_step) line, give up. */ if (row <= w->current_matrix->rows) { - rc = CURSOR_MOVEMENT_CANNOT_BE_USED; + rc = CURSOR_MOVEMENT_MUST_SCROLL; break; } } } - if (rc == CURSOR_MOVEMENT_SUCCESS - || rc == CURSOR_MOVEMENT_CANNOT_BE_USED) + if (must_scroll) ; - else if (MATRIX_ROW_PARTIALLY_VISIBLE_P (w, row) - && make_cursor_line_fully_visible_p) + else if (rc != CURSOR_MOVEMENT_SUCCESS + && MATRIX_ROW_PARTIALLY_VISIBLE_P (w, row) + && make_cursor_line_fully_visible_p) { if (PT == MATRIX_ROW_END_CHARPOS (row) && !row->ends_at_zv_p @@ -13919,7 +13920,8 @@ try_cursor_movement (window, startp, scroll_step) } else if (scroll_p) rc = CURSOR_MOVEMENT_MUST_SCROLL; - else if (!NILP (XBUFFER (w->buffer)->bidi_display_reordering)) + else if (rc != CURSOR_MOVEMENT_SUCCESS + && !NILP (XBUFFER (w->buffer)->bidi_display_reordering)) { /* With bidi-reordered rows, there could be more than one candidate row whose start and end positions @@ -13932,8 +13934,11 @@ try_cursor_movement (window, startp, scroll_step) do { - rv |= set_cursor_from_row (w, row, w->current_matrix, - 0, 0, 0, 0); + if (MATRIX_ROW_START_CHARPOS (row) <= PT + && PT <= MATRIX_ROW_END_CHARPOS (row) + && cursor_row_p (w, row)) + rv |= set_cursor_from_row (w, row, w->current_matrix, + 0, 0, 0, 0); /* As soon as we've found the first suitable row whose ends_at_zv_p flag is set, we are done. */ if (rv @@ -13944,19 +13949,17 @@ try_cursor_movement (window, startp, scroll_step) } ++row; } - while (MATRIX_ROW_BOTTOM_Y (row) < last_y - && MATRIX_ROW_START_CHARPOS (row) <= PT - && PT <= MATRIX_ROW_END_CHARPOS (row) - && cursor_row_p (w, row)); + while ((MATRIX_ROW_CONTINUATION_LINE_P (row) + && MATRIX_ROW_BOTTOM_Y (row) <= last_y) + || (MATRIX_ROW_START_CHARPOS (row) == PT + && MATRIX_ROW_BOTTOM_Y (row) < last_y)); /* If we didn't find any candidate rows, or exited the loop before all the candidates were examined, signal to the caller that this method failed. */ if (rc != CURSOR_MOVEMENT_SUCCESS - && (!rv - || (MATRIX_ROW_START_CHARPOS (row) <= PT - && PT <= MATRIX_ROW_END_CHARPOS (row)))) - rc = CURSOR_MOVEMENT_CANNOT_BE_USED; - else + && (!rv || MATRIX_ROW_CONTINUATION_LINE_P (row))) + rc = CURSOR_MOVEMENT_MUST_SCROLL; + else if (rv) rc = CURSOR_MOVEMENT_SUCCESS; } else