Bidi: remember resolved base direction

There used to be a FIXME comment about this,
but it was removed in commit 27cc03363cfbddb9877cd547327d56405653add7.

Reordering works fine without this *most of the time*, but this is still
a bug. It affects one thing only: trailing whitespace in each line/run.

Of course, we remove trailing U+0020 SPACE characters from each line
regardless of bidi, but Unicode bidi recognizes other whitespace,
and we can now also have trailing U+0020 within in-line runs.

With right-to-left base direction, all trailing whitespace within
each line (after line wrapping) or run are to be placed at the leftmost
end in visual order (even if that whitespace is nominally part
of a left-to-right embed). Conversely, with our current code that
always tells FriBidi the base direction is left-to-right here,
trailing whitespace is always placed at the rightmost end of each line,
even if it is inside right-to-left text (which makes it appear
as *leading* whitespace to the reader).

We treat explicit line breaks \N as paragraph separators,
where each paragraph can have its own base direction. Therefore,
we must remember each paragraph's resolved direction separately.

1 files changed, 24 insertions, 7 deletions

diff --git a/libass/ass_shaper.c b/libass/ass_shaper.c
index 3db939e..6cb2ebf 100644
--- a/libass/ass_shaper.c
+++ b/libass/ass_shaper.c
@@ -49,11 +49,12 @@ struct ass_shaper {
     ASS_ShapingLevel shaping_level;
 
     // FriBidi log2vis
-    int n_glyphs;
+    int n_glyphs, n_pars;
     FriBidiChar *event_text;
     FriBidiCharType *ctypes;
     FriBidiLevel *emblevels;
     FriBidiStrIndex *cmap;
+    FriBidiParType *pbase_dir;
     FriBidiParType base_direction;
 
     // OpenType features
@@ -99,7 +100,7 @@ void ass_shaper_info(ASS_Library *lib)
  * \brief grow arrays, if needed
  * \param new_size requested size
  */
-static bool check_allocations(ASS_Shaper *shaper, size_t new_size)
+static bool check_allocations(ASS_Shaper *shaper, size_t new_size, size_t n_pars)
 {
     if (new_size > shaper->n_glyphs) {
         if (!ASS_REALLOC_ARRAY(shaper->event_text, new_size) ||
@@ -112,6 +113,11 @@ static bool check_allocations(ASS_Shaper *shaper, size_t new_size)
             return false;
         shaper->n_glyphs = new_size;
     }
+    if (shaper->whole_text_layout && n_pars > shaper->n_pars) {
+        if (!ASS_REALLOC_ARRAY(shaper->pbase_dir, n_pars))
+            return false;
+        shaper->n_pars = n_pars;
+    }
     return true;
 }
 
@@ -129,6 +135,7 @@ void ass_shaper_free(ASS_Shaper *shaper)
 #endif
     free(shaper->emblevels);
     free(shaper->cmap);
+    free(shaper->pbase_dir);
     free(shaper);
 }
 
@@ -965,10 +972,15 @@ void ass_shaper_set_whole_text_layout(ASS_Shaper *shaper, bool enable)
 bool ass_shaper_shape(ASS_Shaper *shaper, TextInfo *text_info)
 {
     int i, ret, last_break;
-    FriBidiParType dir;
+    FriBidiParType dir, *pdir;
     GlyphInfo *glyphs = text_info->glyphs;
 
-    if (!check_allocations(shaper, text_info->length))
+    int n_pars = 1;
+    for (i = 0; i < text_info->length - 1; i++)
+        if (glyphs[i].symbol == '\n')
+            n_pars++;
+
+    if (!check_allocations(shaper, text_info->length, n_pars))
         return false;
 
     for (i = 0; i < text_info->length; i++)
@@ -986,6 +998,7 @@ bool ass_shaper_shape(ASS_Shaper *shaper, TextInfo *text_info)
 
     // Get bidi embedding levels
     last_break = 0;
+    pdir = shaper->pbase_dir;
     for (i = 0; i < text_info->length; i++) {
         // embedding levels should be calculated paragraph by paragraph
         if (glyphs[i].symbol == '\n' || i == text_info->length - 1 ||
@@ -1006,6 +1019,8 @@ bool ass_shaper_shape(ASS_Shaper *shaper, TextInfo *text_info)
             if (ret == 0)
                 return false;
             last_break = i + 1;
+            if (shaper->whole_text_layout)
+                *pdir++ = dir;
         }
     }
 
@@ -1079,21 +1094,23 @@ FriBidiStrIndex *ass_shaper_reorder(ASS_Shaper *shaper, TextInfo *text_info)
 
     // Create reorder map line-by-line or run-by-run
     int last_break = 0;
+    FriBidiParType *pdir = shaper->whole_text_layout ?
+        shaper->pbase_dir : &shaper->base_direction;
     GlyphInfo *glyphs = text_info->glyphs;
     for (i = 0; i < text_info->length; i++) {
         if (i == text_info->length - 1 || glyphs[i + 1].linebreak ||
                 (!shaper->whole_text_layout &&
                     (glyphs[i + 1].starts_new_run || glyphs[i].hspacing))) {
-            FriBidiParType dir = FRIBIDI_PAR_ON;
-
             ret = fribidi_reorder_line(0,
-                    shaper->ctypes, i - last_break + 1, last_break, dir,
+                    shaper->ctypes, i - last_break + 1, last_break, *pdir,
                     shaper->emblevels, NULL,
                     shaper->cmap);
             if (ret == 0)
                 return NULL;
 
             last_break = i + 1;
+            if (shaper->whole_text_layout)
+                pdir++;
         }
     }