From: Eli Zaretskii Date: Mon, 16 Dec 2013 18:09:36 +0000 (+0200) Subject: A better fix for bug #16148 and related issues. X-Git-Tag: emacs-24.3.90~173^2^2~42^2~45^2~387^2~385 X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=397e886f3470343dc6bb9bb14776c2de5254e0a9;p=emacs.git A better fix for bug #16148 and related issues. src/xdisp.c (Fmove_point_visually): Fix subtle bugs in the fallback code, revealed in presence of R2L characters, character compositions, and display vectors. src/dispextern.h (struct composition_it): Correct a comment for the 'width' member. --- diff --git a/src/ChangeLog b/src/ChangeLog index f71e4dcbb80..ae154994da1 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,12 @@ +2013-12-16 Eli Zaretskii + + * xdisp.c (Fmove_point_visually): Fix subtle bugs in the fallback + code, revealed in presence of R2L characters, character + compositions, and display vectors. A better fix for Bug#16148. + + * dispextern.h (struct composition_it): Correct a comment for the + 'width' member. + 2013-12-16 Paul Eggert * font.h (valid_font_driver) [!ENABLE_CHECKING]: Define a dummy. diff --git a/src/dispextern.h b/src/dispextern.h index d3ee2472dc6..06177af1e3d 100644 --- a/src/dispextern.h +++ b/src/dispextern.h @@ -2190,9 +2190,8 @@ struct composition_it int nchars, nbytes; /* Indices of the glyphs for the current grapheme cluster. */ int from, to; - /* Width of the current grapheme cluster in units of pixels on a - graphic display and in units of canonical characters on a - terminal display. */ + /* Width of the current grapheme cluster in units of columns it will + occupy on display; see CHAR_WIDTH. */ int width; }; diff --git a/src/xdisp.c b/src/xdisp.c index 450bf5c62dc..a0332a16503 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -20589,23 +20589,37 @@ Value is the new character position of point. */) SET_TEXT_POS (pt, PT, PT_BYTE); start_display (&it, w, pt); - if ((it.cmp_it.id < 0 - && it.method == GET_FROM_STRING - && it.area == TEXT_AREA - && it.string_from_display_prop_p - && (it.sp > 0 && it.stack[it.sp - 1].method == GET_FROM_BUFFER)) - || it.method == GET_FROM_DISPLAY_VECTOR) + if (it.cmp_it.id < 0 + && it.method == GET_FROM_STRING + && it.area == TEXT_AREA + && it.string_from_display_prop_p + && (it.sp > 0 && it.stack[it.sp - 1].method == GET_FROM_BUFFER)) overshoot_expected = true; /* Find the X coordinate of point. We start from the beginning of this or previous line to make sure we are before point in the logical order (since the move_it_* functions can only move forward). */ + reseat: reseat_at_previous_visible_line_start (&it); it.current_x = it.hpos = it.current_y = it.vpos = 0; if (IT_CHARPOS (it) != PT) - move_it_to (&it, overshoot_expected ? PT - 1 : PT, - -1, -1, -1, MOVE_TO_POS); + { + move_it_to (&it, overshoot_expected ? PT - 1 : PT, + -1, -1, -1, MOVE_TO_POS); + /* If we missed point because the character there is + displayed out of a display vector that has more than one + glyph, retry expecting overshoot. */ + if (it.method == GET_FROM_DISPLAY_VECTOR + && it.current.dpvec_index > 0 + && !overshoot_expected) + { + overshoot_expected = true; + goto reseat; + } + else if (IT_CHARPOS (it) != PT && !overshoot_expected) + move_it_in_display_line (&it, PT, -1, MOVE_TO_POS); + } pt_x = it.current_x; pt_vpos = it.vpos; if (dir > 0 || overshoot_expected) @@ -20634,9 +20648,9 @@ Value is the new character position of point. */) else if (pixel_width <= 0) pixel_width = 1; - /* If there's a display string at point, we are actually at the - glyph to the left of point, so we need to correct the X - coordinate. */ + /* If there's a display string (or something similar) at point, + we are actually at the glyph to the left of point, so we need + to correct the X coordinate. */ if (overshoot_expected) { if (it.bidi_p) @@ -20699,15 +20713,37 @@ Value is the new character position of point. */) character at point. */ if (FRAME_WINDOW_P (it.f) && dir < 0) { - struct text_pos new_pos = it.current.pos; + struct text_pos new_pos; enum move_it_result rc = MOVE_X_REACHED; + if (it.current_x == 0) + get_next_display_element (&it); + if (it.what == IT_COMPOSITION) + { + new_pos.charpos = it.cmp_it.charpos; + new_pos.bytepos = -1; + } + else + new_pos = it.current.pos; + while (it.current_x + it.pixel_width <= target_x && rc == MOVE_X_REACHED) { int new_x = it.current_x + it.pixel_width; - new_pos = it.current.pos; + /* For composed characters, we want the position of the + first character in the grapheme cluster (usually, the + composition's base character), whereas it.current + might give us the position of the _last_ one, e.g. if + the composition is rendered in reverse due to bidi + reordering. */ + if (it.what == IT_COMPOSITION) + { + new_pos.charpos = it.cmp_it.charpos; + new_pos.bytepos = -1; + } + else + new_pos = it.current.pos; if (new_x == it.current_x) new_x++; rc = move_it_in_display_line_to (&it, ZV, new_x, @@ -20715,21 +20751,10 @@ Value is the new character position of point. */) if (ITERATOR_AT_END_OF_LINE_P (&it) && !target_is_eol_p) break; } - /* If we ended up on a composed character inside - bidi-reordered text (e.g., Hebrew text with diacritics), - the iterator gives us the buffer position of the last (in - logical order) character of the composed grapheme cluster, - which is not what we want. So we cheat: we compute the - character position of the character that follows (in the - logical order) the one where the above loop stopped. That - character will appear on display to the left of point. */ - if (it.bidi_p - && it.bidi_it.scan_dir == -1 - && new_pos.charpos - IT_CHARPOS (it) > 1) - { - new_pos.charpos = IT_CHARPOS (it) + 1; - new_pos.bytepos = CHAR_TO_BYTE (new_pos.charpos); - } + /* The previous position we saw in the loop is the one we + want. */ + if (new_pos.bytepos == -1) + new_pos.bytepos = CHAR_TO_BYTE (new_pos.charpos); it.current.pos = new_pos; } else