[HarfBuzz] mirroring support in hb_shape

[Jonathan Kew]

Hi Behdad,

I think there's an issue with how (or rather *where*) mirroring is implemented -- currently hb_mirror_chars is called from hb_substitute_default, right before hb_map_glyphs. The problem with this is that by the time hb_substitute_default is called, the buffer has been forced to "native" direction, but AFAICS we need the mirroring to be based on the original direction instead.

This causes problems if we have text such as <RLO>Hello (world)<PDF>, which will form a RTL run; hb_ensure_native_direction will reverse it, in order that OT lookups can work properly, but this will prevent the parens being mirrored because hb_mirror_chars will see the run as being LTR.

To resolve this, I've moved the call to hb_mirror_chars out of hb_substitute_default, and put it before hb_ensure_native_direction, as shown below. Please see if you think this is a reasonable thing to do.

JK

diff --git a/src/hb-shape.c b/src/hb-shape.c
--- a/src/hb-shape.c
+++ b/src/hb-shape.c
@@ -111,17 +111,16 @@ hb_map_glyphs (hb_font_t    *font,
 
 static void
 hb_substitute_default (hb_font_t    *font,
 		       hb_face_t    *face,
 		       hb_buffer_t  *buffer,
 		       hb_feature_t *features,
 		       unsigned int  num_features)
 {
-  hb_mirror_chars (buffer);
   hb_map_glyphs (font, face, buffer);
 }
 
 static hb_bool_t
 hb_substitute_complex (hb_font_t    *font,
 		       hb_face_t    *face,
 		       hb_buffer_t  *buffer,
 		       hb_feature_t *features,
@@ -224,18 +223,20 @@ hb_shape (hb_font_t    *font,
 	  hb_buffer_t  *buffer,
 	  hb_feature_t *features,
 	  unsigned int  num_features)
 {
   hb_direction_t original_direction;
   hb_bool_t substitute_fallback, position_fallback;
 
   hb_form_clusters (buffer);
+
+  hb_mirror_chars (buffer);
+
   original_direction = hb_ensure_native_direction (buffer);
-
   hb_substitute_default (font, face, buffer, features, num_features);
 
   substitute_fallback = !hb_substitute_complex (font, face, buffer, features, num_features);
 
   if (substitute_fallback)
     hb_substitute_fallback (font, face, buffer, features, num_features);
 
   hb_position_default (font, face, buffer, features, num_features);