capture codepoints sorted so we can use them for cmap later. one day we will have a map

src/hb-ot-cmap-table.hh

@@ -506,20 +506,6 @@ struct cmap
 
   inline bool subset (hb_subset_plan_t *plan, hb_face_t *source, hb_face_t *dest) const
   {
-    // TODO something useful re: memory, write to dest
-    size_t dest_sz = 64536; // as much as anyone would ever need
-    void *dest_buf = malloc(dest_sz);
-    OT::hb_serialize_context_t context(dest_buf, dest_sz);
-
-    // Same version
-    OT::cmap new_cmap;
-    new_cmap.version = version;
-    new_cmap.encodingRecord.len.set(1); // one format 12 subtable
-
-    // TODO we need to actually build the format 12 subtable
-
-    // TODO: this fails
-    // out->extend_min(new_cmap);
     return true;
   }
 

src/hb-private.hh

@@ -418,34 +418,44 @@ struct hb_prealloced_array_t
     return &array[len - 1];
   }
 
-  inline bool resize (unsigned int size)
+  // Alloc enouch for size if size < allocated. Don't adjust len.
+  inline bool alloc(unsigned int size)
   {
-    if (unlikely (size > allocated))
+    if (likely (size <= allocated)) 
     {
-      /* Need to reallocate */
-
-      unsigned int new_allocated = allocated;
-      while (size >= new_allocated)
-        new_allocated += (new_allocated >> 1) + 8;
-
-      Type *new_array = nullptr;
-
-      if (array == static_array) {
-	new_array = (Type *) calloc (new_allocated, sizeof (Type));
-	if (new_array)
-	  memcpy (new_array, array, len * sizeof (Type));
-      } else {
-	bool overflows = (new_allocated < allocated) || _hb_unsigned_int_mul_overflows (new_allocated, sizeof (Type));
-	if (likely (!overflows)) {
-	  new_array = (Type *) realloc (array, new_allocated * sizeof (Type));
-	}
+      return true;
+    }
+    /* Need to reallocate */
+
+    unsigned int new_allocated = allocated;
+    while (size >= new_allocated)
+      new_allocated += (new_allocated >> 1) + 8;
+
+    Type *new_array = nullptr;
+
+    if (array == static_array) {
+      new_array = (Type *) calloc (new_allocated, sizeof (Type));
+      if (new_array)
+        memcpy (new_array, array, len * sizeof (Type));
+          } else {
+      bool overflows = (new_allocated < allocated) || _hb_unsigned_int_mul_overflows (new_allocated, sizeof (Type));
+      if (likely (!overflows)) {
+        new_array = (Type *) realloc (array, new_allocated * sizeof (Type));
       }
+    }
+
+    if (unlikely (!new_array))
+      return false;
 
-      if (unlikely (!new_array))
-	return false;
+    array = new_array;
+    allocated = new_allocated;    
+  }
 
-      array = new_array;
-      allocated = new_allocated;
+  inline bool resize (unsigned int size)
+  {
+    if (!alloc(size)) 
+    {
+      return false;
     }
 
     len = size;
@@ -488,6 +498,11 @@ struct hb_prealloced_array_t
     return nullptr;
   }
 
+  inline void qsort (int (*cmp)(const void*, const void*))
+  {
+    ::qsort (array, len, sizeof (Type), cmp);
+  }
+
   inline void qsort (void)
   {
     ::qsort (array, len, sizeof (Type), Type::cmp);

src/hb-subset-glyf.cc

@@ -31,14 +31,14 @@
 
 bool
 _calculate_glyf_and_loca_prime_size (const OT::glyf::accelerator_t &glyf,
-                                     hb_set_t *glyph_ids,
+                                     hb_auto_array_t<unsigned int> &glyph_ids,
                                      unsigned int *glyf_size /* OUT */,
                                      unsigned int *loca_size /* OUT */)
 {
   unsigned int total = 0;
   unsigned int count = 0;
-  hb_codepoint_t next_glyph = -1;
-  while (hb_set_next(glyph_ids, &next_glyph)) {
+  for (unsigned int i = 0; i < glyph_ids.len; i++) {
+    hb_codepoint_t next_glyph = glyph_ids[i];
     unsigned int start_offset, end_offset;
     if (unlikely (!glyf.get_offsets (next_glyph, &start_offset, &end_offset))) {
       *glyf_size = 0;
@@ -58,7 +58,7 @@ _calculate_glyf_and_loca_prime_size (const OT::glyf::accelerator_t &glyf,
 bool
 _write_glyf_and_loca_prime (const OT::glyf::accelerator_t &glyf,
                             const char                    *glyf_data,
-                            const hb_set_t                *glyph_ids,
+                            hb_auto_array_t<unsigned int> &glyph_ids,
                             int                            glyf_prime_size,
                             char                          *glyf_prime_data /* OUT */,
                             int                            loca_prime_size,
@@ -73,9 +73,9 @@ _write_glyf_and_loca_prime (const OT::glyf::accelerator_t &glyf,
   hb_codepoint_t new_glyph_id = 0;
 
   unsigned int end_offset;
-  while (hb_set_next(glyph_ids, &next_glyph)) {
+  for (unsigned int i = 0; i < glyph_ids.len; i++) {
     unsigned int start_offset;
-    if (unlikely (!glyf.get_offsets (next_glyph, &start_offset, &end_offset))) {
+    if (unlikely (!glyf.get_offsets (glyph_ids[i], &start_offset, &end_offset))) {
       return false;
     }
 
@@ -97,7 +97,7 @@ _write_glyf_and_loca_prime (const OT::glyf::accelerator_t &glyf,
 bool
 _hb_subset_glyf_and_loca (const OT::glyf::accelerator_t  &glyf,
                           const char                     *glyf_data,
-                          hb_set_t                       *glyphs_to_retain,
+                          hb_auto_array_t<unsigned int>  &glyphs_to_retain,
                           hb_blob_t                     **glyf_prime /* OUT */,
                           hb_blob_t                     **loca_prime /* OUT */)
 {
@@ -158,7 +158,7 @@ hb_subset_glyf_and_loca (hb_subset_plan_t *plan,
 
   OT::glyf::accelerator_t glyf;
   glyf.init(face);
-  bool result = _hb_subset_glyf_and_loca (glyf, glyf_data, plan->glyphs_to_retain, glyf_prime, loca_prime);
+  bool result = _hb_subset_glyf_and_loca (glyf, glyf_data, plan->gids_to_retain, glyf_prime, loca_prime);
   glyf.fini();
 
   // TODO(grieger): Subset loca

src/hb-subset-plan.cc

@@ -29,47 +29,75 @@
 #include "hb-subset-plan.hh"
 #include "hb-ot-cmap-table.hh"
 
+int hb_codepoint_t_cmp(const void *l, const void *r) {
+  return *((hb_codepoint_t *) l) - *((hb_codepoint_t *) r);
+}
+
 hb_bool_t
 hb_subset_plan_new_gid_for_old_id(hb_subset_plan_t *plan,
                                   hb_codepoint_t old_gid,
                                   hb_codepoint_t *new_gid) {
-  // TODO(Q1) lookup in map from old:new gid
-  // TEMPORARY: just loop over ids to retain and count up
-  hb_codepoint_t current = -1;
-  hb_codepoint_t count = 0;
-  while (hb_set_next(plan->glyphs_to_retain, &current)) {
-    if (old_gid == current) {
-      *new_gid = count;
+
+  // the index in old_gids is the new gid; only up to codepoints.len are valid
+  for (unsigned int i = 0; i < plan->codepoints.len; i++) {
+    if (plan->gids_to_retain[i] == old_gid) {
+      *new_gid = i;
       return true;
     }
-    count++;
   }
   return false;
 }
 
-hb_set_t *
-glyph_ids_to_retain (hb_face_t *face,
-                     hb_set_t  *codepoints)
+void populate_codepoints(hb_set_t *input_codepoints,
+                         hb_auto_array_t<hb_codepoint_t>& plan_codepoints) {
+  plan_codepoints.alloc(hb_set_get_population(input_codepoints));
+  hb_codepoint_t cp = -1;
+  while (hb_set_next(input_codepoints, &cp)) {
+    hb_codepoint_t *wr = plan_codepoints.push();
+    *wr = cp;
+  }
+  plan_codepoints.qsort(hb_codepoint_t_cmp);
+}
+
+void
+populate_gids_to_retain (hb_face_t *face,
+                         hb_auto_array_t<hb_codepoint_t>& codepoints,
+                         hb_auto_array_t<hb_codepoint_t>& old_gids)
 {
   OT::cmap::accelerator_t cmap;
   cmap.init (face);
-  hb_codepoint_t cp = -1;
-  hb_set_t *gids = hb_set_create();
-  while (hb_set_next(codepoints, &cp)) {
+
+  hb_auto_array_t<unsigned int> bad_indices;
+
+  old_gids.alloc(codepoints.len);
+  for (unsigned int i = 0; i < codepoints.len; i++) {
     hb_codepoint_t gid;
-    if (cmap.get_nominal_glyph(cp, &gid)) {
-      DEBUG_MSG(SUBSET, nullptr, "gid for U+%04X is %d", cp, gid);
-      hb_set_add(gids, gid);
-    } else {
-      DEBUG_MSG(SUBSET, nullptr, "Unable to resolve gid for U+%04X", cp);
+    if (!cmap.get_nominal_glyph(codepoints[i], &gid)) {
+      gid = -1;
+      *(bad_indices.push()) = i;
     }
+    *(old_gids.push()) = gid;
+  }
+
+  while (bad_indices.len > 0) {
+    unsigned int i = bad_indices[bad_indices.len - 1];
+    bad_indices.pop();
+    DEBUG_MSG(SUBSET, nullptr, "Drop U+%04X; no gid", codepoints[i]);
+    codepoints.remove(i);
+    old_gids.remove(i);
   }
 
+  for (unsigned int i = 0; i < codepoints.len; i++) {
+      DEBUG_MSG(SUBSET, nullptr, " U+%04X, old_gid %d, new_gid %d", codepoints[i], old_gids[i], i);
+  }
+
+  // TODO always keep .notdef
+
+
   // TODO(Q1) expand with glyphs that make up complex glyphs
   // TODO expand with glyphs reached by G*
   //
   cmap.fini ();
-  return gids;
 }
 
 /**
@@ -88,7 +116,8 @@ hb_subset_plan_create (hb_face_t           *face,
                        hb_subset_input_t   *input)
 {
   hb_subset_plan_t *plan = hb_object_create<hb_subset_plan_t> ();
-  plan->glyphs_to_retain = glyph_ids_to_retain (face, input->codepoints);
+  populate_codepoints(input->codepoints, plan->codepoints);
+  populate_gids_to_retain(face, plan->codepoints, plan->gids_to_retain);
   return plan;
 }
 
@@ -96,7 +125,6 @@ hb_subset_plan_t *
 hb_subset_plan_get_empty ()
 {
   hb_subset_plan_t *plan = hb_object_create<hb_subset_plan_t> ();
-  plan->glyphs_to_retain = hb_set_get_empty();
   return plan;
 }
 
@@ -110,6 +138,7 @@ hb_subset_plan_destroy (hb_subset_plan_t *plan)
 {
   if (!hb_object_destroy (plan)) return;
 
-  hb_set_destroy (plan->glyphs_to_retain);
+  plan->codepoints.finish();
+  plan->gids_to_retain.finish();
   free (plan);
 }

src/hb-subset-plan.hh

@@ -21,7 +21,7 @@
  * ON AN "AS IS" BASIS, AND THE COPYRIGHT HOLDER HAS NO OBLIGATION TO
  * PROVIDE MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
  *
- * Google Author(s): Garret Rieger
+ * Google Author(s): Garret Rieger, Roderick Sheeter
  */
 
 #ifndef HB_SUBSET_PLAN_HH
@@ -35,7 +35,11 @@ struct hb_subset_plan_t {
   hb_object_header_t header;
   ASSERT_POD ();
 
-  hb_set_t *glyphs_to_retain;
+  // TODO(Q1) actual map, drop this crap
+  // Look at me ma, I'm a poor mans map codepoint : new gid
+  // codepoints is sorted and aligned with gids_to_retain.
+  hb_auto_array_t<hb_codepoint_t> codepoints;
+  hb_auto_array_t<hb_codepoint_t> gids_to_retain;
 };
 
 typedef struct hb_subset_plan_t hb_subset_plan_t;

src/hb-subset.cc

@@ -122,8 +122,8 @@ subset (hb_subset_plan_t *plan, hb_face_t *source, hb_face_t *dest)
 
     hb_blob_destroy (table_blob);
 
-    // TODO string not numeric tag
-    DEBUG_MSG(SUBSET, nullptr, "Subset %d %s", TableType::tableTag, result ? "success" : "FAILED!");
+    hb_tag_t tag = TableType::tableTag;
+    DEBUG_MSG(SUBSET, nullptr, "Subset %c%c%c%c %s", HB_UNTAG(tag), result ? "success" : "FAILED!");
     return result;
 }
 
@@ -316,14 +316,23 @@ hb_subset (hb_face_t *source,
 
   hb_subset_plan_t *plan = hb_subset_plan_create (source, profile, input);
 
-  hb_codepoint_t old_gid = -1;
-  while (hb_set_next (plan->glyphs_to_retain, &old_gid)) {
-    hb_codepoint_t new_gid;
-    if (hb_subset_plan_new_gid_for_old_id (plan, old_gid, &new_gid)) {
-      DEBUG_MSG (SUBSET, nullptr, "Remap %d : %d", old_gid, new_gid);
-    } else {
-      DEBUG_MSG (SUBSET, nullptr, "Remap %d : DOOM! No new ID", old_gid);
-    }
+  hb_face_t *face = hb_subset_face_create ();
+
+  /* Copy tables to new face. */
+  {
+    hb_tag_t table_tags[32];
+    unsigned int offset = 0, count;
+    do {
+      count = ARRAY_LENGTH (table_tags);
+      hb_face_get_table_tags (source, offset, &count, table_tags);
+      for (unsigned int i = 0; i < count; i++)
+      {
+      	hb_tag_t tag = table_tags[i];
+      	hb_blob_t *blob = hb_face_reference_table (source, tag);
+      	hb_subset_face_add_table (face, tag, blob);
+      	hb_blob_destroy (blob);
+      }
+    } while (count == ARRAY_LENGTH (table_tags));
   }
 
   hb_face_t *dest = hb_subset_face_create ();