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.

diff --git a/src/.gdbinit b/src/.gdbinit
index a500f2532f51c0f8db087bca94b8cc81b85ff857..8949e66134c75c0e90e55cb7379507118f91db39 100644 (file)
--- 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 7e11c6ec02455d967a8ce15fb7ee9a3015f7d9ff..11ed09b6d545cc76c8d4617dffdc5ea18d84e04c 100644 (file)
--- 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 510ffe50dabaa2b4628ab95ec5c8f1bfe51d3188..0521a062351514d648f4fb87d8de672fb48dcf6a 100644 (file)
--- 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)
 
 \f
 /***********************************************************************
@@ -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 b18b0e04be2621d6180f4e33f67c1d55c94676ed..17d2be0f7f21c44e2aae2f7d310f36f76c8193df 100644 (file)
--- 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