[subset] clone trim logic from fonttools glyf handling

src/hb-ot-glyf-table.hh

@@ -267,7 +267,7 @@ struct glyf
 			       CompositeGlyphHeader::Iterator *composite /* OUT */) const
     {
       unsigned int start_offset, end_offset;
-      if (!get_offsets (glyph, &start_offset, &end_offset))
+      if (!get_offsets (glyph, /* trim */ false, &start_offset, &end_offset))
         return false; /* glyph not found */
 
       return CompositeGlyphHeader::get_iterator ((const char*) this->glyf_table + start_offset,
@@ -275,7 +275,91 @@ struct glyf
 						 composite);
     }
 
+    /* based on FontTools _g_l_y_f.py::trim */
+    inline bool trim_glyph(unsigned int start_offset,
+                           unsigned int *end_offset) const
+    {
+      static const int FLAG_X_SHORT = 0x02;
+      static const int FLAG_Y_SHORT = 0x04;
+      static const int FLAG_REPEAT = 0x08;
+      static const int FLAG_X_SAME = 0x10;
+      static const int FLAG_Y_SAME = 0x20;
+
+      if (*end_offset - start_offset < GlyphHeader::static_size)
+        return false;
+
+      const char *glyph = ((const char *) glyf_table) + start_offset;
+      const char * const glyph_end = glyph + (*end_offset - start_offset);
+      const GlyphHeader &glyph_header = StructAtOffset<GlyphHeader> (glyph, 0);
+      int16_t num_contours = (int16_t) glyph_header.numberOfContours;
+
+      if (num_contours < 0)
+        return true; /* no trimming for composites just yet */
+      else if (num_contours > 0)
+      {
+        unsigned int glyph_len = *end_offset - start_offset;
+        /* simple glyph w/contours, possibly trimmable */
+        glyph += GlyphHeader::static_size + 2 * num_contours;
+
+        if (unlikely (glyph + 2 >= glyph_end)) return false;
+        uint16_t nCoordinates = (uint16_t) StructAtOffset<HBUINT16>(glyph - 2, 0) + 1;
+        uint16_t nInstructions = (uint16_t) StructAtOffset<HBUINT16>(glyph, 0);
+
+        glyph += 2 + nInstructions;
+        if (unlikely (glyph + 2 >= glyph_end)) return false;
+
+        unsigned int coordBytes = 0;
+        unsigned int coordsWithFlags = 0;
+        while (glyph < glyph_end)
+        {
+          uint8_t flag = (uint8_t) *glyph;
+          glyph++; i++;
+
+          unsigned int repeat = 1;
+          if (flag & FLAG_REPEAT)
+          {
+            if (glyph >= glyph_end)
+            {
+              DEBUG_MSG(SUBSET, nullptr, "Bad flag");
+              return false;
+            }
+            repeat = ((uint8_t) *glyph) + 1;
+            glyph++; i++;
+          }
+
+          unsigned int xBytes, yBytes;
+          xBytes = yBytes = 0;
+          if (flag & FLAG_X_SHORT)
+            xBytes = 1;
+          else if ((flag & FLAG_X_SAME) == 0)
+            xBytes = 2;
+
+          if (flag & FLAG_Y_SHORT)
+            yBytes = 1;
+          else if ((flag & FLAG_Y_SAME) == 0)
+            yBytes = 2;
+
+          coordBytes += (xBytes + yBytes) * repeat;
+          coordsWithFlags += repeat;
+          if (coordsWithFlags >= nCoordinates)
+            break;
+        }
+
+        if (coordsWithFlags != nCoordinates)
+        {
+          DEBUG_MSG(SUBSET, nullptr, "Expect %d coords to have flags, got flags for %d", nCoordinates, coordsWithFlags);
+          return false;
+        }
+        glyph += coordBytes;
+
+        if (glyph < glyph_end)
+          *end_offset -= glyph_end - glyph;
+      }
+      return true;
+    }
+
     inline bool get_offsets (hb_codepoint_t  glyph,
+                             bool trim,
                              unsigned int   *start_offset /* OUT */,
                              unsigned int   *end_offset   /* OUT */) const
     {
@@ -291,6 +375,7 @@ struct glyf
       else
       {
         const HBUINT32 *offsets = (const HBUINT32 *) loca_table->dataX;
+
 	*start_offset = offsets[glyph];
 	*end_offset   = offsets[glyph + 1];
       }
@@ -298,6 +383,9 @@ struct glyf
       if (*start_offset > *end_offset || *end_offset > glyf_len)
 	return false;
 
+      if (trim)
+        return trim_glyph(*start_offset, end_offset);
+
       return true;
     }
 
@@ -350,7 +438,7 @@ struct glyf
 			     hb_glyph_extents_t *extents) const
     {
       unsigned int start_offset, end_offset;
-      if (!get_offsets (glyph, &start_offset, &end_offset))
+      if (!get_offsets (glyph, /* trim */ false, &start_offset, &end_offset))
         return false;
 
       if (end_offset - start_offset < GlyphHeader::static_size)

src/hb-subset-glyf.cc

@@ -43,12 +43,18 @@ _calculate_glyf_and_loca_prime_size (const OT::glyf::accelerator_t &glyf,
   for (unsigned int i = 0; i < glyph_ids.len; i++)
   {
     hb_codepoint_t next_glyph = glyph_ids[i];
+    *(instruction_ranges->push()) = 0;
+    *(instruction_ranges->push()) = 0;
+
     unsigned int start_offset, end_offset;
-    if (unlikely (!glyf.get_offsets(next_glyph, &start_offset, &end_offset)))
+    if (unlikely (!glyf.get_offsets(next_glyph, /* trim */ true, &start_offset, &end_offset)))
     {
       DEBUG_MSG(SUBSET, nullptr, "Invalid gid %d", next_glyph);
       continue;
     }
+    if (end_offset - start_offset < OT::glyf::GlyphHeader::static_size)
+      continue; /* 0-length glyph */
+
     unsigned int instruction_start = 0, instruction_end = 0;
     if (drop_hints)
     {
@@ -58,12 +64,13 @@ _calculate_glyf_and_loca_prime_size (const OT::glyf::accelerator_t &glyf,
         DEBUG_MSG(SUBSET, nullptr, "Unable to get instruction offsets for %d", next_glyph);
         return false;
       }
+      instruction_ranges->array[i * 2] = instruction_start;
+      instruction_ranges->array[i * 2 + 1] = instruction_end;
     }
-    *(instruction_ranges->push()) = instruction_start;
-    *(instruction_ranges->push()) = instruction_end;
 
     total += end_offset - start_offset - (instruction_end - instruction_start);
-    fprintf(stderr, "  %d ends at %d (was %d to %d, remove %d)\n", next_glyph, total, start_offset, end_offset, instruction_end - instruction_start);
+    /* round2 so short loca will work */
+    total += total % 2;
   }
 
   *glyf_size = total;
@@ -147,12 +154,13 @@ _write_glyf_and_loca_prime (hb_subset_plan_t              *plan,
   for (unsigned int i = 0; i < glyph_ids.len; i++)
   {
     unsigned int start_offset, end_offset;
-    if (unlikely (!glyf.get_offsets (glyph_ids[i], &start_offset, &end_offset)))
+    if (unlikely (!glyf.get_offsets (glyph_ids[i], /* trim */ true, &start_offset, &end_offset)))
       end_offset = start_offset = 0;
     unsigned int instruction_start = instruction_ranges[i * 2];
     unsigned int instruction_end = instruction_ranges[i * 2 + 1];
 
     int length = end_offset - start_offset - (instruction_end - instruction_start);
+    length += length % 2;
 
     if (glyf_prime_data_next + length > glyf_prime_data + glyf_prime_size)
     {
@@ -164,21 +172,17 @@ _write_glyf_and_loca_prime (hb_subset_plan_t              *plan,
     }
 
     if (instruction_start == instruction_end)
-    {
-      fprintf(stderr, "i=%d copy %d bytes from %d to %ld\n", i, length, start_offset, glyf_prime_data_next - glyf_prime_data);
       memcpy (glyf_prime_data_next, glyf_data + start_offset, length);
-    }
     else
     {
-      fprintf(stderr, "i=%d copy %d bytes from %d to %ld\n", i, instruction_start - start_offset, start_offset, glyf_prime_data_next - glyf_prime_data);
-      fprintf(stderr, "i=%d copy %d bytes from %d to %ld\n", i, end_offset - instruction_end, instruction_end, glyf_prime_data_next + instruction_start - start_offset - glyf_prime_data);
       memcpy (glyf_prime_data_next, glyf_data + start_offset, instruction_start - start_offset);
       memcpy (glyf_prime_data_next + instruction_start - start_offset, glyf_data + instruction_end, end_offset - instruction_end);
       /* if the instructions end at the end this was a composite glyph */
       if (instruction_end == end_offset)
         ; // TODO(rsheeter) remove WE_HAVE_INSTRUCTIONS from last flags
       else
-        ; // TODO(rsheeter) zero instructionLength
+        /* zero instruction length, which is just before instruction_start */
+        memset (glyf_prime_data_next + instruction_start - start_offset - 2, 0, 2);
     }
 
     success = success && _write_loca_entry (i,

src/hb-subset-plan.cc

@@ -79,6 +79,9 @@ hb_subset_plan_add_table (hb_subset_plan_t *plan,
                           hb_tag_t tag,
                           hb_blob_t *contents)
 {
+  hb_blob_t *source_blob = plan->source->reference_table (tag);
+  DEBUG_MSG(SUBSET, nullptr, "add table %c%c%c%c, dest %d bytes, source %d bytes", HB_UNTAG(tag), hb_blob_get_length (contents), hb_blob_get_length (source_blob));
+  hb_blob_destroy (source_blob);
   return hb_subset_face_add_table(plan->dest, tag, contents);
 }
 

src/hb-subset.cc

@@ -230,11 +230,12 @@ _subset_table (hb_subset_plan_t *plan,
       result = _subset<const OT::glyf> (plan);
       break;
     case HB_OT_TAG_head:
-      // SKIP head, it's handled by glyf
+      // TODO that won't work well if there is no glyf
+      DEBUG_MSG(SUBSET, nullptr, "skip head, handled by glyf");
       result = true;
       break;
     case HB_OT_TAG_hhea:
-      // SKIP hhea, it's handled by hmtx
+      DEBUG_MSG(SUBSET, nullptr, "skip hhea handled by hmtx");
       return true;
     case HB_OT_TAG_hmtx:
       result = _subset<const OT::hmtx> (plan);
@@ -243,7 +244,7 @@ _subset_table (hb_subset_plan_t *plan,
       result = _subset<const OT::maxp> (plan);
       break;
     case HB_OT_TAG_loca:
-      // SKIP loca, it's handle by glyf
+      DEBUG_MSG(SUBSET, nullptr, "skip loca handled by glyf");
       return true;
     case HB_OT_TAG_cmap:
       result = _subset<const OT::cmap> (plan);
@@ -254,16 +255,12 @@ _subset_table (hb_subset_plan_t *plan,
     default:
       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;
-      }
+      DEBUG_MSG(SUBSET, nullptr, "subset %c%c%c%c %s", HB_UNTAG(tag), result ? "ok" : "FAILED");
       break;
   }
-  DEBUG_MSG(SUBSET, nullptr, "subset %c%c%c%c %s", HB_UNTAG(tag), result ? "ok" : "FAILED");
   return result;
 }
 

test/api/hb-subset-test.h

@@ -122,6 +122,7 @@ hb_subset_test_check (hb_face_t *expected,
                       hb_face_t *actual,
                       hb_tag_t   table)
 {
+  fprintf(stderr, "compare %c%c%c%c\n", HB_UNTAG(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_blobs_equal (expected_blob, actual_blob);

test/api/test-subset-glyf.c

@@ -105,8 +105,8 @@ test_subset_glyf_strip_hints (void)
   hb_face_t *face_abc_subset = hb_subset_test_create_subset (face_abc, input);
   hb_set_destroy (codepoints);
 
-  hb_subset_test_check (face_ac, face_abc_subset, HB_TAG ('g','l','y','f'));
   hb_subset_test_check (face_ac, face_abc_subset, HB_TAG ('l','o','c', 'a'));
+  hb_subset_test_check (face_ac, face_abc_subset, HB_TAG ('g','l','y','f'));
   hb_subset_test_check (face_ac, face_abc_subset, HB_TAG ('m','a','x', 'p'));
 
   hb_face_destroy (face_abc_subset);
@@ -123,10 +123,10 @@ main (int argc, char **argv)
 {
   hb_test_init (&argc, &argv);
 
-  hb_test_add (test_subset_glyf);
-  hb_test_add (test_subset_glyf_with_components);
   hb_test_add (test_subset_glyf_noop);
+  hb_test_add (test_subset_glyf);
   hb_test_add (test_subset_glyf_strip_hints);
+  hb_test_add (test_subset_glyf_with_components);
 
   return hb_test_run();
 }