From 6a7a38521f940216f1e9e2fa2bf22f7b45ce2aef Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Mon, 9 Jan 2023 21:29:19 -0700 Subject: [PATCH] Revert "[aat] Support feature ranges" This reverts commit 1b7994cb3a3c35f3618d7f40c7289496bdab6f06. Broke Zapfino with partial ligature disabling. Debugging. --- src/hb-aat-layout-common.hh | 105 ++++++++++----------------- src/hb-aat-layout-kerx-table.hh | 4 +- src/hb-aat-layout-morx-table.hh | 27 +++---- src/hb-aat-layout.cc | 14 +--- src/hb-aat-layout.hh | 4 +- src/hb-aat-map.cc | 125 ++++++++------------------------ src/hb-aat-map.hh | 45 +++--------- src/hb-ot-shape.cc | 43 ++++++++--- src/hb-ot-shape.hh | 2 + 9 files changed, 130 insertions(+), 239 deletions(-) diff --git a/src/hb-aat-layout-common.hh b/src/hb-aat-layout-common.hh index 95e6b05d9..6cbed8269 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -28,7 +28,6 @@ #define HB_AAT_LAYOUT_COMMON_HH #include "hb-aat-layout.hh" -#include "hb-aat-map.hh" #include "hb-open-type.hh" namespace OT { @@ -40,43 +39,6 @@ namespace AAT { using namespace OT; -struct ankr; - -struct hb_aat_apply_context_t : - hb_dispatch_context_t -{ - const char *get_name () { return "APPLY"; } - template - return_t dispatch (const T &obj) { return obj.apply (this); } - static return_t default_return_value () { return false; } - bool stop_sublookup_iteration (return_t r) const { return r; } - - const hb_ot_shape_plan_t *plan; - hb_font_t *font; - hb_face_t *face; - hb_buffer_t *buffer; - hb_sanitize_context_t sanitizer; - const ankr *ankr_table; - const OT::GDEF *gdef_table; - const hb_sorted_vector_t *range_flags = nullptr; - hb_mask_t subtable_flags = 0; - - /* Unused. For debug tracing only. */ - unsigned int lookup_index; - - HB_INTERNAL hb_aat_apply_context_t (const hb_ot_shape_plan_t *plan_, - hb_font_t *font_, - hb_buffer_t *buffer_, - hb_blob_t *blob = const_cast (&Null (hb_blob_t))); - - HB_INTERNAL ~hb_aat_apply_context_t (); - - HB_INTERNAL void set_ankr_table (const AAT::ankr *ankr_table_); - - void set_lookup_index (unsigned int i) { lookup_index = i; } -}; - - /* * Lookup Table */ @@ -778,44 +740,16 @@ struct StateTableDriver num_glyphs (face_->get_num_glyphs ()) {} template - void drive (context_t *c, hb_aat_apply_context_t *ac) + void drive (context_t *c) { if (!c->in_place) buffer->clear_output (); int state = StateTableT::STATE_START_OF_TEXT; - auto *last_range = ac->range_flags && (*ac->range_flags) ? &(*ac->range_flags)[0] : nullptr; for (buffer->idx = 0; buffer->successful;) { - if (last_range) - { - auto *range = last_range; - if (buffer->idx < buffer->len) - { - unsigned cluster = buffer->cur().cluster; - while (cluster < range->cluster_first) - range--; - while (cluster > range->cluster_last) - range++; - - if (range != last_range) - state = StateTableT::STATE_START_OF_TEXT; - - last_range = range; - } - - if (!(range->flags & ac->subtable_flags)) - { - if (buffer->idx == buffer->len || unlikely (!buffer->successful)) - break; - - (void) buffer->next_glyph (); - continue; - } - } - unsigned int klass = buffer->idx < buffer->len ? - machine.get_class (buffer->cur().codepoint, num_glyphs) : + machine.get_class (buffer->info[buffer->idx].codepoint, num_glyphs) : (unsigned) StateTableT::CLASS_END_OF_TEXT; DEBUG_MSG (APPLY, nullptr, "c%u at %u", klass, buffer->idx); const EntryT &entry = machine.get_entry (state, klass); @@ -911,6 +845,41 @@ struct StateTableDriver }; +struct ankr; + +struct hb_aat_apply_context_t : + hb_dispatch_context_t +{ + const char *get_name () { return "APPLY"; } + template + return_t dispatch (const T &obj) { return obj.apply (this); } + static return_t default_return_value () { return false; } + bool stop_sublookup_iteration (return_t r) const { return r; } + + const hb_ot_shape_plan_t *plan; + hb_font_t *font; + hb_face_t *face; + hb_buffer_t *buffer; + hb_sanitize_context_t sanitizer; + const ankr *ankr_table; + const OT::GDEF *gdef_table; + + /* Unused. For debug tracing only. */ + unsigned int lookup_index; + + HB_INTERNAL hb_aat_apply_context_t (const hb_ot_shape_plan_t *plan_, + hb_font_t *font_, + hb_buffer_t *buffer_, + hb_blob_t *blob = const_cast (&Null (hb_blob_t))); + + HB_INTERNAL ~hb_aat_apply_context_t (); + + HB_INTERNAL void set_ankr_table (const AAT::ankr *ankr_table_); + + void set_lookup_index (unsigned int i) { lookup_index = i; } +}; + + } /* namespace AAT */ diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index c96ca300e..995492cd5 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -350,7 +350,7 @@ struct KerxSubTableFormat1 driver_context_t dc (this, c); StateTableDriver driver (machine, c->buffer, c->font->face); - driver.drive (&dc, c); + driver.drive (&dc); return_trace (true); } @@ -594,7 +594,7 @@ struct KerxSubTableFormat4 driver_context_t dc (this, c); StateTableDriver driver (machine, c->buffer, c->font->face); - driver.drive (&dc, c); + driver.drive (&dc); return_trace (true); } diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh index 7d2e958fb..8b9190d0b 100644 --- a/src/hb-aat-layout-morx-table.hh +++ b/src/hb-aat-layout-morx-table.hh @@ -169,7 +169,7 @@ struct RearrangementSubtable driver_context_t dc (this); StateTableDriver driver (machine, c->buffer, c->face); - driver.drive (&dc, c); + driver.drive (&dc); return_trace (dc.ret); } @@ -325,7 +325,7 @@ struct ContextualSubtable driver_context_t dc (this, c); StateTableDriver driver (machine, c->buffer, c->face); - driver.drive (&dc, c); + driver.drive (&dc); return_trace (dc.ret); } @@ -577,7 +577,7 @@ struct LigatureSubtable driver_context_t dc (this, c); StateTableDriver driver (machine, c->buffer, c->face); - driver.drive (&dc, c); + driver.drive (&dc); return_trace (dc.ret); } @@ -820,7 +820,7 @@ struct InsertionSubtable driver_context_t dc (this, c); StateTableDriver driver (machine, c->buffer, c->face); - driver.drive (&dc, c); + driver.drive (&dc); return_trace (dc.ret); } @@ -968,7 +968,7 @@ struct Chain // Check whether this type/setting pair was requested in the map, and if so, apply its flags. // (The search here only looks at the type and setting fields of feature_info_t.) hb_aat_map_builder_t::feature_info_t info = { type, setting, false, 0 }; - if (map->current_features.bsearch (info)) + if (map->features.bsearch (info)) { flags &= feature.disableFlags; flags |= feature.enableFlags; @@ -994,7 +994,8 @@ struct Chain return flags; } - void apply (hb_aat_apply_context_t *c) const + void apply (hb_aat_apply_context_t *c, + hb_mask_t flags) const { const ChainSubtable *subtable = &StructAfter> (featureZ.as_array (featureCount)); unsigned int count = subtableCount; @@ -1002,9 +1003,8 @@ struct Chain { bool reverse; - if (c->range_flags->length == 1 && !(subtable->subFeatureFlags & (*c->range_flags)[0].flags)) + if (!(subtable->subFeatureFlags & flags)) goto skip; - c->subtable_flags = subtable->subFeatureFlags; if (!(subtable->get_coverage() & ChainSubtable::AllDirections) && HB_DIRECTION_IS_VERTICAL (c->buffer->props.direction) != @@ -1120,18 +1120,14 @@ struct mortmorx { const Chain *chain = &firstChain; unsigned int count = chainCount; - map->chain_flags.resize (count); for (unsigned int i = 0; i < count; i++) { - map->chain_flags[i].push (hb_aat_map_t::range_flags_t {chain->compile_flags (mapper), - mapper->range_first, - mapper->range_last}); + map->chain_flags.push (chain->compile_flags (mapper)); chain = &StructAfter> (*chain); } } - void apply (hb_aat_apply_context_t *c, - const hb_aat_map_t &map) const + void apply (hb_aat_apply_context_t *c) const { if (unlikely (!c->buffer->successful)) return; c->set_lookup_index (0); @@ -1139,8 +1135,7 @@ struct mortmorx unsigned int count = chainCount; for (unsigned int i = 0; i < count; i++) { - c->range_flags = &map.chain_flags[i]; - chain->apply (c); + chain->apply (c, c->plan->aat_map.chain_flags[i]); if (unlikely (!c->buffer->successful)) return; chain = &StructAfter> (*chain); } diff --git a/src/hb-aat-layout.cc b/src/hb-aat-layout.cc index c9147ff73..78427b0d5 100644 --- a/src/hb-aat-layout.cc +++ b/src/hb-aat-layout.cc @@ -244,23 +244,15 @@ hb_aat_layout_has_substitution (hb_face_t *face) void hb_aat_layout_substitute (const hb_ot_shape_plan_t *plan, hb_font_t *font, - hb_buffer_t *buffer, - const hb_feature_t *features, - unsigned num_features) + hb_buffer_t *buffer) { - hb_aat_map_builder_t builder (font->face, plan->props); - for (unsigned i = 0; i < num_features; i++) - builder.add_feature (features[i]); - hb_aat_map_t map; - builder.compile (map); - hb_blob_t *morx_blob = font->face->table.morx.get_blob (); const AAT::morx& morx = *morx_blob->as (); if (morx.has_data ()) { AAT::hb_aat_apply_context_t c (plan, font, buffer, morx_blob); if (!buffer->message (font, "start table morx")) return; - morx.apply (&c, map); + morx.apply (&c); (void) buffer->message (font, "end table morx"); return; } @@ -271,7 +263,7 @@ hb_aat_layout_substitute (const hb_ot_shape_plan_t *plan, { AAT::hb_aat_apply_context_t c (plan, font, buffer, mort_blob); if (!buffer->message (font, "start table mort")) return; - mort.apply (&c, map); + mort.apply (&c); (void) buffer->message (font, "end table mort"); return; } diff --git a/src/hb-aat-layout.hh b/src/hb-aat-layout.hh index 15c382aa9..5e4e3bda1 100644 --- a/src/hb-aat-layout.hh +++ b/src/hb-aat-layout.hh @@ -53,9 +53,7 @@ hb_aat_layout_compile_map (const hb_aat_map_builder_t *mapper, HB_INTERNAL void hb_aat_layout_substitute (const hb_ot_shape_plan_t *plan, hb_font_t *font, - hb_buffer_t *buffer, - const hb_feature_t *features, - unsigned num_features); + hb_buffer_t *buffer); HB_INTERNAL void hb_aat_layout_zero_width_deleted_glyphs (hb_buffer_t *buffer); diff --git a/src/hb-aat-map.cc b/src/hb-aat-map.cc index f18972439..2c38c3502 100644 --- a/src/hb-aat-map.cc +++ b/src/hb-aat-map.cc @@ -36,29 +36,27 @@ #include "hb-aat-layout-feat-table.hh" -void hb_aat_map_builder_t::add_feature (const hb_feature_t &feature) +void hb_aat_map_builder_t::add_feature (hb_tag_t tag, unsigned value) { if (!face->table.feat->has_data ()) return; - if (feature.tag == HB_TAG ('a','a','l','t')) + if (tag == HB_TAG ('a','a','l','t')) { if (!face->table.feat->exposes_feature (HB_AAT_LAYOUT_FEATURE_TYPE_CHARACTER_ALTERNATIVES)) return; - feature_range_t *range = features.push(); - range->start = feature.start; - range->end = feature.end; - range->info.type = HB_AAT_LAYOUT_FEATURE_TYPE_CHARACTER_ALTERNATIVES; - range->info.setting = (hb_aat_layout_feature_selector_t) feature.value; - range->info.seq = features.length; - range->info.is_exclusive = true; + feature_info_t *info = features.push(); + info->type = HB_AAT_LAYOUT_FEATURE_TYPE_CHARACTER_ALTERNATIVES; + info->setting = (hb_aat_layout_feature_selector_t) value; + info->seq = features.length; + info->is_exclusive = true; return; } - const hb_aat_feature_mapping_t *mapping = hb_aat_layout_find_feature_mapping (feature.tag); + const hb_aat_feature_mapping_t *mapping = hb_aat_layout_find_feature_mapping (tag); if (!mapping) return; - const AAT::FeatureName* feature_name = &face->table.feat->get_feature (mapping->aatFeatureType); - if (!feature_name->has_data ()) + const AAT::FeatureName* feature = &face->table.feat->get_feature (mapping->aatFeatureType); + if (!feature->has_data ()) { /* Special case: Chain::compile_flags will fall back to the deprecated version of * small-caps if necessary, so we need to check for that possibility. @@ -66,101 +64,38 @@ void hb_aat_map_builder_t::add_feature (const hb_feature_t &feature) if (mapping->aatFeatureType == HB_AAT_LAYOUT_FEATURE_TYPE_LOWER_CASE && mapping->selectorToEnable == HB_AAT_LAYOUT_FEATURE_SELECTOR_LOWER_CASE_SMALL_CAPS) { - feature_name = &face->table.feat->get_feature (HB_AAT_LAYOUT_FEATURE_TYPE_LETTER_CASE); - if (!feature_name->has_data ()) return; + feature = &face->table.feat->get_feature (HB_AAT_LAYOUT_FEATURE_TYPE_LETTER_CASE); + if (!feature->has_data ()) return; } else return; } - feature_range_t *range = features.push(); - range->start = feature.start; - range->end = feature.end; - range->info.type = mapping->aatFeatureType; - range->info.setting = feature.value ? mapping->selectorToEnable : mapping->selectorToDisable; - range->info.seq = features.length; - range->info.is_exclusive = feature_name->is_exclusive (); + feature_info_t *info = features.push(); + info->type = mapping->aatFeatureType; + info->setting = value ? mapping->selectorToEnable : mapping->selectorToDisable; + info->seq = features.length; + info->is_exclusive = feature->is_exclusive (); } void hb_aat_map_builder_t::compile (hb_aat_map_t &m) { - /* Compute active features per range, and compile each. */ + /* Sort features and merge duplicates */ if (features.length) { - /* Sort features by start/end events. */ - hb_vector_t feature_events; - for (unsigned int i = 0; i < features.length; i++) - { - auto &feature = features[i]; - - feature_event_t *event; - - event = feature_events.push (); - event->index = features[i].start; - event->start = true; - event->feature = feature.info; - - event = feature_events.push (); - event->index = features[i].end; - event->start = false; - event->feature = feature.info; - } - feature_events.qsort (); - /* Add a strategic final event. */ - { - feature_info_t feature; - feature.seq = features.length + 1; - - feature_event_t *event = feature_events.push (); - event->index = 0; /* This value does magic. */ - event->start = false; - event->feature = feature; - } - - /* Scan events and save features for each range. */ - hb_sorted_vector_t active_features; - unsigned int last_index = 0; - for (unsigned int i = 0; i < feature_events.length; i++) - { - feature_event_t *event = &feature_events[i]; - - if (event->index != last_index) - { - /* Save a snapshot of active features and the range. */ - - /* Sort features and merge duplicates */ - current_features = active_features; - range_first = last_index; - range_last = event->index - 1; - if (current_features.length) - { - current_features.qsort (); - unsigned int j = 0; - for (unsigned int i = 1; i < current_features.length; i++) - if (current_features[i].type != current_features[j].type || - /* Nonexclusive feature selectors come in even/odd pairs to turn a setting on/off - * respectively, so we mask out the low-order bit when checking for "duplicates" - * (selectors referring to the same feature setting) here. */ - (!current_features[i].is_exclusive && ((current_features[i].setting & ~1) != (current_features[j].setting & ~1)))) - current_features[++j] = current_features[i]; - current_features.shrink (j + 1); - } - - hb_aat_layout_compile_map (this, &m); - - last_index = event->index; - } - - if (event->start) - { - active_features.push (event->feature); - } else { - feature_info_t *feature = active_features.lsearch (event->feature); - if (feature) - active_features.remove_ordered (feature - active_features.arrayZ); - } - } + features.qsort (); + unsigned int j = 0; + for (unsigned int i = 1; i < features.length; i++) + if (features[i].type != features[j].type || + /* Nonexclusive feature selectors come in even/odd pairs to turn a setting on/off + * respectively, so we mask out the low-order bit when checking for "duplicates" + * (selectors referring to the same feature setting) here. */ + (!features[i].is_exclusive && ((features[i].setting & ~1) != (features[j].setting & ~1)))) + features[++j] = features[i]; + features.shrink (j + 1); } + + hb_aat_layout_compile_map (this, &m); } diff --git a/src/hb-aat-map.hh b/src/hb-aat-map.hh index ac14b9c38..d0ee7d672 100644 --- a/src/hb-aat-map.hh +++ b/src/hb-aat-map.hh @@ -35,15 +35,16 @@ struct hb_aat_map_t friend struct hb_aat_map_builder_t; public: - struct range_flags_t + + void init () { - hb_mask_t flags; - unsigned cluster_first; - unsigned cluster_last; // end - 1 - }; + hb_memset (this, 0, sizeof (*this)); + chain_flags.init (); + } + void fini () { chain_flags.fini (); } public: - hb_vector_t> chain_flags; + hb_vector_t chain_flags; }; struct hb_aat_map_builder_t @@ -55,7 +56,7 @@ struct hb_aat_map_builder_t face (face_), props (props_) {} - HB_INTERNAL void add_feature (const hb_feature_t &feature); + HB_INTERNAL void add_feature (hb_tag_t tag, unsigned int value=1); HB_INTERNAL void compile (hb_aat_map_t &m); @@ -77,7 +78,7 @@ struct hb_aat_map_builder_t return (a->seq < b->seq ? -1 : a->seq > b->seq ? 1 : 0); } - /* compares type & setting only */ + /* compares type & setting only, not is_exclusive flag or seq number */ int cmp (const feature_info_t& f) const { return (f.type != type) ? (f.type < type ? -1 : 1) : @@ -85,38 +86,12 @@ struct hb_aat_map_builder_t } }; - struct feature_range_t - { - feature_info_t info; - unsigned start; - unsigned end; - }; - - private: - struct feature_event_t - { - unsigned int index; - bool start; - feature_info_t feature; - - HB_INTERNAL static int cmp (const void *pa, const void *pb) { - const feature_event_t *a = (const feature_event_t *) pa; - const feature_event_t *b = (const feature_event_t *) pb; - return a->index < b->index ? -1 : a->index > b->index ? 1 : - a->start < b->start ? -1 : a->start > b->start ? 1 : - feature_info_t::cmp (&a->feature, &b->feature); - } - }; - public: hb_face_t *face; hb_segment_properties_t props; public: - hb_sorted_vector_t features; - hb_sorted_vector_t current_features; - unsigned range_first; - unsigned range_last; + hb_sorted_vector_t features; }; diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index 3d207e068..bbdfc214a 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -80,7 +80,8 @@ hb_ot_shape_planner_t::hb_ot_shape_planner_t (hb_face_t *fac const hb_segment_properties_t &props) : face (face), props (props), - map (face, props) + map (face, props), + aat_map (face, props) #ifndef HB_NO_AAT_SHAPE , apply_morx (_hb_apply_morx (face, props)) #endif @@ -104,6 +105,10 @@ hb_ot_shape_planner_t::compile (hb_ot_shape_plan_t &plan, plan.props = props; plan.shaper = shaper; map.compile (plan.map, key); +#ifndef HB_NO_AAT_SHAPE + if (apply_morx) + aat_map.compile (plan.aat_map); +#endif #ifndef HB_NO_OT_SHAPE_FRACTIONS plan.frac_mask = plan.map.get_1_mask (HB_TAG ('f','r','a','c')); @@ -217,6 +222,9 @@ hb_ot_shape_plan_t::init0 (hb_face_t *face, const hb_shape_plan_key_t *key) { map.init (); +#ifndef HB_NO_AAT_SHAPE + aat_map.init (); +#endif hb_ot_shape_planner_t planner (face, key->props); @@ -233,6 +241,9 @@ hb_ot_shape_plan_t::init0 (hb_face_t *face, if (unlikely (!data)) { map.fini (); +#ifndef HB_NO_AAT_SHAPE + aat_map.fini (); +#endif return false; } } @@ -247,13 +258,21 @@ hb_ot_shape_plan_t::fini () shaper->data_destroy (const_cast (data)); map.fini (); +#ifndef HB_NO_AAT_SHAPE + aat_map.fini (); +#endif } void hb_ot_shape_plan_t::substitute (hb_font_t *font, hb_buffer_t *buffer) const { - map.substitute (this, font, buffer); +#ifndef HB_NO_AAT_SHAPE + if (unlikely (apply_morx)) + hb_aat_layout_substitute (this, font, buffer); + else +#endif + map.substitute (this, font, buffer); } void @@ -387,6 +406,18 @@ hb_ot_shape_collect_features (hb_ot_shape_planner_t *planner, feature->value); } +#ifndef HB_NO_AAT_SHAPE + if (planner->apply_morx) + { + hb_aat_map_builder_t *aat_map = &planner->aat_map; + for (unsigned int i = 0; i < num_user_features; i++) + { + const hb_feature_t *feature = &user_features[i]; + aat_map->add_feature (feature->tag, feature->value); + } + } +#endif + if (planner->shaper->override_features) planner->shaper->override_features (planner); } @@ -909,13 +940,7 @@ hb_ot_substitute_plan (const hb_ot_shape_context_t *c) if (c->plan->fallback_glyph_classes) hb_synthesize_glyph_classes (c->buffer); -#ifndef HB_NO_AAT_SHAPE - if (unlikely (c->plan->apply_morx)) - hb_aat_layout_substitute (c->plan, c->font, c->buffer, - c->user_features, c->num_user_features); - else -#endif - c->plan->substitute (c->font, buffer); + c->plan->substitute (c->font, buffer); } static inline void diff --git a/src/hb-ot-shape.hh b/src/hb-ot-shape.hh index f84aa5c49..ace28602f 100644 --- a/src/hb-ot-shape.hh +++ b/src/hb-ot-shape.hh @@ -65,6 +65,7 @@ struct hb_ot_shape_plan_t hb_segment_properties_t props; const struct hb_ot_shaper_t *shaper; hb_ot_map_t map; + hb_aat_map_t aat_map; const void *data; #ifndef HB_NO_OT_SHAPE_FRACTIONS hb_mask_t frac_mask, numr_mask, dnom_mask; @@ -151,6 +152,7 @@ struct hb_ot_shape_planner_t hb_face_t *face; hb_segment_properties_t props; hb_ot_map_builder_t map; + hb_aat_map_builder_t aat_map; #ifndef HB_NO_AAT_SHAPE bool apply_morx : 1; #else