From e7ad017ca6607d9576c4292d82b6c417710a4e54 Mon Sep 17 00:00:00 2001
From: Behdad Esfahbod <behdad@behdad.org>
Date: Sun, 25 Jun 2023 22:25:24 -0600
Subject: [PATCH] [subset/cff] Allocate more memory for retain-gids

To avoid serializing twice.

20% speedup in:
BM_subset/subset_glyphs/SourceSansPro-Regular.otf/retaingids/10
---
 src/hb-subset.cc | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/src/hb-subset.cc b/src/hb-subset.cc
index d5c439b6f..8e8a5eb0b 100644
--- a/src/hb-subset.cc
+++ b/src/hb-subset.cc
@@ -213,15 +213,36 @@ _get_table_tags (const hb_subset_plan_t* plan,
 static unsigned
 _plan_estimate_subset_table_size (hb_subset_plan_t *plan,
 				  unsigned table_len,
-				  bool same_size)
+				  hb_tag_t table_tag)
 {
   unsigned src_glyphs = plan->source->get_num_glyphs ();
   unsigned dst_glyphs = plan->glyphset ()->get_population ();
 
-  if (unlikely (!src_glyphs) || same_size)
-    return 8192 + table_len;
+  unsigned bulk = 8192;
+  /* Tables that we want to allocate same space as the source table. For GSUB/GPOS it's
+   * because those are expensive to subset, so giving them more room is fine. */
+  bool same_size = table_tag == HB_OT_TAG_GSUB ||
+		   table_tag == HB_OT_TAG_GPOS ||
+		   table_tag == HB_OT_TAG_name;
 
-  return 8192 + (unsigned) (table_len * sqrt ((double) dst_glyphs / src_glyphs));
+  if (plan->flags & HB_SUBSET_FLAGS_RETAIN_GIDS)
+  {
+    if (table_tag == HB_OT_TAG_CFF1)
+    {
+      /* Add some extra room for the CFF charset. */
+      bulk += src_glyphs * 16;
+    }
+    else if (table_tag == HB_OT_TAG_CFF2)
+    {
+      /* Just extra CharString offsets. */
+      bulk += src_glyphs * 4;
+    }
+  }
+
+  if (unlikely (!src_glyphs) || same_size)
+    return bulk + table_len;
+
+  return bulk + (unsigned) (table_len * sqrt ((double) dst_glyphs / src_glyphs));
 }
 
 /*
@@ -306,13 +327,7 @@ _subset (hb_subset_plan_t *plan, hb_vector_t<char> &buf)
     return false;
   }
 
-  /* Tables that we want to allocate same space as the source table. For GSUB/GPOS it's
-   * because those are expensive to subset, so giving them more room is fine. */
-  bool same_size_table = TableType::tableTag == HB_OT_TAG_GSUB ||
-			 TableType::tableTag == HB_OT_TAG_GPOS ||
-			 TableType::tableTag == HB_OT_TAG_name;
-
-  unsigned buf_size = _plan_estimate_subset_table_size (plan, blob->length, same_size_table);
+  unsigned buf_size = _plan_estimate_subset_table_size (plan, blob->length, TableType::tableTag);
   DEBUG_MSG (SUBSET, nullptr,
              "OT::%c%c%c%c initial estimated table size: %u bytes.", HB_UNTAG (tag), buf_size);
   if (unlikely (!buf.alloc (buf_size)))