[instance] update collect_lookups/prune_features()/closure_features() with variations

Some features will be substituted with variations, so we do not collect
lookups from the original feature tables.

src/hb-ot-layout-common.hh

@@ -2977,10 +2977,16 @@ struct FeatureTableSubstitution
   }
 
   void collect_lookups (const hb_set_t *feature_indexes,
+			const hb_hashmap_t<unsigned, const Feature*> *feature_substitutes_map,
 			hb_set_t       *lookup_indexes /* OUT */) const
   {
     + hb_iter (substitutions)
     | hb_filter (feature_indexes, &FeatureTableSubstitutionRecord::featureIndex)
+    | hb_filter ([feature_substitutes_map] (const FeatureTableSubstitutionRecord& record)
+                 {
+                   if (feature_substitutes_map == nullptr) return true;
+                   return !feature_substitutes_map->has (record.featureIndex);
+                 })
     | hb_apply ([this, lookup_indexes] (const FeatureTableSubstitutionRecord& r)
 		{ r.collect_lookups (this, lookup_indexes); })
     ;
@@ -3047,9 +3053,10 @@ struct FeatureVariationRecord
 
   void collect_lookups (const void     *base,
 			const hb_set_t *feature_indexes,
+			const hb_hashmap_t<unsigned, const Feature*> *feature_substitutes_map,
 			hb_set_t       *lookup_indexes /* OUT */) const
   {
-    return (base+substitutions).collect_lookups (feature_indexes, lookup_indexes);
+    return (base+substitutions).collect_lookups (feature_indexes, feature_substitutes_map, lookup_indexes);
   }
 
   void closure_features (const void     *base,
@@ -3149,17 +3156,25 @@ struct FeatureVariations
   }
 
   void collect_lookups (const hb_set_t *feature_indexes,
+			const hb_hashmap_t<unsigned, const Feature*> *feature_substitutes_map,
 			hb_set_t       *lookup_indexes /* OUT */) const
   {
     for (const FeatureVariationRecord& r : varRecords)
-      r.collect_lookups (this, feature_indexes, lookup_indexes);
+      r.collect_lookups (this, feature_indexes, feature_substitutes_map, lookup_indexes);
   }
 
   void closure_features (const hb_map_t *lookup_indexes,
+			 const hb_hashmap_t<unsigned, hb::shared_ptr<hb_set_t>> *feature_record_cond_idx_map,
 			 hb_set_t       *feature_indexes /* OUT */) const
   {
-    for (const FeatureVariationRecord& record : varRecords)
-      record.closure_features (this, lookup_indexes, feature_indexes);
+    unsigned int count = varRecords.len;
+    for (unsigned int i = 0; i < count; i++)
+    {
+      if (feature_record_cond_idx_map != nullptr &&
+          !feature_record_cond_idx_map->has (i))
+        continue;
+      varRecords[i].closure_features (this, lookup_indexes, feature_indexes);
+    }
   }
 
   bool subset (hb_subset_context_t *c,

src/hb-ot-layout-gsubgpos.hh

@@ -4236,10 +4236,11 @@ struct GSUBGPOS
   }
 
   void feature_variation_collect_lookups (const hb_set_t *feature_indexes,
+					  const hb_hashmap_t<unsigned, const Feature*> *feature_substitutes_map,
 					  hb_set_t       *lookup_indexes /* OUT */) const
   {
 #ifndef HB_NO_VAR
-    get_feature_variations ().collect_lookups (feature_indexes, lookup_indexes);
+    get_feature_variations ().collect_lookups (feature_indexes, feature_substitutes_map, lookup_indexes);
 #endif
   }
 
@@ -4283,6 +4284,8 @@ struct GSUBGPOS
   }
 
   void prune_features (const hb_map_t *lookup_indices, /* IN */
+		       const hb_hashmap_t<unsigned, hb::shared_ptr<hb_set_t>> *feature_record_cond_idx_map, /* IN */
+		       const hb_hashmap_t<unsigned, const Feature*> *feature_substitutes_map, /* IN */
 		       hb_set_t       *feature_indices /* IN/OUT */) const
   {
 #ifndef HB_NO_VAR
@@ -4290,7 +4293,7 @@ struct GSUBGPOS
     // if the FeatureVariation's table and the alternate version(s) intersect the
     // set of lookup indices.
     hb_set_t alternate_feature_indices;
-    get_feature_variations ().closure_features (lookup_indices, &alternate_feature_indices);
+    get_feature_variations ().closure_features (lookup_indices, feature_record_cond_idx_map, &alternate_feature_indices);
     if (unlikely (alternate_feature_indices.in_error()))
     {
       feature_indices->err ();
@@ -4300,7 +4303,6 @@ struct GSUBGPOS
 
     for (unsigned i : feature_indices->iter())
     {
-      const Feature& f = get_feature (i);
       hb_tag_t tag =  get_feature_tag (i);
       if (tag == HB_TAG ('p', 'r', 'e', 'f'))
         // Note: Never ever drop feature 'pref', even if it's empty.
@@ -4310,11 +4312,16 @@ struct GSUBGPOS
         continue;
 
 
-      if (!f.featureParams.is_null () &&
+      const Feature *f = &(get_feature (i));
+      const Feature** p = nullptr;
+      if (feature_substitutes_map->has (i, &p))
+        f = *p;
+
+      if (!f->featureParams.is_null () &&
           tag == HB_TAG ('s', 'i', 'z', 'e'))
         continue;
 
-      if (!f.intersects_lookup_indexes (lookup_indices)
+      if (!f->intersects_lookup_indexes (lookup_indices)
 #ifndef HB_NO_VAR
           && !alternate_feature_indices.has (i)
 #endif

src/hb-ot-layout.cc

@@ -1271,7 +1271,7 @@ hb_ot_layout_collect_lookups (hb_face_t      *face,
        hb_set_next (&feature_indexes, &feature_index);)
     g.get_feature (feature_index).add_lookup_indexes_to (lookup_indexes);
 
-  g.feature_variation_collect_lookups (&feature_indexes, lookup_indexes);
+  g.feature_variation_collect_lookups (&feature_indexes, nullptr, lookup_indexes);
 }
 
 

src/hb-subset-plan.cc

@@ -129,7 +129,9 @@ template <typename T>
 static void _collect_layout_indices (hb_subset_plan_t     *plan,
                                      const T&              table,
                                      hb_set_t		  *lookup_indices, /* OUT */
-                                     hb_set_t		  *feature_indices /* OUT */)
+                                     hb_set_t		  *feature_indices, /* OUT */
+                                     hb_hashmap_t<unsigned, hb::shared_ptr<hb_set_t>> *feature_record_cond_idx_map, /* OUT */
+                                     hb_hashmap_t<unsigned, const OT::Feature*> *feature_substitutes_map /* OUT */)
 {
   unsigned num_features = table.get_feature_count ();
   hb_vector_t<hb_tag_t> features;
@@ -154,16 +156,37 @@ static void _collect_layout_indices (hb_subset_plan_t     *plan,
                                  retain_all_features ? nullptr : features.arrayZ,
                                  feature_indices);
 
+#ifndef HB_NO_VAR
+  // collect feature substitutes with variations
+  if (!plan->user_axes_location->is_empty ())
+  {
+    hb_hashmap_t<hb::shared_ptr<hb_map_t>, unsigned> conditionset_map;
+    OT::hb_collect_feature_substitutes_with_var_context_t c =
+    {
+      plan->axes_old_index_tag_map,
+      plan->axes_location,
+      feature_record_cond_idx_map,
+      feature_substitutes_map,
+      feature_indices,
+      true,
+      0,
+      &conditionset_map
+    };
+    table.collect_feature_substitutes_with_variations (&c);
+  }
+#endif
+
   for (unsigned feature_index : *feature_indices)
   {
-    //TODO: replace HB_OT_LAYOUT_NO_VARIATIONS_INDEX with variation_index for
-    //instancing
-    const OT::Feature &f = table.get_feature_variation (feature_index, HB_OT_LAYOUT_NO_VARIATIONS_INDEX);
-    f.add_lookup_indexes_to (lookup_indices);
+    const OT::Feature* f = &(table.get_feature (feature_index));
+    const OT::Feature **p = nullptr;
+    if (feature_substitutes_map->has (feature_index, &p))
+      f = *p;
+
+    f->add_lookup_indexes_to (lookup_indices);
   }
 
-  //TODO: update for instancing: only collect lookups from feature_indexes that have no variations
-  table.feature_variation_collect_lookups (feature_indices, lookup_indices);
+  table.feature_variation_collect_lookups (feature_indices, feature_substitutes_map, lookup_indices);
 }
 
 
@@ -171,6 +194,7 @@ static inline void
 _GSUBGPOS_find_duplicate_features (const OT::GSUBGPOS &g,
 				   const hb_map_t *lookup_indices,
 				   const hb_set_t *feature_indices,
+				   const hb_hashmap_t<unsigned, const OT::Feature*> *feature_substitutes_map,
 				   hb_map_t *duplicate_feature_map /* OUT */)
 {
   if (feature_indices->is_empty ()) return;
@@ -195,16 +219,22 @@ _GSUBGPOS_find_duplicate_features (const OT::GSUBGPOS &g,
     hb_set_t* same_tag_features = unique_features.get (t);
     for (unsigned other_f_index : same_tag_features->iter ())
     {
-      const OT::Feature& f = g.get_feature (i);
-      const OT::Feature& other_f = g.get_feature (other_f_index);
+      const OT::Feature* f = &(g.get_feature (i));
+      const OT::Feature **p = nullptr;
+      if (feature_substitutes_map->has (i, &p))
+        f = *p;
+
+      const OT::Feature* other_f = &(g.get_feature (other_f_index));
+      if (feature_substitutes_map->has (other_f_index, &p))
+        f = *p;
 
       auto f_iter =
-      + hb_iter (f.lookupIndex)
+      + hb_iter (f->lookupIndex)
       | hb_filter (lookup_indices)
       ;
 
       auto other_f_iter =
-      + hb_iter (other_f.lookupIndex)
+      + hb_iter (other_f->lookupIndex)
       | hb_filter (lookup_indices)
       ;
 
@@ -238,7 +268,7 @@ _closure_glyphs_lookups_features (hb_subset_plan_t   *plan,
 				  hb_map_t	     *lookups,
 				  hb_map_t	     *features,
 				  script_langsys_map *langsys_map,
-				  hb_hashmap_t<unsigned, hb::unique_ptr<hb_set_t>> *feature_record_cond_idx_map,
+				  hb_hashmap_t<unsigned, hb::shared_ptr<hb_set_t>> *feature_record_cond_idx_map,
 				  hb_hashmap_t<unsigned, const OT::Feature*> *feature_substitutes_map)
 {
   hb_blob_ptr_t<T> table = plan->source_table<T> ();
@@ -247,7 +277,9 @@ _closure_glyphs_lookups_features (hb_subset_plan_t   *plan,
   _collect_layout_indices<T> (plan,
                               *table,
                               &lookup_indices,
-                              &feature_indices);
+                              &feature_indices,
+                              feature_record_cond_idx_map,
+                              feature_substitutes_map);
 
   if (table_tag == HB_OT_TAG_GSUB)
     hb_ot_layout_lookups_substitute_closure (plan->source,
@@ -259,9 +291,12 @@ _closure_glyphs_lookups_features (hb_subset_plan_t   *plan,
   _remap_indexes (&lookup_indices, lookups);
 
   // prune features
-  table->prune_features (lookups, &feature_indices);
+  table->prune_features (lookups,
+                         plan->user_axes_location->is_empty () ? nullptr : feature_record_cond_idx_map,
+                         feature_substitutes_map,
+                         &feature_indices);
   hb_map_t duplicate_feature_map;
-  _GSUBGPOS_find_duplicate_features (*table, lookups, &feature_indices, &duplicate_feature_map);
+  _GSUBGPOS_find_duplicate_features (*table, lookups, &feature_indices, feature_substitutes_map, &duplicate_feature_map);
 
   feature_indices.clear ();
   table->prune_langsys (&duplicate_feature_map, plan->layout_scripts, langsys_map, &feature_indices);
@@ -756,8 +791,8 @@ hb_subset_plan_create_or_fail (hb_face_t	 *face,
   plan->gsub_features = hb_map_create ();
   plan->gpos_features = hb_map_create ();
 
-  plan->check_success (plan->gsub_feature_record_cond_idx_map = hb_hashmap_create<unsigned, hb::unique_ptr<hb_set_t>> ());
-  plan->check_success (plan->gpos_feature_record_cond_idx_map = hb_hashmap_create<unsigned, hb::unique_ptr<hb_set_t>> ());
+  plan->check_success (plan->gsub_feature_record_cond_idx_map = hb_hashmap_create<unsigned, hb::shared_ptr<hb_set_t>> ());
+  plan->check_success (plan->gpos_feature_record_cond_idx_map = hb_hashmap_create<unsigned, hb::shared_ptr<hb_set_t>> ());
 
   plan->check_success (plan->gsub_feature_substitutes_map = hb_hashmap_create<unsigned, const OT::Feature*> ());
   plan->check_success (plan->gpos_feature_substitutes_map = hb_hashmap_create<unsigned, const OT::Feature*> ());

src/hb-subset-plan.hh

@@ -154,8 +154,8 @@ struct hb_subset_plan_t
   hb_map_t *gpos_features;
 
   //active feature variation records/condition index with variations
-  hb_hashmap_t<unsigned, hb::unique_ptr<hb_set_t>> *gsub_feature_record_cond_idx_map;
-  hb_hashmap_t<unsigned, hb::unique_ptr<hb_set_t>> *gpos_feature_record_cond_idx_map;
+  hb_hashmap_t<unsigned, hb::shared_ptr<hb_set_t>> *gsub_feature_record_cond_idx_map;
+  hb_hashmap_t<unsigned, hb::shared_ptr<hb_set_t>> *gpos_feature_record_cond_idx_map;
 
   //feature index-> address of substituation feature table mapping with
   //variations