[subset] de-duplicate the logic that finds unicodes corresponding to requested glyphs.

Move the logic into subset planning and then re-use the results in cmap and OS2 subsetting. Removes depedency on cmap from os2.

src/hb-ot-cmap-table.hh

@@ -1367,41 +1367,14 @@ struct cmap
     for (const EncodingRecord& _ : encodingrec_iter)
     {
       unsigned format = (base+_.subtable).u.format;
-      if (!plan->glyphs_requested->is_empty ())
-      {
-	hb_set_t unicodes_set;
-	hb_map_t cp_glyphid_map;
-	(base+_.subtable).collect_mapping (&unicodes_set, &cp_glyphid_map);
+      if (format != 4 && format != 12 && format != 14) continue;
 
-	auto table_iter =
-	+ hb_zip (unicodes_set.iter(), unicodes_set.iter() | hb_map(cp_glyphid_map))
-	| hb_filter (plan->_glyphset, hb_second)
-	| hb_filter ([plan] (const hb_pair_t<hb_codepoint_t, hb_codepoint_t>& p)
-		     {
-		       return plan->unicodes->has (p.first) ||
-			      plan->glyphs_requested->has (p.second);
-		     })
-	| hb_map ([plan] (const hb_pair_t<hb_codepoint_t, hb_codepoint_t>& p_org)
-		  {
-		    return hb_pair_t<hb_codepoint_t, hb_codepoint_t> (p_org.first, plan->glyph_map->get(p_org.second));
-		  })
-	;
+      hb_set_t unicodes_set;
+      (base+_.subtable).collect_unicodes (&unicodes_set);
 
-	if (format == 4) c->copy (_, table_iter, 4u, base, plan, &format4objidx);
-	else if (format == 12) c->copy (_, table_iter, 12u, base, plan, &format12objidx);
-	else if (format == 14) c->copy (_, table_iter, 14u, base, plan, &format14objidx);
-      }
-      /* when --gids option is not used, we iterate input unicodes instead of
-       * all codepoints in each subtable, which is more efficient */
-      else
-      {
-	hb_set_t unicodes_set;
-	(base+_.subtable).collect_unicodes (&unicodes_set);
-
-	if (format == 4) c->copy (_, + it | hb_filter (unicodes_set, hb_first), 4u, base, plan, &format4objidx);
-	else if (format == 12) c->copy (_, + it | hb_filter (unicodes_set, hb_first), 12u, base, plan, &format12objidx);
-	else if (format == 14) c->copy (_, it, 14u, base, plan, &format14objidx);
-      }
+      if (format == 4) c->copy (_, + it | hb_filter (unicodes_set, hb_first), 4u, base, plan, &format4objidx);
+      else if (format == 12) c->copy (_, + it | hb_filter (unicodes_set, hb_first), 12u, base, plan, &format12objidx);
+      else if (format == 14) c->copy (_, it, 14u, base, plan, &format14objidx);
     }
 
     c->check_assign(this->encodingRecord.len,

src/hb-ot-os2-table.hh

@@ -30,7 +30,6 @@
 
 #include "hb-open-type.hh"
 #include "hb-ot-os2-unicode-ranges.hh"
-#include "hb-ot-cmap-table.hh"
 
 #include "hb-set.hh"
 
@@ -174,32 +173,14 @@ struct OS2
     if (unlikely (!os2_prime)) return_trace (false);
     if (!c->plan->prune_unicode_ranges) return_trace (true);
 
-    hb_set_t unicodes;
-    if (!c->plan->glyphs_requested->is_empty ())
-    {
-      hb_map_t unicode_glyphid_map;
-
-      OT::cmap::accelerator_t cmap;
-      cmap.init (c->plan->source);
-      cmap.collect_mapping (&unicodes, &unicode_glyphid_map);
-      cmap.fini ();
-
-      hb_set_set (&unicodes, c->plan->unicodes);
-
-      + unicode_glyphid_map.iter ()
-      | hb_filter (c->plan->glyphs_requested, hb_second)
-      | hb_map (hb_first)
-      | hb_sink (unicodes)
-      ;
-    }
     /* when --gids option is not used, no need to do collect_mapping that is
        * iterating all codepoints in each subtable, which is not efficient */
     uint16_t min_cp, max_cp;
-    find_min_and_max_codepoint (unicodes.is_empty () ? c->plan->unicodes : &unicodes, &min_cp, &max_cp);
+    find_min_and_max_codepoint (c->plan->unicodes, &min_cp, &max_cp);
     os2_prime->usFirstCharIndex = min_cp;
     os2_prime->usLastCharIndex = max_cp;
 
-    _update_unicode_ranges (unicodes.is_empty () ? c->plan->unicodes : &unicodes, os2_prime->ulUnicodeRange);
+    _update_unicode_ranges (c->plan->unicodes, os2_prime->ulUnicodeRange);
 
     return_trace (true);
   }

src/hb-subset-plan.cc

@@ -231,31 +231,14 @@ _remove_invalid_gids (hb_set_t *glyphs,
 }
 
 static void
-_populate_gids_to_retain (hb_subset_plan_t* plan,
-			  const hb_set_t *unicodes,
-			  const hb_set_t *input_glyphs_to_retain,
-			  bool close_over_gsub,
-			  bool close_over_gpos,
-			  bool close_over_gdef)
+_populate_unicodes_to_retain (const hb_set_t *unicodes,
+                              const hb_set_t *glyphs,
+                              hb_subset_plan_t *plan)
 {
   OT::cmap::accelerator_t cmap;
-  OT::glyf::accelerator_t glyf;
-#ifndef HB_NO_SUBSET_CFF
-  OT::cff1::accelerator_t cff;
-#endif
-  OT::COLR::accelerator_t colr;
   cmap.init (plan->source);
-  glyf.init (plan->source);
-#ifndef HB_NO_SUBSET_CFF
-  cff.init (plan->source);
-#endif
-  colr.init (plan->source);
 
-  plan->_glyphset_gsub->add (0); // Not-def
-  hb_set_union (plan->_glyphset_gsub, input_glyphs_to_retain);
-
-  hb_codepoint_t cp = HB_SET_VALUE_INVALID;
-  while (unicodes->next (&cp))
+  for (hb_codepoint_t cp : *unicodes)
   {
     hb_codepoint_t gid;
     if (!cmap.get_nominal_glyph (cp, &gid))
@@ -268,6 +251,46 @@ _populate_gids_to_retain (hb_subset_plan_t* plan,
     plan->_glyphset_gsub->add (gid);
   }
 
+  if (glyphs->is_empty ())
+  {
+    cmap.fini ();
+    return;
+  }
+
+  hb_map_t unicode_glyphid_map;
+  cmap.collect_mapping (hb_set_get_empty (), &unicode_glyphid_map);
+  cmap.fini ();
+
+  for (hb_pair_t<hb_codepoint_t, hb_codepoint_t> cp_gid :
+       + unicode_glyphid_map.iter () | hb_filter (glyphs, hb_second))
+  {
+    plan->unicodes->add (cp_gid.first);
+    plan->codepoint_to_glyph->set (cp_gid.first, cp_gid.second);
+  }
+}
+
+static void
+_populate_gids_to_retain (hb_subset_plan_t* plan,
+			  const hb_set_t *unicodes,
+			  const hb_set_t *input_glyphs_to_retain,
+			  bool close_over_gsub,
+			  bool close_over_gpos,
+			  bool close_over_gdef)
+{
+  OT::glyf::accelerator_t glyf;
+#ifndef HB_NO_SUBSET_CFF
+  OT::cff1::accelerator_t cff;
+#endif
+  OT::COLR::accelerator_t colr;
+  glyf.init (plan->source);
+#ifndef HB_NO_SUBSET_CFF
+  cff.init (plan->source);
+#endif
+  colr.init (plan->source);
+
+  plan->_glyphset_gsub->add (0); // Not-def
+  hb_set_union (plan->_glyphset_gsub, input_glyphs_to_retain);
+
   _cmap_closure (plan->source, plan->unicodes, plan->_glyphset_gsub);
 
 #ifndef HB_NO_SUBSET_LAYOUT
@@ -328,7 +351,6 @@ _populate_gids_to_retain (hb_subset_plan_t* plan,
   cff.fini ();
 #endif
   glyf.fini ();
-  cmap.fini ();
 }
 
 static void
@@ -439,6 +461,8 @@ hb_subset_plan_create (hb_face_t         *face,
     return plan;
   }
 
+  _populate_unicodes_to_retain (input->unicodes, input->glyphs, plan);
+
   _populate_gids_to_retain (plan,
 			    input->unicodes,
 			    input->glyphs,