[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.
diff --git a/src/hb-ot-shape-complex-arabic.cc b/src/hb-ot-shape-complex-arabic.cc index 3c59743..1b4ebc9 100644 --- a/src/hb-ot-shape-complex-arabic.cc +++ b/src/hb-ot-shape-complex-arabic.cc
@@ -68,31 +68,30 @@ ) ? 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 @@ { 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 @@ 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 @@ 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;