[subset] fix for collect_features and remove_redundant_lamngsys

previously remove_redundant_sys () is missing in harfbuzz, after
redundant langsys removal, some features are removed as well in
prune_features() in fonttools. This change is trying to get the same
result between harfbuzz and fonttools.

src/hb-ot-layout-common.hh

@@ -94,6 +94,60 @@ static void ClassDef_remap_and_serialize (hb_serialize_context_t *c,
                                           bool use_class_zero,
 					  hb_map_t *klass_map /*INOUT*/);
 
+
+struct hb_prune_langsys_context_t
+{
+  hb_prune_langsys_context_t (const void         *table_,
+                              hb_hashmap_t<unsigned, hb_set_t *, (unsigned)-1, nullptr> *script_langsys_map_,
+                              const hb_map_t     *duplicate_feature_map_,
+                              hb_set_t           *new_collected_feature_indexes_)
+      :table (table_),
+      script_langsys_map (script_langsys_map_),
+      duplicate_feature_map (duplicate_feature_map_),
+      new_feature_indexes (new_collected_feature_indexes_),
+      script_count (0),langsys_count (0) {}
+
+  bool visitedScript (const void *s)
+  {
+    if (script_count++ > HB_MAX_SCRIPTS)
+      return true;
+
+    return visited (s, visited_script);
+  }
+
+  bool visitedLangsys (const void *l)
+  {
+    if (langsys_count++ > HB_MAX_LANGSYS)
+      return true;
+
+    return visited (l, visited_langsys);
+  }
+
+  private:
+  template <typename T>
+  bool visited (const T *p, hb_set_t &visited_set)
+  {
+    hb_codepoint_t delta = (hb_codepoint_t) ((uintptr_t) p - (uintptr_t) table);
+     if (visited_set.has (delta))
+      return true;
+
+    visited_set.add (delta);
+    return false;
+  }
+
+  public:
+  const void *table;
+  hb_hashmap_t<unsigned, hb_set_t *, (unsigned)-1, nullptr> *script_langsys_map;
+  const hb_map_t     *duplicate_feature_map;
+  hb_set_t           *new_feature_indexes;
+
+  private:
+  hb_set_t visited_script;
+  hb_set_t visited_langsys;
+  unsigned script_count;
+  unsigned langsys_count;
+};
+
 struct hb_subset_layout_context_t :
   hb_dispatch_context_t<hb_subset_layout_context_t, hb_empty_t, HB_DEBUG_SUBSET>
 {
@@ -125,16 +179,21 @@ struct hb_subset_layout_context_t :
   hb_subset_context_t *subset_context;
   const hb_tag_t table_tag;
   const hb_map_t *lookup_index_map;
+  const hb_hashmap_t<unsigned, hb_set_t *, (unsigned)-1, nullptr> *script_langsys_map;
   const hb_map_t *feature_index_map;
+  unsigned cur_script_index;
 
   hb_subset_layout_context_t (hb_subset_context_t *c_,
 			      hb_tag_t tag_,
 			      hb_map_t *lookup_map_,
-			      hb_map_t *feature_map_) :
+			      hb_hashmap_t<unsigned, hb_set_t *, (unsigned)-1, nullptr> *script_langsys_map_,
+			      hb_map_t *feature_index_map_) :
 				subset_context (c_),
 				table_tag (tag_),
 				lookup_index_map (lookup_map_),
-				feature_index_map (feature_map_),
+				script_langsys_map (script_langsys_map_),
+				feature_index_map (feature_index_map_),
+				cur_script_index (0xFFFFu),
 				script_count (0),
 				langsys_count (0),
 				feature_index_count (0),
@@ -407,6 +466,30 @@ struct RecordListOfFeature : RecordListOf<Feature>
   }
 };
 
+struct Script;
+struct RecordListOfScript : RecordListOf<Script>
+{
+  bool subset (hb_subset_context_t *c,
+               hb_subset_layout_context_t *l) const
+  {
+    TRACE_SUBSET (this);
+    auto *out = c->serializer->start_embed (*this);
+    if (unlikely (!out || !c->serializer->extend_min (out))) return_trace (false);
+
+    unsigned count = this->len;
+    for (auto _ : + hb_zip (*this, hb_range (count)))
+    {
+      auto snap = c->serializer->snapshot ();
+      l->cur_script_index = _.second;
+      bool ret = _.first.subset (l, this);
+      if (!ret) c->serializer->revert (snap);
+      else out->len++;
+    }
+    
+    return_trace (true);
+  }
+};
+
 struct RangeRecord
 {
   int cmp (hb_codepoint_t g) const
@@ -506,18 +589,46 @@ struct LangSys
     return_trace (c->embed (*this));
   }
 
-  bool operator == (const LangSys& o) const
+  bool compare (const LangSys& o, const hb_map_t *feature_index_map) const
   {
-    if (featureIndex.len != o.featureIndex.len ||
-	reqFeatureIndex != o.reqFeatureIndex)
+    if (reqFeatureIndex != o.reqFeatureIndex)
       return false;
 
-    for (const auto _ : + hb_zip (featureIndex, o.featureIndex))
+    auto iter =
+    + hb_iter (featureIndex)
+    | hb_filter (feature_index_map)
+    | hb_map (feature_index_map)
+    ;
+
+    auto o_iter =
+    + hb_iter (o.featureIndex)
+    | hb_filter (feature_index_map)
+    | hb_map (feature_index_map)
+    ;
+
+    if (iter.len () != o_iter.len ())
+      return false;
+
+    for (const auto _ : + hb_zip (iter, o_iter))
       if (_.first != _.second) return false;
 
     return true;
   }
 
+  void collect_features (hb_prune_langsys_context_t *c) const
+  {
+    if (!has_required_feature () && !get_feature_count ()) return;
+    if (c->visitedLangsys (this)) return;
+    if (has_required_feature () &&
+        c->duplicate_feature_map->has (reqFeatureIndex))
+      c->new_feature_indexes->add (get_required_feature_index ());
+
+    + hb_iter (featureIndex)
+    | hb_filter (c->duplicate_feature_map)
+    | hb_sink (c->new_feature_indexes)
+    ;
+  }
+
   bool subset (hb_subset_context_t        *c,
 	       hb_subset_layout_context_t *l,
 	       const Tag                  *tag = nullptr) const
@@ -581,6 +692,42 @@ struct Script
   bool has_default_lang_sys () const           { return defaultLangSys != 0; }
   const LangSys& get_default_lang_sys () const { return this+defaultLangSys; }
 
+  void prune_langsys (hb_prune_langsys_context_t *c,
+                      unsigned script_index) const
+  {
+    if (!has_default_lang_sys () && !get_lang_sys_count ()) return;
+    if (c->visitedScript (this)) return;
+
+    if (!c->script_langsys_map->has (script_index))
+      c->script_langsys_map->set (script_index, hb_set_create ());
+    
+    unsigned langsys_count = get_lang_sys_count ();
+    if (has_default_lang_sys ())
+    {
+      //only collect features from non-redundant langsys
+      const LangSys& d = get_default_lang_sys ();
+      d.collect_features (c);
+
+      for (auto _ : + hb_zip (langSys, hb_range (langsys_count)))
+      {
+        const LangSys& l = this+_.first.offset;
+        if (l.compare (d, c->duplicate_feature_map)) continue;
+
+        l.collect_features (c);
+        c->script_langsys_map->get (script_index)->add (_.second);
+      }
+    }
+    else
+    {
+      for (auto _ : + hb_zip (langSys, hb_range (langsys_count)))
+      {
+        const LangSys& l = this+_.first.offset;
+        l.collect_features (c);
+        c->script_langsys_map->get (script_index)->add (_.second);
+      }
+    }
+  }
+
   bool subset (hb_subset_context_t         *c,
 	       hb_subset_layout_context_t  *l,
 	       const Tag                   *tag) const
@@ -609,16 +756,17 @@ struct Script
       }
     }
 
-    + langSys.iter ()
-    | hb_filter ([=] (const Record<LangSys>& record) {return l->visitLangSys (); })
-    | hb_filter ([&] (const Record<LangSys>& record)
-		 {
-		   const LangSys& d = this+defaultLangSys;
-		   const LangSys& l = this+record.offset;
-		   return !(l == d);
-		 })
-    | hb_apply (subset_record_array (l, &(out->langSys), this))
-    ;
+    const hb_set_t *active_langsys = l->script_langsys_map->get (l->cur_script_index);
+    if (active_langsys)
+    {
+      unsigned count = langSys.len;
+      + hb_zip (langSys, hb_range (count))
+      | hb_filter (active_langsys, hb_second)
+      | hb_map (hb_first)
+      | hb_filter ([=] (const Record<LangSys>& record) {return l->visitLangSys (); })
+      | hb_apply (subset_record_array (l, &(out->langSys), this))
+      ;
+    }
 
     return_trace (bool (out->langSys.len) || defaultLang || l->table_tag == HB_OT_TAG_GSUB);
   }
@@ -641,7 +789,7 @@ struct Script
   DEFINE_SIZE_ARRAY_SIZED (4, langSys);
 };
 
-typedef RecordListOf<Script> ScriptList;
+typedef RecordListOfScript ScriptList;
 
 
 /* https://docs.microsoft.com/en-us/typography/opentype/spec/features_pt#size */

src/hb-ot-layout-gpos-table.hh

@@ -2666,7 +2666,7 @@ struct GPOS : GSUBGPOS
 
   bool subset (hb_subset_context_t *c) const
   {
-    hb_subset_layout_context_t l (c, tableTag, c->plan->gpos_lookups, c->plan->gpos_features);
+    hb_subset_layout_context_t l (c, tableTag, c->plan->gpos_lookups, c->plan->gpos_langsys, c->plan->gpos_features);
     return GSUBGPOS::subset<PosLookup> (&l);
   }
 

src/hb-ot-layout-gsub-table.hh

@@ -1596,7 +1596,7 @@ struct GSUB : GSUBGPOS
 
   bool subset (hb_subset_context_t *c) const
   {
-    hb_subset_layout_context_t l (c, tableTag, c->plan->gsub_lookups, c->plan->gsub_features);
+    hb_subset_layout_context_t l (c, tableTag, c->plan->gsub_lookups, c->plan->gsub_langsys, c->plan->gsub_features);
     return GSUBGPOS::subset<SubstLookup> (&l);
   }
 

src/hb-ot-layout-gsubgpos.hh

@@ -52,7 +52,7 @@ struct hb_intersects_context_t :
   const hb_set_t *glyphs;
 
   hb_intersects_context_t (const hb_set_t *glyphs_) :
-			     glyphs (glyphs_) {}
+                            glyphs (glyphs_) {}
 };
 
 struct hb_have_non_1to1_context_t :
@@ -3587,6 +3587,20 @@ struct GSUBGPOS
     hb_set_subtract (lookup_indexes, &inactive_lookups);
   }
 
+  void prune_langsys (const hb_map_t *duplicate_feature_map,
+                      hb_hashmap_t<unsigned, hb_set_t *, (unsigned)-1, nullptr> *script_langsys_map,
+                      hb_set_t       *new_feature_indexes /* OUT */) const
+  {
+    hb_prune_langsys_context_t c (this, script_langsys_map, duplicate_feature_map, new_feature_indexes);
+    
+    unsigned count = get_script_count ();
+    for (unsigned script_index = 0; script_index < count; script_index++)
+    {
+      const Script& s = get_script (script_index);
+      s.prune_langsys (&c, script_index);
+    }
+  }
+
   template <typename TLookup>
   bool subset (hb_subset_layout_context_t *c) const
   {
@@ -3627,8 +3641,65 @@ struct GSUBGPOS
     return_trace (true);
   }
 
+  void find_duplicate_features (const hb_map_t *lookup_indices,
+                                const hb_set_t *feature_indices,
+                                hb_map_t *duplicate_feature_map /* OUT */) const
+  {
+    //find out duplicate features after subset
+    unsigned prev = 0xFFFFu;
+    for (unsigned i : feature_indices->iter ())
+    {
+      if (prev == 0xFFFFu)
+      {
+        duplicate_feature_map->set (i, i);
+        prev = i;
+        continue;
+      }
+
+      hb_tag_t t = get_feature_tag (i);
+      hb_tag_t prev_t = get_feature_tag (prev);
+      if (t != prev_t)
+      {
+        duplicate_feature_map->set (i, i);
+        prev = i;
+        continue;
+      }
+
+      const Feature& f = get_feature (i);
+      const Feature& prev_f = get_feature (prev);
+
+      auto f_iter =
+      + hb_iter (f.lookupIndex)
+      | hb_filter (lookup_indices)
+      ;
+
+      auto prev_iter =
+      + hb_iter (prev_f.lookupIndex)
+      | hb_filter (lookup_indices)
+      ;
+
+      if (f_iter.len () != prev_iter.len ())
+      {
+        duplicate_feature_map->set (i, i);
+        prev = i;
+        continue;
+      }
+
+      bool is_equal = true;
+      for (auto _ : + hb_zip (f_iter, prev_iter))
+        if (_.first != _.second) { is_equal = false; break; }
+
+      if (is_equal == true) duplicate_feature_map->set (i, prev);
+      else
+      {
+        duplicate_feature_map->set (i, i);
+        prev = i;
+      }
+    }
+  }
+
   void prune_features (const hb_map_t *lookup_indices, /* IN */
-                       hb_set_t       *feature_indices /* IN/OUT */) const
+		       hb_set_t       *feature_indices /* IN/OUT */) const
   {
 #ifndef HB_NO_VAR
     // This is the set of feature indices which have alternate versions defined

src/hb-subset-plan.cc

@@ -39,6 +39,7 @@
 #include "hb-ot-stat-table.hh"
 
 
+typedef hb_hashmap_t<unsigned, hb_set_t *, (unsigned)-1, nullptr> script_langsys_map;
 #ifndef HB_NO_SUBSET_CFF
 static inline void
 _add_cff_seac_components (const OT::cff1::accelerator_t &cff,
@@ -70,7 +71,8 @@ static inline void
 _gsub_closure_glyphs_lookups_features (hb_face_t *face,
 				       hb_set_t *gids_to_retain,
 				       hb_map_t *gsub_lookups,
-				       hb_map_t *gsub_features)
+				       hb_map_t *gsub_features,
+				       script_langsys_map *gsub_langsys)
 {
   hb_set_t lookup_indices;
   hb_ot_layout_collect_lookups (face,
@@ -96,7 +98,13 @@ _gsub_closure_glyphs_lookups_features (hb_face_t *face,
                                  nullptr,
                                  nullptr,
                                  &feature_indices);
+
   gsub->prune_features (gsub_lookups, &feature_indices);
+  hb_map_t duplicate_feature_map;
+  gsub->find_duplicate_features (gsub_lookups, &feature_indices, &duplicate_feature_map);
+
+  feature_indices.clear ();
+  gsub->prune_langsys (&duplicate_feature_map, gsub_langsys, &feature_indices);
   _remap_indexes (&feature_indices, gsub_features);
 
   gsub.destroy ();
@@ -106,7 +114,8 @@ static inline void
 _gpos_closure_lookups_features (hb_face_t      *face,
 				const hb_set_t *gids_to_retain,
 				hb_map_t       *gpos_lookups,
-				hb_map_t       *gpos_features)
+				hb_map_t       *gpos_features,
+				script_langsys_map *gpos_langsys)
 {
   hb_set_t lookup_indices;
   hb_ot_layout_collect_lookups (face,
@@ -129,8 +138,15 @@ _gpos_closure_lookups_features (hb_face_t      *face,
                                  nullptr,
                                  nullptr,
                                  &feature_indices);
+
   gpos->prune_features (gpos_lookups, &feature_indices);
+  hb_map_t duplicate_feature_map;
+  gpos->find_duplicate_features (gpos_lookups, &feature_indices, &duplicate_feature_map);
+
+  feature_indices.clear ();
+  gpos->prune_langsys (&duplicate_feature_map, gpos_langsys, &feature_indices);
   _remap_indexes (&feature_indices, gpos_features);
+
   gpos.destroy ();
 }
 #endif
@@ -231,10 +247,10 @@ _populate_gids_to_retain (hb_subset_plan_t* plan,
 #ifndef HB_NO_SUBSET_LAYOUT
   if (close_over_gsub)
     // closure all glyphs/lookups/features needed for GSUB substitutions.
-    _gsub_closure_glyphs_lookups_features (plan->source, plan->_glyphset_gsub, plan->gsub_lookups, plan->gsub_features);
+    _gsub_closure_glyphs_lookups_features (plan->source, plan->_glyphset_gsub, plan->gsub_lookups, plan->gsub_features, plan->gsub_langsys);
 
   if (close_over_gpos)
-    _gpos_closure_lookups_features (plan->source, plan->_glyphset_gsub, plan->gpos_lookups, plan->gpos_features);
+    _gpos_closure_lookups_features (plan->source, plan->_glyphset_gsub, plan->gpos_lookups, plan->gpos_features, plan->gpos_langsys);
 #endif
   _remove_invalid_gids (plan->_glyphset_gsub, plan->source->get_num_glyphs ());
 
@@ -356,6 +372,12 @@ hb_subset_plan_create (hb_face_t         *face,
   plan->reverse_glyph_map = hb_map_create ();
   plan->gsub_lookups = hb_map_create ();
   plan->gpos_lookups = hb_map_create ();
+  
+  plan->gsub_langsys = hb_object_create<script_langsys_map> ();
+  plan->gsub_langsys->init_shallow ();
+
+  plan->gpos_langsys = hb_object_create<script_langsys_map> ();
+  plan->gpos_langsys->init_shallow ();
   plan->gsub_features = hb_map_create ();
   plan->gpos_features = hb_map_create ();
   plan->layout_variation_indices = hb_set_create ();
@@ -407,6 +429,19 @@ hb_subset_plan_destroy (hb_subset_plan_t *plan)
   hb_set_destroy (plan->layout_variation_indices);
   hb_map_destroy (plan->layout_variation_idx_map);
 
+  for (auto _ : plan->gsub_langsys->iter ())
+    hb_set_destroy (_.second);
+  
+  hb_object_destroy (plan->gsub_langsys);
+  plan->gsub_langsys->fini_shallow ();
+  free (plan->gsub_langsys);
+
+  for (auto _ : plan->gpos_langsys->iter ())
+    hb_set_destroy (_.second);
+  
+  hb_object_destroy (plan->gpos_langsys);
+  plan->gpos_langsys->fini_shallow ();
+  free (plan->gpos_langsys);
 
   free (plan);
 }

src/hb-subset-plan.hh

@@ -79,7 +79,11 @@ struct hb_subset_plan_t
   hb_map_t *gsub_lookups;
   hb_map_t *gpos_lookups;
 
-  //active features we'd like to retain
+  //active langsys we'd like to retain
+  hb_hashmap_t<unsigned, hb_set_t *, (unsigned)-1, nullptr> *gsub_langsys;
+  hb_hashmap_t<unsigned, hb_set_t *, (unsigned)-1, nullptr> *gpos_langsys;
+
+  //active features after removing redundant langsys and prune_features
   hb_map_t *gsub_features;
   hb_map_t *gpos_features;