Continue working on handling of properties in bidi iteration.

Region display and extension seems to work.
Solved a crash in bidirectional display of etc/HELLO.
(HELLO display still not 100% OK, e.g. near Kannada.)

 .gdbinit (pitx): Display some bidi information about the
 iterator.

 dispextern.h (BIDI_AT_BASE_LEVEL): Enclose definition in
 parentheses.

 xdisp.c (handle_stop_backwards): Save and restore it->current
 and it->position, instead of expecting the caller to do that.
 (next_element_from_buffer): When moving across stop_charpos,
 record it in prev_stop.  When IT_CHARPOS backs up, call
 handle_stop_backwards only if above the base embedding level.
 This solves the crash while displaying etc/HELLO in bidi mode.

4 files changed, 48 insertions, 26 deletions

diff --git a/src/.gdbinit b/src/.gdbinit
index a500f2532f5..8949e66134c 100644
--- a/src/.gdbinit
+++ b/src/.gdbinit
@@ -271,6 +271,9 @@ define pitx
     end
   end
   printf "\n"
+  if ($it->bidi_p)
+    printf "BIDI: base_stop=%d prev_stop=%d level=%d\n", $it->base_level_stop, $it->prev_stop, $it->bidi_it.resolved_level
+  end
   if ($it->region_beg_charpos >= 0)
     printf "reg=%d-%d ", $it->region_beg_charpos, $it->region_end_charpos
   end
diff --git a/src/ChangeLog.bidi b/src/ChangeLog.bidi
index 7e11c6ec024..11ed09b6d54 100644
--- a/src/ChangeLog.bidi
+++ b/src/ChangeLog.bidi
@@ -1,3 +1,18 @@
+2010-01-02  Eli Zaretskii  <eliz@gnu.org>
+
+        * .gdbinit (pitx): Display some bidi information about the
+        iterator.
+
+        * dispextern.h (BIDI_AT_BASE_LEVEL): Enclose definition in
+        parentheses.
+
+        * xdisp.c (handle_stop_backwards): Save and restore it->current
+        and it->position, instead of expecting the caller to do that.
+        (next_element_from_buffer): When moving across stop_charpos,
+        record it in prev_stop.  When IT_CHARPOS backs up, call
+        handle_stop_backwards only if above the base embedding level.
+        This solves the crash while displaying etc/HELLO in bidi mode.
+
 2009-12-26  Eli Zaretskii  <eliz@gnu.org>
 
         * xdisp.c (handle_stop_backwards): Call compute_stop_pos in the
diff --git a/src/dispextern.h b/src/dispextern.h
index 510ffe50dab..0521a062351 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1812,7 +1812,7 @@ struct bidi_it {
 /* Value is non-zero when the bidi iterator is at base paragraph
    embedding level.  */
 #define BIDI_AT_BASE_LEVEL(BIDI_IT) \
-  (BIDI_IT).resolved_level == (BIDI_IT).level_stack[0].level
+  ((BIDI_IT).resolved_level == (BIDI_IT).level_stack[0].level)
 
 
 /***********************************************************************
@@ -2013,10 +2013,11 @@ struct it
   EMACS_INT stop_charpos;
 
   /* Previous stop position, i.e. the last one before the current
-     buffer position.  */
+     iterator position in `current'.  */
   EMACS_INT prev_stop;
 
-  /* Last stop_pos at the current paragraph's embedding level.  */
+  /* Last stop position iterated across whose embedding level is equal
+     to the current paragraph's embedding level.  */
   EMACS_INT base_level_stop;
 
   /* Maximum string or buffer position + 1.  ZV when iterating over
diff --git a/src/xdisp.c b/src/xdisp.c
index b18b0e04be2..17d2be0f7f2 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -6550,19 +6550,18 @@ next_element_from_stretch (it)
 
 /* Scan forward from CHARPOS in the current buffer, until we find a
    stop position > current IT's position.  Then handle the stop
-   position before that.
-
-   This is called when we are reordering bidirectional text.  The
-   caller should save and restore IT and in particular the bidi_p
-   flag, because this function modifies them.  */
+   position before that.  This is called when we bump into a stop
+   position while reordering bidirectional text.  */
 
 static void
 handle_stop_backwards (it, charpos)
      struct it *it;
      EMACS_INT charpos;
 {
-  struct text_pos pos1;
   EMACS_INT where_we_are = IT_CHARPOS (*it);
+  struct display_pos save_current = it->current;
+  struct text_pos save_position = it->position;
+  struct text_pos pos1;
   EMACS_INT next_stop;
 
   /* Scan in strict logical order.  */
@@ -6584,6 +6583,9 @@ handle_stop_backwards (it, charpos)
   it->stop_charpos = it->prev_stop;
   handle_stop (it);
   it->stop_charpos = next_stop;
+  it->bidi_p = 1;
+  it->current = save_current;
+  it->position = save_position;
 }
 
 /* Load IT with the next display element from current_buffer.  Value
@@ -6685,38 +6687,39 @@ next_element_from_buffer (it)
              them all now, in buffer's logical order, until we find
              and handle the last stop_charpos that precedes our
              current position.  */
-          struct it save_it = *it;
-
           handle_stop_backwards (it, it->stop_charpos);
-          it->bidi_p = 1;
-          it->current = save_it.current;
-          it->position = save_it.position;
           return GET_NEXT_DISPLAY_ELEMENT (it);
         }
       else
         {
-          /* If we are at base paragraph embedding level, take note of
-             the last stop position seen at this level.  */
-          if (BIDI_AT_BASE_LEVEL (it->bidi_it))
-            it->base_level_stop = it->stop_charpos;
+          if (it->bidi_p)
+            {
+              /* Take note of the stop position we just moved across,
+                 for when we will move back across it.  */
+              it->prev_stop = it->stop_charpos;
+              /* If we are at base paragraph embedding level, take
+                 note of the last stop position seen at this
+                 level.  */
+              if (BIDI_AT_BASE_LEVEL (it->bidi_it))
+                it->base_level_stop = it->stop_charpos;
+            }
           handle_stop (it);
           return GET_NEXT_DISPLAY_ELEMENT (it);
         }
     }
-  else if (it->bidi_p && IT_CHARPOS (*it) < it->prev_stop)
+  else if (it->bidi_p
+           /* We can sometimes back up for reasons that have nothing
+              to do with bidi reordering.  E.g., compositions.  The
+              code below is only needed when we are above the base
+              embedding level, so test for that explicitly.  */
+           && !BIDI_AT_BASE_LEVEL (it->bidi_it)
+           && IT_CHARPOS (*it) < it->prev_stop)
     {
-      struct it save_it = *it;
-
       if (it->base_level_stop <= 0)
         it->base_level_stop = 1;
       if (IT_CHARPOS (*it) < it->base_level_stop)
         abort ();
-      if (BIDI_AT_BASE_LEVEL (it->bidi_it))
-        abort ();
       handle_stop_backwards (it, it->base_level_stop);
-      it->bidi_p = 1;
-      it->current = save_it.current;
-      it->position = save_it.position;
       return GET_NEXT_DISPLAY_ELEMENT (it);
     }
   else