From e2b878055bb1c0b84e1f7cfbf3d3f80bfc6811ea Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 22 Dec 2016 14:40:19 -0600 Subject: [PATCH] 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] --- src/hb-ot-shape-complex-arabic.cc | 1 + src/hb-ot-shape-complex-default.cc | 1 + src/hb-ot-shape-complex-hangul.cc | 1 + src/hb-ot-shape-complex-hebrew.cc | 13 +++++++++++++ src/hb-ot-shape-complex-indic.cc | 1 + src/hb-ot-shape-complex-myanmar.cc | 2 ++ src/hb-ot-shape-complex-private.hh | 8 ++++++++ src/hb-ot-shape-complex-thai.cc | 1 + src/hb-ot-shape-complex-tibetan.cc | 1 + src/hb-ot-shape-complex-use.cc | 1 + src/hb-ot-shape.cc | 29 ++++++++++++++++------------- 11 files changed, 46 insertions(+), 13 deletions(-) diff --git a/src/hb-ot-shape-complex-arabic.cc b/src/hb-ot-shape-complex-arabic.cc index 4da89905..56ec5cd6 100644 --- 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 be60e56f..42830ab6 100644 --- 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 5f4d98b7..eb95a28c 100644 --- 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 32159002..96f24946 100644 --- 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 94556f65..ec6e82c4 100644 --- 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 577d790c..bb68622e 100644 --- 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 fb0c7040..39572dfe 100644 --- 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 4322b0d1..e6f80f59 100644 --- 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 a77b531d..aadf59f5 100644 --- 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 045ead52..13c7ab3b 100644 --- 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 cf916cc8..ddd6662e 100644 --- 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; -- GitLab