[HarfBuzz] harfbuzz: Branch 'master' - 8 commits

Behdad Esfahbod behdad at kemper.freedesktop.org
Wed Feb 21 08:35:55 UTC 2018


 src/hb-open-file-private.hh        |    2 -
 src/hb-open-type-private.hh        |    4 +-
 src/hb-ot-glyf-table.hh            |   42 ++++++++++++++++++++++++-
 src/hb-ot-layout-common-private.hh |    6 +--
 src/hb-ot-maxp-table.hh            |    3 -
 src/hb-ot-shape.cc                 |    2 -
 src/hb-subset-glyf.cc              |   62 ++++++++++++++++++++++++++++---------
 src/hb-subset.cc                   |   50 -----------------------------
 8 files changed, 99 insertions(+), 72 deletions(-)

New commits:
commit eada749e4642ea90300c9c68c226fa76a3e35a75
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Wed Feb 21 00:35:23 2018 -0800

    Use HB_SET_VALUE_INVALID consistently

diff --git a/src/hb-ot-layout-common-private.hh b/src/hb-ot-layout-common-private.hh
index cb66c81a..c5e7f521 100644
--- a/src/hb-ot-layout-common-private.hh
+++ b/src/hb-ot-layout-common-private.hh
@@ -1057,7 +1057,7 @@ struct ClassDefFormat1
     if (klass == 0)
     {
       /* Match if there's any glyph that is not listed! */
-      hb_codepoint_t g = -1;
+      hb_codepoint_t g = HB_SET_VALUE_INVALID;
       if (!hb_set_next (glyphs, &g))
         return false;
       if (g < startGlyph)
@@ -1128,7 +1128,7 @@ struct ClassDefFormat2
     if (klass == 0)
     {
       /* Match if there's any glyph that is not listed! */
-      hb_codepoint_t g = (hb_codepoint_t) -1;
+      hb_codepoint_t g = HB_SET_VALUE_INVALID;
       for (unsigned int i = 0; i < count; i++)
       {
 	if (!hb_set_next (glyphs, &g))
@@ -1137,7 +1137,7 @@ struct ClassDefFormat2
 	  return true;
 	g = rangeRecord[i].end;
       }
-      if (g != (hb_codepoint_t) -1 && hb_set_next (glyphs, &g))
+      if (g != HB_SET_VALUE_INVALID && hb_set_next (glyphs, &g))
         return true;
       /* Fall through. */
     }
diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc
index fc5dd108..d9ba0f6b 100644
--- a/src/hb-ot-shape.cc
+++ b/src/hb-ot-shape.cc
@@ -957,7 +957,7 @@ hb_ot_shape_glyphs_closure (hb_font_t          *font,
   hb_set_t *copy = hb_set_create ();
   do {
     copy->set (glyphs);
-    for (hb_codepoint_t lookup_index = -1; hb_set_next (lookups, &lookup_index);)
+    for (hb_codepoint_t lookup_index = HB_SET_VALUE_INVALID; hb_set_next (lookups, &lookup_index);)
       hb_ot_layout_lookup_substitute_closure (font->face, lookup_index, glyphs);
   } while (!copy->is_equal (glyphs));
   hb_set_destroy (copy);
commit 2cc845f311b6dc4f0feda8b8fc5609fbd51b5923
Author: Garret Rieger <grieger at google.com>
Date:   Tue Feb 20 18:13:41 2018 -0800

    [subset] fix calculation of range shiftz. Should be 16 * len - searchRange not 16 * (len - searchRange).

diff --git a/src/hb-open-type-private.hh b/src/hb-open-type-private.hh
index 080dcca1..54eda4c5 100644
--- a/src/hb-open-type-private.hh
+++ b/src/hb-open-type-private.hh
@@ -1113,7 +1113,9 @@ struct BinSearchHeader
     assert (len == v);
     entrySelectorZ.set (MAX (1u, _hb_bit_storage (v)) - 1);
     searchRangeZ.set (16 * (1u << entrySelectorZ));
-    rangeShiftZ.set (16 * MAX (0, (int) v - searchRangeZ));
+    rangeShiftZ.set (v * 16 > searchRangeZ
+                     ? 16 * v - searchRangeZ
+                     : 0);
   }
 
   protected:
commit 8e614ade5aef102baed56f91c2fcb1f3d1788ea9
Author: Garret Rieger <grieger at google.com>
Date:   Tue Feb 20 17:36:54 2018 -0800

    [subset] Reverse table order for font serialization to match what OTS expects.

diff --git a/src/hb-open-file-private.hh b/src/hb-open-file-private.hh
index f01ab871..88ce65a4 100644
--- a/src/hb-open-file-private.hh
+++ b/src/hb-open-file-private.hh
@@ -54,7 +54,7 @@ struct TTCHeader;
 typedef struct TableRecord
 {
   int cmp (Tag t) const
-  { return t.cmp (tag); }
+  { return -t.cmp (tag); }
 
   static int cmp (const void *pa, const void *pb)
   {
commit a998eeee4ad7bba4a1574c9735618891b6bd0948
Author: Garret Rieger <grieger at google.com>
Date:   Tue Feb 20 16:48:52 2018 -0800

    [subset] sanity check glyph data writes during glyph subsetting to ensure they are inbounds.

diff --git a/src/hb-subset-glyf.cc b/src/hb-subset-glyf.cc
index c337e65b..d57b4115 100644
--- a/src/hb-subset-glyf.cc
+++ b/src/hb-subset-glyf.cc
@@ -89,7 +89,6 @@ _write_loca_entry (unsigned int  id,
   return false;
 }
 
-
 static void
 _update_components (hb_subset_plan_t * plan,
 		    char * glyph_start,
@@ -124,7 +123,6 @@ _write_glyf_and_loca_prime (hb_subset_plan_t              *plan,
                             unsigned int                   loca_prime_size,
                             char                          *loca_prime_data /* OUT */)
 {
-  // TODO(grieger): Sanity check writes to make sure they are in-bounds.
   hb_prealloced_array_t<hb_codepoint_t> &glyph_ids = plan->gids_to_retain_sorted;
   char *glyf_prime_data_next = glyf_prime_data;
 
@@ -136,6 +134,15 @@ _write_glyf_and_loca_prime (hb_subset_plan_t              *plan,
       end_offset = start_offset = 0;
 
     int length = end_offset - start_offset;
+
+    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",
+                 i);
+      return false;
+    }
     memcpy (glyf_prime_data_next, glyf_data + start_offset, length);
 
     success = success && _write_loca_entry (i,
commit 0ab73e594275cf064e09b9df2e1df337a589745d
Author: Garret Rieger <grieger at google.com>
Date:   Tue Feb 20 15:33:03 2018 -0800

    [subset] Sanity check that loca writes are inbounds.

diff --git a/src/hb-subset-glyf.cc b/src/hb-subset-glyf.cc
index 7dfe0ba4..c337e65b 100644
--- a/src/hb-subset-glyf.cc
+++ b/src/hb-subset-glyf.cc
@@ -62,15 +62,34 @@ _calculate_glyf_and_loca_prime_size (const OT::glyf::accelerator_t &glyf,
   return true;
 }
 
-static void
-_write_loca_entry (unsigned int id, unsigned int offset, bool is_short, void *loca_prime) {
-  if (is_short) {
-    ((OT::HBUINT16*) loca_prime) [id].set (offset / 2);
-  } else {
-    ((OT::HBUINT32*) loca_prime) [id].set (offset);
+static bool
+_write_loca_entry (unsigned int  id,
+                   unsigned int  offset,
+                   bool          is_short,
+                   void         *loca_prime,
+                   unsigned int  loca_size)
+{
+  unsigned int entry_size = is_short ? sizeof (OT::HBUINT16) : sizeof (OT::HBUINT32);
+  if ((id + 1) * entry_size <= loca_size)
+  {
+    if (is_short) {
+      ((OT::HBUINT16*) loca_prime) [id].set (offset / 2);
+    } else {
+      ((OT::HBUINT32*) loca_prime) [id].set (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,
+             loca_size);
+  return false;
 }
 
+
 static void
 _update_components (hb_subset_plan_t * plan,
 		    char * glyph_start,
@@ -100,14 +119,16 @@ _write_glyf_and_loca_prime (hb_subset_plan_t              *plan,
 			    const OT::glyf::accelerator_t &glyf,
                             const char                    *glyf_data,
                             bool                           use_short_loca,
-                            int                            glyf_prime_size,
+                            unsigned int                   glyf_prime_size,
                             char                          *glyf_prime_data /* OUT */,
-                            int                            loca_prime_size,
+                            unsigned int                   loca_prime_size,
                             char                          *loca_prime_data /* OUT */)
 {
+  // TODO(grieger): Sanity check writes to make sure they are in-bounds.
   hb_prealloced_array_t<hb_codepoint_t> &glyph_ids = plan->gids_to_retain_sorted;
   char *glyf_prime_data_next = glyf_prime_data;
 
+  bool success = true;
   for (unsigned int i = 0; i < glyph_ids.len; i++)
   {
     unsigned int start_offset, end_offset;
@@ -117,15 +138,22 @@ _write_glyf_and_loca_prime (hb_subset_plan_t              *plan,
     int length = end_offset - start_offset;
     memcpy (glyf_prime_data_next, glyf_data + start_offset, length);
 
-    _write_loca_entry (i, glyf_prime_data_next - glyf_prime_data, use_short_loca, loca_prime_data);
+    success = success && _write_loca_entry (i,
+                                            glyf_prime_data_next - glyf_prime_data,
+                                            use_short_loca,
+                                            loca_prime_data,
+                                            loca_prime_size);
     _update_components (plan, glyf_prime_data_next, end_offset - start_offset);
 
     glyf_prime_data_next += length;
   }
 
-  _write_loca_entry (glyph_ids.len, glyf_prime_data_next - glyf_prime_data, use_short_loca, loca_prime_data);
-
-  return true;
+  success = success && _write_loca_entry (glyph_ids.len,
+                                          glyf_prime_data_next - glyf_prime_data,
+                                          use_short_loca,
+                                          loca_prime_data,
+                                          loca_prime_size);
+  return success;
 }
 
 static bool
@@ -136,9 +164,7 @@ _hb_subset_glyf_and_loca (const OT::glyf::accelerator_t  &glyf,
                           hb_blob_t                     **glyf_prime /* OUT */,
                           hb_blob_t                     **loca_prime /* OUT */)
 {
-  // TODO(grieger): Sanity check writes to make sure they are in-bounds.
   // TODO(grieger): Sanity check allocation size for the new table.
-  // TODO(grieger): Don't fail on bad offsets, just dump them.
   hb_prealloced_array_t<hb_codepoint_t> &glyphs_to_retain = plan->gids_to_retain_sorted;
 
   unsigned int glyf_prime_size;
@@ -159,6 +185,7 @@ _hb_subset_glyf_and_loca (const OT::glyf::accelerator_t  &glyf,
                                              glyf_prime_size, glyf_prime_data,
                                              loca_prime_size, loca_prime_data))) {
     free (glyf_prime_data);
+    free (loca_prime_data);
     return false;
   }
 
commit 73e20ec6e9ad86bea023fc8b6fc10287889ed048
Merge: 6ae4013f 69e443b2
Author: Garret Rieger <grieger at google.com>
Date:   Tue Feb 20 17:34:59 2018 -0700

    Merge pull request #812 from googlefonts/cleanup
    
    Clean up of glyf subsetting.

commit 69e443b254fceb29f26f6a0c0129fe3c93c19cfb
Author: Garret Rieger <grieger at google.com>
Date:   Tue Feb 20 14:29:21 2018 -0800

    [subset] Switch to hb_blob_copy_writable_or_fail in glyf subsetting.

diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh
index 454b4e68..50a71115 100644
--- a/src/hb-ot-glyf-table.hh
+++ b/src/hb-ot-glyf-table.hh
@@ -104,25 +104,18 @@ struct glyf
   _add_head_and_set_loca_version (hb_face_t *source, bool use_short_loca, hb_face_t *dest)
   {
     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);
-    hb_bool_t has_head = (head != nullptr);
-
-    if (has_head) {
-      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_READONLY,
-                                                   head_prime,
-                                                   free);
-      has_head = hb_subset_face_add_table (dest, HB_OT_TAG_head, head_prime_blob);
-      hb_blob_destroy (head_prime_blob);
-    }
-
+    hb_blob_t *head_prime_blob = hb_blob_copy_writable_or_fail (head_blob);
     hb_blob_destroy (head_blob);
-    return has_head;
+
+    if (unlikely (!head_prime_blob))
+      return false;
+
+    OT::head *head_prime = (OT::head *) hb_blob_get_data_writable (head_prime_blob, nullptr);
+    head_prime->indexToLocFormat.set (use_short_loca ? 0 : 1);
+    bool success = hb_subset_face_add_table (dest, HB_OT_TAG_head, head_prime_blob);
+
+    hb_blob_destroy (head_prime_blob);
+    return success;
   }
 
   struct GlyphHeader
diff --git a/src/hb-ot-maxp-table.hh b/src/hb-ot-maxp-table.hh
index 3ffa57b1..12929229 100644
--- a/src/hb-ot-maxp-table.hh
+++ b/src/hb-ot-maxp-table.hh
@@ -70,8 +70,7 @@ struct maxp
     if (unlikely (!maxp_prime_blob)) {
       return false;
     }
-    unsigned int length;
-    OT::maxp *maxp_prime = (OT::maxp *) hb_blob_get_data (maxp_prime_blob, &length);
+    OT::maxp *maxp_prime = (OT::maxp *) hb_blob_get_data (maxp_prime_blob, nullptr);
 
     maxp_prime->set_num_glyphs (plan->gids_to_retain_sorted.len);
 
commit e3e0ac98238b78530a625a6b7e7647dbabbe1c4d
Author: Garret Rieger <grieger at google.com>
Date:   Tue Feb 20 14:07:40 2018 -0800

    [subset] Move glyf subsetting code into hb-ot-glyf-table.hh

diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh
index a73fd4aa..454b4e68 100644
--- a/src/hb-ot-glyf-table.hh
+++ b/src/hb-ot-glyf-table.hh
@@ -29,7 +29,9 @@
 
 #include "hb-open-type-private.hh"
 #include "hb-ot-head-table.hh"
-
+#include "hb-subset-glyf.hh"
+#include "hb-subset-plan.hh"
+#include "hb-subset-private.hh"
 
 namespace OT {
 
@@ -78,6 +80,51 @@ struct glyf
     return_trace (true);
   }
 
+  inline bool subset (hb_subset_plan_t *plan) const
+  {
+    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 && hb_subset_plan_add_table (plan, HB_OT_TAG_glyf, glyf_prime);
+      success = success && hb_subset_plan_add_table (plan, HB_OT_TAG_loca, loca_prime);
+      success = success && _add_head_and_set_loca_version (plan->source, use_short_loca, plan->dest);
+    } else {
+      success = false;
+    }
+    hb_blob_destroy (loca_prime);
+    hb_blob_destroy (glyf_prime);
+
+    return success;
+  }
+
+  static bool
+  _add_head_and_set_loca_version (hb_face_t *source, bool use_short_loca, hb_face_t *dest)
+  {
+    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);
+    hb_bool_t has_head = (head != nullptr);
+
+    if (has_head) {
+      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_READONLY,
+                                                   head_prime,
+                                                   free);
+      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;
+  }
+
   struct GlyphHeader
   {
     HBINT16		numberOfContours;	/* If the number of contours is
diff --git a/src/hb-subset.cc b/src/hb-subset.cc
index 1fe266fc..418e481f 100644
--- a/src/hb-subset.cc
+++ b/src/hb-subset.cc
@@ -220,62 +220,14 @@ hb_subset_face_add_table (hb_face_t *face, hb_tag_t tag, hb_blob_t *blob)
 }
 
 static bool
-_add_head_and_set_loca_version (hb_face_t *source, bool use_short_loca, hb_face_t *dest)
-{
-  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);
-  hb_bool_t has_head = (head != nullptr);
-
-  if (has_head) {
-    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_READONLY,
-                                                 head_prime,
-                                                 free);
-    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_blob_t *glyf_prime = nullptr;
-  hb_blob_t *loca_prime = nullptr;
-
-  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, &use_short_loca, &glyf_prime, &loca_prime)) {
-    success = success && hb_subset_plan_add_table (plan, HB_OT_TAG_glyf, glyf_prime);
-    success = success && hb_subset_plan_add_table (plan, HB_OT_TAG_loca, loca_prime);
-    success = success && _add_head_and_set_loca_version (plan->source, use_short_loca, plan->dest);
-  } else {
-    success = false;
-  }
-  hb_blob_destroy (loca_prime);
-  hb_blob_destroy (glyf_prime);
-
-  return success;
-}
-
-static bool
 _subset_table (hb_subset_plan_t *plan,
                hb_tag_t          tag)
 {
-  // TODO (grieger): Handle updating the head table (loca format + num glyphs)
   DEBUG_MSG(SUBSET, nullptr, "begin subset %c%c%c%c", HB_UNTAG(tag));
   bool result = true;
   switch (tag) {
     case HB_OT_TAG_glyf:
-      result = _subset_glyf (plan);
+      result = _subset<const OT::glyf> (plan);
       break;
     case HB_OT_TAG_head:
       // SKIP head, it's handled by glyf


More information about the HarfBuzz mailing list