From 8f1bf23cc9a8912c452f7571e2a3f35a192a8120 Mon Sep 17 00:00:00 2001
From: Garret Rieger <grieger@google.com>
Date: Tue, 27 Sep 2022 22:50:54 +0000
Subject: [PATCH] [subset] optimize glyf subsetting w/ retain gids.

When retain gids is enabled the subset plan may require the output of many empty glyphs. This change optimizes the glyf subsetting code when the number of retained glyphs << number of output glyphs. Unnessecary lookups to the glyph map are reduced by iterating through the glyph map instead of the output glyph set.
---
 src/OT/glyf/Glyph.hh       | 14 +++++++----
 src/OT/glyf/SubsetGlyph.hh |  1 -
 src/OT/glyf/glyf.hh        | 48 +++++++++++++++++---------------------
 3 files changed, 32 insertions(+), 31 deletions(-)

diff --git a/src/OT/glyf/Glyph.hh b/src/OT/glyf/Glyph.hh
index 73fefc5d8..503112270 100644
--- a/src/OT/glyf/Glyph.hh
+++ b/src/OT/glyf/Glyph.hh
@@ -346,10 +346,16 @@ struct Glyph
 
   hb_bytes_t get_bytes () const { return bytes; }
 
-  Glyph (hb_bytes_t bytes_ = hb_bytes_t (),
-	 hb_codepoint_t gid_ = (hb_codepoint_t) -1) : bytes (bytes_),
-						      header (bytes.as<GlyphHeader> ()),
-						      gid (gid_)
+  Glyph () : bytes (),
+             header (bytes.as<GlyphHeader> ()),
+             gid (-1),
+             type(EMPTY)
+  {}
+
+  Glyph (hb_bytes_t bytes_,
+	 hb_codepoint_t gid_ = (unsigned) -1) : bytes (bytes_),
+                                                header (bytes.as<GlyphHeader> ()),
+                                                gid (gid_)
   {
     int num_contours = header->numberOfContours;
     if (unlikely (num_contours == 0)) type = EMPTY;
diff --git a/src/OT/glyf/SubsetGlyph.hh b/src/OT/glyf/SubsetGlyph.hh
index 7ddefc5a9..88fc93c43 100644
--- a/src/OT/glyf/SubsetGlyph.hh
+++ b/src/OT/glyf/SubsetGlyph.hh
@@ -14,7 +14,6 @@ namespace glyf_impl {
 
 struct SubsetGlyph
 {
-  hb_codepoint_t new_gid;
   hb_codepoint_t old_gid;
   Glyph source_glyph;
   hb_bytes_t dest_start;  /* region of source_glyph to copy first */
diff --git a/src/OT/glyf/glyf.hh b/src/OT/glyf/glyf.hh
index 45316dffd..5fb32f67f 100644
--- a/src/OT/glyf/glyf.hh
+++ b/src/OT/glyf/glyf.hh
@@ -72,7 +72,7 @@ struct glyf
     if (unlikely (!c->serializer->check_success (glyf_prime))) return_trace (false);
 
     hb_vector_t<glyf_impl::SubsetGlyph> glyphs;
-    _populate_subset_glyphs (c->plan, &glyphs);
+    _populate_subset_glyphs (c->plan, glyphs);
 
     if (!c->plan->pinned_at_default)
     {
@@ -108,8 +108,8 @@ struct glyf
 
   void
   _populate_subset_glyphs (const hb_subset_plan_t   *plan,
-			   hb_vector_t<glyf_impl::SubsetGlyph> *glyphs /* OUT */) const;
-  
+			   hb_vector_t<glyf_impl::SubsetGlyph> &glyphs /* OUT */) const;
+
   bool
   _compile_subset_glyphs_with_deltas (const hb_subset_plan_t *plan,
                                       hb_vector_t<glyf_impl::SubsetGlyph> *glyphs /* OUT */) const;
@@ -377,34 +377,30 @@ struct glyf_accelerator_t
 
 inline void
 glyf::_populate_subset_glyphs (const hb_subset_plan_t   *plan,
-			       hb_vector_t<glyf_impl::SubsetGlyph> *glyphs /* OUT */) const
+			       hb_vector_t<glyf_impl::SubsetGlyph>& glyphs /* OUT */) const
 {
   OT::glyf_accelerator_t glyf (plan->source);
+  unsigned num_glyphs = plan->num_output_glyphs ();
+  if (!glyphs.resize (num_glyphs)) return;
 
-  + hb_range (plan->num_output_glyphs ())
-  | hb_map ([&] (hb_codepoint_t new_gid)
-	{
-	  glyf_impl::SubsetGlyph subset_glyph = {0};
-	  subset_glyph.new_gid = new_gid;
+  for (auto p : plan->glyph_map->iter ())
+  {
+    unsigned new_gid = p.second;
+    glyf_impl::SubsetGlyph& subset_glyph = glyphs.arrayZ[new_gid];
+    subset_glyph.old_gid = p.first;
 
-	  /* should never fail: all old gids should be mapped */
-	  if (!plan->old_gid_for_new_gid (new_gid, &subset_glyph.old_gid))
-	    return subset_glyph;
+    if (unlikely (new_gid == 0 &&
+                  !(plan->flags & HB_SUBSET_FLAGS_NOTDEF_OUTLINE)) &&
+                  plan->pinned_at_default)
+      subset_glyph.source_glyph = glyf_impl::Glyph ();
+    else
+      subset_glyph.source_glyph = glyf.glyph_for_gid (subset_glyph.old_gid, true);
 
-	  if (new_gid == 0 &&
-	      !(plan->flags & HB_SUBSET_FLAGS_NOTDEF_OUTLINE) &&
-	      plan->pinned_at_default)
-	    subset_glyph.source_glyph = glyf_impl::Glyph ();
-	  else
-	    subset_glyph.source_glyph = glyf.glyph_for_gid (subset_glyph.old_gid, true);
-	  if (plan->flags & HB_SUBSET_FLAGS_NO_HINTING)
-	    subset_glyph.drop_hints_bytes ();
-	  else
-	    subset_glyph.dest_start = subset_glyph.source_glyph.get_bytes ();
-	  return subset_glyph;
-	})
-  | hb_sink (glyphs)
-  ;
+    if (plan->flags & HB_SUBSET_FLAGS_NO_HINTING)
+      subset_glyph.drop_hints_bytes ();
+    else
+      subset_glyph.dest_start = subset_glyph.source_glyph.get_bytes ();
+  }
 }
 
 inline bool