Merge pull request #1722 from googlefonts/glyf

[subset] Use iterators in glyf/loca subsetting

src/Makefile.sources

@@ -241,9 +241,6 @@ HB_SUBSET_sources = \
 	hb-subset-cff1.hh \
 	hb-subset-cff2.cc \
 	hb-subset-cff2.hh \
-	hb-subset-glyf.cc \
-	hb-subset-glyf.hh \
-	hb-subset-glyf.hh \
 	hb-subset-input.cc \
 	hb-subset-input.hh \
 	hb-subset-plan.cc \

src/hb-ot-glyf-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, Garret Rieger, Roderick Sheeter
  */
 
 #ifndef HB_OT_GLYF_TABLE_HH
@@ -29,7 +29,6 @@
 
 #include "hb-open-type.hh"
 #include "hb-ot-head-table.hh"
-#include "hb-subset-glyf.hh"
 
 namespace OT {
 
@@ -81,26 +80,169 @@ struct glyf
     return_trace (true);
   }
 
-  bool subset (hb_subset_plan_t *plan) const
+  template<typename Iterator,
+	   hb_requires (hb_is_source_of (Iterator, unsigned int))>
+  static bool
+  _add_loca_and_head (hb_subset_plan_t * plan, Iterator padded_offsets)
+  {
+    unsigned int max_offset = + padded_offsets | hb_reduce (hb_max, 0);
+    unsigned num_offsets = padded_offsets.len () + 1;
+    bool use_short_loca = max_offset < 0x1FFFF;
+    unsigned entry_size = use_short_loca ? 2 : 4;
+    char *loca_prime_data = (char *) calloc (entry_size, num_offsets);
+
+    if (unlikely (!loca_prime_data)) return false;
+
+    if (use_short_loca)
+      _write_loca (padded_offsets, 1, hb_array ((HBUINT16*) loca_prime_data, num_offsets));
+    else
+      _write_loca (padded_offsets, 0, hb_array ((HBUINT32*) loca_prime_data, num_offsets));
+
+    hb_blob_t * loca_blob = hb_blob_create (loca_prime_data,
+					    entry_size * num_offsets,
+					    HB_MEMORY_MODE_WRITABLE,
+					    loca_prime_data,
+					    free);
+
+    bool result = plan->add_table (HB_OT_TAG_loca, loca_blob)
+		  && _add_head_and_set_loca_version(plan, use_short_loca);
+
+    hb_blob_destroy (loca_blob);
+    return result;
+  }
+
+  template<typename IteratorIn, typename IteratorOut,
+	   hb_requires (hb_is_source_of (IteratorIn, unsigned int)),
+	   hb_requires (hb_is_sink_of (IteratorOut, unsigned))>
+  static void
+  _write_loca (IteratorIn it, unsigned right_shift, IteratorOut dest)
+  {
+    unsigned int offset = 0;
+    dest << 0;
+    + it
+    | hb_map ([=, &offset] (unsigned int padded_size) {
+      offset += padded_size;
+      DEBUG_MSG(SUBSET, nullptr, "loca entry offset %d", offset);
+      return offset >> right_shift;
+    })
+    | hb_sink (dest)
+    ;
+  }
+
+  // requires source of SubsetGlyph complains the identifier isn't declared
+  template <typename Iterator>
+  bool serialize(hb_serialize_context_t *c,
+		 Iterator it,
+		 const hb_subset_plan_t *plan)
+  {
+    TRACE_SERIALIZE (this);
+
+    + it
+    | hb_apply ([=] (const SubsetGlyph& _) { _.serialize (c, plan); })
+    ;
+
+    return_trace (true);
+  }
+
+  bool subset (hb_subset_context_t *c) const
+  {
+    TRACE_SUBSET (this);
+
+    glyf *glyf_prime = c->serializer->start_embed <glyf> ();
+    if (unlikely (!c->serializer->check_success (glyf_prime))) return_trace (false);
+
+    // Byte region(s) per glyph to output
+    // unpadded, hints removed if so requested
+    // If we fail to process a glyph we produce an empty (0-length) glyph
+    hb_vector_t<SubsetGlyph> glyphs;
+    _populate_subset_glyphs (c->plan, &glyphs);
+
+    glyf_prime->serialize (c->serializer, hb_iter (glyphs), c->plan);
+
+    auto padded_offsets =
+    + hb_iter (glyphs)
+    | hb_map (&SubsetGlyph::padded_size)
+    ;
+
+    return_trace (c->serializer->check_success (_add_loca_and_head (c->plan, padded_offsets)));
+  }
+
+  template <typename SubsetGlyph>
+  void
+  _populate_subset_glyphs (const hb_subset_plan_t * plan,
+			   hb_vector_t<SubsetGlyph> * glyphs /* OUT */) const
+  {
+    OT::glyf::accelerator_t glyf;
+    glyf.init (plan->source);
+
+    + hb_range (plan->num_output_glyphs ())
+    | hb_map ([&] (hb_codepoint_t new_gid) {
+      SubsetGlyph subset_glyph;
+      subset_glyph.new_gid = new_gid;
+
+      // 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;
+
+      subset_glyph.source_glyph = glyf.bytes_for_glyph ((const char *) this, subset_glyph.old_gid);
+      if (plan->drop_hints) subset_glyph.drop_hints (glyf);
+      else subset_glyph.dest_start = subset_glyph.source_glyph;
+
+      return subset_glyph;
+    })
+    | hb_sink (glyphs)
+    ;
+
+    glyf.fini();
+  }
+
+  static void
+  _fix_component_gids (const hb_subset_plan_t *plan,
+		       hb_bytes_t glyph)
   {
-    hb_blob_t *glyf_prime = nullptr;
-    hb_blob_t *loca_prime = nullptr;
-
-    bool success = true;
-    bool use_short_loca = false;
-    if (hb_subset_glyf_and_loca (plan, &use_short_loca, &glyf_prime, &loca_prime)) {
-      success = success && plan->add_table (HB_OT_TAG_glyf, glyf_prime);
-      success = success && plan->add_table (HB_OT_TAG_loca, loca_prime);
-      success = success && _add_head_and_set_loca_version (plan, use_short_loca);
-    } else {
-      success = false;
+    OT::glyf::CompositeGlyphHeader::Iterator iterator;
+    if (OT::glyf::CompositeGlyphHeader::get_iterator (&glyph,
+						      glyph.length,
+						      &iterator))
+    {
+      do
+      {
+	hb_codepoint_t new_gid;
+	if (!plan->new_gid_for_old_gid (iterator.current->glyphIndex,
+					&new_gid))
+	  continue;
+	((OT::glyf::CompositeGlyphHeader *) iterator.current)->glyphIndex = new_gid;
+      } while (iterator.move_to_next ());
     }
-    hb_blob_destroy (loca_prime);
-    hb_blob_destroy (glyf_prime);
+  }
 
-    return success;
+  static void
+  _zero_instruction_length (hb_bytes_t glyph)
+  {
+    const GlyphHeader &glyph_header = StructAtOffset<GlyphHeader> (&glyph, 0);
+    int16_t num_contours = (int16_t) glyph_header.numberOfContours;
+    if (num_contours <= 0) return;  // only for simple glyphs
+
+    const HBUINT16 &instruction_length = StructAtOffset<HBUINT16> (&glyph, GlyphHeader::static_size + 2 * num_contours);
+    (HBUINT16 &) instruction_length = 0;
   }
 
+  static bool _remove_composite_instruction_flag (hb_bytes_t glyph)
+  {
+    const GlyphHeader &glyph_header = StructAtOffset<GlyphHeader> (&glyph, 0);
+    if (glyph_header.numberOfContours >= 0) return true;  // only for composites
+
+    /* remove WE_HAVE_INSTRUCTIONS from flags in dest */
+    OT::glyf::CompositeGlyphHeader::Iterator composite_it;
+    if (unlikely (!OT::glyf::CompositeGlyphHeader::get_iterator (&glyph, glyph.length, &composite_it))) return false;
+    const OT::glyf::CompositeGlyphHeader *composite_header;
+    do {
+      composite_header = composite_it.current;
+      OT::HBUINT16 *flags = const_cast<OT::HBUINT16 *> (&composite_header->flags);
+      *flags = (uint16_t) *flags & ~OT::glyf::CompositeGlyphHeader::WE_HAVE_INSTRUCTIONS;
+    } while (composite_it.move_to_next ());
+    return true;
+}
+
   static bool
   _add_head_and_set_loca_version (hb_subset_plan_t *plan, bool use_short_loca)
   {
@@ -171,6 +313,7 @@ struct glyf
       return size;
     }
 
+    // TODO rewrite using new iterator framework if possible
     struct Iterator
     {
       const char *glyph_start;
@@ -381,61 +524,55 @@ struct glyf
       return true;
     }
 
-    bool get_instruction_offsets (unsigned int start_offset,
-				  unsigned int end_offset,
-				  unsigned int *instruction_start /* OUT */,
-				  unsigned int *instruction_end /* OUT */) const
+    bool get_instruction_length (hb_bytes_t glyph,
+				 unsigned int * length /* OUT */) const
     {
-      if (end_offset - start_offset < GlyphHeader::static_size)
+      /* Empty glyph; no instructions. */
+      if (glyph.length < GlyphHeader::static_size)
       {
-	*instruction_start = 0;
-	*instruction_end = 0;
-	return true; /* Empty glyph; no instructions. */
+	*length = 0;
+	// only 0 byte glyphs are healthy when missing GlyphHeader
+	return glyph.length == 0;
       }
-      const GlyphHeader &glyph_header = StructAtOffset<GlyphHeader> (glyf_table, start_offset);
+      const GlyphHeader &glyph_header = StructAtOffset<GlyphHeader> (&glyph, 0);
       int16_t num_contours = (int16_t) glyph_header.numberOfContours;
       if (num_contours < 0)
       {
+	unsigned int start = glyph.length;
+	unsigned int end = glyph.length;
+	unsigned int glyph_offset = &glyph - glyf_table;
 	CompositeGlyphHeader::Iterator composite_it;
-	if (unlikely (!CompositeGlyphHeader::get_iterator (
-	    (const char*) this->glyf_table + start_offset,
-	     end_offset - start_offset, &composite_it))) return false;
+	if (unlikely (!CompositeGlyphHeader::get_iterator (&glyph, glyph.length, &composite_it))) return false;
 	const CompositeGlyphHeader *last;
 	do {
 	  last = composite_it.current;
 	} while (composite_it.move_to_next ());
 
 	if ((uint16_t) last->flags & CompositeGlyphHeader::WE_HAVE_INSTRUCTIONS)
-	  *instruction_start = ((char *) last - (char *) glyf_table->dataZ.arrayZ) + last->get_size ();
-	else
-	  *instruction_start = end_offset;
-	*instruction_end = end_offset;
-	if (unlikely (*instruction_start > *instruction_end))
+	  start = ((char *) last - (char *) glyf_table->dataZ.arrayZ) + last->get_size () - glyph_offset;
+	if (unlikely (start > end))
 	{
-	  DEBUG_MSG(SUBSET, nullptr, "Invalid instruction offset, %d is outside [%d, %d]", *instruction_start, start_offset, end_offset);
+	  DEBUG_MSG(SUBSET, nullptr, "Invalid instruction offset, %d is outside %d byte buffer", start, glyph.length);
 	  return false;
 	}
+	*length = end - start;
       }
       else
       {
-	unsigned int instruction_length_offset = start_offset + GlyphHeader::static_size + 2 * num_contours;
-	if (unlikely (instruction_length_offset + 2 > end_offset))
+	unsigned int instruction_length_offset = GlyphHeader::static_size + 2 * num_contours;
+	if (unlikely (instruction_length_offset + 2 > glyph.length))
 	{
 	  DEBUG_MSG(SUBSET, nullptr, "Glyph size is too short, missing field instructionLength.");
 	  return false;
 	}
 
-	const HBUINT16 &instruction_length = StructAtOffset<HBUINT16> (glyf_table, instruction_length_offset);
-	unsigned int start = instruction_length_offset + 2;
-	unsigned int end = start + (uint16_t) instruction_length;
-	if (unlikely (end > end_offset)) // Out of bounds of the current glyph
+	const HBUINT16 &instruction_length = StructAtOffset<HBUINT16> (&glyph, instruction_length_offset);
+	if (unlikely (instruction_length_offset + instruction_length > glyph.length)) // Out of bounds of the current glyph
 	{
 	  DEBUG_MSG(SUBSET, nullptr, "The instructions array overruns the glyph's boundaries.");
 	  return false;
 	}
-
-	*instruction_start = start;
-	*instruction_end = end;
+	*length = (uint16_t) instruction_length;
       }
       return true;
     }
@@ -459,6 +596,25 @@ struct glyf
       return true;
     }
 
+  hb_bytes_t bytes_for_glyph (const char * glyf, hb_codepoint_t gid)
+  {
+    unsigned int start_offset, end_offset;
+    if (unlikely (!(get_offsets (gid, &start_offset, &end_offset) &&
+      remove_padding (start_offset, &end_offset))))
+    {
+      DEBUG_MSG(SUBSET, nullptr, "Unable to get offset or remove padding for %d", gid);
+      return hb_bytes_t ();
+    }
+    hb_bytes_t glyph = hb_bytes_t (glyf + start_offset, end_offset - start_offset);
+    if (glyph.length == 0) return glyph;
+    if (unlikely (glyph.length < GlyphHeader::static_size))
+    {
+      DEBUG_MSG(SUBSET, nullptr, "Glyph size smaller than minimum header %d", gid);
+      return hb_bytes_t ();
+    }
+    return glyph;
+  }
+
     private:
     bool short_offset;
     unsigned int num_glyphs;
@@ -466,6 +622,93 @@ struct glyf
     hb_blob_ptr_t<glyf> glyf_table;
   };
 
+
+  struct SubsetGlyph
+  {
+    hb_codepoint_t new_gid;
+    hb_codepoint_t old_gid;
+    hb_bytes_t source_glyph;
+    hb_bytes_t dest_start;  // region of source_glyph to copy first
+    hb_bytes_t dest_end;    // region of source_glyph to copy second
+
+
+  bool serialize (hb_serialize_context_t *c,
+		  const hb_subset_plan_t *plan) const
+  {
+    TRACE_SERIALIZE (this);
+
+    hb_bytes_t dest_glyph = dest_start.copy(c);
+    dest_glyph = hb_bytes_t (&dest_glyph, dest_glyph.length + dest_end.copy(c).length);
+    unsigned int pad_length = padding ();
+    DEBUG_MSG(SUBSET, nullptr, "serialize %d byte glyph, width %d pad %d", dest_glyph.length, dest_glyph.length  + pad_length, pad_length);
+
+    HBUINT8 pad;
+    pad = 0;
+    while (pad_length > 0)
+    {
+      c->embed(pad);
+      pad_length--;
+    }
+
+    if (dest_glyph.length)
+    {
+      _fix_component_gids (plan, dest_glyph);
+      if (plan->drop_hints)
+      {
+	_zero_instruction_length (dest_glyph);
+	c->check_success (_remove_composite_instruction_flag (dest_glyph));
+      }
+    }
+
+    return_trace (true);
+  }
+
+    void drop_hints (const OT::glyf::accelerator_t& glyf)
+    {
+      if (source_glyph.length == 0) return;
+
+      unsigned int instruction_length = 0;
+      if (!glyf.get_instruction_length (source_glyph, &instruction_length))
+      {
+	 DEBUG_MSG(SUBSET, nullptr, "Unable to read instruction length for new_gid %d", new_gid);
+	return ;
+      }
+
+      const GlyphHeader& header = StructAtOffset<GlyphHeader> (&source_glyph, 0);
+      int16_t num_contours = (int16_t) header.numberOfContours;
+      DEBUG_MSG(SUBSET, nullptr, "new_gid %d (%d contours) drop %d instruction bytes from %d byte source glyph", new_gid, num_contours, instruction_length, source_glyph.length);
+      if (num_contours < 0)
+      {
+	// composite, just chop instructions off the end
+	dest_start = hb_bytes_t (&source_glyph, source_glyph.length - instruction_length);
+      }
+      else
+      {
+	// simple glyph
+	dest_start = hb_bytes_t (&source_glyph, GlyphHeader::static_size + 2 * header.numberOfContours + 2);
+	dest_end = hb_bytes_t (&source_glyph + dest_start.length + instruction_length,
+			       source_glyph.length - dest_start.length - instruction_length);
+DEBUG_MSG(SUBSET, nullptr, "source_len %d start len %d instruction_len %d end len %d", source_glyph.length, dest_start.length, instruction_length, dest_end.length);
+      }
+    }
+
+    unsigned int length () const
+    {
+      return dest_start.length + dest_end.length;
+    }
+
+    // pad to 2 to ensure 2-byte loca will be ok
+    unsigned int padding () const
+    {
+      return length () % 2;
+    }
+
+    unsigned int padded_size () const
+    {
+      return length () + padding ();
+    }
+  };
+
   protected:
   UnsizedArrayOf<HBUINT8>	dataZ;		/* Glyphs data. */
   public:

src/hb-subset-glyf.cc

-/*
- * 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, Roderick Sheeter
- */
-
-#include "hb-open-type.hh"
-#include "hb-ot-glyf-table.hh"
-#include "hb-set.h"
-#include "hb-subset-glyf.hh"
-
-struct loca_data_t
-{
-  bool          is_short;
-  void         *data;
-  unsigned int  size;
-
-  inline bool
-  _write_loca_entry (unsigned int  id,
-                     unsigned int  offset)
-  {
-    unsigned int entry_size = is_short ? sizeof (OT::HBUINT16) : sizeof (OT::HBUINT32);
-    if ((id + 1) * entry_size <= size)
-    {
-      if (is_short) {
-        ((OT::HBUINT16*) data) [id] = offset / 2;
-      } else {
-        ((OT::HBUINT32*) data) [id] = offset;
-      }
-      return true;
-    }
-
-    // Offset was not written because the write is out of bounds.
-    DEBUG_MSG(SUBSET,
-              nullptr,
-              "WARNING: Attempted to write an out of bounds loca entry at index %d. Loca size is %d.",
-              id,
-              size);
-    return false;
-  }
-};
-
-/*
- * If hints are being dropped find the range which in glyf at which
- * the hinting instructions are located. Add them to the instruction_ranges
- * vector.
- */
-static bool
-_add_instructions_range (const OT::glyf::accelerator_t &glyf,
-                         hb_codepoint_t                 glyph_id,
-                         unsigned int                   glyph_start_offset,
-                         unsigned int                   glyph_end_offset,
-                         bool                           drop_hints,
-                         hb_vector_t<unsigned int>     *instruction_ranges /* OUT */)
-{
-  if (!instruction_ranges->resize (instruction_ranges->length + 2))
-  {
-    DEBUG_MSG(SUBSET, nullptr, "Failed to resize instruction_ranges.");
-    return false;
-  }
-  unsigned int *instruction_start = &(*instruction_ranges)[instruction_ranges->length - 2];
-  *instruction_start = 0;
-  unsigned int *instruction_end = &(*instruction_ranges)[instruction_ranges->length - 1];
-  *instruction_end = 0;
-
-  if (drop_hints)
-  {
-    if (unlikely (!glyf.get_instruction_offsets (glyph_start_offset, glyph_end_offset,
-                                                 instruction_start, instruction_end)))
-    {
-      DEBUG_MSG(SUBSET, nullptr, "Unable to get instruction offsets for %d", glyph_id);
-      return false;
-    }
-  }
-
-  return true;
-}
-
-static bool
-_calculate_glyf_and_loca_prime_size (const OT::glyf::accelerator_t &glyf,
-                                     const hb_subset_plan_t        *plan,
-                                     loca_data_t                   *loca_data, /* OUT */
-				     unsigned int                  *glyf_size /* OUT */,
-				     hb_vector_t<unsigned int>     *instruction_ranges /* OUT */)
-{
-  unsigned int total = 0;
-
-  hb_codepoint_t next_glyph = HB_SET_VALUE_INVALID;
-  while (plan->glyphset ()->next (&next_glyph))
-  {
-    unsigned int start_offset = 0, end_offset = 0;
-    if (unlikely (!(glyf.get_offsets (next_glyph, &start_offset, &end_offset) &&
-		    glyf.remove_padding (start_offset, &end_offset))))
-    {
-      DEBUG_MSG(SUBSET, nullptr, "Invalid gid %d", next_glyph);
-      start_offset = end_offset = 0;
-    }
-
-    bool is_zero_length = end_offset - start_offset < OT::glyf::GlyphHeader::static_size;
-    if (!_add_instructions_range (glyf,
-                                  next_glyph,
-                                  start_offset,
-                                  end_offset,
-                                  plan->drop_hints && !is_zero_length,
-                                  instruction_ranges))
-      return false;
-
-    if (is_zero_length)
-      continue; /* 0-length glyph */
-
-    total += end_offset - start_offset
-             - ((*instruction_ranges)[instruction_ranges->length - 1]
-                - (*instruction_ranges)[instruction_ranges->length - 2]);
-    /* round2 so short loca will work */
-    total += total % 2;
-  }
-
-  *glyf_size = total;
-  loca_data->is_short = (total <= 131070);
-  loca_data->size = (plan->num_output_glyphs () + 1)
-      * (loca_data->is_short ? sizeof (OT::HBUINT16) : sizeof (OT::HBUINT32));
-
-  DEBUG_MSG(SUBSET, nullptr, "preparing to subset glyf: final size %d, loca size %d, using %s loca",
-	    total,
-	    loca_data->size,
-	    loca_data->is_short ? "short" : "long");
-  return true;
-}
-
-static void
-_update_components (const hb_subset_plan_t *plan,
-		    char                   *glyph_start,
-		    unsigned int            length)
-{
-  OT::glyf::CompositeGlyphHeader::Iterator iterator;
-  if (OT::glyf::CompositeGlyphHeader::get_iterator (glyph_start,
-						    length,
-						    &iterator))
-  {
-    do
-    {
-      hb_codepoint_t new_gid;
-      if (!plan->new_gid_for_old_gid (iterator.current->glyphIndex,
-				      &new_gid))
-	continue;
-
-      ((OT::glyf::CompositeGlyphHeader *) iterator.current)->glyphIndex = new_gid;
-    } while (iterator.move_to_next ());
-  }
-}
-
-static bool _remove_composite_instruction_flag (char *glyf_prime, unsigned int length)
-{
-  /* remove WE_HAVE_INSTRUCTIONS from flags in dest */
-  OT::glyf::CompositeGlyphHeader::Iterator composite_it;
-  if (unlikely (!OT::glyf::CompositeGlyphHeader::get_iterator (glyf_prime, length, &composite_it))) return false;
-  const OT::glyf::CompositeGlyphHeader *glyph;
-  do {
-    glyph = composite_it.current;
-    OT::HBUINT16 *flags = const_cast<OT::HBUINT16 *> (&glyph->flags);
-    *flags = (uint16_t) *flags & ~OT::glyf::CompositeGlyphHeader::WE_HAVE_INSTRUCTIONS;
-  } while (composite_it.move_to_next ());
-  return true;
-}
-
-static bool
-_write_glyf_and_loca_prime (const hb_subset_plan_t        *plan,
-			    const OT::glyf::accelerator_t &glyf,
-			    const char                    *glyf_data,
-			    hb_vector_t<unsigned int>     &instruction_ranges,
-			    unsigned int                   glyf_prime_size,
-			    char                          *glyf_prime_data /* OUT */,
-			    loca_data_t                   *loca_prime /* OUT */)
-{
-  char *glyf_prime_data_next = glyf_prime_data;
-
-  bool success = true;
-
-
-  unsigned int i = 0;
-  hb_codepoint_t new_gid;
-  for (new_gid = 0; new_gid < plan->num_output_glyphs (); new_gid++)
-  {
-    hb_codepoint_t old_gid;
-    if (!plan->old_gid_for_new_gid (new_gid, &old_gid))
-    {
-      // Empty glyph, add a loca entry and carry on.
-      loca_prime->_write_loca_entry (new_gid,
-                                     glyf_prime_data_next - glyf_prime_data);
-      continue;
-    }
-
-
-    unsigned int start_offset = 0, end_offset = 0;
-    if (unlikely (!(glyf.get_offsets (old_gid, &start_offset, &end_offset) &&
-		    glyf.remove_padding (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);
-
-    if (glyf_prime_data_next + length > glyf_prime_data + glyf_prime_size)
-    {
-      DEBUG_MSG(SUBSET,
-                nullptr,
-                "WARNING: Attempted to write an out of bounds glyph entry for gid %d (length %d)",
-                i, length);
-      return false;
-    }
-
-    if (instruction_start == instruction_end)
-      memcpy (glyf_prime_data_next, glyf_data + start_offset, length);
-    else
-    {
-      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, else simple */
-      if (instruction_end == end_offset)
-      {
-	if (unlikely (!_remove_composite_instruction_flag (glyf_prime_data_next, length))) return false;
-      }
-      else
-	/* zero instruction length, which is just before instruction_start */
-	memset (glyf_prime_data_next + instruction_start - start_offset - 2, 0, 2);
-    }
-
-    success = success && loca_prime->_write_loca_entry (new_gid,
-                                                        glyf_prime_data_next - glyf_prime_data);
-    _update_components (plan, glyf_prime_data_next, length);
-
-    // TODO: don't align to two bytes if using long loca.
-    glyf_prime_data_next += length + (length % 2); // Align to 2 bytes for short loca.
-
-    i++;
-  }
-
-  // loca table has n+1 entries where the last entry signifies the end location of the last
-  // glyph.
-  success = success && loca_prime->_write_loca_entry (new_gid,
-                                                      glyf_prime_data_next - glyf_prime_data);
-  return success;
-}
-
-static bool
-_hb_subset_glyf_and_loca (const OT::glyf::accelerator_t  &glyf,
-			  const char                     *glyf_data,
-			  hb_subset_plan_t               *plan,
-			  bool                           *use_short_loca,
-			  hb_blob_t                     **glyf_prime_blob /* OUT */,
-			  hb_blob_t                     **loca_prime_blob /* OUT */)
-{
-  // TODO(grieger): Sanity check allocation size for the new table.
-  loca_data_t loca_prime;
-  unsigned int glyf_prime_size;
-  hb_vector_t<unsigned int> instruction_ranges;
-  instruction_ranges.init ();
-
-  if (unlikely (!_calculate_glyf_and_loca_prime_size (glyf,
-                                                      plan,
-                                                      &loca_prime,
-						      &glyf_prime_size,
-						      &instruction_ranges))) {
-    instruction_ranges.fini ();
-    return false;
-  }
-  *use_short_loca = loca_prime.is_short;
-
-  char *glyf_prime_data = (char *) calloc (1, glyf_prime_size);
-  loca_prime.data = (void *) calloc (1, loca_prime.size);
-  if (unlikely (!_write_glyf_and_loca_prime (plan, glyf, glyf_data,
-					     instruction_ranges,
-					     glyf_prime_size, glyf_prime_data,
-					     &loca_prime))) {
-    free (glyf_prime_data);
-    free (loca_prime.data);
-    instruction_ranges.fini ();
-    return false;
-  }
-  instruction_ranges.fini ();
-
-  *glyf_prime_blob = hb_blob_create (glyf_prime_data,
-                                     glyf_prime_size,
-                                     HB_MEMORY_MODE_READONLY,
-                                     glyf_prime_data,
-                                     free);
-  *loca_prime_blob = hb_blob_create ((char *) loca_prime.data,
-                                     loca_prime.size,
-                                     HB_MEMORY_MODE_READONLY,
-                                     loca_prime.data,
-                                     free);
-  return true;
-}
-
-/**
- * hb_subset_glyf:
- * Subsets the glyph table according to a provided plan.
- *
- * Return value: subsetted glyf table.
- *
- * Since: 1.7.5
- **/
-bool
-hb_subset_glyf_and_loca (hb_subset_plan_t *plan,
-			 bool             *use_short_loca, /* OUT */
-			 hb_blob_t       **glyf_prime, /* OUT */
-			 hb_blob_t       **loca_prime /* OUT */)
-{
-  hb_blob_t *glyf_blob = hb_sanitize_context_t ().reference_table<OT::glyf> (plan->source);
-  const char *glyf_data = hb_blob_get_data (glyf_blob, nullptr);
-
-  OT::glyf::accelerator_t glyf;
-  glyf.init (plan->source);
-  bool result = _hb_subset_glyf_and_loca (glyf,
-					  glyf_data,
-					  plan,
-					  use_short_loca,
-					  glyf_prime,
-					  loca_prime);
-
-  hb_blob_destroy (glyf_blob);
-  glyf.fini ();
-
-  return result;
-}

src/hb-subset-glyf.hh

-/*
- * 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_GLYF_HH
-#define HB_SUBSET_GLYF_HH
-
-#include "hb.hh"
-
-#include "hb-subset.hh"
-
-HB_INTERNAL bool
-hb_subset_glyf_and_loca (hb_subset_plan_t *plan,
-			 bool             *use_short_loca, /* OUT */
-			 hb_blob_t       **glyf_prime      /* OUT */,
-			 hb_blob_t       **loca_prime      /* OUT */);
-
-#endif /* HB_SUBSET_GLYF_HH */

src/hb-subset-plan.hh

@@ -71,6 +71,8 @@ struct hb_subset_plan_t
 
   /*
    * The set of input glyph ids which will be retained in the subset.
+   * Does NOT include ids kept due to retain_gids. You probably want to use
+   * glyph_map/reverse_glyph_map.
    */
   inline const hb_set_t *
   glyphset () const

src/hb-subset.cc

@@ -28,7 +28,6 @@
 #include "hb-open-type.hh"
 
 #include "hb-subset.hh"
-#include "hb-subset-glyf.hh"
 
 #include "hb-open-file.hh"
 #include "hb-ot-cmap-table.hh"
@@ -156,7 +155,7 @@ _subset_table (hb_subset_plan_t *plan,
   bool result = true;
   switch (tag) {
     case HB_OT_TAG_glyf:
-      result = _subset<const OT::glyf> (plan);
+      result = _subset2<const OT::glyf> (plan);
       break;
     case HB_OT_TAG_hdmx:
       result = _subset2<const OT::hdmx> (plan);

test/api/hb-subset-test.h

@@ -91,9 +91,9 @@ hb_subset_test_check (hb_face_t *expected,
                       hb_tag_t   table)
 {
   hb_blob_t *expected_blob, *actual_blob;
-  //fprintf(stderr, "comparing %c%c%c%c ", HB_UNTAG(table));
   expected_blob = hb_face_reference_table (expected, table);
   actual_blob = hb_face_reference_table (actual, table);
+  fprintf(stderr, "comparing %c%c%c%c, expected %d bytes, actual %d bytes\n", HB_UNTAG(table), hb_blob_get_length(expected_blob), hb_blob_get_length (actual_blob));
   hb_test_assert_blobs_equal (expected_blob, actual_blob);
   hb_blob_destroy (expected_blob);
   hb_blob_destroy (actual_blob);

test/api/hb-test.h

@@ -173,6 +173,16 @@ static inline void hb_test_assert_blobs_equal (hb_blob_t *expected_blob, hb_blob
   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);
+  if (memcmp (raw_expected, raw_actual, expected_length) != 0)
+  {
+    for (unsigned int i = 0; i < expected_length; i++)
+    {
+      int expected = *(raw_expected + i);
+      int actual = *(raw_actual + i);
+      if (expected != actual) fprintf(stderr, "+%u %02x != %02x\n", i, expected, actual);
+      else fprintf(stderr, "+%u %02x\n", i, expected);
+    }
+  }
   g_assert_cmpint(0, ==, memcmp(raw_expected, raw_actual, expected_length));
 }
 

test/api/test-subset-glyf.c

@@ -70,9 +70,9 @@ test_subset_glyf (void)
   face_abc_subset = hb_subset_test_create_subset (face_abc, hb_subset_test_create_input (codepoints));
   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'));
   check_maxp_num_glyphs(face_abc_subset, 3, true);
+  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_face_destroy (face_abc_subset);
   hb_face_destroy (face_abc);
@@ -194,9 +194,9 @@ test_subset_glyf_noop (void)
   face_abc_subset = hb_subset_test_create_subset (face_abc, hb_subset_test_create_input (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'));
   check_maxp_num_glyphs(face_abc_subset, 4, true);
+  hb_subset_test_check (face_abc, face_abc_subset, HB_TAG ('l','o','c', 'a'));
+  hb_subset_test_check (face_abc, face_abc_subset, HB_TAG ('g','l','y','f'));
 
   hb_face_destroy (face_abc_subset);
   hb_face_destroy (face_abc);
@@ -218,9 +218,9 @@ test_subset_glyf_strip_hints_simple (void)
   face_abc_subset = hb_subset_test_create_subset (face_abc, input);
   hb_set_destroy (codepoints);
 
+  check_maxp_num_glyphs(face_abc_subset, 3, false);
   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'));
-  check_maxp_num_glyphs(face_abc_subset, 3, false);
 
   hb_face_destroy (face_abc_subset);
   hb_face_destroy (face_abc);
@@ -243,9 +243,9 @@ test_subset_glyf_strip_hints_composite (void)
   face_generated_subset = hb_subset_test_create_subset (face_components, input);
   hb_set_destroy (codepoints);
 
-  hb_subset_test_check (face_subset, face_generated_subset, HB_TAG ('g','l','y','f'));
-  hb_subset_test_check (face_subset, face_generated_subset, HB_TAG ('l','o','c', 'a'));
   check_maxp_num_glyphs(face_generated_subset, 4, false);
+  hb_subset_test_check (face_subset, face_generated_subset, HB_TAG ('l','o','c', 'a'));
+  hb_subset_test_check (face_subset, face_generated_subset, HB_TAG ('g','l','y','f'));
 
   hb_face_destroy (face_generated_subset);
   hb_face_destroy (face_subset);
@@ -300,9 +300,9 @@ test_subset_glyf_retain_gids (void)
   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'));
   check_maxp_num_glyphs(face_abc_subset, 4, true);
+  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_face_destroy (face_abc_subset);
   hb_face_destroy (face_abc);
@@ -324,9 +324,9 @@ test_subset_glyf_retain_gids_truncates (void)
   face_abc_subset = hb_subset_test_create_subset (face_abc, input);
   hb_set_destroy (codepoints);
 
-  hb_subset_test_check (face_a, face_abc_subset, HB_TAG ('g','l','y','f'));
-  hb_subset_test_check (face_a, face_abc_subset, HB_TAG ('l','o','c', 'a'));
   check_maxp_num_glyphs(face_abc_subset, 2, true);
+  hb_subset_test_check (face_a, face_abc_subset, HB_TAG ('l','o','c', 'a'));
+  hb_subset_test_check (face_a, face_abc_subset, HB_TAG ('g','l','y','f'));
 
   hb_face_destroy (face_abc_subset);
   hb_face_destroy (face_abc);