Disable OTL processing for Hebrew if GPOS doesn't have Hebrew subtable

New approach to fix this:
https://github.com/behdad/harfbuzz/commit/69f9fbc4200442a35484d3c790ae8f4979be5d60

Previous approach was reverted as it was too broad.  See context:
https://github.com/behdad/harfbuzz/issues/347#issuecomment-267838368

With U+05E9,U+05B8,U+05C1,U+05DC and Arial Unicode, we now (correctly) disable
GDEF and GPOS, so we get results very close to Uniscribe, but slightly different
since our fallback position logic is not exactly the same:

Before:		[gid1166=3+991|gid1142=0+737|gid5798=0+1434]
After:		[gid1166=3+991|gid1142=0@402,-26+0|gid5798=0+1434]
Uniscribe:	[gid1166=3+991|gid1142=0@348,0+0|gid5798=0+1434]

diff --git a/src/hb-ot-shape-complex-arabic.cc b/src/hb-ot-shape-complex-arabic.cc
index 4da8990..56ec5cd 100644 (file)
--- a/src/hb-ot-shape-complex-arabic.cc
+++ b/src/hb-ot-shape-complex-arabic.cc
@@ -618,6 +618,7 @@ const hb_ot_complex_shaper_t _hb_ot_complex_shaper_arabic =
   NULL, /* decompose */
   NULL, /* compose */
   setup_masks_arabic,
+  NULL, /* disable_otl */
   HB_OT_SHAPE_ZERO_WIDTH_MARKS_BY_GDEF_LATE,
   true, /* fallback_position */
 };

diff --git a/src/hb-ot-shape-complex-default.cc b/src/hb-ot-shape-complex-default.cc
index be60e56..42830ab 100644 (file)
--- a/src/hb-ot-shape-complex-default.cc
+++ b/src/hb-ot-shape-complex-default.cc
@@ -40,6 +40,7 @@ const hb_ot_complex_shaper_t _hb_ot_complex_shaper_default =
   NULL, /* decompose */
   NULL, /* compose */
   NULL, /* setup_masks */
+  NULL, /* disable_otl */
   HB_OT_SHAPE_ZERO_WIDTH_MARKS_BY_GDEF_LATE,
   true, /* fallback_position */
 };

diff --git a/src/hb-ot-shape-complex-hangul.cc b/src/hb-ot-shape-complex-hangul.cc
index 5f4d98b..eb95a28 100644 (file)
--- a/src/hb-ot-shape-complex-hangul.cc
+++ b/src/hb-ot-shape-complex-hangul.cc
@@ -419,6 +419,7 @@ const hb_ot_complex_shaper_t _hb_ot_complex_shaper_hangul =
   NULL, /* decompose */
   NULL, /* compose */
   setup_masks_hangul,
+  NULL, /* disable_otl */
   HB_OT_SHAPE_ZERO_WIDTH_MARKS_NONE,
   false, /* fallback_position */
 };

diff --git a/src/hb-ot-shape-complex-hebrew.cc b/src/hb-ot-shape-complex-hebrew.cc
index 3215900..96f2494 100644 (file)
--- a/src/hb-ot-shape-complex-hebrew.cc
+++ b/src/hb-ot-shape-complex-hebrew.cc
@@ -154,6 +154,18 @@ compose_hebrew (const hb_ot_shape_normalize_context_t *c,
   return found;
 }
 
+static bool
+disable_otl_hebrew (const hb_ot_shape_plan_t *plan)
+{
+  /* For Hebrew shaper, use fallback if GPOS does not have 'hebr'
+   * script.  This matches Uniscribe better, and makes fonts like
+   * Arial that have GSUB/GPOS/GDEF but no data for Hebrew work.
+   * See:
+   * https://github.com/behdad/harfbuzz/issues/347#issuecomment-267838368
+   */
+  return plan->map.chosen_script[1] != HB_TAG ('h','e','b','r');
+}
+
 
 const hb_ot_complex_shaper_t _hb_ot_complex_shaper_hebrew =
 {
@@ -168,6 +180,7 @@ const hb_ot_complex_shaper_t _hb_ot_complex_shaper_hebrew =
   NULL, /* decompose */
   compose_hebrew,
   NULL, /* setup_masks */
+  disable_otl_hebrew,
   HB_OT_SHAPE_ZERO_WIDTH_MARKS_BY_GDEF_LATE,
   true, /* fallback_position */
 };

diff --git a/src/hb-ot-shape-complex-indic.cc b/src/hb-ot-shape-complex-indic.cc
index 94556f6..ec6e82c 100644 (file)
--- a/src/hb-ot-shape-complex-indic.cc
+++ b/src/hb-ot-shape-complex-indic.cc
@@ -1819,6 +1819,7 @@ const hb_ot_complex_shaper_t _hb_ot_complex_shaper_indic =
   decompose_indic,
   compose_indic,
   setup_masks_indic,
+  NULL, /* disable_otl */
   HB_OT_SHAPE_ZERO_WIDTH_MARKS_NONE,
   false, /* fallback_position */
 };

diff --git a/src/hb-ot-shape-complex-myanmar.cc b/src/hb-ot-shape-complex-myanmar.cc
index 577d790..bb68622 100644 (file)
--- a/src/hb-ot-shape-complex-myanmar.cc
+++ b/src/hb-ot-shape-complex-myanmar.cc
@@ -521,6 +521,7 @@ const hb_ot_complex_shaper_t _hb_ot_complex_shaper_myanmar_old =
   NULL, /* decompose */
   NULL, /* compose */
   NULL, /* setup_masks */
+  NULL, /* disable_otl */
   HB_OT_SHAPE_ZERO_WIDTH_MARKS_BY_GDEF_LATE,
   true, /* fallback_position */
 };
@@ -538,6 +539,7 @@ const hb_ot_complex_shaper_t _hb_ot_complex_shaper_myanmar =
   NULL, /* decompose */
   NULL, /* compose */
   setup_masks_myanmar,
+  NULL, /* disable_otl */
   HB_OT_SHAPE_ZERO_WIDTH_MARKS_BY_GDEF_EARLY,
   false, /* fallback_position */
 };

diff --git a/src/hb-ot-shape-complex-private.hh b/src/hb-ot-shape-complex-private.hh
index fb0c704..39572df 100644 (file)
--- a/src/hb-ot-shape-complex-private.hh
+++ b/src/hb-ot-shape-complex-private.hh
@@ -146,6 +146,14 @@ struct hb_ot_complex_shaper_t
                       hb_buffer_t              *buffer,
                       hb_font_t                *font);
 
+  /* disable_otl()
+   * Called during shape().
+   * If set and returns true, GDEF/GSUB/GPOS of the font are ignored
+   * and fallback operations used.
+   * May be NULL.
+   */
+  bool (*disable_otl) (const hb_ot_shape_plan_t *plan);
+
   hb_ot_shape_zero_width_marks_type_t zero_width_marks;
 
   bool fallback_position;

diff --git a/src/hb-ot-shape-complex-thai.cc b/src/hb-ot-shape-complex-thai.cc
index 4322b0d..e6f80f5 100644 (file)
--- a/src/hb-ot-shape-complex-thai.cc
+++ b/src/hb-ot-shape-complex-thai.cc
@@ -376,6 +376,7 @@ const hb_ot_complex_shaper_t _hb_ot_complex_shaper_thai =
   NULL, /* decompose */
   NULL, /* compose */
   NULL, /* setup_masks */
+  NULL, /* disable_otl */
   HB_OT_SHAPE_ZERO_WIDTH_MARKS_BY_GDEF_LATE,
   false,/* fallback_position */
 };

diff --git a/src/hb-ot-shape-complex-tibetan.cc b/src/hb-ot-shape-complex-tibetan.cc
index a77b531..aadf59f 100644 (file)
--- a/src/hb-ot-shape-complex-tibetan.cc
+++ b/src/hb-ot-shape-complex-tibetan.cc
@@ -57,6 +57,7 @@ const hb_ot_complex_shaper_t _hb_ot_complex_shaper_tibetan =
   NULL, /* decompose */
   NULL, /* compose */
   NULL, /* setup_masks */
+  NULL, /* disable_otl */
   HB_OT_SHAPE_ZERO_WIDTH_MARKS_BY_GDEF_LATE,
   true, /* fallback_position */
 };

diff --git a/src/hb-ot-shape-complex-use.cc b/src/hb-ot-shape-complex-use.cc
index 045ead5..13c7ab3 100644 (file)
--- a/src/hb-ot-shape-complex-use.cc
+++ b/src/hb-ot-shape-complex-use.cc
@@ -585,6 +585,7 @@ const hb_ot_complex_shaper_t _hb_ot_complex_shaper_use =
   NULL, /* decompose */
   compose_use,
   setup_masks_use,
+  NULL, /* disable_otl */
   HB_OT_SHAPE_ZERO_WIDTH_MARKS_BY_GDEF_EARLY,
   false, /* fallback_position */
 };

diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc
index cf916cc..ddd6662 100644 (file)
--- a/src/hb-ot-shape.cc
+++ b/src/hb-ot-shape.cc
@@ -218,6 +218,8 @@ struct hb_ot_shape_context_t
   unsigned int        num_user_features;
 
   /* Transient stuff */
+  bool fallback_positioning;
+  bool fallback_glyph_classes;
   hb_direction_t target_direction;
 };
 
@@ -571,7 +573,7 @@ hb_ot_substitute_default (hb_ot_shape_context_t *c)
   hb_ot_shape_setup_masks (c);
 
   /* This is unfortunate to go here, but necessary... */
-  if (!hb_ot_layout_has_positioning (c->face))
+  if (c->fallback_positioning)
     _hb_ot_shape_fallback_position_recategorize_marks (c->plan, c->font, buffer);
 
   hb_ot_map_glyphs_fast (buffer);
@@ -667,14 +669,12 @@ hb_ot_position_default (hb_ot_shape_context_t *c)
     _hb_ot_shape_fallback_spaces (c->plan, c->font, c->buffer);
 }
 
-static inline bool
+static inline void
 hb_ot_position_complex (hb_ot_shape_context_t *c)
 {
   hb_ot_layout_position_start (c->font, c->buffer);
 
-  bool ret = false;
   unsigned int count = c->buffer->len;
-  bool has_positioning = (bool) hb_ot_layout_has_positioning (c->face);
 
   /* If the font has no GPOS, AND, no fallback positioning will
    * happen, AND, direction is forward, then when zeroing mark
@@ -685,8 +685,9 @@ hb_ot_position_complex (hb_ot_shape_context_t *c)
    * If fallback positinoing happens or GPOS is present, we don't
    * care.
    */
-  bool adjust_offsets_when_zeroing = !(has_positioning || c->plan->shaper->fallback_position ||
-                                       HB_DIRECTION_IS_BACKWARD (c->buffer->props.direction));
+  bool adjust_offsets_when_zeroing = c->fallback_positioning &&
+                                    !c->plan->shaper->fallback_position &&
+                                    HB_DIRECTION_IS_FORWARD (c->buffer->props.direction);
 
   switch (c->plan->shaper->zero_width_marks)
   {
@@ -700,7 +701,7 @@ hb_ot_position_complex (hb_ot_shape_context_t *c)
       break;
   }
 
-  if (has_positioning)
+  if (likely (!c->fallback_positioning))
   {
     hb_glyph_info_t *info = c->buffer->info;
     hb_glyph_position_t *pos = c->buffer->pos;
@@ -723,7 +724,6 @@ hb_ot_position_complex (hb_ot_shape_context_t *c)
                                          &pos[i].x_offset,
                                          &pos[i].y_offset);
 
-    ret = true;
   }
 
   switch (c->plan->shaper->zero_width_marks)
@@ -742,8 +742,6 @@ hb_ot_position_complex (hb_ot_shape_context_t *c)
   hb_ot_layout_position_finish_advances (c->font, c->buffer);
   hb_ot_zero_width_default_ignorables (c);
   hb_ot_layout_position_finish_offsets (c->font, c->buffer);
-
-  return ret;
 }
 
 static inline void
@@ -753,9 +751,9 @@ hb_ot_position (hb_ot_shape_context_t *c)
 
   hb_ot_position_default (c);
 
-  hb_bool_t fallback = !hb_ot_position_complex (c);
+  hb_ot_position_complex (c);
 
-  if (fallback && c->plan->shaper->fallback_position)
+  if (c->fallback_positioning && c->plan->shaper->fallback_position)
     _hb_ot_shape_fallback_position (c->plan, c->font, c->buffer);
 
   if (HB_DIRECTION_IS_BACKWARD (c->buffer->props.direction))
@@ -763,7 +761,7 @@ hb_ot_position (hb_ot_shape_context_t *c)
 
   /* Visual fallback goes here. */
 
-  if (fallback)
+  if (c->fallback_positioning)
     _hb_ot_shape_fallback_kern (c->plan, c->font, c->buffer);
 
   _hb_buffer_deallocate_gsubgpos_vars (c->buffer);
@@ -783,6 +781,11 @@ hb_ot_shape_internal (hb_ot_shape_context_t *c)
                              (unsigned) HB_BUFFER_MAX_LEN_MIN);
   }
 
+  bool disable_otl = c->plan->shaper->disable_otl && c->plan->shaper->disable_otl (c->plan);
+  //c->fallback_substitute     = disable_otl || !hb_ot_layout_has_substitution (c->face);
+  c->fallback_positioning    = disable_otl || !hb_ot_layout_has_positioning (c->face);
+  c->fallback_glyph_classes  = disable_otl || !hb_ot_layout_has_glyph_classes (c->face);
+
   /* Save the original direction, we use it later. */
   c->target_direction = c->buffer->props.direction;