Merge pull request #4168 from googlefonts/subset_name_collect

[subset] name_id closure

src/OT/Color/CPAL/CPAL.hh

@@ -73,6 +73,30 @@ struct CPALV1Tail
   }
 
   public:
+  void collect_name_ids (const void *base,
+                         unsigned palette_count,
+                         unsigned color_count,
+                         const hb_map_t *color_index_map,
+                         hb_set_t *nameids_to_retain /* OUT */) const
+  {
+    if (paletteLabelsZ)
+    {
+      + (base+paletteLabelsZ).as_array (palette_count)
+      | hb_sink (nameids_to_retain)
+      ;
+    }
+
+    if (colorLabelsZ)
+    {
+      const hb_array_t<const NameID> colorLabels = (base+colorLabelsZ).as_array (color_count);
+      for (unsigned i = 0; i < color_count; i++)
+      {
+        if (!color_index_map->has (i)) continue;
+        nameids_to_retain->add (colorLabels[i]);
+      }
+    }
+  }
+
   bool serialize (hb_serialize_context_t *c,
                   unsigned palette_count,
                   unsigned color_count,
@@ -95,13 +119,10 @@ struct CPALV1Tail
     if (colorLabelsZ)
     {
       c->push ();
-      for (const auto _ : colorLabels)
+      for (unsigned i = 0; i < color_count; i++)
       {
-	const hb_codepoint_t *v;
+        if (!color_index_map->has (i)) continue;
-        if (!color_index_map->has (_, &v)) continue;
+        if (!c->copy<NameID> (colorLabels[i]))
-        NameID new_color_idx;
-	new_color_idx = *v;
-        if (!c->copy<NameID> (new_color_idx))
         {
           c->pop_discard ();
           return_trace (false);
@@ -189,6 +210,13 @@ struct CPAL
     return numColors;
   }
 
+  void collect_name_ids (const hb_map_t *color_index_map,
+                         hb_set_t *nameids_to_retain /* OUT */) const
+  {
+    if (version == 1)
+      v1 ().collect_name_ids (this, numPalettes, numColors, color_index_map, nameids_to_retain);
+  }
+
   private:
   const CPALV1Tail& v1 () const
   {

src/hb-ot-layout-common.hh

@@ -529,6 +529,9 @@ struct FeatureParamsSize
       return_trace (true);
   }
 
+  void collect_name_ids (hb_set_t *nameids_to_retain /* OUT */) const
+  { nameids_to_retain->add (subfamilyNameID); }
+
   bool subset (hb_subset_context_t *c) const
   {
     TRACE_SUBSET (this);
@@ -585,6 +588,9 @@ struct FeatureParamsStylisticSet
     return_trace (c->check_struct (this));
   }
 
+  void collect_name_ids (hb_set_t *nameids_to_retain /* OUT */) const
+  { nameids_to_retain->add (uiNameID); }
+
   bool subset (hb_subset_context_t *c) const
   {
     TRACE_SUBSET (this);
@@ -632,6 +638,20 @@ struct FeatureParamsCharacterVariants
   unsigned get_size () const
   { return min_size + characters.len * HBUINT24::static_size; }
 
+  void collect_name_ids (hb_set_t *nameids_to_retain /* OUT */) const
+  {
+    if (featUILableNameID) nameids_to_retain->add (featUILableNameID);
+    if (featUITooltipTextNameID) nameids_to_retain->add (featUITooltipTextNameID);
+    if (sampleTextNameID) nameids_to_retain->add (sampleTextNameID);
+
+    if (!firstParamUILabelNameID || !numNamedParameters || numNamedParameters >= 0x7FFF)
+      return;
+
+    unsigned last_name_id = (unsigned) firstParamUILabelNameID + (unsigned) numNamedParameters - 1;
+    if (last_name_id >= 256 && last_name_id <= 32767)
+      nameids_to_retain->add_range (firstParamUILabelNameID, last_name_id);
+  }
+
   bool subset (hb_subset_context_t *c) const
   {
     TRACE_SUBSET (this);
@@ -694,6 +714,19 @@ struct FeatureParams
     return_trace (true);
   }
 
+  void collect_name_ids (hb_tag_t tag, hb_set_t *nameids_to_retain /* OUT */) const
+  {
+#ifdef HB_NO_LAYOUT_FEATURE_PARAMS
+    return;
+#endif
+    if (tag == HB_TAG ('s','i','z','e'))
+      return (u.size.collect_name_ids (nameids_to_retain));
+    if ((tag & 0xFFFF0000u) == HB_TAG ('s','s','\0','\0')) /* ssXX */
+      return (u.stylisticSet.collect_name_ids (nameids_to_retain));
+    if ((tag & 0xFFFF0000u) == HB_TAG ('c','v','\0','\0')) /* cvXX */
+      return (u.characterVariants.collect_name_ids (nameids_to_retain));
+  }
+
   bool subset (hb_subset_context_t *c, const Tag* tag) const
   {
     TRACE_SUBSET (this);
@@ -762,6 +795,12 @@ struct Feature
   bool intersects_lookup_indexes (const hb_map_t *lookup_indexes) const
   { return lookupIndex.intersects (lookup_indexes); }
 
+  void collect_name_ids (hb_tag_t tag, hb_set_t *nameids_to_retain /* OUT */) const
+  {
+    if (featureParams)
+      get_feature_params ().collect_name_ids (tag, nameids_to_retain);
+  }
+
   bool subset (hb_subset_context_t         *c,
 	       hb_subset_layout_context_t  *l,
 	       const Tag                   *tag = nullptr) const

src/hb-ot-layout-gsubgpos.hh

@@ -4461,6 +4461,18 @@ struct GSUBGPOS
     }
   }
 
+  void collect_name_ids (const hb_map_t *feature_index_map,
+                         hb_set_t *nameids_to_retain /* OUT */) const
+  {
+    unsigned count = get_feature_count ();
+    for (unsigned i = 0 ; i < count; i++)
+    {
+      if (!feature_index_map->has (i)) continue;
+      hb_tag_t tag = get_feature_tag (i);
+      get_feature (i).collect_name_ids (tag, nameids_to_retain);
+    }
+  }
+
   template <typename T>
   struct accelerator_t
   {

src/hb-ot-stat-table.hh

@@ -536,6 +536,8 @@ struct STAT
     | hb_map (&AxisValue::get_value_name_id)
     | hb_sink (nameids_to_retain)
     ;
+
+    nameids_to_retain->add (elidedFallbackNameID);
   }
 
   bool subset (hb_subset_context_t *c) const

src/hb-subset-plan.cc

@@ -38,6 +38,7 @@
 #include "hb-ot-cff1-table.hh"
 #include "OT/Color/COLR/COLR.hh"
 #include "OT/Color/COLR/colrv1-closure.hh"
+#include "OT/Color/CPAL/CPAL.hh"
 #include "hb-ot-var-fvar-table.hh"
 #include "hb-ot-var-avar-table.hh"
 #include "hb-ot-stat-table.hh"
@@ -620,6 +621,37 @@ _glyf_add_gid_and_children (const OT::glyf_accelerator_t &glyf,
   return operation_count;
 }
 
+static void
+_nameid_closure (hb_subset_plan_t* plan,
+		 hb_set_t* drop_tables)
+{
+#ifndef HB_NO_STYLE
+  plan->source->table.STAT->collect_name_ids (&plan->user_axes_location, &plan->name_ids);
+#endif
+#ifndef HB_NO_VAR
+  if (!plan->all_axes_pinned)
+    plan->source->table.fvar->collect_name_ids (&plan->user_axes_location, &plan->name_ids);
+#endif
+
+  if (!drop_tables->has (HB_OT_TAG_CPAL))
+    plan->source->table.CPAL->collect_name_ids (&plan->colr_palettes, &plan->name_ids);
+
+#ifndef HB_NO_SUBSET_LAYOUT
+  if (!drop_tables->has (HB_OT_TAG_GPOS))
+  {
+    hb_blob_ptr_t<GPOS> gpos = plan->source_table<GPOS> ();
+    gpos->collect_name_ids (&plan->gpos_features, &plan->name_ids);
+    gpos.destroy ();
+  }
+  if (!drop_tables->has (HB_OT_TAG_GSUB))
+  {
+    hb_blob_ptr_t<GSUB> gsub = plan->source_table<GSUB> ();
+    gsub->collect_name_ids (&plan->gsub_features, &plan->name_ids);
+    gsub.destroy ();
+  }
+#endif
+}
+
 static void
 _populate_gids_to_retain (hb_subset_plan_t* plan,
 		          hb_set_t* drop_tables)
@@ -673,6 +705,7 @@ _populate_gids_to_retain (hb_subset_plan_t* plan,
 
   plan->_glyphset_colred = cur_glyphset;
 
+  _nameid_closure (plan, drop_tables);
   /* Populate a full set of glyphs to retain by adding all referenced
    * composite glyphs. */
   if (glyf.has_data ())
@@ -756,21 +789,6 @@ _create_old_gid_to_new_gid_map (const hb_face_t *face,
   ;
 }
 
-static void
-_nameid_closure (hb_face_t *face,
-		 hb_set_t  *nameids,
-		 bool all_axes_pinned,
-		 hb_hashmap_t<hb_tag_t, float> *user_axes_location)
-{
-#ifndef HB_NO_STYLE
-  face->table.STAT->collect_name_ids (user_axes_location, nameids);
-#endif
-#ifndef HB_NO_VAR
-  if (!all_axes_pinned)
-    face->table.fvar->collect_name_ids (user_axes_location, nameids);
-#endif
-}
-
 #ifndef HB_NO_VAR
 static void
 _normalize_axes_location (hb_face_t *face, hb_subset_plan_t *plan)
@@ -905,7 +923,6 @@ hb_subset_plan_t::hb_subset_plan_t (hb_face_t *face,
         glyph_map->get(unicode_to_new_gid_list.arrayZ[i].second);
   }
 
-  _nameid_closure (face, &name_ids, all_axes_pinned, &user_axes_location);
   if (unlikely (in_error ()))
     return;
 

test/subset/data/Makefile.am

@@ -67,6 +67,7 @@ EXTRA_DIST += \
 	expected/instance_comp_glyph_empty_child \
 	expected/post_apply_mvar_delta \
 	expected/apply_cvar_delta \
+	expected/collect_name_ids \
 	fonts \
 	profiles \
 	$(NULL)

test/subset/data/Makefile.sources

@@ -58,6 +58,7 @@ TESTS = \
 	tests/instance_comp_glyph_empty_child.tests \
 	tests/post_apply_mvar_delta.tests \
 	tests/apply_cvar_delta.tests \
+	tests/collect_name_ids.tests \
 	$(NULL)
 
 # TODO: re-enable once colrv1 subsetting is stabilized.

test/subset/data/tests/collect_name_ids.tests

@@ -0,0 +1,11 @@
+FONTS:
+SourceSerif4Variable-Roman_subset.otf
+
+PROFILES:
+keep-all-layout-features.txt
+
+SUBSETS:
+*
+
+OPTIONS:
+no_fonttools

test/subset/meson.build

@@ -60,6 +60,7 @@ tests = [
   'instance_comp_glyph_empty_child',
   'post_apply_mvar_delta',
   'apply_cvar_delta',
+  'collect_name_ids',
 ]
 
 repack_tests = [