[arabic] Apply init/medi/isol/fini/... in separate stages

Follows the order of the Arabic/Syriac specs.  Also don't stop
between rlig and calt in non-Arabic scripts.

Micro-tests for Arabic and Mongolian added for the latter.

src/hb-ot-shape-complex-arabic.cc

@@ -68,31 +68,30 @@ static unsigned int get_joining_type (hb_codepoint_t u, hb_unicode_general_categ
 	 ) ?  JOINING_TYPE_T : JOINING_TYPE_U;
 }
 
+#define FEATURE_IS_SYRIAC(tag) hb_in_range<unsigned char> ((unsigned char) (tag), '2', '3')
+
 static const hb_tag_t arabic_features[] =
 {
-  HB_TAG('i','n','i','t'),
-  HB_TAG('m','e','d','i'),
-  HB_TAG('f','i','n','a'),
   HB_TAG('i','s','o','l'),
-  /* Syriac */
-  HB_TAG('m','e','d','2'),
+  HB_TAG('f','i','n','a'),
   HB_TAG('f','i','n','2'),
   HB_TAG('f','i','n','3'),
+  HB_TAG('m','e','d','i'),
+  HB_TAG('m','e','d','2'),
+  HB_TAG('i','n','i','t'),
   HB_TAG_NONE
 };
 
 
 /* Same order as the feature array */
 enum {
-  INIT,
-  MEDI,
-  FINA,
   ISOL,
-
-  /* Syriac */
-  MED2,
+  FINA,
   FIN2,
   FIN3,
+  MEDI,
+  MED2,
+  INIT,
 
   NONE,
 
@@ -145,14 +144,23 @@ collect_features_arabic (hb_ot_shape_planner_t *plan)
 {
   hb_ot_map_builder_t *map = &plan->map;
 
-  /* For Language forms (in ArabicOT speak), we do the iso/fina/medi/init together,
-   * then rlig and calt each in their own stage.  This makes IranNastaliq's ALLAH
-   * ligature work correctly. It's unfortunate though...
+  /* We apply features according to the Arabic spec, with pauses
+   * in between most.
    *
-   * This also makes Arial Bold in Windows7 work.  See:
+   * The pause between init/medi/... and rlig is required.  See eg:
    * https://bugzilla.mozilla.org/show_bug.cgi?id=644184
    *
-   * TODO: Add test cases for these two.
+   * The pauses between init/medi/... themselves are not necessarily
+   * needed as only one of those features is applied to any character.
+   * The only difference it makes is when fonts have contextual
+   * substitutions.  We now follow the order of the spec, which makes
+   * for better experience if that's what Uniscribe is doing.
+   *
+   * At least for Arabic, looks like Uniscribe has a pause between
+   * rlig and calt.  Otherwise the IranNastaliq's ALLAH ligature won't
+   * work.  However, testing shows that rlig and calt are applied
+   * together for Mongolian in Uniscribe.  As such, we only add a
+   * pause for Arabic, not other scripts.
    */
 
   map->add_gsub_pause (nuke_joiners);
@@ -163,12 +171,15 @@ collect_features_arabic (hb_ot_shape_planner_t *plan)
   map->add_gsub_pause (NULL);
 
   for (unsigned int i = 0; i < ARABIC_NUM_FEATURES; i++)
-    map->add_feature (arabic_features[i], 1, i < 4 ? F_HAS_FALLBACK : F_NONE); /* The first four features have fallback. */
-
-  map->add_gsub_pause (NULL);
+  {
+    bool has_fallback = plan->props.script == HB_SCRIPT_ARABIC && !FEATURE_IS_SYRIAC (arabic_features[i]);
+    map->add_feature (arabic_features[i], 1, has_fallback ? F_HAS_FALLBACK : F_NONE);
+    map->add_gsub_pause (NULL);
+  }
 
   map->add_feature (HB_TAG('r','l','i','g'), 1, F_GLOBAL|F_HAS_FALLBACK);
-  map->add_gsub_pause (arabic_fallback_shape);
+  if (plan->props.script == HB_SCRIPT_ARABIC)
+    map->add_gsub_pause (arabic_fallback_shape);
 
   map->add_global_bool_feature (HB_TAG('c','a','l','t'));
   map->add_gsub_pause (NULL);
@@ -202,8 +213,9 @@ data_create_arabic (const hb_ot_shape_plan_t *plan)
   arabic_plan->do_fallback = plan->props.script == HB_SCRIPT_ARABIC;
   for (unsigned int i = 0; i < ARABIC_NUM_FEATURES; i++) {
     arabic_plan->mask_array[i] = plan->map.get_1_mask (arabic_features[i]);
-    if (i < 4)
-      arabic_plan->do_fallback = arabic_plan->do_fallback && plan->map.needs_fallback (arabic_features[i]);
+    arabic_plan->do_fallback = arabic_plan->do_fallback &&
+			       !FEATURE_IS_SYRIAC (arabic_features[i]) &&
+			       plan->map.needs_fallback (arabic_features[i]);
   }
 
   return arabic_plan;

test/shaping/Makefile.am

@@ -36,8 +36,10 @@ CLEANFILES += \
 	$(NULL)
 
 TESTS = \
+	tests/arabic-feature-order.tests \
 	tests/context-matching.tests \
 	tests/indic-pref-blocking.tests \
+	tests/mongolian-variation-selector.tests \
 	$(NULL)
 
 TEST_EXTENSIONS = \

test/shaping/fonts/sha1sum/MANIFEST

@@ -1,6 +1,9 @@
 226bc2deab3846f1a682085f70c67d0421014144.ttf
 37033cc5cf37bb223d7355153016b6ccece93b28.ttf
 4cce528e99f600ed9c25a2b69e32eb94a03b4ae8.ttf
+813c2f8e5512187fd982417a7fb4286728e6f4a8.ttf
+8a9fea2a7384f2116e5b84a9b31f83be7850ce21.ttf
+a919b33197965846f21074b24e30250d67277bce.ttf
 d629e7fedc0b350222d7987345fe61613fa3929a.ttf
 e207635780b42f898d58654b65098763e340f5c7.ttf
 ef86fe710cfea877bbe0dbb6946a1f88d0661031.ttf

test/shaping/tests/MANIFEST

@@ -1,3 +1,4 @@
+arabic-feature-order.tests
 context-matching.tests
 indic-pref-blocking.tests
 mongolian-variation-selector.tests

test/shaping/tests/arabic-feature-order.tests

@@ -0,0 +1,3 @@
+fonts/sha1sum/813c2f8e5512187fd982417a7fb4286728e6f4a8.ttf:U+1820,U+180B:[uni2048.E81A=0+1550]
+fonts/sha1sum/8a9fea2a7384f2116e5b84a9b31f83be7850ce21.ttf:U+1820,U+180B:[uni2048.E81A=0+1550]
+fonts/sha1sum/a919b33197965846f21074b24e30250d67277bce.ttf:U+0644,U+0644,U+0647:[Lellah=0+1503]