Merge pull request #792 from googlefonts/master

Support more tables in subsetter

src/hb-open-file-private.hh

@@ -133,15 +133,16 @@ typedef struct OffsetTable
 			 unsigned int table_count)
   {
     TRACE_SERIALIZE (this);
-    // alloc 12 for the OTHeader
+    /* alloc 12 for the OTHeader */
     if (unlikely (!c->extend_min (*this))) return_trace (false);
-    // write sfntVersion (bytes 0..3)
+    /* write sfntVersion (bytes 0..3) */
     sfnt_version.set (sfnt_tag);
-    // take space for numTables, searchRange, entrySelector, RangeShift
+    /* take space for numTables, searchRange, entrySelector, RangeShift
-    // and the TableRecords themselves
+     * and the TableRecords themselves
+     */
     if (unlikely (!tables.serialize (c, table_count))) return_trace (false);
 
-    // write OffsetTables, alloc for and write actual table blobs
+    /* write OffsetTables, alloc for and write actual table blobs */
     for (unsigned int i = 0; i < table_count; i++)
     {
       TableRecord &rec = tables.array[i];
@@ -153,9 +154,9 @@ typedef struct OffsetTable
       // take room for the table
       void *p = c->allocate_size<void> (rec.length);
       if (unlikely (!p)) {return false;}
-      // copy the actual table
+      /* copy the actual table */
       memcpy (p, hb_blob_get_data (blob, nullptr), rec.length);
-      // 4-byte allignment
+      /* 4-byte allignment */
       if (rec.length % 4)
 	p = c->allocate_size<void> (4 - rec.length % 4);
     }

src/hb-ot-cmap-table.hh

@@ -271,18 +271,13 @@ struct CmapSubtableLongSegmented
     return_trace (c->check_struct (this) && groups.sanitize (c));
   }
 
-  inline bool serialize(hb_serialize_context_t *context,
+  inline bool serialize (hb_serialize_context_t *context,
-                        unsigned int group_count,
+                         hb_prealloced_array_t<CmapSubtableLongGroup> &group_data)
-                        Supplier<CmapSubtableLongGroup> &group_supplier)
   {
     TRACE_SERIALIZE (this);
-    if (unlikely(!context->extend_min (*this))) return_trace (false);
+    if (unlikely (!context->extend_min (*this))) return_trace (false);
-    if (unlikely(!groups.serialize(context, group_count))) return_trace (false);
+    Supplier<CmapSubtableLongGroup> supplier (group_data.array, group_data.len);
-    for (unsigned int i = 0; i < group_count; i++)
+    if (unlikely (!groups.serialize (context, supplier, group_data.len))) return_trace (false);
-    {
-      const CmapSubtableLongGroup &group = group_supplier[i];
-      memcpy(&groups[i], &group, sizeof(group));
-    }
     return true;
   }
 
@@ -440,12 +435,12 @@ struct CmapSubtable
 			 hb_codepoint_t *glyph) const
   {
     switch (u.format) {
-    case  0: return u.format0 .get_glyph(codepoint, glyph);
+    case  0: return u.format0 .get_glyph (codepoint, glyph);
-    case  4: return u.format4 .get_glyph(codepoint, glyph);
+    case  4: return u.format4 .get_glyph (codepoint, glyph);
-    case  6: return u.format6 .get_glyph(codepoint, glyph);
+    case  6: return u.format6 .get_glyph (codepoint, glyph);
-    case 10: return u.format10.get_glyph(codepoint, glyph);
+    case 10: return u.format10.get_glyph (codepoint, glyph);
-    case 12: return u.format12.get_glyph(codepoint, glyph);
+    case 12: return u.format12.get_glyph (codepoint, glyph);
-    case 13: return u.format13.get_glyph(codepoint, glyph);
+    case 13: return u.format13.get_glyph (codepoint, glyph);
     case 14:
     default: return false;
     }
@@ -522,24 +517,28 @@ struct cmap
 		  encodingRecord.sanitize (c, this));
   }
 
-  inline void populate_groups(hb_prealloced_array_t<hb_codepoint_t> &codepoints,
+  inline bool populate_groups (hb_subset_plan_t *plan,
-                              hb_prealloced_array_t<CmapSubtableLongGroup> *groups) const
+			       hb_prealloced_array_t<CmapSubtableLongGroup> *groups) const
   {
     CmapSubtableLongGroup *group = nullptr;
-    for (unsigned int i = 0; i < codepoints.len; i++) {
+    for (unsigned int i = 0; i < plan->codepoints.len; i++) {
-      hb_codepoint_t cp = codepoints[i];
+
-      if (!group)
+      hb_codepoint_t cp = plan->codepoints[i];
+      if (!group || cp - 1 != group->endCharCode)
       {
-        group = groups->push();
+        group = groups->push ();
-        group->startCharCode.set(cp);
+        group->startCharCode.set (cp);
-        group->endCharCode.set(cp);
+        group->endCharCode.set (cp);
-        group->glyphID.set(i);  // index in codepoints is new gid
+        hb_codepoint_t new_gid;
-      } else if (cp -1 == group->endCharCode)
+        if (unlikely (!hb_subset_plan_new_gid_for_codepoint (plan, cp, &new_gid)))
-      {
+        {
-        group->endCharCode.set(cp);
+          DEBUG_MSG(SUBSET, nullptr, "Unable to find new gid for %04x", cp);
+          return false;
+        }
+        group->glyphID.set (new_gid);
       } else
       {
-        group = nullptr;
+        group->endCharCode.set (cp);
       }
     }
 
@@ -548,51 +547,54 @@ struct cmap
       CmapSubtableLongGroup& group = (*groups)[i];
       DEBUG_MSG(SUBSET, nullptr, "  %d: U+%04X-U+%04X, gid %d-%d", i, (uint32_t) group.startCharCode, (uint32_t) group.endCharCode, (uint32_t) group.glyphID, (uint32_t) group.glyphID + ((uint32_t) group.endCharCode - (uint32_t) group.startCharCode));
     }
-  }
 
-  hb_bool_t _subset (hb_prealloced_array_t<CmapSubtableLongGroup> &groups,
-                     size_t dest_sz,
-                     void *dest) const
-  {
-    hb_serialize_context_t context(dest, dest_sz);
-
-    OT::cmap *cmap = context.start_serialize<OT::cmap> ();
-    if (unlikely(!context.extend_min(*cmap)))
-    {
-      return false;
-    }
-
-    cmap->version.set(0);
-
-    if (unlikely(!cmap->encodingRecord.serialize(&context, /* numTables */ 1)))
-    {
-      return false;
-    }
-
-    EncodingRecord &rec = cmap->encodingRecord[0];
-    rec.platformID.set (3); // Windows
-    rec.encodingID.set (1); // Unicode BMP
-
-    CmapSubtable &subtable = rec.subtable.serialize(&context, &rec.subtable);
-    subtable.u.format.set(12);
-
-    CmapSubtableFormat12 &format12 = subtable.u.format12;
-    format12.format.set(12);
-    format12.reservedZ.set(0);
-
-    OT::Supplier<CmapSubtableLongGroup> group_supplier  (&groups[0], groups.len, sizeof (CmapSubtableLongGroup));
-    if (unlikely(!format12.serialize(&context, groups.len, group_supplier)))
-      return false;
-
-    context.end_serialize ();
     return true;
   }
 
-  hb_blob_t * subset (hb_subset_plan_t *plan, hb_face_t *source) const
+  inline bool _subset (hb_prealloced_array_t<CmapSubtableLongGroup> &groups,
+		       size_t dest_sz,
+		       void *dest) const
+  {
+    hb_serialize_context_t context (dest, dest_sz);
+
+    OT::cmap *cmap = context.start_serialize<OT::cmap> ();
+    if (unlikely (!context.extend_min (*cmap)))
+    {
+      return false;
+    }
+
+    cmap->version.set (0);
+
+    if (unlikely (!cmap->encodingRecord.serialize (&context, /* numTables */ 1))) return false;
+
+    EncodingRecord &rec = cmap->encodingRecord[0];
+    rec.platformID.set (3); // Windows
+    rec.encodingID.set (10); // Unicode UCS-4
+
+    /* capture offset to subtable */
+    CmapSubtable &subtable = rec.subtable.serialize (&context, cmap);
+
+    subtable.u.format.set (12);
+
+    CmapSubtableFormat12 &format12 = subtable.u.format12;
+    if (unlikely (!context.extend_min (format12))) return false;
+
+    format12.format.set (12);
+    format12.reservedZ.set (0);
+    format12.lengthZ.set (16 + 12 * groups.len);
+
+    if (unlikely (!format12.serialize (&context, groups))) return false;
+
+    context.end_serialize ();
+
+    return true;
+  }
+
+  inline bool subset (hb_subset_plan_t *plan) const
   {
     hb_auto_array_t<CmapSubtableLongGroup> groups;
 
-    populate_groups(plan->codepoints, &groups);
+    if (unlikely (!populate_groups (plan, &groups))) return false;
 
     // We now know how big our blob needs to be
     // TODO use APIs from the structs to get size?
@@ -600,24 +602,25 @@ struct cmap
                    + 8 // 1 EncodingRecord
                    + 16 // Format 12 header
                    + 12 * groups.len; // SequentialMapGroup records
-    void *dest = calloc(dest_sz, 1);
+    void *dest = calloc (dest_sz, 1);
-    if (unlikely(!dest)) {
+    if (unlikely (!dest)) {
       DEBUG_MSG(SUBSET, nullptr, "Unable to alloc %ld for cmap subset output", dest_sz);
-      return nullptr;
+      return false;
     }
 
-    if (unlikely(!_subset(groups, dest_sz, dest)))
+    if (unlikely (!_subset (groups, dest_sz, dest)))
     {
-      free(dest);
+      free (dest);
-      return nullptr;
+      return false;
     }
 
     // all done, write the blob into dest
-    return hb_blob_create((const char *)dest, 
+    hb_blob_t *cmap_prime = hb_blob_create ((const char *)dest,
-                          dest_sz,
+                                            dest_sz,
-                          HB_MEMORY_MODE_READONLY,
+                                            HB_MEMORY_MODE_READONLY,
-                          /* userdata */ nullptr,
+                                            /* userdata */ nullptr,
-                          free);
+                                            free);
+    return hb_subset_plan_add_table (plan, HB_OT_TAG_cmap, cmap_prime);
   }
 
   struct accelerator_t

src/hb-ot-hhea-table.hh

@@ -41,7 +41,7 @@ namespace OT {
 #define HB_OT_TAG_hhea HB_TAG('h','h','e','a')
 #define HB_OT_TAG_vhea HB_TAG('v','h','e','a')
 
-
+template <typename T>
 struct _hea
 {
   inline bool sanitize (hb_sanitize_context_t *c) const
@@ -84,10 +84,10 @@ struct _hea
   DEFINE_SIZE_STATIC (36);
 };
 
-struct hhea : _hea {
+struct hhea : _hea<hhea> {
   static const hb_tag_t tableTag	= HB_OT_TAG_hhea;
 };
-struct vhea : _hea {
+struct vhea : _hea<vhea> {
   static const hb_tag_t tableTag	= HB_OT_TAG_vhea;
 };
 

src/hb-ot-hmtx-table.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): Behdad Esfahbod
+ * Google Author(s): Behdad Esfahbod, Roderick Sheeter
  */
 
 #ifndef HB_OT_HMTX_TABLE_HH
@@ -53,7 +53,7 @@ struct LongMetric
   DEFINE_SIZE_STATIC (4);
 };
 
-template <typename T>
+template <typename T, typename H>
 struct hmtxvmtx
 {
   inline bool sanitize (hb_sanitize_context_t *c) const
@@ -64,8 +64,117 @@ struct hmtxvmtx
     return_trace (true);
   }
 
+
+  inline bool subset_update_header (hb_subset_plan_t *plan,
+                                    unsigned int num_hmetrics) const
+  {
+    hb_blob_t *src_blob = OT::Sanitizer<H> ().sanitize (plan->source->reference_table (H::tableTag));
+    hb_blob_t *dest_blob = hb_blob_copy_writable_or_fail(src_blob);
+    hb_blob_destroy (src_blob);
+
+    if (unlikely (!dest_blob)) {
+      return false;
+    }
+
+    unsigned int length;
+    H *table = (H *) hb_blob_get_data (dest_blob, &length);
+    table->numberOfLongMetrics.set (num_hmetrics);
+
+    bool result = hb_subset_plan_add_table (plan, H::tableTag, dest_blob);
+    hb_blob_destroy (dest_blob);
+
+    return result;
+  }
+
+  inline bool subset (hb_subset_plan_t *plan) const
+  {
+    typename T::accelerator_t _mtx;
+    _mtx.init (plan->source);
+
+    /* All the trailing glyphs with the same advance can use one LongMetric
+     * and just keep LSB */
+    hb_prealloced_array_t<hb_codepoint_t> &gids = plan->gids_to_retain_sorted;
+    unsigned int num_advances = gids.len;
+    unsigned int last_advance = _mtx.get_advance (gids[num_advances - 1]);
+    while (num_advances > 1
+        && last_advance == _mtx.get_advance (gids[num_advances - 2]))
+    {
+      num_advances--;
+    }
+
+    /* alloc the new table */
+    size_t dest_sz = num_advances * 4
+                  + (gids.len - num_advances) * 2;
+    void *dest = (void *) calloc (dest_sz, 1);
+    if (unlikely (!dest))
+    {
+      return false;
+    }
+    DEBUG_MSG(SUBSET, nullptr, "%c%c%c%c in src has %d advances, %d lsbs", HB_UNTAG(T::tableTag), _mtx.num_advances, _mtx.num_metrics - _mtx.num_advances);
+    DEBUG_MSG(SUBSET, nullptr, "%c%c%c%c in dest has %d advances, %d lsbs, %u bytes", HB_UNTAG(T::tableTag), num_advances, gids.len - num_advances, (unsigned int) dest_sz);
+
+    const char *source_table = hb_blob_get_data (_mtx.blob, nullptr);
+    // Copy everything over
+    LongMetric * old_metrics = (LongMetric *) source_table;
+    FWORD *lsbs = (FWORD *) (old_metrics + _mtx.num_advances);
+    char * dest_pos = (char *) dest;
+    for (unsigned int i = 0; i < gids.len; i++)
+    {
+      /* the last metric or the one for gids[i] */
+      LongMetric *src_metric = old_metrics + MIN ((hb_codepoint_t) _mtx.num_advances - 1, gids[i]);
+      if (gids[i] < _mtx.num_advances)
+      {
+        /* src is a LongMetric */
+        if (i < num_advances)
+        {
+          /* dest is a LongMetric, copy it */
+          *((LongMetric *) dest_pos) = *src_metric;
+        }
+        else
+        {
+          /* dest just lsb */
+          *((FWORD *) dest_pos) = src_metric->lsb;
+        }
+      }
+      else
+      {
+        FWORD src_lsb = *(lsbs + gids[i] - _mtx.num_advances);
+        if (i < num_advances)
+        {
+          /* dest needs a full LongMetric */
+          LongMetric *metric = (LongMetric *)dest_pos;
+          metric->advance = src_metric->advance;
+          metric->lsb = src_lsb;
+        }
+        else
+        {
+          /* dest just needs an lsb */
+          *((FWORD *) dest_pos) = src_lsb;
+        }
+      }
+      dest_pos += (i < num_advances ? 4 : 2);
+    }
+    _mtx.fini ();
+
+    // Amend header num hmetrics
+    if (unlikely (!subset_update_header (plan, num_advances)))
+    {
+      free (dest);
+      return false;
+    }
+
+    hb_blob_t *result = hb_blob_create ((const char *)dest,
+                                        dest_sz,
+                                        HB_MEMORY_MODE_READONLY,
+                                        /* userdata */ nullptr,
+                                        free);
+    return hb_subset_plan_add_table (plan, T::tableTag, result);
+  }
+
   struct accelerator_t
   {
+    friend struct hmtxvmtx;
+
     inline void init (hb_face_t *face,
 		      unsigned int default_advance_ = 0)
     {
@@ -74,7 +183,7 @@ struct hmtxvmtx
       bool got_font_extents = false;
       if (T::os2Tag)
       {
-	hb_blob_t *os2_blob = Sanitizer<os2>().sanitize (face->reference_table (T::os2Tag));
+	hb_blob_t *os2_blob = Sanitizer<os2> ().sanitize (face->reference_table (T::os2Tag));
 	const os2 *os2_table = Sanitizer<os2>::lock_instance (os2_blob);
 #define USE_TYPO_METRICS (1u<<7)
 	if (0 != (os2_table->fsSelection & USE_TYPO_METRICS))
@@ -87,8 +196,8 @@ struct hmtxvmtx
 	hb_blob_destroy (os2_blob);
       }
 
-      hb_blob_t *_hea_blob = Sanitizer<_hea>().sanitize (face->reference_table (T::headerTag));
+      hb_blob_t *_hea_blob = Sanitizer<H> ().sanitize (face->reference_table (H::tableTag));
-      const _hea *_hea_table = Sanitizer<_hea>::lock_instance (_hea_blob);
+      const H *_hea_table = Sanitizer<H>::lock_instance (_hea_blob);
       num_advances = _hea_table->numberOfLongMetrics;
       if (!got_font_extents)
       {
@@ -101,7 +210,7 @@ struct hmtxvmtx
 
       has_font_extents = got_font_extents;
 
-      blob = Sanitizer<hmtxvmtx>().sanitize (face->reference_table (T::tableTag));
+      blob = Sanitizer<hmtxvmtx> ().sanitize (face->reference_table (T::tableTag));
 
       /* Cap num_metrics() and num_advances() based on table length. */
       unsigned int len = hb_blob_get_length (blob);
@@ -119,7 +228,7 @@ struct hmtxvmtx
       }
       table = Sanitizer<hmtxvmtx>::lock_instance (blob);
 
-      var_blob = Sanitizer<HVARVVAR>().sanitize (face->reference_table (T::variationsTag));
+      var_blob = Sanitizer<HVARVVAR> ().sanitize (face->reference_table (T::variationsTag));
       var_table = Sanitizer<HVARVVAR>::lock_instance (var_blob);
     }
 
@@ -129,22 +238,31 @@ struct hmtxvmtx
       hb_blob_destroy (var_blob);
     }
 
-    inline unsigned int get_advance (hb_codepoint_t  glyph,
+    inline unsigned int get_advance (hb_codepoint_t  glyph) const
-				     hb_font_t      *font) const
     {
       if (unlikely (glyph >= num_metrics))
       {
-	/* If num_metrics is zero, it means we don't have the metrics table
+        /* If num_metrics is zero, it means we don't have the metrics table
-	 * for this direction: return default advance.  Otherwise, it means that the
+         * for this direction: return default advance.  Otherwise, it means that the
-	 * glyph index is out of bound: return zero. */
+         * glyph index is out of bound: return zero. */
-	if (num_metrics)
+        if (num_metrics)
-	  return 0;
+          return 0;
-	else
+        else
-	  return default_advance;
+          return default_advance;
       }
 
-      return table->longMetric[MIN (glyph, (uint32_t) num_advances - 1)].advance
+      return table->longMetric[MIN (glyph, (uint32_t) num_advances - 1)].advance;
-	   + (font->num_coords ? var_table->get_advance_var (glyph, font->coords, font->num_coords) : 0); // TODO Optimize?!
+    }
+
+    inline unsigned int get_advance (hb_codepoint_t  glyph,
+                                     hb_font_t      *font) const
+    {
+      unsigned int advance = get_advance (glyph);
+      if (likely(glyph < num_metrics))
+      {
+        advance += (font->num_coords ? var_table->get_advance_var (glyph, font->coords, font->num_coords) : 0); // TODO Optimize?!
+      }
+      return advance;	        
     }
 
     public:
@@ -153,11 +271,12 @@ struct hmtxvmtx
     unsigned short descender;
     unsigned short line_gap;
 
-    private:
+    protected:
     unsigned int num_metrics;
     unsigned int num_advances;
     unsigned int default_advance;
 
+    private:
     const hmtxvmtx *table;
     hb_blob_t *blob;
     const HVARVVAR *var_table;
@@ -190,15 +309,13 @@ struct hmtxvmtx
   DEFINE_SIZE_ARRAY (0, longMetric);
 };
 
-struct hmtx : hmtxvmtx<hmtx> {
+struct hmtx : hmtxvmtx<hmtx, hhea> {
   static const hb_tag_t tableTag	= HB_OT_TAG_hmtx;
-  static const hb_tag_t headerTag	= HB_OT_TAG_hhea;
   static const hb_tag_t variationsTag	= HB_OT_TAG_HVAR;
   static const hb_tag_t os2Tag		= HB_OT_TAG_os2;
 };
-struct vmtx : hmtxvmtx<vmtx> {
+struct vmtx : hmtxvmtx<vmtx, vhea> {
   static const hb_tag_t tableTag	= HB_OT_TAG_vmtx;
-  static const hb_tag_t headerTag	= HB_OT_TAG_vhea;
   static const hb_tag_t variationsTag	= HB_OT_TAG_VVAR;
   static const hb_tag_t os2Tag		= HB_TAG_NONE;
 };

src/hb-ot-maxp-table.hh

@@ -28,7 +28,7 @@
 #define HB_OT_MAXP_TABLE_HH
 
 #include "hb-open-type-private.hh"
-
+#include "hb-subset-plan.hh"
 
 namespace OT {
 
@@ -48,6 +48,11 @@ struct maxp
     return numGlyphs;
   }
 
+  inline void set_num_glyphs (unsigned int count)
+  {
+    numGlyphs.set (count);
+  }
+
   inline bool sanitize (hb_sanitize_context_t *c) const
   {
     TRACE_SANITIZE (this);
@@ -56,6 +61,25 @@ struct maxp
 			  (version.major == 0 && version.minor == 0x5000u)));
   }
 
+  inline bool subset (hb_subset_plan_t *plan) const
+  {
+    hb_blob_t *maxp_blob = OT::Sanitizer<OT::maxp>().sanitize (hb_face_reference_table (plan->source, HB_OT_TAG_maxp));
+    hb_blob_t *maxp_prime_blob = hb_blob_copy_writable_or_fail (maxp_blob);
+    hb_blob_destroy (maxp_blob);
+
+    if (unlikely (!maxp_prime_blob)) {
+      return false;
+    }
+    unsigned int length;
+    OT::maxp *maxp_prime = (OT::maxp *) hb_blob_get_data (maxp_prime_blob, &length);
+
+    maxp_prime->set_num_glyphs (plan->gids_to_retain_sorted.len);
+
+    bool result = hb_subset_plan_add_table(plan, HB_OT_TAG_maxp, maxp_prime_blob);
+    hb_blob_destroy (maxp_prime_blob);
+    return result;
+  }
+
   /* We only implement version 0.5 as none of the extra fields in version 1.0 are useful. */
   protected:
   FixedVersion<>version;		/* Version of the maxp table (0.5 or 1.0),

src/hb-ot-os2-table.hh

@@ -49,6 +49,53 @@ struct os2
     return_trace (c->check_struct (this));
   }
 
+  inline bool subset (hb_subset_plan_t *plan) const
+  {
+    hb_blob_t *os2_blob = OT::Sanitizer<OT::os2>().sanitize (hb_face_reference_table (plan->source, HB_OT_TAG_os2));
+    hb_blob_t *os2_prime_blob = hb_blob_create_sub_blob (os2_blob, 0, -1);
+    // TODO(grieger): move to hb_blob_copy_writable_or_fail
+    hb_blob_destroy (os2_blob);
+
+    OT::os2 *os2_prime = (OT::os2 *) hb_blob_get_data_writable (os2_prime_blob, nullptr);
+    if (unlikely (!os2_prime)) {
+      hb_blob_destroy (os2_prime_blob);
+      return false;
+    }
+
+    uint16_t min_cp, max_cp;
+    find_min_and_max_codepoint (plan->codepoints, &min_cp, &max_cp);
+    os2_prime->usFirstCharIndex.set (min_cp);
+    os2_prime->usLastCharIndex.set (max_cp);
+
+    bool result = hb_subset_plan_add_table(plan, HB_OT_TAG_os2, os2_prime_blob);
+    hb_blob_destroy (os2_prime_blob);
+    return result;
+  }
+
+  static inline void find_min_and_max_codepoint (const hb_prealloced_array_t<hb_codepoint_t> &codepoints,
+                                                 uint16_t *min_cp, /* OUT */
+                                                 uint16_t *max_cp  /* OUT */)
+  {
+    hb_codepoint_t min = -1, max = 0;
+
+    for (int i = 0; i < codepoints.len; i++)
+    {
+      hb_codepoint_t cp = codepoints[i];
+      if (cp < min)
+        min = cp;
+      if (cp > max)
+        max = cp;
+    }
+
+    if (min > 0xFFFF)
+      min = 0xFFFF;
+    if (max > 0xFFFF)
+      max = 0xFFFF;
+
+    *min_cp = min;
+    *max_cp = max;
+  }
+
   public:
   HBUINT16	version;
 

src/hb-subset-glyf.cc

@@ -157,16 +157,15 @@ _hb_subset_glyf_and_loca (const OT::glyf::accelerator_t  &glyf,
  **/
 bool
 hb_subset_glyf_and_loca (hb_subset_plan_t *plan,
-                         hb_face_t        *face,
                          bool             *use_short_loca, /* OUT */
                          hb_blob_t       **glyf_prime, /* OUT */
                          hb_blob_t       **loca_prime /* OUT */)
 {
-  hb_blob_t *glyf_blob = OT::Sanitizer<OT::glyf>().sanitize (face->reference_table (HB_OT_TAG_glyf));
+  hb_blob_t *glyf_blob = OT::Sanitizer<OT::glyf>().sanitize (plan->source->reference_table (HB_OT_TAG_glyf));
   const char *glyf_data = hb_blob_get_data(glyf_blob, nullptr);
 
   OT::glyf::accelerator_t glyf;
-  glyf.init(face);
+  glyf.init(plan->source);
   bool result = _hb_subset_glyf_and_loca (glyf,
                                           glyf_data,
                                           plan->gids_to_retain_sorted,
@@ -175,7 +174,5 @@ hb_subset_glyf_and_loca (hb_subset_plan_t *plan,
                                           loca_prime);
   glyf.fini();
 
-  *use_short_loca = false;
-
   return result;
 }

src/hb-subset-glyf.hh

@@ -33,7 +33,6 @@
 
 HB_INTERNAL bool
 hb_subset_glyf_and_loca (hb_subset_plan_t *plan,
-                         hb_face_t        *face,
                          bool             *use_short_loca, /* OUT */
                          hb_blob_t       **glyf_prime /* OUT */,
                          hb_blob_t       **loca_prime /* OUT */);

src/hb-subset-plan.cc

@@ -38,21 +38,48 @@ _hb_codepoint_t_cmp (const void *pa, const void *pb)
   return a < b ? -1 : a > b ? +1 : 0;
 }
 
+hb_bool_t
+hb_subset_plan_new_gid_for_codepoint (hb_subset_plan_t *plan,
+                                      hb_codepoint_t codepoint,
+                                      hb_codepoint_t *new_gid)
+{
+  // TODO actual map, delete this garbage.
+  for (unsigned int i = 0; i < plan->codepoints.len; i++)
+  {
+    if (plan->codepoints[i] != codepoint) continue;
+    if (!hb_subset_plan_new_gid_for_old_id(plan, plan->gids_to_retain[i], new_gid))
+    {
+      return false;
+    }
+    return true;
+  }
+  return false;
+}
+
 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)
 {
   // the index in old_gids is the new gid; only up to codepoints.len are valid
-  for (unsigned int i = 0; i < plan->gids_to_retain_sorted.len; i++) {
+  for (unsigned int i = 0; i < plan->gids_to_retain.len; i++) {
-    if (plan->gids_to_retain_sorted[i] == old_gid) {
+    if (plan->gids_to_retain[i] == old_gid) {
-      *new_gid = i;
+      // +1: assign new gids from 1..N; 0 is special
+      *new_gid = i + 1;
       return true;
     }
   }
   return false;
 }
 
+hb_bool_t
+hb_subset_plan_add_table (hb_subset_plan_t *plan,
+                          hb_tag_t tag,
+                          hb_blob_t *contents)
+{
+  return hb_subset_face_add_table(plan->dest, tag, contents);
+}
+
 static void
 _populate_codepoints (hb_set_t *input_codepoints,
                       hb_prealloced_array_t<hb_codepoint_t>& plan_codepoints)
@@ -91,6 +118,7 @@ _populate_gids_to_retain (hb_face_t *face,
     *(old_gids.push ()) = gid;
   }
 
+  /* Generally there shouldn't be any */
   while (bad_indices.len > 0) {
     unsigned int i = bad_indices[bad_indices.len - 1];
     bad_indices.pop ();
@@ -109,10 +137,6 @@ _populate_gids_to_retain (hb_face_t *face,
     *(old_gids_sorted.push ()) = 0;
   old_gids_sorted.qsort (_hb_codepoint_t_cmp);
 
-  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(Q1) expand with glyphs that make up complex glyphs
   // TODO expand with glyphs reached by G*
   //
@@ -139,12 +163,15 @@ hb_subset_plan_create (hb_face_t           *face,
   plan->codepoints.init();
   plan->gids_to_retain.init();
   plan->gids_to_retain_sorted.init();
+  plan->source = hb_face_reference (face);
+  plan->dest = hb_subset_face_create ();
 
   _populate_codepoints (input->unicodes, plan->codepoints);
   _populate_gids_to_retain (face,
                             plan->codepoints,
                             plan->gids_to_retain,
                             plan->gids_to_retain_sorted);
+
   return plan;
 }
 
@@ -162,5 +189,8 @@ hb_subset_plan_destroy (hb_subset_plan_t *plan)
   plan->gids_to_retain.finish ();
   plan->gids_to_retain_sorted.finish ();
 
+  hb_face_destroy (plan->source);
+  hb_face_destroy (plan->dest);
+
   free (plan);
 }

src/hb-subset-plan.hh

@@ -41,6 +41,10 @@ struct hb_subset_plan_t {
   hb_prealloced_array_t<hb_codepoint_t> codepoints;
   hb_prealloced_array_t<hb_codepoint_t> gids_to_retain;
   hb_prealloced_array_t<hb_codepoint_t> gids_to_retain_sorted;
+
+  // Plan is only good for a specific source/dest so keep them with it
+  hb_face_t *source;
+  hb_face_t *dest;
 };
 
 typedef struct hb_subset_plan_t hb_subset_plan_t;
@@ -55,6 +59,16 @@ hb_subset_plan_new_gid_for_old_id(hb_subset_plan_t *plan,
                                   hb_codepoint_t old_gid,
                                   hb_codepoint_t *new_gid /* OUT */);
 
+HB_INTERNAL hb_bool_t
+hb_subset_plan_new_gid_for_codepoint(hb_subset_plan_t *plan,
+                                     hb_codepoint_t codepont,
+                                     hb_codepoint_t *new_gid /* OUT */);
+
+HB_INTERNAL hb_bool_t
+hb_subset_plan_add_table(hb_subset_plan_t *plan,
+                         hb_tag_t tag,
+                         hb_blob_t *contents);
+
 HB_INTERNAL void
 hb_subset_plan_destroy (hb_subset_plan_t *plan);
 

src/hb-subset-private.hh

@@ -34,6 +34,8 @@
 
 #include "hb-font-private.hh"
 
+typedef struct hb_subset_face_data_t hb_subset_face_data_t;
+
 struct hb_subset_input_t {
   hb_object_header_t header;
   ASSERT_POD ();
@@ -50,4 +52,10 @@ struct hb_subset_input_t {
    */
 };
 
+HB_INTERNAL hb_face_t *
+hb_subset_face_create (void);
+
+HB_INTERNAL hb_bool_t
+hb_subset_face_add_table (hb_face_t *face, hb_tag_t tag, hb_blob_t *blob);
+
 #endif /* HB_SUBSET_PRIVATE_HH */

src/hb-subset.cc

@@ -27,6 +27,8 @@
 #include "hb-object-private.hh"
 #include "hb-open-type-private.hh"
 
+#include "hb-private.hh"
+
 #include "hb-subset-glyf.hh"
 #include "hb-subset-private.hh"
 #include "hb-subset-plan.hh"
@@ -34,6 +36,11 @@
 #include "hb-open-file-private.hh"
 #include "hb-ot-cmap-table.hh"
 #include "hb-ot-glyf-table.hh"
+#include "hb-ot-head-table.hh"
+#include "hb-ot-hhea-table.hh"
+#include "hb-ot-hmtx-table.hh"
+#include "hb-ot-maxp-table.hh"
+#include "hb-ot-os2-table.hh"
 
 
 #ifndef HB_NO_VISIBILITY
@@ -73,18 +80,18 @@ hb_subset_profile_destroy (hb_subset_profile_t *profile)
 }
 
 template<typename TableType>
-static hb_blob_t *
+static bool
-_subset (hb_subset_plan_t *plan, hb_face_t *source)
+_subset (hb_subset_plan_t *plan)
 {
     OT::Sanitizer<TableType> sanitizer;
-    hb_blob_t *source_blob = sanitizer.sanitize (source->reference_table (TableType::tableTag));
+    hb_blob_t *source_blob = sanitizer.sanitize (plan->source->reference_table (TableType::tableTag));
     const TableType *table = OT::Sanitizer<TableType>::lock_instance (source_blob);
-    hb_blob_t *result = table->subset(plan, source);
+    hb_bool_t result = table->subset(plan);
 
     hb_blob_destroy (source_blob);
 
     hb_tag_t tag = TableType::tableTag;
-    DEBUG_MSG(SUBSET, nullptr, "Subset %c%c%c%c %s", HB_UNTAG(tag), result ? "success" : "FAILED!");
+    DEBUG_MSG(SUBSET, nullptr, "OT::%c%c%c%c::subset %s", HB_UNTAG(tag), result ? "success" : "FAILED!");
     return result;
 }
 
@@ -185,7 +192,7 @@ _hb_subset_face_reference_table (hb_face_t *face, hb_tag_t tag, void *user_data)
   return nullptr;
 }
 
-static hb_face_t *
+hb_face_t *
 hb_subset_face_create (void)
 {
   hb_subset_face_data_t *data = _hb_subset_face_data_create ();
@@ -196,7 +203,7 @@ hb_subset_face_create (void)
 				    _hb_subset_face_data_destroy);
 }
 
-static bool
+hb_bool_t
 hb_subset_face_add_table (hb_face_t *face, hb_tag_t tag, hb_blob_t *blob)
 {
   if (unlikely (face->destroy != _hb_subset_face_data_destroy))
@@ -218,30 +225,28 @@ _add_head_and_set_loca_version (hb_face_t *source, bool use_short_loca, hb_face_
 {
   hb_blob_t *head_blob = OT::Sanitizer<OT::head>().sanitize (hb_face_reference_table (source, HB_OT_TAG_head));
   const OT::head *head = OT::Sanitizer<OT::head>::lock_instance (head_blob);
-  bool has_head = (head != nullptr);
+  hb_bool_t has_head = (head != nullptr);
 
   if (has_head) {
-    OT::head *head_prime = (OT::head *) calloc (OT::head::static_size, 1);
+    OT::head *head_prime = (OT::head *) malloc (OT::head::static_size);
     memcpy (head_prime, head, OT::head::static_size);
     head_prime->indexToLocFormat.set (use_short_loca ? 0 : 1);
 
     hb_blob_t *head_prime_blob = hb_blob_create ((const char*) head_prime,
                                                  OT::head::static_size,
-                                                 HB_MEMORY_MODE_WRITABLE,
+                                                 HB_MEMORY_MODE_READONLY,
                                                  head_prime,
                                                  free);
-    has_head = has_head && hb_subset_face_add_table (dest, HB_OT_TAG_head, head_prime_blob);
+    has_head = hb_subset_face_add_table (dest, HB_OT_TAG_head, head_prime_blob);
-
     hb_blob_destroy (head_prime_blob);
   }
 
   hb_blob_destroy (head_blob);
-
   return has_head;
 }
 
 static bool
-_subset_glyf (hb_subset_plan_t *plan, hb_face_t *source, hb_face_t *dest)
+_subset_glyf (hb_subset_plan_t *plan)
 {
   hb_blob_t *glyf_prime = nullptr;
   hb_blob_t *loca_prime = nullptr;
@@ -249,10 +254,10 @@ _subset_glyf (hb_subset_plan_t *plan, hb_face_t *source, hb_face_t *dest)
   bool success = true;
   bool use_short_loca = false;
   // TODO(grieger): Migrate to subset function on the table like cmap.
-  if (hb_subset_glyf_and_loca (plan, source, &use_short_loca, &glyf_prime, &loca_prime)) {
+  if (hb_subset_glyf_and_loca (plan, &use_short_loca, &glyf_prime, &loca_prime)) {
-    success = success && hb_subset_face_add_table (dest, HB_OT_TAG_glyf, glyf_prime);
+    success = success && hb_subset_plan_add_table (plan, HB_OT_TAG_glyf, glyf_prime);
-    success = success && hb_subset_face_add_table (dest, HB_OT_TAG_loca, loca_prime);
+    success = success && hb_subset_plan_add_table (plan, HB_OT_TAG_loca, loca_prime);
-    success = success && _add_head_and_set_loca_version (source, use_short_loca, dest);
+    success = success && _add_head_and_set_loca_version (plan->source, use_short_loca, plan->dest);
   } else {
     success = false;
   }
@@ -264,36 +269,67 @@ _subset_glyf (hb_subset_plan_t *plan, hb_face_t *source, hb_face_t *dest)
 
 static bool
 _subset_table (hb_subset_plan_t *plan,
-               hb_face_t        *source,
+               hb_tag_t          tag)
-               hb_tag_t          tag,
-               hb_blob_t        *source_blob,
-               hb_face_t        *dest)
 {
   // TODO (grieger): Handle updating the head table (loca format + num glyphs)
   DEBUG_MSG(SUBSET, nullptr, "begin subset %c%c%c%c", HB_UNTAG(tag));
-  hb_blob_t *dest_blob;
+  bool result = true;
   switch (tag) {
     case HB_OT_TAG_glyf:
-      return _subset_glyf (plan, source, dest);
+      result = _subset_glyf (plan);
+      break;
     case HB_OT_TAG_head:
       // SKIP head, it's handled by glyf
+      result = true;
+      break;
+    case HB_OT_TAG_hhea:
+      // SKIP hhea, it's handled by hmtx
       return true;
+    case HB_OT_TAG_hmtx:
+      result = _subset<const OT::hmtx> (plan);
+      break;
+    case HB_OT_TAG_maxp:
+      result = _subset<const OT::maxp> (plan);
+      break;
     case HB_OT_TAG_loca:
       // SKIP loca, it's handle by glyf
       return true;
     case HB_OT_TAG_cmap:
-      dest_blob = _subset<const OT::cmap> (plan, source);
+      result = _subset<const OT::cmap> (plan);
+      break;
+    case HB_OT_TAG_os2:
+      result = _subset<const OT::os2> (plan);
       break;
     default:
-      dest_blob = source_blob;
+      hb_blob_t *source_table = hb_face_reference_table(plan->source, tag);
+      if (likely(source_table))
+      {
+        result = hb_subset_plan_add_table(plan, tag, source_table);
+      }
+      else
+      {
+        result = false;
+      }
       break;
-  } 
+  }
-  DEBUG_MSG(SUBSET, nullptr, "subset %c%c%c%c %s", HB_UNTAG(tag), dest_blob ? "ok" : "FAILED");
+  DEBUG_MSG(SUBSET, nullptr, "subset %c%c%c%c %s", HB_UNTAG(tag), result ? "ok" : "FAILED");
-  if (unlikely(!dest_blob)) return false;
-  if (unlikely(!hb_subset_face_add_table (dest, tag, dest_blob))) return false;
   return true;
 }
 
+static bool
+_should_drop_table(hb_tag_t tag)
+{
+    switch (tag) {
+      case HB_TAG ('G', 'D', 'E', 'F'): /* temporary */
+      case HB_TAG ('G', 'P', 'O', 'S'): /* temporary */
+      case HB_TAG ('G', 'S', 'U', 'B'): /* temporary */
+      case HB_TAG ('D', 'S', 'I', 'G'):
+        return true;
+      default:
+        return false;
+  }
+}
+
 /**
  * hb_subset:
  * @source: font face data to be subset.
@@ -304,14 +340,13 @@ _subset_table (hb_subset_plan_t *plan,
  **/
 hb_face_t *
 hb_subset (hb_face_t *source,
-	   hb_subset_profile_t *profile,
+           hb_subset_profile_t *profile,
            hb_subset_input_t *input)
 {
   if (unlikely (!profile || !input || !source)) return hb_face_get_empty();
 
   hb_subset_plan_t *plan = hb_subset_plan_create (source, profile, input);
 
-  hb_face_t *dest = hb_subset_face_create ();
   hb_tag_t table_tags[32];
   unsigned int offset = 0, count;
   bool success = true;
@@ -321,13 +356,16 @@ hb_subset (hb_face_t *source,
     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);
+      if (_should_drop_table(tag))
-      success = success && _subset_table (plan, source, tag, blob, dest);
+      {
-      hb_blob_destroy (blob);
+        DEBUG_MSG(SUBSET, nullptr, "drop %c%c%c%c", HB_UNTAG(tag));
+        continue;
+      }
+      success = success && _subset_table (plan, tag);
     }
   } while (count == ARRAY_LENGTH (table_tags));
 
-  // TODO(grieger): Remove once basic subsetting is working + tests updated.
+  hb_face_t *result = success ? hb_face_reference(plan->dest) : hb_face_get_empty();
-  hb_face_destroy (dest);
+  hb_subset_plan_destroy (plan);
-  return success ? hb_face_reference (source) : hb_face_get_empty ();
+  return result;
 }

test/api/Makefile.am

@@ -16,7 +16,7 @@ if HAVE_GLIB
 AM_CPPFLAGS = -DSRCDIR="\"$(srcdir)\"" -I$(top_srcdir)/src/ -I$(top_builddir)/src/ $(GLIB_CFLAGS)
 LDADD = $(top_builddir)/src/libharfbuzz.la $(GLIB_LIBS)
 
-EXTRA_DIST += hb-test.h
+EXTRA_DIST += hb-test.h hb-subset-test.h
 
 check_PROGRAMS = $(TEST_PROGS)
 noinst_PROGRAMS = $(TEST_PROGS)
@@ -29,14 +29,19 @@ TEST_PROGS = \
 	test-object \
 	test-set \
 	test-shape \
-	test-subset \
+	test-subset-cmap \
 	test-subset-glyf \
+	test-subset-hmtx \
+	test-subset-os2 \
 	test-unicode \
 	test-version \
 	$(NULL)
 
 test_subset_LDADD = $(LDADD) $(top_builddir)/src/libharfbuzz-subset.la
+test_subset_cmap_LDADD = $(LDADD) $(top_builddir)/src/libharfbuzz-subset.la
 test_subset_glyf_LDADD = $(LDADD) $(top_builddir)/src/libharfbuzz-subset.la
+test_subset_hmtx_LDADD = $(LDADD) $(top_builddir)/src/libharfbuzz-subset.la
+test_subset_os2_LDADD = $(LDADD) $(top_builddir)/src/libharfbuzz-subset.la
 
 test_unicode_CPPFLAGS = \
 	$(AM_CPPFLAGS) \
@@ -62,6 +67,14 @@ TEST_PROGS += \
 test_ot_math_LDADD = $(LDADD) $(FREETYPE_LIBS)
 test_ot_math_CPPFLAGS = $(AM_CPPFLAGS) $(FREETYPE_CFLAGS)
 EXTRA_DIST += \
+	fonts/Inconsolata-Regular.ab.ttf \
+	fonts/Inconsolata-Regular.abc.ttf \
+	fonts/Inconsolata-Regular.abc.widerc.ttf \
+	fonts/Inconsolata-Regular.ac.ttf \
+	fonts/Inconsolata-Regular.ac.widerc.ttf \
+	fonts/Roboto-Regular.abc.cmap-format12-only.ttf \
+	fonts/Roboto-Regular.ac.cmap-format12-only.ttf \
+	fonts/Roboto-Regular.b.ttf \
 	fonts/Roboto-Regular.abc.ttf \
 	fonts/Roboto-Regular.ac.ttf \
 	fonts/MathTestFontEmpty.otf \

test/api/fonts/README

@@ -0,0 +1,3 @@
+cmap-format12-only files created by ttx & remove all other cmap entries
+
+Inconsolata-Regular.abc.widerc.ttf has the hmtx width of "c" set to 600; everything else is 500. Subsetting out c should reduce numberOfHMetrics to 1.

test/api/hb-subset-test.h

@@ -0,0 +1,125 @@
+/*
+ * Copyright © 2018  Google, Inc.
+ *
+ *  This is part of HarfBuzz, a text shaping library.
+ *
+ * Permission is hereby granted, without written agreement and without
+ * license or royalty fees, to use, copy, modify, and distribute this
+ * software and its documentation for any purpose, provided that the
+ * above copyright notice and the following two paragraphs appear in
+ * all copies of this software.
+ *
+ * IN NO EVENT SHALL THE COPYRIGHT HOLDER BE LIABLE TO ANY PARTY FOR
+ * DIRECT, INDIRECT, SPECIAL, INCIDENTAL, OR CONSEQUENTIAL DAMAGES
+ * ARISING OUT OF THE USE OF THIS SOFTWARE AND ITS DOCUMENTATION, EVEN
+ * IF THE COPYRIGHT HOLDER HAS BEEN ADVISED OF THE POSSIBILITY OF SUCH
+ * DAMAGE.
+ *
+ * THE COPYRIGHT HOLDER SPECIFICALLY DISCLAIMS ANY WARRANTIES, INCLUDING,
+ * BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
+ * FITNESS FOR A PARTICULAR PURPOSE.  THE SOFTWARE PROVIDED HEREUNDER IS
+ * 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
+ */
+
+#ifndef HB_SUBSET_TEST_H
+#define HB_SUBSET_TEST_H
+
+#include <stdio.h>
+
+#include "hb-test.h"
+
+
+HB_BEGIN_DECLS
+
+static inline char *
+hb_subset_test_read_file (const char *path,
+                          size_t     *length /* OUT */)
+{
+  FILE *fp = fopen (path, "rb");
+
+  size_t file_length = 0;
+  char *buffer = NULL;
+  if (fp && fseek (fp, 0, SEEK_END) == 0) {
+    file_length = ftell(fp);
+    rewind (fp);
+  }
+
+  if (file_length > 0) {
+    buffer = (char *) calloc (file_length + 1, sizeof(char));
+    if (fread (buffer, 1, file_length, fp) == file_length) {
+      *length = file_length;
+    } else {
+      free (buffer);
+      buffer = NULL;
+    }
+  }
+
+  if (fp)
+    fclose(fp);
+
+  return buffer;
+}
+
+static inline hb_face_t *
+hb_subset_test_open_font (const char *font_path)
+{
+#if GLIB_CHECK_VERSION(2,37,2)
+  gchar* path = g_test_build_filename(G_TEST_DIST, font_path, NULL);
+#else
+  gchar* path = g_strdup(fontFile);
+#endif
+
+  size_t length;
+  char *font_data = hb_subset_test_read_file(path, &length);
+
+  if (font_data != NULL) {
+    hb_blob_t *blob = hb_blob_create (font_data,
+                                      length,
+                                      HB_MEMORY_MODE_READONLY,
+                                      font_data,
+                                      free);
+    hb_face_t *face = hb_face_create (blob, 0);
+    hb_blob_destroy (blob);
+    return face;
+  }
+  g_assert (false);
+}
+
+static inline hb_face_t *
+hb_subset_test_create_subset (hb_face_t *source,
+                              const hb_set_t  *codepoints)
+{
+  hb_subset_profile_t *profile = hb_subset_profile_create();
+  hb_subset_input_t *input = hb_subset_input_create_or_fail ();
+
+  hb_set_t * input_codepoints = hb_subset_input_unicode_set (input);
+
+  hb_set_union (input_codepoints, codepoints);
+
+  hb_face_t *subset = hb_subset (source, profile, input);
+  g_assert (subset);
+
+  hb_subset_profile_destroy (profile);
+  hb_subset_input_destroy (input);
+  return subset;
+}
+
+static inline void
+hb_subset_test_check (hb_face_t *expected,
+                      hb_face_t *actual,
+                      hb_tag_t   table)
+{
+  hb_blob_t *expected_blob = hb_face_reference_table (expected, table);
+  hb_blob_t *actual_blob = hb_face_reference_table (actual, table);
+  hb_test_assert_blob_eq(expected_blob, actual_blob);  
+  hb_blob_destroy (expected_blob);
+  hb_blob_destroy (actual_blob);
+}
+
+
+HB_END_DECLS
+
+#endif /* HB_SUBSET_TEST_H */

test/api/hb-test.h

@@ -161,9 +161,14 @@ typedef void (*hb_test_fixture_func_t) (void);
 #define g_test_fail() g_error("Test failed")
 #endif
 
-#ifndef g_assert_cmpmem
+static inline void hb_test_assert_blob_eq(hb_blob_t *expected_blob, hb_blob_t *actual_blob)
-#define g_assert_cmpmem(m1, l1, m2, l2) g_assert_true (l1 == l2 && memcmp (m1, m2, l1) == 0)
+{
-#endif
+  unsigned int expected_length, actual_length;
+  const char *raw_expected = hb_blob_get_data (expected_blob, &expected_length);
+  const char *raw_actual = hb_blob_get_data (actual_blob, &actual_length);
+  g_assert_cmpint(expected_length, ==, actual_length);
+  g_assert_cmpint(0, ==, memcmp(raw_expected, raw_actual, expected_length));
+}
 
 
 static inline void

test/api/test-subset-cmap.c

@@ -0,0 +1,82 @@
+/*
+ * Copyright © 2018  Google, Inc.
+ *
+ *  This is part of HarfBuzz, a text shaping library.
+ *
+ * Permission is hereby granted, without written agreement and without
+ * license or royalty fees, to use, copy, modify, and distribute this
+ * software and its documentation for any purpose, provided that the
+ * above copyright notice and the following two paragraphs appear in
+ * all copies of this software.
+ *
+ * IN NO EVENT SHALL THE COPYRIGHT HOLDER BE LIABLE TO ANY PARTY FOR
+ * DIRECT, INDIRECT, SPECIAL, INCIDENTAL, OR CONSEQUENTIAL DAMAGES
+ * ARISING OUT OF THE USE OF THIS SOFTWARE AND ITS DOCUMENTATION, EVEN
+ * IF THE COPYRIGHT HOLDER HAS BEEN ADVISED OF THE POSSIBILITY OF SUCH
+ * DAMAGE.
+ *
+ * THE COPYRIGHT HOLDER SPECIFICALLY DISCLAIMS ANY WARRANTIES, INCLUDING,
+ * BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
+ * FITNESS FOR A PARTICULAR PURPOSE.  THE SOFTWARE PROVIDED HEREUNDER IS
+ * ON AN "AS IS" BASIS, AND THE COPYRIGHT HOLDER HAS NO OBLIGATION TO
+ * PROVIDE MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
+ *
+ * Google Author(s): Roderick Sheeter
+ */
+
+#include <stdbool.h>
+
+#include "hb-test.h"
+#include "hb-subset-test.h"
+
+/* Unit tests for cmap subsetting */
+
+static void
+test_subset_cmap (void)
+{
+  hb_face_t *face_abc = hb_subset_test_open_font ("fonts/Roboto-Regular.abc.ttf");
+  hb_face_t *face_ac = hb_subset_test_open_font ("fonts/Roboto-Regular.ac.cmap-format12-only.ttf");
+
+  hb_set_t *codepoints = hb_set_create ();
+  hb_set_add (codepoints, 97);
+  hb_set_add (codepoints, 99);
+  hb_face_t *face_abc_subset = hb_subset_test_create_subset (face_abc, codepoints);
+  hb_set_destroy (codepoints);
+
+  hb_subset_test_check (face_ac, face_abc_subset, HB_TAG ('c','m','a','p'));
+
+  hb_face_destroy (face_abc_subset);
+  hb_face_destroy (face_abc);
+  hb_face_destroy (face_ac);
+}
+
+static void
+test_subset_cmap_noop (void)
+{
+  hb_face_t *face_abc = hb_subset_test_open_font("fonts/Roboto-Regular.abc.cmap-format12-only.ttf");
+
+  hb_set_t *codepoints = hb_set_create();
+  hb_set_add (codepoints, 97);
+  hb_set_add (codepoints, 98);
+  hb_set_add (codepoints, 99);
+  hb_face_t *face_abc_subset = hb_subset_test_create_subset (face_abc, codepoints);
+  hb_set_destroy (codepoints);
+
+  hb_subset_test_check (face_abc, face_abc_subset, HB_TAG ('c','m','a','p'));
+
+  hb_face_destroy (face_abc_subset);
+  hb_face_destroy (face_abc);
+}
+
+// TODO(rsheeter) test cmap to no codepoints
+
+int
+main (int argc, char **argv)
+{
+  hb_test_init (&argc, &argv);
+
+  hb_test_add (test_subset_cmap);
+  hb_test_add (test_subset_cmap_noop);
+
+  return hb_test_run();
+}

test/api/test-subset-glyf.c

@@ -27,115 +27,59 @@
 #include <stdbool.h>
 
 #include "hb-test.h"
-
+#include "hb-subset-test.h"
-static char *
-read_file (const char *path,
-           size_t     *length)
-{
-  FILE *fp = fopen (path, "rb");
-
-  size_t file_length = 0;
-  char *buffer = NULL;
-  if (fp && fseek (fp, 0, SEEK_END) == 0) {
-    file_length = ftell(fp);
-    rewind (fp);
-  }
-
-  if (file_length > 0) {
-    buffer = (char *) calloc (file_length + 1, sizeof(char));
-    if (fread (buffer, 1, file_length, fp) == file_length) {
-      *length = file_length;
-    } else {
-      free (buffer);
-      buffer = NULL;
-    }
-  }
-
-  if (fp)
-    fclose(fp);
-
-  return buffer;
-}
-
-static hb_face_t *
-open_font (const char *font_path)
-{
-#if GLIB_CHECK_VERSION(2,37,2)
-  gchar* path = g_test_build_filename(G_TEST_DIST, font_path, NULL);
-#else
-  gchar* path = g_strdup(fontFile);
-#endif
-
-  size_t length;
-  char *font_data = read_file(path, &length);
-
-  if (font_data != NULL) {
-    hb_blob_t *blob = hb_blob_create (font_data,
-                                      length,
-                                      HB_MEMORY_MODE_READONLY,
-                                      font_data,
-                                      free);
-    hb_face_t *face = hb_face_create (blob, 0);
-    hb_blob_destroy (blob);
-    return face;
-  }
-
-  return NULL;
-}
-
 
 /* Unit tests for hb-subset-glyf.h */
 
 static void
 test_subset_glyf (void)
 {
-  hb_face_t *face_abc = open_font("fonts/Roboto-Regular.abc.ttf");
+  hb_face_t *face_abc = hb_subset_test_open_font ("fonts/Roboto-Regular.abc.ttf");
-  hb_face_t *face_ac = open_font("fonts/Roboto-Regular.ac.ttf");
+  hb_face_t *face_ac = hb_subset_test_open_font ("fonts/Roboto-Regular.ac.ttf");
-  hb_face_t *face_abc_subset;
-  hb_blob_t *glyf_expected_blob;
-  hb_blob_t *glyf_actual_blob;
-  hb_subset_profile_t *profile = hb_subset_profile_create();
-  hb_subset_input_t *input = hb_subset_input_create_or_fail ();
-  hb_set_t *codepoints = hb_set_reference (hb_subset_input_unicode_set (input));
-
-  g_assert (input);
-  g_assert (face_abc);
-  g_assert (face_ac);
 
+  hb_set_t *codepoints = hb_set_create();
   hb_set_add (codepoints, 97);
   hb_set_add (codepoints, 99);
+  hb_face_t *face_abc_subset = hb_subset_test_create_subset (face_abc, codepoints);
+  hb_set_destroy (codepoints);
 
-#if 0 /* Enable when actually works. */
+  hb_subset_test_check (face_ac, face_abc_subset, HB_TAG ('g','l','y','f'));
-  face_abc_subset = hb_subset (face_abc, profile, input);
+  hb_subset_test_check (face_ac, face_abc_subset, HB_TAG ('l','o','c', 'a'));
-  g_assert (face_abc_subset);
+  hb_subset_test_check (face_ac, face_abc_subset, HB_TAG ('m','a','x', 'p'));
-
-  glyf_expected_blob = hb_face_reference_table (face_ac, HB_TAG('g','l','y','f'));
-  glyf_actual_blob = hb_face_reference_table (face_abc_subset, HB_TAG('g','l','y','f'));
-
-  // TODO(grieger): Uncomment below once this actually works.
-  //int expected_length, actual_length;
-  // g_assert_cmpmem(hb_blob_get_data (glyf_expected_blob, &expected_length), expected_length,
-  //   hb_blob_get_data (glyf_actual_blob, &actual_length), actual_length);
-
-  hb_blob_destroy (glyf_actual_blob);
-  hb_blob_destroy (glyf_expected_blob);
 
   hb_face_destroy (face_abc_subset);
-#endif
-
-  hb_subset_input_destroy (input);
-  hb_subset_profile_destroy (profile);
-  hb_set_destroy (codepoints);
   hb_face_destroy (face_abc);
   hb_face_destroy (face_ac);
 }
 
+static void
+test_subset_glyf_noop (void)
+{
+  hb_face_t *face_abc = hb_subset_test_open_font("fonts/Roboto-Regular.abc.ttf");
+
+  hb_set_t *codepoints = hb_set_create();
+  hb_set_add (codepoints, 97);
+  hb_set_add (codepoints, 98);
+  hb_set_add (codepoints, 99);
+  hb_face_t *face_abc_subset = hb_subset_test_create_subset (face_abc, codepoints);
+  hb_set_destroy (codepoints);
+
+  hb_subset_test_check (face_abc, face_abc_subset, HB_TAG ('g','l','y','f'));
+  hb_subset_test_check (face_abc, face_abc_subset, HB_TAG ('l','o','c', 'a'));
+
+  hb_face_destroy (face_abc_subset);
+  hb_face_destroy (face_abc);
+}
+
+// TODO(grieger): test for long loca generation.
+
 int
 main (int argc, char **argv)
 {
   hb_test_init (&argc, &argv);
 
   hb_test_add (test_subset_glyf);
+  hb_test_add (test_subset_glyf_noop);
 
   return hb_test_run();
 }

test/api/test-subset-hmtx.c

@@ -0,0 +1,163 @@
+/*
+ * Copyright © 2018  Google, Inc.
+ *
+ *  This is part of HarfBuzz, a text shaping library.
+ *
+ * Permission is hereby granted, without written agreement and without
+ * license or royalty fees, to use, copy, modify, and distribute this
+ * software and its documentation for any purpose, provided that the
+ * above copyright notice and the following two paragraphs appear in
+ * all copies of this software.
+ *
+ * IN NO EVENT SHALL THE COPYRIGHT HOLDER BE LIABLE TO ANY PARTY FOR
+ * DIRECT, INDIRECT, SPECIAL, INCIDENTAL, OR CONSEQUENTIAL DAMAGES
+ * ARISING OUT OF THE USE OF THIS SOFTWARE AND ITS DOCUMENTATION, EVEN
+ * IF THE COPYRIGHT HOLDER HAS BEEN ADVISED OF THE POSSIBILITY OF SUCH
+ * DAMAGE.
+ *
+ * THE COPYRIGHT HOLDER SPECIFICALLY DISCLAIMS ANY WARRANTIES, INCLUDING,
+ * BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
+ * FITNESS FOR A PARTICULAR PURPOSE.  THE SOFTWARE PROVIDED HEREUNDER IS
+ * ON AN "AS IS" BASIS, AND THE COPYRIGHT HOLDER HAS NO OBLIGATION TO
+ * PROVIDE MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
+ *
+ * Google Author(s): Roderick Sheeter
+ */
+
+#include <stdbool.h>
+
+#include "hb-test.h"
+#include "hb-subset-test.h"
+
+/* Unit tests for hmtx subsetting */
+
+static void check_num_hmetrics(hb_face_t *face, uint16_t expected_num_hmetrics)
+{
+  hb_blob_t *hhea_blob = hb_face_reference_table (face, HB_TAG ('h','h','e','a'));
+  hb_blob_t *hmtx_blob = hb_face_reference_table (face, HB_TAG ('h','m','t','x'));
+
+  // TODO I sure wish I could just use the hmtx table struct!
+  unsigned int hmtx_len = hb_blob_get_length(hmtx_blob);
+  unsigned int hhea_len;
+  uint8_t *raw_hhea = (uint8_t *) hb_blob_get_data(hhea_blob, &hhea_len);
+  uint16_t num_hmetrics = (raw_hhea[hhea_len - 2] << 8) + raw_hhea[hhea_len - 1];
+  g_assert_cmpuint(expected_num_hmetrics, ==, num_hmetrics);
+
+  hb_blob_destroy (hhea_blob);
+  hb_blob_destroy (hmtx_blob);
+}
+
+static void
+test_subset_hmtx_simple_subset (void)
+{
+  hb_face_t *face_abc = hb_subset_test_open_font ("fonts/Roboto-Regular.abc.ttf");
+  hb_face_t *face_ac = hb_subset_test_open_font ("fonts/Roboto-Regular.ac.ttf");
+
+  hb_set_t *codepoints = hb_set_create ();
+  hb_set_add (codepoints, 'a');
+  hb_set_add (codepoints, 'c');
+  hb_face_t *face_abc_subset = hb_subset_test_create_subset (face_abc, codepoints);
+  hb_set_destroy (codepoints);
+
+  check_num_hmetrics(face_abc_subset, 3); /* nothing has same width */
+  hb_subset_test_check (face_ac, face_abc_subset, HB_TAG ('h','m','t','x'));
+
+  hb_face_destroy (face_abc_subset);
+  hb_face_destroy (face_abc);
+  hb_face_destroy (face_ac);
+}
+
+
+static void
+test_subset_hmtx_monospace (void)
+{
+  hb_face_t *face_abc = hb_subset_test_open_font ("fonts/Inconsolata-Regular.abc.ttf");
+  hb_face_t *face_ac = hb_subset_test_open_font ("fonts/Inconsolata-Regular.ac.ttf");
+
+  hb_set_t *codepoints = hb_set_create ();
+  hb_set_add (codepoints, 'a');
+  hb_set_add (codepoints, 'c');
+  hb_face_t *face_abc_subset = hb_subset_test_create_subset (face_abc, codepoints);
+  hb_set_destroy (codepoints);
+
+  check_num_hmetrics(face_abc_subset, 1); /* everything has same width */
+  hb_subset_test_check (face_ac, face_abc_subset, HB_TAG ('h','m','t','x'));
+
+  hb_face_destroy (face_abc_subset);
+  hb_face_destroy (face_abc);
+  hb_face_destroy (face_ac);
+}
+
+
+static void
+test_subset_hmtx_keep_num_metrics (void)
+{
+  hb_face_t *face_abc = hb_subset_test_open_font ("fonts/Inconsolata-Regular.abc.widerc.ttf");
+  hb_face_t *face_ac = hb_subset_test_open_font ("fonts/Inconsolata-Regular.ac.widerc.ttf");
+
+  hb_set_t *codepoints = hb_set_create ();
+  hb_set_add (codepoints, 'a');
+  hb_set_add (codepoints, 'c');
+  hb_face_t *face_abc_subset = hb_subset_test_create_subset (face_abc, codepoints);
+  hb_set_destroy (codepoints);
+
+  check_num_hmetrics(face_abc_subset, 3); /* c is wider */
+  hb_subset_test_check (face_ac, face_abc_subset, HB_TAG ('h','m','t','x'));
+
+  hb_face_destroy (face_abc_subset);
+  hb_face_destroy (face_abc);
+  hb_face_destroy (face_ac);
+}
+
+static void
+test_subset_hmtx_decrease_num_metrics (void)
+{
+  hb_face_t *face_abc = hb_subset_test_open_font ("fonts/Inconsolata-Regular.abc.widerc.ttf");
+  hb_face_t *face_ab = hb_subset_test_open_font ("fonts/Inconsolata-Regular.ab.ttf");
+
+  hb_set_t *codepoints = hb_set_create ();
+  hb_set_add (codepoints, 'a');
+  hb_set_add (codepoints, 'b');
+  hb_face_t *face_abc_subset = hb_subset_test_create_subset (face_abc, codepoints);
+  hb_set_destroy (codepoints);
+
+  check_num_hmetrics(face_abc_subset, 1); /* everything left has same width */
+  hb_subset_test_check (face_ab, face_abc_subset, HB_TAG ('h','m','t','x'));
+
+  hb_face_destroy (face_abc_subset);
+  hb_face_destroy (face_abc);
+  hb_face_destroy (face_ab);
+}
+
+static void
+test_subset_hmtx_noop (void)
+{
+  hb_face_t *face_abc = hb_subset_test_open_font("fonts/Roboto-Regular.abc.ttf");
+
+  hb_set_t *codepoints = hb_set_create();
+  hb_set_add (codepoints, 'a');
+  hb_set_add (codepoints, 'b');
+  hb_set_add (codepoints, 'c');
+  hb_face_t *face_abc_subset = hb_subset_test_create_subset (face_abc, codepoints);
+  hb_set_destroy (codepoints);
+
+  check_num_hmetrics(face_abc_subset, 4); /* nothing has same width */
+  hb_subset_test_check (face_abc, face_abc_subset, HB_TAG ('h','m','t','x'));
+
+  hb_face_destroy (face_abc_subset);
+  hb_face_destroy (face_abc);
+}
+
+int
+main (int argc, char **argv)
+{
+  hb_test_init (&argc, &argv);
+
+  hb_test_add (test_subset_hmtx_simple_subset);
+  hb_test_add (test_subset_hmtx_monospace);
+  hb_test_add (test_subset_hmtx_keep_num_metrics);
+  hb_test_add (test_subset_hmtx_decrease_num_metrics);
+  hb_test_add (test_subset_hmtx_noop);
+
+  return hb_test_run();
+}

test/api/test-subset-os2.c

@@ -1,5 +1,5 @@
 /*
- * Copyright © 2011  Google, Inc.
+ * Copyright © 2018  Google, Inc.
  *
  *  This is part of HarfBuzz, a text shaping library.
  *
@@ -24,53 +24,36 @@
  * Google Author(s): Garret Rieger
  */
 
+#include <stdbool.h>
+
 #include "hb-test.h"
-
+#include "hb-subset-test.h"
-/* Unit tests for hb-subset.h */
-
-static const char test_data[] = { 0, 1, 0, 0,	/* sfntVersion */
-				  0, 0,		/* numTables */
-				  0, 0x10,	/* searchRange */
-				  0, 0,		/* entrySelector */
-				  0, 0,		/* rangeShift */
-				};
 
 static void
-test_subset (void)
+test_subset_os2 (void)
 {
-  unsigned int output_length;
+  hb_face_t *face_abc = hb_subset_test_open_font("fonts/Roboto-Regular.abc.ttf");
-  hb_blob_t *font_blob = hb_blob_create(test_data, sizeof(test_data),
+  hb_face_t *face_b = hb_subset_test_open_font("fonts/Roboto-Regular.b.ttf");
-					HB_MEMORY_MODE_READONLY, NULL, NULL);
-  hb_face_t *face = hb_face_create(font_blob, 0);
 
-  hb_subset_profile_t *profile = hb_subset_profile_create();
+  hb_set_t *codepoints = hb_set_create();
-  hb_subset_input_t *input = hb_subset_input_create_or_fail ();
+  hb_set_add (codepoints, 98);
+  hb_face_t *face_abc_subset = hb_subset_test_create_subset (face_abc, codepoints);
+  hb_set_destroy (codepoints);
 
-  hb_face_t *out_face = hb_subset(face, profile, input);
+  hb_subset_test_check (face_b, face_abc_subset, HB_TAG ('O','S','/','2'));
-  hb_blob_t *output;
 
-  g_assert (input);
+  hb_face_destroy (face_abc_subset);
-  g_assert(out_face);
+  hb_face_destroy (face_abc);
-  g_assert(out_face != hb_face_get_empty ());
+  hb_face_destroy (face_b);
-  output = hb_face_reference_blob (out_face);
-
-  const char *output_data = hb_blob_get_data(output, &output_length);
-  g_assert_cmpmem (test_data, sizeof (test_data), output_data, output_length);
-
-  hb_blob_destroy(output);
-  hb_subset_input_destroy(input);
-  hb_subset_profile_destroy(profile);
-  hb_face_destroy(out_face);
-  hb_face_destroy(face);
-  hb_blob_destroy(font_blob);
 }
 
+
 int
 main (int argc, char **argv)
 {
   hb_test_init (&argc, &argv);
 
-  hb_test_add (test_subset);
+  hb_test_add (test_subset_os2);
 
   return hb_test_run();
 }

test/subset/generate-expected-outputs.py

@@ -18,6 +18,7 @@ def usage():
 def generate_expected_output(input_file, unicodes, output_path):
 	check_call(["fonttools", "subset",
 							input_file,
+							"--drop-tables+=DSIG,GPOS,GSUB,GDEF",
 							"--unicodes=%s" % unicodes,
 							"--output-file=%s" % output_path])
 

test/subset/run-tests.py

@@ -7,6 +7,7 @@ from __future__ import print_function
 
 import io
 import os
+import re
 import subprocess
 import sys
 import tempfile
@@ -57,6 +58,9 @@ def run_test(test):
 	if return_code:
 		return fail_test(test, cli_args, "ttx (actual) returned %d" % (return_code))
 
+	expected_ttx = strip_check_sum (expected_ttx)
+	actual_ttx = strip_check_sum (expected_ttx)
+
 	if not actual_ttx == expected_ttx:
 		return fail_test(test, cli_args, 'ttx for expected and actual does not match.')
 
@@ -68,6 +72,10 @@ def run_ttx(file):
 		    file]
 	return cmd(cli_args)
 
+def strip_check_sum (ttx_string):
+	return re.sub ('checksumAdjustment value=["]0x(\d+)["]',
+		       'checksumAdjustment value="0x00000000"',
+		       ttx_string, count=1)
 
 args = sys.argv[1:]
 if not args or sys.argv[1].find('hb-subset') == -1 or not os.path.exists (sys.argv[1]):