Retrospective commit from 2009-09-26.

Continued working on initialization.
Started working on paragraph direction initialization.

 bidi.c (bidi_paragraph_init): Don't set bidi_it->ch_len.  Abort
 if called not at beginning of a new paragraph.
 (bidi_get_next_char_visually): Prepare and use a sentinel iterator
 state when first_elt flag is set.
 dispextern.h (struct bidi_it): New struct member first_elt.
 bidi.c (bidi_init_it): Initialize bidi_it->first_elt.
 (bidi_copy_it): Don't copy the first_elt flag.
 xdisp.c (reseat_1): Initialize bidi_it.first_elt.  Move bidi
 scan start code from here...
 (next_element_from_buffer): ...to here.  Use bidi_it.first_elt flag.

4 files changed, 100 insertions, 55 deletions

diff --git a/src/ChangeLog.bidi b/src/ChangeLog.bidi
index bcdb1ad309a..bc7a473af49 100644
--- a/src/ChangeLog.bidi
+++ b/src/ChangeLog.bidi
@@ -1,3 +1,20 @@
+2009-09-26  Eli Zaretskii  <eliz@gnu.org>
+
+        * bidi.c (bidi_paragraph_init): Don't set bidi_it->ch_len.  Abort
+        if called not at beginning of a new paragraph.
+        (bidi_get_next_char_visually): Prepare and use a sentinel iterator
+        state when first_elt flag is set.
+
+        * dispextern.h (struct bidi_it): New struct member first_elt.
+
+        * bidi.c (bidi_init_it): Initialize bidi_it->first_elt.
+        (bidi_copy_it): Don't copy the first_elt flag.
+
+        * xdisp.c (reseat_1): Initialize bidi_it.first_elt.  Move bidi
+        scan start code from here...
+        (next_element_from_buffer): ...to here.  Use bidi_it.first_elt
+        flag.
+
 2009-09-20  Eli Zaretskii  <eliz@gnu.org>
 
         * xdisp.c (reseat_1): Handle position < BEGV.
diff --git a/src/bidi.c b/src/bidi.c
index b146d562c37..bcbbb485e1a 100644
--- a/src/bidi.c
+++ b/src/bidi.c
@@ -482,10 +482,14 @@ bidi_mirror_char (int c)
 static inline void
 bidi_copy_it (struct bidi_it *to, struct bidi_it *from)
 {
+  int save_first_elt = to->first_elt;
   int i;
 
   /* Copy everything except the level stack.  */
   memcpy (to, from, ((int)&((struct bidi_it *)0)->level_stack[0]));
+  to->first_elt = save_first_elt;
+  if (to->first_elt != 0 && to->first_elt != 1)
+    to->first_elt = 0;
 
   /* Copy the active part of the level stack.  */
   to->level_stack[0] = from->level_stack[0]; /* level zero is always in use */
@@ -739,14 +743,21 @@ void
 bidi_paragraph_init (bidi_dir_t dir, struct bidi_it *bidi_it)
 {
   int pos = bidi_it->charpos, bytepos = bidi_it->bytepos;
-  int ch;
+  int ch, ch_len;
 
-  /* We should never be called at EOB.  */
-  if (bytepos >= ZV_BYTE)
+  /* We should never be called at EOB or before BEGV.  */
+  if (bytepos >= ZV_BYTE || bytepos < BEGV_BYTE)
+    abort ();
+
+  /* We should always be called at the beginning of a new
+     paragraph.  */
+  if (!(bytepos == BEGV_BYTE
+        || FETCH_CHAR (bytepos) == '\n'
+        || FETCH_CHAR (bytepos - 1) == '\n'))
     abort ();
 
   ch = FETCH_CHAR (bytepos);
-  bidi_it->ch_len = CHAR_BYTES (ch);
+  ch_len = CHAR_BYTES (ch);
   bidi_it->level_stack[0].level = 0; /* default for L2R */
   if (dir == R2L)
     bidi_it->level_stack[0].level = 1;
@@ -758,7 +769,7 @@ bidi_paragraph_init (bidi_dir_t dir, struct bidi_it *bidi_it)
          start the loop below from there, since UAX#9 says to find the
          first strong directional character in the paragraph.  */
 
-      for (type = bidi_get_type (ch), pos++, bytepos += bidi_it->ch_len;
+      for (type = bidi_get_type (ch), pos++, bytepos += ch_len;
            /* NOTE: UAX#9 says to search only for L, AL, or R types of
               characters, and ignore RLE, RLO, LRE, and LRO.  However,
               I'm not sure it makes sense to omit those 4; should try
@@ -808,6 +819,7 @@ bidi_init_it (int charpos, int bytepos, struct bidi_it *bidi_it)
   if (! bidi_initialized)
     bidi_initialize ();
   bidi_set_paragraph_end (bidi_it);
+  bidi_it->first_elt = 1;
   bidi_it->charpos = charpos;
   bidi_it->bytepos = bytepos;
   bidi_it->type = NEUTRAL_B;
@@ -908,12 +920,15 @@ bidi_resolve_explicit_1 (struct bidi_it *bidi_it)
   int new_level;
   bidi_dir_t override;
 
-  if (bidi_it->bytepos < BEGV_BYTE) /* after reseat to BEGV */
+  if (bidi_it->bytepos < BEGV_BYTE      /* after reseat to BEGV? */
+      || bidi_it->first_elt)
     {
-      bidi_it->charpos = BEGV;
-      bidi_it->bytepos = BEGV_BYTE;
+      bidi_it->first_elt = 0;
+      if (bidi_it->charpos < BEGV)
+        bidi_it->charpos = BEGV;
+      bidi_it->bytepos = CHAR_TO_BYTE (bidi_it->charpos);
     }
-  else if (bidi_it->bytepos < ZV_BYTE) /* don't move at ZV */
+  else if (bidi_it->bytepos < ZV_BYTE)  /* don't move at ZV */
     {
       bidi_it->charpos++;
       if (bidi_it->ch_len == 0)
@@ -1712,7 +1727,7 @@ void
 bidi_get_next_char_visually (struct bidi_it *bidi_it)
 {
   int old_level, new_level, next_level;
-  struct bidi_it prev_bidi_it;
+  struct bidi_it sentinel;
 
   if (bidi_it->scan_dir == 0)
     {
@@ -1721,8 +1736,18 @@ bidi_get_next_char_visually (struct bidi_it *bidi_it)
 
   if (bidi_it->new_paragraph)
     bidi_paragraph_init (bidi_overriding_paragraph_direction, bidi_it);
+  /* Prepare the sentinel iterator state.  */
   if (bidi_cache_idx == 0)
-    bidi_copy_it (&prev_bidi_it, bidi_it);
+    {
+      bidi_copy_it (&sentinel, bidi_it);
+      if (bidi_it->first_elt)
+        {
+          sentinel.charpos--;   /* cached charpos needs to be monotonic */
+          sentinel.bytepos--;
+          sentinel.ch = '\n';   /* doesn't matter, but why not? */
+          sentinel.ch_len = 1;
+        }
+    }
 
   old_level = bidi_it->resolved_level;
   new_level = bidi_level_of_next_char (bidi_it);
@@ -1740,10 +1765,10 @@ bidi_get_next_char_visually (struct bidi_it *bidi_it)
       int expected_next_level = old_level + incr;
 
       /* If we don't have anything cached yet, we need to cache the
-         previous character we've seen, since we'll need it to record
-         where to jump when the last non-base level is exhausted.  */
+         sentinel state, since we'll need it to record where to jump
+         when the last non-base level is exhausted.  */
       if (bidi_cache_idx == 0)
-        bidi_cache_iterator_state (&prev_bidi_it, 1);
+        bidi_cache_iterator_state (&sentinel, 1);
       /* Jump (or walk) to the other edge of this level.  */
       bidi_find_other_level_edge (bidi_it, level_to_search, !ascending);
       /* Switch scan direction and peek at the next character in the
diff --git a/src/dispextern.h b/src/dispextern.h
index d4f572227a3..389d0acc23c 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -739,14 +739,18 @@ struct glyph_row
   /* First position in this row.  This is the text position, including
      overlay position information etc, where the display of this row
      started, and can thus be less the position of the first glyph
-     (e.g. due to invisible text or horizontal scrolling).  */
+     (e.g. due to invisible text or horizontal scrolling).  BIDI Note:
+     This is the smallest character position in the row, i.e. not
+     necessarily the character that is displayed the leftmost.  */
   struct display_pos start;
 
   /* Text position at the end of this row.  This is the position after
      the last glyph on this row.  It can be greater than the last
      glyph position + 1, due to truncation, invisible text etc.  In an
      up-to-date display, this should always be equal to the start
-     position of the next row.  */
+     position of the next row.  BIDI Note: this is the character whose
+     buffer position is the largest, not necessarily the one displayed
+     the rightmost.  */
   struct display_pos end;
 
   /* Non-zero means the overlay arrow bitmap is on this line.
@@ -924,12 +928,18 @@ struct glyph_row *matrix_row P_ ((struct glyph_matrix *, int));
      (MATRIX_ROW ((MATRIX), (ROW))->used[TEXT_AREA])
 
 /* Return the character/ byte position at which the display of ROW
-   starts.  */
+   starts.  BIDI Note: this is the smallest character/byte position
+   among characters in ROW, i.e. the first logical-order character
+   displayed by ROW, which is not necessarily the smallest horizontal
+   position.  */
 
 #define MATRIX_ROW_START_CHARPOS(ROW) ((ROW)->start.pos.charpos)
 #define MATRIX_ROW_START_BYTEPOS(ROW) ((ROW)->start.pos.bytepos)
 
-/* Return the character/ byte position at which ROW ends.  */
+/* Return the character/ byte position at which ROW ends.  BIDI Note:
+   this is the largest character/byte position among characters in
+   ROW, i.e. the last logical-order character displayed by ROW, which
+   is not necessarily the largest horizontal position.  */
 
 #define MATRIX_ROW_END_CHARPOS(ROW) ((ROW)->end.pos.charpos)
 #define MATRIX_ROW_END_BYTEPOS(ROW) ((ROW)->end.pos.bytepos)
@@ -1750,26 +1760,27 @@ struct bidi_stack {
 
 /* Data type for iterating over bidi text.  */
 struct bidi_it {
+  int first_elt;                /* if non-zero, examine current char first */
   int bytepos;                  /* iterator's position in buffer */
   int charpos;
-  int ch;                       /* the character itself */
-  int ch_len;                   /* the length of its multibyte sequence */
+  int ch;                       /* character itself */
+  int ch_len;                   /* length of its multibyte sequence */
   bidi_type_t type;             /* bidi type of this character, after
                                    resolving weak and neutral types */
   bidi_type_t type_after_w1;    /* original type, after overrides and W1 */
   bidi_type_t orig_type;        /* original type, as found in the buffer */
   int resolved_level;           /* final resolved level of this character */
-  int invalid_levels;           /* how many PDFs should we ignore */
-  int invalid_rl_levels;        /* how many PDFs from RLE/RLO should ignore */
+  int invalid_levels;           /* how many PDFs to ignore */
+  int invalid_rl_levels;        /* how many PDFs from RLE/RLO to ignore */
   int new_paragraph;            /* if non-zero, a new paragraph begins here */
-  int prev_was_pdf;             /* if non-zero, prev char was PDF */
-  struct bidi_saved_info prev;  /* info about the previous character */
+  int prev_was_pdf;             /* if non-zero, previous char was PDF */
+  struct bidi_saved_info prev;  /* info about previous character */
   struct bidi_saved_info last_strong; /* last-seen strong directional char */
   struct bidi_saved_info next_for_neutral; /* surrounding characters for... */
   struct bidi_saved_info prev_for_neutral; /* ...resolving neutrals */
   struct bidi_saved_info next_for_ws; /* character after sequence of ws */
   int next_en_pos;              /* position of next EN char for ET */
-  int ignore_bn_limit;          /* position until which we should ignore BNs */
+  int ignore_bn_limit;          /* position until which to ignore BNs */
   bidi_dir_t sor;               /* direction of start-of-run in effect */
   int scan_dir;                 /* direction of text scan */
   int stack_idx;                /* index of current data on the stack */
diff --git a/src/xdisp.c b/src/xdisp.c
index 05cc61e9759..ae4a0305034 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -2821,11 +2821,7 @@ init_iterator (it, w, charpos, bytepos, row, base_face_id)
 
       /* Compute byte position if not specified.  */
       if (bytepos < charpos)
-        {
-          IT_BYTEPOS (*it) = CHAR_TO_BYTE (charpos);
-          if (it->bidi_p)
-            it->bidi_it.bytepos = IT_BYTEPOS (*it);
-        }
+        IT_BYTEPOS (*it) = CHAR_TO_BYTE (charpos);
       else
         IT_BYTEPOS (*it) = bytepos;
 
@@ -5526,32 +5522,8 @@ reseat_1 (it, pos, set_stop_p)
   it->sp = 0;
   it->string_from_display_prop_p = 0;
   it->face_before_selective_p = 0;
-
   if (it->bidi_p)
-    {
-      /* FIXME: L2R below is just for easyness of testing, as we
-         currently support only left-to-right paragraphs.  The value
-         should be user-definable and/or come from some ``higher
-         protocol''. In the absence of any other guidance, the default
-         for this initialization should be NEUTRAL_DIR.  */
-      it->bidi_it.charpos = CHARPOS (pos);
-      it->bidi_it.bytepos = BYTEPOS (pos);
-      bidi_paragraph_init (L2R, &it->bidi_it);
-      /* With bidi reordering, the first character to display might
-         not be the character at POS.  We need to find the next
-         character in visual order starting from the preceding
-         character.  */
-      if ((it->bidi_it.charpos = CHARPOS (pos) - 1) >= BEGV)
-        {
-          it->bidi_it.bytepos = CHAR_TO_BYTE (CHARPOS (pos) - 1);
-          it->bidi_it.ch_len = CHAR_BYTES (FETCH_CHAR (it->bidi_it.bytepos));
-        }
-      else
-        it->bidi_it.bytepos = 0; /* signal bidi.c not to move */
-      bidi_get_next_char_visually (&it->bidi_it);
-      SET_TEXT_POS (pos, it->bidi_it.charpos, it->bidi_it.bytepos);
-      it->current.pos = it->position = pos;
-    }
+    it->bidi_it.first_elt = 1;
 
   if (set_stop_p)
     it->stop_charpos = CHARPOS (pos);
@@ -6529,6 +6501,26 @@ next_element_from_buffer (it)
 
   xassert (IT_CHARPOS (*it) >= BEGV);
 
+  /* With bidi reordering, the character to display might not be
+     the character at IT_CHARPOS.  */
+  if (it->bidi_p && it->bidi_it.first_elt)
+    {
+      /* FIXME: L2R below is just for easyness of testing, as we
+         currently support only left-to-right paragraphs.  The value
+         should be user-definable and/or come from some ``higher
+         protocol''. In the absence of any other guidance, the default
+         for this initialization should be NEUTRAL_DIR.  */
+      it->bidi_it.charpos = IT_CHARPOS (*it);
+      it->bidi_it.bytepos = IT_BYTEPOS (*it);
+      bidi_paragraph_init (L2R, &it->bidi_it);
+      bidi_get_next_char_visually (&it->bidi_it);
+      it->bidi_it.first_elt = 0;
+      /*  Adjust IT's position information to where we moved.  */
+      IT_CHARPOS (*it) = it->bidi_it.charpos;
+      IT_BYTEPOS (*it) = it->bidi_it.bytepos;
+      SET_TEXT_POS (it->position, IT_CHARPOS (*it), IT_BYTEPOS (*it));
+    }
+
   if (IT_CHARPOS (*it) >= it->stop_charpos)
     {
       if (IT_CHARPOS (*it) >= it->end_charpos)