Merge pull request #4023 from harfbuzz/aat-features2

[aat] Support feature ranges

src/hb-aat-layout-common.hh

@@ -28,6 +28,7 @@
 #define HB_AAT_LAYOUT_COMMON_HH
 
 #include "hb-aat-layout.hh"
+#include "hb-aat-map.hh"
 #include "hb-open-type.hh"
 
 namespace OT {
@@ -39,6 +40,43 @@ namespace AAT {
 using namespace OT;
 
 
+struct ankr;
+
+struct hb_aat_apply_context_t :
+       hb_dispatch_context_t<hb_aat_apply_context_t, bool, HB_DEBUG_APPLY>
+{
+  const char *get_name () { return "APPLY"; }
+  template <typename T>
+  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<hb_aat_map_t::range_flags_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<hb_blob_t *> (&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
  */
@@ -740,16 +778,44 @@ struct StateTableDriver
 	      num_glyphs (face_->get_num_glyphs ()) {}
 
   template <typename context_t>
-  void drive (context_t *c)
+  void drive (context_t *c, hb_aat_apply_context_t *ac)
   {
     if (!c->in_place)
       buffer->clear_output ();
 
     int state = StateTableT::STATE_START_OF_TEXT;
+    // If there's only one range, we already checked the flag.
+    auto *last_range = ac->range_flags && (ac->range_flags->length > 1) ? &(*ac->range_flags)[0] : nullptr;
     for (buffer->idx = 0; buffer->successful;)
     {
+      /* This block is copied in NoncontextualSubtable::apply. Keep in sync. */
+      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++;
+
+
+	  last_range = range;
+	}
+	if (!(range->flags & ac->subtable_flags))
+	{
+	  if (buffer->idx == buffer->len || unlikely (!buffer->successful))
+	    break;
+
+	  state = StateTableT::STATE_START_OF_TEXT;
+	  (void) buffer->next_glyph ();
+	  continue;
+	}
+      }
+
       unsigned int klass = buffer->idx < buffer->len ?
-			   machine.get_class (buffer->info[buffer->idx].codepoint, num_glyphs) :
+			   machine.get_class (buffer->cur().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);
@@ -845,41 +911,6 @@ struct StateTableDriver
 };
 
 
-struct ankr;
-
-struct hb_aat_apply_context_t :
-       hb_dispatch_context_t<hb_aat_apply_context_t, bool, HB_DEBUG_APPLY>
-{
-  const char *get_name () { return "APPLY"; }
-  template <typename T>
-  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<hb_blob_t *> (&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 */
 
 

src/hb-aat-layout-kerx-table.hh

@@ -350,7 +350,7 @@ struct KerxSubTableFormat1
     driver_context_t dc (this, c);
 
     StateTableDriver<Types, EntryData> driver (machine, c->buffer, c->font->face);
-    driver.drive (&dc);
+    driver.drive (&dc, c);
 
     return_trace (true);
   }
@@ -594,7 +594,7 @@ struct KerxSubTableFormat4
     driver_context_t dc (this, c);
 
     StateTableDriver<Types, EntryData> driver (machine, c->buffer, c->font->face);
-    driver.drive (&dc);
+    driver.drive (&dc, c);
 
     return_trace (true);
   }
@@ -869,6 +869,8 @@ struct KerxTable
 
   bool apply (AAT::hb_aat_apply_context_t *c) const
   {
+    c->buffer->unsafe_to_concat ();
+
     typedef typename T::SubTable SubTable;
 
     bool ret = false;

src/hb-aat-layout-morx-table.hh

@@ -169,7 +169,7 @@ struct RearrangementSubtable
     driver_context_t dc (this);
 
     StateTableDriver<Types, EntryData> driver (machine, c->buffer, c->face);
-    driver.drive (&dc);
+    driver.drive (&dc, c);
 
     return_trace (dc.ret);
   }
@@ -325,7 +325,7 @@ struct ContextualSubtable
     driver_context_t dc (this, c);
 
     StateTableDriver<Types, EntryData> driver (machine, c->buffer, c->face);
-    driver.drive (&dc);
+    driver.drive (&dc, c);
 
     return_trace (dc.ret);
   }
@@ -577,7 +577,7 @@ struct LigatureSubtable
     driver_context_t dc (this, c);
 
     StateTableDriver<Types, EntryData> driver (machine, c->buffer, c->face);
-    driver.drive (&dc);
+    driver.drive (&dc, c);
 
     return_trace (dc.ret);
   }
@@ -618,8 +618,27 @@ struct NoncontextualSubtable
 
     hb_glyph_info_t *info = c->buffer->info;
     unsigned int count = c->buffer->len;
+    // If there's only one range, we already checked the flag.
+    auto *last_range = c->range_flags && (c->range_flags->length > 1) ? &(*c->range_flags)[0] : nullptr;
     for (unsigned int i = 0; i < count; i++)
     {
+      /* This block copied from StateTableDriver::drive. Keep in sync. */
+      if (last_range)
+      {
+	auto *range = last_range;
+	{
+	  unsigned cluster = info[i].cluster;
+	  while (cluster < range->cluster_first)
+	    range--;
+	  while (cluster > range->cluster_last)
+	    range++;
+
+	  last_range = range;
+	}
+	if (!(range->flags & c->subtable_flags))
+	  continue;
+      }
+
       const HBGlyphID16 *replacement = substitute.get_value (info[i].codepoint, num_glyphs);
       if (replacement)
       {
@@ -820,7 +839,7 @@ struct InsertionSubtable
     driver_context_t dc (this, c);
 
     StateTableDriver<Types, EntryData> driver (machine, c->buffer, c->face);
-    driver.drive (&dc);
+    driver.drive (&dc, c);
 
     return_trace (dc.ret);
   }
@@ -968,7 +987,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->features.bsearch (info))
+	if (map->current_features.bsearch (info))
 	{
 	  flags &= feature.disableFlags;
 	  flags |= feature.enableFlags;
@@ -994,8 +1013,7 @@ struct Chain
     return flags;
   }
 
-  void apply (hb_aat_apply_context_t *c,
+  void apply (hb_aat_apply_context_t *c) const
-	      hb_mask_t flags) const
   {
     const ChainSubtable<Types> *subtable = &StructAfter<ChainSubtable<Types>> (featureZ.as_array (featureCount));
     unsigned int count = subtableCount;
@@ -1003,8 +1021,10 @@ struct Chain
     {
       bool reverse;
 
-      if (!(subtable->subFeatureFlags & flags))
+      if (hb_none (hb_iter (c->range_flags) |
+		   hb_map ([&subtable] (const hb_aat_map_t::range_flags_t _) -> bool { return subtable->subFeatureFlags & (_.flags); })))
 	goto skip;
+      c->subtable_flags = subtable->subFeatureFlags;
 
       if (!(subtable->get_coverage() & ChainSubtable<Types>::AllDirections) &&
 	  HB_DIRECTION_IS_VERTICAL (c->buffer->props.direction) !=
@@ -1120,22 +1140,31 @@ struct mortmorx
   {
     const Chain<Types> *chain = &firstChain;
     unsigned int count = chainCount;
+    if (unlikely (!map->chain_flags.resize (count)))
+      return;
     for (unsigned int i = 0; i < count; i++)
     {
-      map->chain_flags.push (chain->compile_flags (mapper));
+      map->chain_flags[i].push (hb_aat_map_t::range_flags_t {chain->compile_flags (mapper),
+							     mapper->range_first,
+							     mapper->range_last});
       chain = &StructAfter<Chain<Types>> (*chain);
     }
   }
 
-  void apply (hb_aat_apply_context_t *c) const
+  void apply (hb_aat_apply_context_t *c,
+	      const hb_aat_map_t &map) const
   {
     if (unlikely (!c->buffer->successful)) return;
+
+    c->buffer->unsafe_to_concat ();
+
     c->set_lookup_index (0);
     const Chain<Types> *chain = &firstChain;
     unsigned int count = chainCount;
     for (unsigned int i = 0; i < count; i++)
     {
-      chain->apply (c, c->plan->aat_map.chain_flags[i]);
+      c->range_flags = &map.chain_flags[i];
+      chain->apply (c);
       if (unlikely (!c->buffer->successful)) return;
       chain = &StructAfter<Chain<Types>> (*chain);
     }

src/hb-aat-layout.cc

@@ -244,15 +244,23 @@ 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)
+			  hb_buffer_t *buffer,
+			  const hb_feature_t *features,
+			  unsigned num_features)
 {
+  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<AAT::morx> ();
   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);
+    morx.apply (&c, map);
     (void) buffer->message (font, "end table morx");
     return;
   }
@@ -263,7 +271,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);
+    mort.apply (&c, map);
     (void) buffer->message (font, "end table mort");
     return;
   }

src/hb-aat-layout.hh

@@ -53,7 +53,9 @@ 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);
+			  hb_buffer_t *buffer,
+			  const hb_feature_t *features,
+			  unsigned num_features);
 
 HB_INTERNAL void
 hb_aat_layout_zero_width_deleted_glyphs (hb_buffer_t *buffer);

src/hb-aat-map.cc

@@ -36,27 +36,29 @@
 #include "hb-aat-layout-feat-table.hh"
 
 
-void hb_aat_map_builder_t::add_feature (hb_tag_t tag, unsigned value)
+void hb_aat_map_builder_t::add_feature (const hb_feature_t &feature)
 {
   if (!face->table.feat->has_data ()) return;
 
-  if (tag == HB_TAG ('a','a','l','t'))
+  if (feature.tag == HB_TAG ('a','a','l','t'))
   {
     if (!face->table.feat->exposes_feature (HB_AAT_LAYOUT_FEATURE_TYPE_CHARACTER_ALTERNATIVES))
       return;
-    feature_info_t *info = features.push();
+    feature_range_t *range = features.push();
-    info->type = HB_AAT_LAYOUT_FEATURE_TYPE_CHARACTER_ALTERNATIVES;
+    range->start = feature.start;
-    info->setting = (hb_aat_layout_feature_selector_t) value;
+    range->end = feature.end;
-    info->seq = features.length;
+    range->info.type = HB_AAT_LAYOUT_FEATURE_TYPE_CHARACTER_ALTERNATIVES;
-    info->is_exclusive = true;
+    range->info.setting = (hb_aat_layout_feature_selector_t) feature.value;
+    range->info.seq = features.length;
+    range->info.is_exclusive = true;
     return;
   }
 
-  const hb_aat_feature_mapping_t *mapping = hb_aat_layout_find_feature_mapping (tag);
+  const hb_aat_feature_mapping_t *mapping = hb_aat_layout_find_feature_mapping (feature.tag);
   if (!mapping) return;
 
-  const AAT::FeatureName* feature = &face->table.feat->get_feature (mapping->aatFeatureType);
+  const AAT::FeatureName* feature_name = &face->table.feat->get_feature (mapping->aatFeatureType);
-  if (!feature->has_data ())
+  if (!feature_name->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.
@@ -64,38 +66,106 @@ void hb_aat_map_builder_t::add_feature (hb_tag_t tag, unsigned value)
     if (mapping->aatFeatureType == HB_AAT_LAYOUT_FEATURE_TYPE_LOWER_CASE &&
 	mapping->selectorToEnable == HB_AAT_LAYOUT_FEATURE_SELECTOR_LOWER_CASE_SMALL_CAPS)
     {
-      feature = &face->table.feat->get_feature (HB_AAT_LAYOUT_FEATURE_TYPE_LETTER_CASE);
+      feature_name = &face->table.feat->get_feature (HB_AAT_LAYOUT_FEATURE_TYPE_LETTER_CASE);
-      if (!feature->has_data ()) return;
+      if (!feature_name->has_data ()) return;
     }
     else return;
   }
 
-  feature_info_t *info = features.push();
+  feature_range_t *range = features.push();
-  info->type = mapping->aatFeatureType;
+  range->start = feature.start;
-  info->setting = value ? mapping->selectorToEnable : mapping->selectorToDisable;
+  range->end = feature.end;
-  info->seq = features.length;
+  range->info.type = mapping->aatFeatureType;
-  info->is_exclusive = feature->is_exclusive ();
+  range->info.setting = feature.value ? mapping->selectorToEnable : mapping->selectorToDisable;
+  range->info.seq = features.length;
+  range->info.is_exclusive = feature_name->is_exclusive ();
 }
 
 void
 hb_aat_map_builder_t::compile (hb_aat_map_t  &m)
 {
-  /* Sort features and merge duplicates */
+  /* Compute active features per range, and compile each. */
-  if (features.length)
+
+  /* Sort features by start/end events. */
+  hb_vector_t<feature_event_t> feature_events;
+  for (unsigned int i = 0; i < features.length; i++)
   {
-    features.qsort ();
+    auto &feature = features[i];
+
+    if (features[i].start == features[i].end)
+      continue;
+
+    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 = -1; /* This value does magic. */
+    event->start = false;
+    event->feature = feature;
+  }
+
+  /* Scan events and save features for each range. */
+  hb_sorted_vector_t<feature_info_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 < features.length; i++)
+	for (unsigned int i = 1; i < current_features.length; i++)
-      if (features[i].type != features[j].type ||
+	  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. */
-	  (!features[i].is_exclusive && ((features[i].setting & ~1) != (features[j].setting & ~1))))
+	      (!current_features[i].is_exclusive && ((current_features[i].setting & ~1) != (current_features[j].setting & ~1))))
-	features[++j] = features[i];
+	    current_features[++j] = current_features[i];
-    features.shrink (j + 1);
+	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);
+    }
+  }
+
+  for (auto &chain_flags : m.chain_flags)
+    // With our above setup this value is one less than desired; adjust it.
+    chain_flags.tail().cluster_last = HB_FEATURE_GLOBAL_END;
 }
 
 

src/hb-aat-map.hh

@@ -35,16 +35,15 @@ struct hb_aat_map_t
   friend struct hb_aat_map_builder_t;
 
   public:
-
+  struct range_flags_t
-  void init ()
   {
-    hb_memset (this, 0, sizeof (*this));
+    hb_mask_t flags;
-    chain_flags.init ();
+    unsigned cluster_first;
-  }
+    unsigned cluster_last; // end - 1
-  void fini () { chain_flags.fini (); }
+  };
 
   public:
-  hb_vector_t<hb_mask_t> chain_flags;
+  hb_vector_t<hb_sorted_vector_t<range_flags_t>> chain_flags;
 };
 
 struct hb_aat_map_builder_t
@@ -56,7 +55,7 @@ struct hb_aat_map_builder_t
 				      face (face_),
 				      props (props_) {}
 
-  HB_INTERNAL void add_feature (hb_tag_t tag, unsigned int value=1);
+  HB_INTERNAL void add_feature (const hb_feature_t &feature);
 
   HB_INTERNAL void compile (hb_aat_map_t  &m);
 
@@ -78,7 +77,7 @@ struct hb_aat_map_builder_t
 	    return (a->seq < b->seq ? -1 : a->seq > b->seq ? 1 : 0);
     }
 
-    /* compares type & setting only, not is_exclusive flag or seq number */
+    /* compares type & setting only */
     int cmp (const feature_info_t& f) const
     {
       return (f.type != type) ? (f.type < type ? -1 : 1) :
@@ -86,12 +85,38 @@ 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<feature_info_t> features;
+  hb_sorted_vector_t<feature_range_t> features;
+  hb_sorted_vector_t<feature_info_t> current_features;
+  unsigned range_first;
+  unsigned range_last;
 };
 
 

src/hb-ot-shape.cc

@@ -80,8 +80,7 @@ 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
@@ -105,10 +104,6 @@ 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'));
@@ -222,9 +217,6 @@ 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);
@@ -241,9 +233,6 @@ 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;
     }
   }
@@ -258,20 +247,12 @@ hb_ot_shape_plan_t::fini ()
     shaper->data_destroy (const_cast<void *> (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
 {
-#ifndef HB_NO_AAT_SHAPE
-  if (unlikely (apply_morx))
-    hb_aat_layout_substitute (this, font, buffer);
-  else
-#endif
   map.substitute (this, font, buffer);
 }
 
@@ -406,18 +387,6 @@ 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);
 }
@@ -940,6 +909,12 @@ 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);
 }
 

src/hb-ot-shape.hh

@@ -65,7 +65,6 @@ 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;
@@ -152,7 +151,6 @@ 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

test/shape/data/in-house/tests/macos.tests

@@ -1,3 +1,6 @@
+# https://github.com/harfbuzz/harfbuzz/issues/4020
+/System/Library/Fonts/LucidaGrande.ttc;--features=-liga[3:5];U+0066,U+0066,U+0020,U+0066,U+0066,U+0066,U+0020,U+0066,U+0066;[ff=0+1443|space=2+648|f=3+753|f=4+753|f=5+753|space=6+648|ff=7+1443]
+
 # https://github.com/harfbuzz/harfbuzz/issues/1373
 /System/Library/Fonts/Supplemental/BigCaslon.ttf;;U+0107;[cacute=0+432]
 /System/Library/Fonts/Supplemental/BigCaslon.ttf;--language=pl;U+0107;[cacute.polish=0+432]
@@ -11,11 +14,11 @@
 # https://github.com/harfbuzz/harfbuzz/issues/3535
 /System/Library/Fonts/LucidaGrande.ttc;;U+20DD,U+1F174,U+1F175;[circlecmb=0+0|.notdef=1+1536|.notdef=2+1536]
 
-# https;//github.com/harfbuzz/harfbuzz/issues/3008
+# https://github.com/harfbuzz/harfbuzz/issues/3008
 /System/Library/Fonts/ヒラギノ丸ゴ\ ProN\ W4.ttc;--features=palt;U+FF11;[gid781=0@-78,0+842]
 
-# https;//github.com/harfbuzz/harfbuzz/pull/2871
+# https://github.com/harfbuzz/harfbuzz/pull/2871
-/System/Library/Fonts/LucidaGrande.ttc;--font-funcs ot --show-flags;U+0041,U+0042,U+0043,U+0044;[A=0+1413|B=1+1178|C=2+1417|D=3+1534]
+/System/Library/Fonts/LucidaGrande.ttc;--font-funcs ot --show-flags;U+0041,U+0042,U+0043,U+0044;[A=0+1413|B=1+1178#2|C=2+1417#2|D=3+1534#2]
 
 # 10.12.6 https;//gist.github.com/ebraminio/1704341fa16b06979e605aafd88198cf
 /System/Library/Fonts/Helvetica.dfont@c7bec2785a4c402b7809b5e35337c3d24c18e281;--font-funcs ot;U+006D,U+0300;[m=0+1706|gravecmb=0@-284,10+0]