[HarfBuzz] harfbuzz: Branch 'bitops' - 16 commits

Behdad Esfahbod behdad at kemper.freedesktop.org
Sun Feb 18 18:51:07 UTC 2018


 TODO                                                |   29 ----
 src/hb-buffer-private.hh                            |    3 
 src/hb-coretext.cc                                  |    4 
 src/hb-directwrite.cc                               |    2 
 src/hb-graphite2.cc                                 |    1 
 src/hb-ot-glyf-table.hh                             |  119 +++++++++++++++++++-
 src/hb-ot-os2-table.hh                              |   52 ++++----
 src/hb-private.hh                                   |    4 
 src/hb-subset-glyf.cc                               |   38 +++++-
 src/hb-subset-plan.cc                               |   72 ++++++++----
 src/hb-subset-plan.hh                               |    6 +
 src/hb-uniscribe.cc                                 |    2 
 test/api/Makefile.am                                |   21 ---
 test/api/fonts/Roboto-Regular.components.subset.ttf |binary
 test/api/fonts/Roboto-Regular.components.ttf        |binary
 test/api/test-subset-glyf.c                         |   21 +++
 test/api/test-subset-hmtx.c                         |    1 
 17 files changed, 257 insertions(+), 118 deletions(-)

New commits:
commit 97a71102153d28982297a190739c7d82e76b109e
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Sun Feb 18 10:50:24 2018 -0800

    Fix BitScanForward() usage
    
    Should fix Win64 bot.

diff --git a/src/hb-private.hh b/src/hb-private.hh
index 583b5615..daa496e9 100644
--- a/src/hb-private.hh
+++ b/src/hb-private.hh
@@ -458,14 +458,14 @@ _hb_ctz (T v)
   {
     unsigned long where;
     _BitScanForward (&where, v);
-    return 1 + where;
+    return where;
   }
 # if _WIN64
   if (sizeof (T) <= 8)
   {
     unsigned long where;
     _BitScanForward64 (&where, v);
-    return 1 + where;
+    return where;
   }
 # endif
 #endif
commit fe8f40a4180e7b02831a264c0b3c66763156abb6
Merge: cd11107b 21646cc4
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Sun Feb 18 10:45:33 2018 -0800

    Merge branch 'master' into bitops

commit 21646cc4a6160088933774e179df9be4865a9f4b
Author: David Corbett <corbett.dav at husky.neu.edu>
Date:   Fri Feb 16 12:08:55 2018 -0500

    Do not mark the first glyph as unsafe to break
    
    Fixes #791.

diff --git a/src/hb-buffer-private.hh b/src/hb-buffer-private.hh
index a72376de..af4767f5 100644
--- a/src/hb-buffer-private.hh
+++ b/src/hb-buffer-private.hh
@@ -344,8 +344,7 @@ struct hb_buffer_t {
   inline void
   unsafe_to_break_all (void)
   {
-    for (unsigned int i = 0; i < len; i++)
-      info[i].mask |= HB_GLYPH_FLAG_UNSAFE_TO_BREAK;
+    unsafe_to_break_impl (0, len);
   }
   inline void
   safe_to_break_all (void)
diff --git a/src/hb-coretext.cc b/src/hb-coretext.cc
index 752dea8a..aba7cf44 100644
--- a/src/hb-coretext.cc
+++ b/src/hb-coretext.cc
@@ -1244,8 +1244,6 @@ resize_and_retry:
 	pos->x_offset = info->var1.i32;
 	pos->y_offset = info->var2.i32;
 
-	info->mask = HB_GLYPH_FLAG_UNSAFE_TO_BREAK;
-
 	info++, pos++;
       }
     else
@@ -1255,8 +1253,6 @@ resize_and_retry:
 	pos->x_offset = info->var1.i32;
 	pos->y_offset = info->var2.i32;
 
-	info->mask = HB_GLYPH_FLAG_UNSAFE_TO_BREAK;
-
 	info++, pos++;
       }
 
diff --git a/src/hb-directwrite.cc b/src/hb-directwrite.cc
index 5429255a..69a8aa20 100644
--- a/src/hb-directwrite.cc
+++ b/src/hb-directwrite.cc
@@ -878,8 +878,6 @@ retry_getglyphs:
     pos->x_offset =
       x_mult * (isRightToLeft ? -info->var1.i32 : info->var1.i32);
     pos->y_offset = y_mult * info->var2.i32;
-
-    info->mask = HB_GLYPH_FLAG_UNSAFE_TO_BREAK;
   }
 
   if (isRightToLeft)
diff --git a/src/hb-graphite2.cc b/src/hb-graphite2.cc
index 3b55b475..46fe1399 100644
--- a/src/hb-graphite2.cc
+++ b/src/hb-graphite2.cc
@@ -360,7 +360,6 @@ _hb_graphite2_shape (hb_shape_plan_t    *shape_plan,
       hb_glyph_info_t *info = &buffer->info[clusters[i].base_glyph + j];
       info->codepoint = gids[clusters[i].base_glyph + j];
       info->cluster = clusters[i].cluster;
-      info->mask = HB_GLYPH_FLAG_UNSAFE_TO_BREAK;
       info->var1.i32 = clusters[i].advance;     // all glyphs in the cluster get the same advance
     }
   }
diff --git a/src/hb-uniscribe.cc b/src/hb-uniscribe.cc
index 5e05baa8..cd25769d 100644
--- a/src/hb-uniscribe.cc
+++ b/src/hb-uniscribe.cc
@@ -1025,8 +1025,6 @@ retry:
     pos->x_advance = x_mult * (int32_t) info->mask;
     pos->x_offset = x_mult * (backward ? -info->var1.i32 : info->var1.i32);
     pos->y_offset = y_mult * info->var2.i32;
-
-    info->mask = HB_GLYPH_FLAG_UNSAFE_TO_BREAK;
   }
 
   if (backward)
commit 04dedec96b76600eecdb739b72814a4a56b270ae
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Fri Feb 16 18:32:57 2018 -0800

    [test] Remove unused var

diff --git a/test/api/test-subset-hmtx.c b/test/api/test-subset-hmtx.c
index 099b57de..36d7d9e8 100644
--- a/test/api/test-subset-hmtx.c
+++ b/test/api/test-subset-hmtx.c
@@ -37,7 +37,6 @@ static void check_num_hmetrics(hb_face_t *face, uint16_t expected_num_hmetrics)
   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];
commit 181b7471074cc814e0f498fc05fd6850c3f5e403
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Fri Feb 16 17:08:44 2018 -0800

    Update outdated TODO file

diff --git a/TODO b/TODO
index 4f37f605..53ffbe9d 100644
--- a/TODO
+++ b/TODO
@@ -1,24 +1,14 @@
 General fixes:
 =============
 
-- AAT 'morx' implementation.
-
-- Return "safe-to-break" bit from shaping.
-
 - Implement 'rand' feature.
 
-- mask propagation? (when ligation, "or" the masks).
-
 
 API issues:
 ===========
 
 - API to accept a list of languages?
 
-- Add init_func to font_funcs.  Adjust ft.
-
-- 'const' for getter APIs? (use mutable internally)
-
 - Remove hb_ot_shape_glyphs_closure()?
 
 
@@ -39,7 +29,7 @@ API additions
 
 - Add query / enumeration API for aalt-like features?
 
-- SFNT api? get_num_faces? get_table_tags? (there's something in stash)
+- SFNT api? get_num_faces?
 
 - Add segmentation API
 
@@ -50,20 +40,3 @@ hb-view / hb-shape enhancements:
 ===============================
 
 - Add --width, --height, --auto-size, --ink-box, --align, etc?
-
-
-Tests to write:
-==============
-
-- ot-layout enumeration API (needs font)
-
-- Finish test-shape.c, grep for TODO
-
-- Finish test-unicode.c, grep for TODO
-
-- GObject, FreeType, etc
-
-- hb_cache_t and relatives
-
-- hb_feature_to/from_string
-- hb_buffer_[sg]et_contents
commit 6d56db8983e03fbebbeb61282bef8cb1f9abb8e2
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Fri Feb 16 17:02:57 2018 -0800

    [test/api] Distribute all fonts

diff --git a/test/api/Makefile.am b/test/api/Makefile.am
index 358cd57f..ecf44754 100644
--- a/test/api/Makefile.am
+++ b/test/api/Makefile.am
@@ -12,6 +12,8 @@ lib:
 
 EXTRA_DIST += CMakeLists.txt
 
+EXTRA_DIST += fonts
+
 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)
@@ -66,25 +68,6 @@ TEST_PROGS += \
 	$(NULL)
 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 \
-	fonts/MathTestFontFull.otf \
-	fonts/MathTestFontNone.otf \
-	fonts/MathTestFontPartial1.otf \
-	fonts/MathTestFontPartial2.otf \
-	fonts/MathTestFontPartial3.otf \
-	fonts/MathTestFontPartial4.otf \
-	$(NULL)
 endif # HAVE_FREETYPE
 
 endif # HAVE_OT
commit e5ab34fd3a104f7ff2f0b36c66770c88b2ea1051
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Fri Feb 16 16:58:17 2018 -0800

    Misc fixes
    
    Should bring bag djgpp bot.

diff --git a/src/hb-ot-os2-table.hh b/src/hb-ot-os2-table.hh
index 18dc4ab0..2d9d2149 100644
--- a/src/hb-ot-os2-table.hh
+++ b/src/hb-ot-os2-table.hh
@@ -78,7 +78,7 @@ struct os2
   {
     hb_codepoint_t min = -1, max = 0;
 
-    for (int i = 0; i < codepoints.len; i++)
+    for (unsigned int i = 0; i < codepoints.len; i++)
     {
       hb_codepoint_t cp = codepoints[i];
       if (cp < min)
@@ -100,47 +100,47 @@ struct os2
   HBUINT16	version;
 
   /* Version 0 */
-  HBINT16		xAvgCharWidth;
+  HBINT16	xAvgCharWidth;
   HBUINT16	usWeightClass;
   HBUINT16	usWidthClass;
   HBUINT16	fsType;
-  HBINT16		ySubscriptXSize;
-  HBINT16		ySubscriptYSize;
-  HBINT16		ySubscriptXOffset;
-  HBINT16		ySubscriptYOffset;
-  HBINT16		ySuperscriptXSize;
-  HBINT16		ySuperscriptYSize;
-  HBINT16		ySuperscriptXOffset;
-  HBINT16		ySuperscriptYOffset;
-  HBINT16		yStrikeoutSize;
-  HBINT16		yStrikeoutPosition;
-  HBINT16		sFamilyClass;
-  HBUINT8		panose[10];
-  HBUINT32		ulUnicodeRange[4];
+  HBINT16	ySubscriptXSize;
+  HBINT16	ySubscriptYSize;
+  HBINT16	ySubscriptXOffset;
+  HBINT16	ySubscriptYOffset;
+  HBINT16	ySuperscriptXSize;
+  HBINT16	ySuperscriptYSize;
+  HBINT16	ySuperscriptXOffset;
+  HBINT16	ySuperscriptYOffset;
+  HBINT16	yStrikeoutSize;
+  HBINT16	yStrikeoutPosition;
+  HBINT16	sFamilyClass;
+  HBUINT8	panose[10];
+  HBUINT32	ulUnicodeRange[4];
   Tag		achVendID;
   HBUINT16	fsSelection;
   HBUINT16	usFirstCharIndex;
   HBUINT16	usLastCharIndex;
-  HBINT16		sTypoAscender;
-  HBINT16		sTypoDescender;
-  HBINT16		sTypoLineGap;
+  HBINT16	sTypoAscender;
+  HBINT16	sTypoDescender;
+  HBINT16	sTypoLineGap;
   HBUINT16	usWinAscent;
   HBUINT16	usWinDescent;
 
   /* Version 1 */
-  //HBUINT32 ulCodePageRange1;
-  //HBUINT32 ulCodePageRange2;
+  //HBUINT32	ulCodePageRange1;
+  //HBUINT32	ulCodePageRange2;
 
   /* Version 2 */
-  //HBINT16 sxHeight;
-  //HBINT16 sCapHeight;
-  //HBUINT16  usDefaultChar;
-  //HBUINT16  usBreakChar;
-  //HBUINT16  usMaxContext;
+  //HBINT16	sxHeight;
+  //HBINT16	sCapHeight;
+  //HBUINT16	usDefaultChar;
+  //HBUINT16	usBreakChar;
+  //HBUINT16	usMaxContext;
 
   /* Version 5 */
-  //HBUINT16  usLowerOpticalPointSize;
-  //HBUINT16  usUpperOpticalPointSize;
+  //HBUINT16	usLowerOpticalPointSize;
+  //HBUINT16	usUpperOpticalPointSize;
 
   public:
   DEFINE_SIZE_STATIC (78);
diff --git a/src/hb-subset-plan.cc b/src/hb-subset-plan.cc
index d017cca1..b9e299ad 100644
--- a/src/hb-subset-plan.cc
+++ b/src/hb-subset-plan.cc
@@ -130,9 +130,11 @@ _populate_gids_to_retain (hb_face_t *face,
   hb_auto_array_t<unsigned int> bad_indices;
 
   old_gids.alloc (codepoints.len);
-  for (unsigned int i = 0; i < codepoints.len; i++) {
+  for (unsigned int i = 0; i < codepoints.len; i++)
+  {
     hb_codepoint_t gid;
-    if (!cmap.get_nominal_glyph (codepoints[i], &gid)) {
+    if (!cmap.get_nominal_glyph (codepoints[i], &gid))
+    {
       gid = -1;
       *(bad_indices.push ()) = i;
     }
@@ -140,7 +142,8 @@ _populate_gids_to_retain (hb_face_t *face,
   }
 
   /* Generally there shouldn't be any */
-  while (bad_indices.len > 0) {
+  while (bad_indices.len > 0)
+  {
     unsigned int i = bad_indices[bad_indices.len - 1];
     bad_indices.pop ();
     DEBUG_MSG(SUBSET, nullptr, "Drop U+%04X; no gid", codepoints[i]);
@@ -154,18 +157,13 @@ _populate_gids_to_retain (hb_face_t *face,
   hb_set_t * all_gids_to_retain = hb_set_create ();
   _add_gid_and_children (glyf, 0, all_gids_to_retain);
   for (unsigned int i = 0; i < old_gids.len; i++)
-  {
     _add_gid_and_children (glyf, old_gids[i], all_gids_to_retain);
-  }
 
   // Transfer to a sorted list.
   old_gids_sorted.alloc (hb_set_get_population (all_gids_to_retain));
-  unsigned int gid = HB_SET_VALUE_INVALID;
+  hb_codepoint_t gid = HB_SET_VALUE_INVALID;
   while (hb_set_next (all_gids_to_retain, &gid))
-  {
     *(old_gids_sorted.push ()) = gid;
-  }
-  old_gids_sorted.qsort (_hb_codepoint_t_cmp);
 
   glyf.fini ();
   cmap.fini ();
commit df9e22656de746bde65dee775a66f1a80f1c2e32
Author: Garret Rieger <grieger at google.com>
Date:   Fri Feb 16 17:02:51 2018 -0700

    [subset] add a glyf subsetting test for a font with composite glyphs.

diff --git a/test/api/fonts/Roboto-Regular.components.subset.ttf b/test/api/fonts/Roboto-Regular.components.subset.ttf
new file mode 100644
index 00000000..e759d776
Binary files /dev/null and b/test/api/fonts/Roboto-Regular.components.subset.ttf differ
diff --git a/test/api/fonts/Roboto-Regular.components.ttf b/test/api/fonts/Roboto-Regular.components.ttf
new file mode 100644
index 00000000..816e3a28
Binary files /dev/null and b/test/api/fonts/Roboto-Regular.components.ttf differ
diff --git a/test/api/test-subset-glyf.c b/test/api/test-subset-glyf.c
index 3c9d8fe9..96e37bbc 100644
--- a/test/api/test-subset-glyf.c
+++ b/test/api/test-subset-glyf.c
@@ -53,6 +53,26 @@ test_subset_glyf (void)
 }
 
 static void
+test_subset_glyf_with_components (void)
+{
+  hb_face_t *face_components = hb_subset_test_open_font ("fonts/Roboto-Regular.components.ttf");
+  hb_face_t *face_subset = hb_subset_test_open_font ("fonts/Roboto-Regular.components.subset.ttf");
+
+  hb_set_t *codepoints = hb_set_create();
+  hb_set_add (codepoints, 0x1fc);
+  hb_face_t *face_generated_subset = hb_subset_test_create_subset (face_components, codepoints);
+  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'));
+  hb_subset_test_check (face_subset, face_generated_subset, HB_TAG ('m','a','x', 'p'));
+
+  hb_face_destroy (face_generated_subset);
+  hb_face_destroy (face_subset);
+  hb_face_destroy (face_components);
+}
+
+static void
 test_subset_glyf_noop (void)
 {
   hb_face_t *face_abc = hb_subset_test_open_font("fonts/Roboto-Regular.abc.ttf");
@@ -79,6 +99,7 @@ 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);
 
   return hb_test_run();
commit c36d015b0e9c363431cd9d228b776ad419fde474
Author: Garret Rieger <grieger at google.com>
Date:   Fri Feb 16 17:02:15 2018 -0700

    [subset] missing return.

diff --git a/test/api/hb-subset-test.h b/test/api/hb-subset-test.h
index 12d402d3..e8625674 100644
--- a/test/api/hb-subset-test.h
+++ b/test/api/hb-subset-test.h
@@ -86,6 +86,7 @@ hb_subset_test_open_font (const char *font_path)
     return face;
   }
   g_assert (false);
+  return NULL;
 }
 
 static inline hb_face_t *
commit 2130392dcc30784ee34c487ab16316006c91f16d
Author: Garret Rieger <grieger at google.com>
Date:   Fri Feb 16 17:01:00 2018 -0700

    [subset] Add support for updating reference gids in components to their new values.

diff --git a/src/hb-subset-glyf.cc b/src/hb-subset-glyf.cc
index f1bca10d..7dfe0ba4 100644
--- a/src/hb-subset-glyf.cc
+++ b/src/hb-subset-glyf.cc
@@ -28,6 +28,7 @@
 #include "hb-ot-glyf-table.hh"
 #include "hb-set.h"
 #include "hb-subset-glyf.hh"
+#include "hb-subset-plan.hh"
 
 static bool
 _calculate_glyf_and_loca_prime_size (const OT::glyf::accelerator_t &glyf,
@@ -70,16 +71,41 @@ _write_loca_entry (unsigned int id, unsigned int offset, bool is_short, void *lo
   }
 }
 
+static void
+_update_components (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 (!hb_subset_plan_new_gid_for_old_id (plan,
+					      iterator.current->glyphIndex,
+					      &new_gid))
+	continue;
+
+      ((OT::glyf::CompositeGlyphHeader *) iterator.current)->glyphIndex.set (new_gid);
+    } while (iterator.move_to_next());
+  }
+}
+
 static bool
-_write_glyf_and_loca_prime (const OT::glyf::accelerator_t &glyf,
+_write_glyf_and_loca_prime (hb_subset_plan_t              *plan,
+			    const OT::glyf::accelerator_t &glyf,
                             const char                    *glyf_data,
-                            hb_prealloced_array_t<hb_codepoint_t> &glyph_ids,
                             bool                           use_short_loca,
                             int                            glyf_prime_size,
                             char                          *glyf_prime_data /* OUT */,
                             int                            loca_prime_size,
                             char                          *loca_prime_data /* OUT */)
 {
+  hb_prealloced_array_t<hb_codepoint_t> &glyph_ids = plan->gids_to_retain_sorted;
   char *glyf_prime_data_next = glyf_prime_data;
 
   for (unsigned int i = 0; i < glyph_ids.len; i++)
@@ -92,6 +118,7 @@ _write_glyf_and_loca_prime (const OT::glyf::accelerator_t &glyf,
     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);
+    _update_components (plan, glyf_prime_data_next, end_offset - start_offset);
 
     glyf_prime_data_next += length;
   }
@@ -104,7 +131,7 @@ _write_glyf_and_loca_prime (const OT::glyf::accelerator_t &glyf,
 static bool
 _hb_subset_glyf_and_loca (const OT::glyf::accelerator_t  &glyf,
                           const char                     *glyf_data,
-                          hb_prealloced_array_t<hb_codepoint_t>&glyphs_to_retain,
+			  hb_subset_plan_t               *plan,
                           bool                           *use_short_loca,
                           hb_blob_t                     **glyf_prime /* OUT */,
                           hb_blob_t                     **loca_prime /* OUT */)
@@ -112,6 +139,7 @@ _hb_subset_glyf_and_loca (const OT::glyf::accelerator_t  &glyf,
   // 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;
   unsigned int loca_prime_size;
@@ -126,7 +154,7 @@ _hb_subset_glyf_and_loca (const OT::glyf::accelerator_t  &glyf,
 
   char *glyf_prime_data = (char *) malloc (glyf_prime_size);
   char *loca_prime_data = (char *) malloc (loca_prime_size);
-  if (unlikely (!_write_glyf_and_loca_prime (glyf, glyf_data, glyphs_to_retain,
+  if (unlikely (!_write_glyf_and_loca_prime (plan, glyf, glyf_data,
                                              *use_short_loca,
                                              glyf_prime_size, glyf_prime_data,
                                              loca_prime_size, loca_prime_data))) {
@@ -168,7 +196,7 @@ hb_subset_glyf_and_loca (hb_subset_plan_t *plan,
   glyf.init(plan->source);
   bool result = _hb_subset_glyf_and_loca (glyf,
                                           glyf_data,
-                                          plan->gids_to_retain_sorted,
+                                          plan,
                                           use_short_loca,
                                           glyf_prime,
                                           loca_prime);
commit 49544eb860e523838892d6ce88eeca72ffd19da4
Author: Garret Rieger <grieger at google.com>
Date:   Fri Feb 16 16:56:15 2018 -0700

    [subset] Refactor composite glyf iteration code into an Iterator outside of the accelerator.

diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh
index 7af981e4..a73fd4aa 100644
--- a/src/hb-ot-glyf-table.hh
+++ b/src/hb-ot-glyf-table.hh
@@ -133,6 +133,58 @@ struct glyf
       return size;
     }
 
+    struct Iterator
+    {
+      const char *glyph_start;
+      const char *glyph_end;
+      const CompositeGlyphHeader *current;
+
+      inline bool move_to_next ()
+      {
+	if (current->flags & CompositeGlyphHeader::MORE_COMPONENTS)
+	{
+	  const CompositeGlyphHeader *possible =
+	    &StructAfter<CompositeGlyphHeader, CompositeGlyphHeader> (*current);
+	  if (!in_range (possible))
+	    return false;
+	  current = possible;
+	  return true;
+	}
+	return false;
+      }
+
+      inline bool in_range (const CompositeGlyphHeader *composite) const
+      {
+	return (const char *) composite >= glyph_start
+	  && ((const char *) composite + CompositeGlyphHeader::min_size) <= glyph_end
+	  && ((const char *) composite + composite->get_size()) <= glyph_end;
+      }
+    };
+
+    static inline bool get_iterator (const char * glyph_data,
+				     unsigned int length,
+				     CompositeGlyphHeader::Iterator *iterator /* OUT */)
+    {
+      if (length < GlyphHeader::static_size)
+	return false; /* Empty glyph; zero extents. */
+
+      const GlyphHeader &glyph_header = StructAtOffset<GlyphHeader> (glyph_data, 0);
+      if (glyph_header.numberOfContours < 0)
+      {
+        const CompositeGlyphHeader *possible =
+	  &StructAfter<CompositeGlyphHeader, GlyphHeader> (glyph_header);
+
+	iterator->glyph_start = glyph_data;
+	iterator->glyph_end = (const char *) glyph_data + length;
+	if (!iterator->in_range (possible))
+          return false;
+        iterator->current = possible;
+        return true;
+      }
+
+      return false;
+    }
+
     DEFINE_SIZE_MIN (4);
   };
 
@@ -166,69 +218,21 @@ struct glyf
       hb_blob_destroy (glyf_blob);
     }
 
-    inline bool in_table (const char *offset, unsigned int len) const
-    {
-      return (offset - (const char *) glyf_table + len) <= glyf_len;
-    }
-
-    inline bool in_table (const CompositeGlyphHeader *header) const
-    {
-      return in_table ((const char *) header, CompositeGlyphHeader::min_size)
-          && in_table ((const char *) header, header->get_size());
-    }
-
-    inline bool in_glyph (const CompositeGlyphHeader *header,
-                          unsigned int start_offset,
-                          unsigned int end_offset) const
-    {
-      do
-      {
-        unsigned int offset_in_glyf = (const char *) header - (const char*) glyf_table;
-        if (offset_in_glyf < start_offset
-            || offset_in_glyf + header->get_size() > end_offset)
-          return false;
-      } while (next_composite (&header));
-      return true;
-    }
-
     /*
      * Returns true if the referenced glyph is a valid glyph and a composite glyph.
      * If true is returned a pointer to the composite glyph will be written into
      * composite.
      */
-    inline bool get_composite (hb_codepoint_t glyph, const CompositeGlyphHeader ** composite /* OUT */) const
+    inline bool get_composite (hb_codepoint_t glyph,
+			       CompositeGlyphHeader::Iterator *composite /* OUT */) const
     {
       unsigned int start_offset, end_offset;
       if (!get_offsets (glyph, &start_offset, &end_offset))
         return false; /* glyph not found */
 
-      if (end_offset - start_offset < GlyphHeader::static_size)
-	return false; /* Empty glyph; zero extents. */
-
-      const GlyphHeader &glyph_header = StructAtOffset<GlyphHeader> (glyf_table, start_offset);
-      if (glyph_header.numberOfContours < 0) {
-        const CompositeGlyphHeader *possible = &StructAfter<CompositeGlyphHeader, GlyphHeader> (glyph_header);
-        if (!in_table (possible)
-            || !in_glyph (possible, start_offset, end_offset))
-          return false;
-        *composite = possible;
-        return true;
-      }
-
-      return false;
-    }
-
-    inline bool next_composite (const CompositeGlyphHeader ** next /* IN/OUT */) const
-    {
-      if ((*next)->flags & CompositeGlyphHeader::MORE_COMPONENTS)
-      {
-        const CompositeGlyphHeader *possible = &StructAfter<CompositeGlyphHeader, CompositeGlyphHeader> (**next);
-        if (!in_table (possible))
-          return false;
-        *next = possible;
-        return true;
-      }
-      return false;
+      return CompositeGlyphHeader::get_iterator ((const char*) this->glyf_table + start_offset,
+						 end_offset - start_offset,
+						 composite);
     }
 
     inline bool get_offsets (hb_codepoint_t  glyph,
diff --git a/src/hb-subset-plan.cc b/src/hb-subset-plan.cc
index fca65c5a..d017cca1 100644
--- a/src/hb-subset-plan.cc
+++ b/src/hb-subset-plan.cc
@@ -106,13 +106,13 @@ _add_gid_and_children (const OT::glyf::accelerator_t &glyf,
 
   hb_set_add (gids_to_retain, gid);
 
-  const OT::glyf::CompositeGlyphHeader *composite;
+  OT::glyf::CompositeGlyphHeader::Iterator composite;
   if (glyf.get_composite (gid, &composite))
   {
     do
     {
-      _add_gid_and_children (glyf, (hb_codepoint_t) composite->glyphIndex, gids_to_retain);
-    } while (glyf.next_composite (&composite));
+      _add_gid_and_children (glyf, (hb_codepoint_t) composite.current->glyphIndex, gids_to_retain);
+    } while (composite.move_to_next());
   }
 }
 
commit dc6d67df1395faf38d7587b1dd3c6661ee7cd6f0
Author: Garret Rieger <grieger at google.com>
Date:   Fri Feb 16 15:20:14 2018 -0700

    [subset] Use gids_to_retain_sorted to produce old gid -> new gid mapping since it now has the more complete set.

diff --git a/src/hb-subset-plan.cc b/src/hb-subset-plan.cc
index 58b9f1eb..fca65c5a 100644
--- a/src/hb-subset-plan.cc
+++ b/src/hb-subset-plan.cc
@@ -63,10 +63,11 @@ hb_subset_plan_new_gid_for_old_id (hb_subset_plan_t *plan,
                                    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.len; i++) {
-    if (plan->gids_to_retain[i] == old_gid) {
-      // +1: assign new gids from 1..N; 0 is special
-      *new_gid = i + 1;
+  for (unsigned int i = 0; i < plan->gids_to_retain_sorted.len; i++)
+  {
+    if (plan->gids_to_retain_sorted[i] == old_gid)
+    {
+      *new_gid = i;
       return true;
     }
   }
@@ -158,6 +159,7 @@ _populate_gids_to_retain (hb_face_t *face,
   }
 
   // Transfer to a sorted list.
+  old_gids_sorted.alloc (hb_set_get_population (all_gids_to_retain));
   unsigned int gid = HB_SET_VALUE_INVALID;
   while (hb_set_next (all_gids_to_retain, &gid))
   {
commit dcac9fe96429d4e272a3fbd60a6162f988f58f71
Author: Garret Rieger <grieger at google.com>
Date:   Fri Feb 16 11:27:03 2018 -0700

    [subset] Use complex glyph closure to populate gids_to_retain_sorted.

diff --git a/src/hb-subset-plan.cc b/src/hb-subset-plan.cc
index ea46e5f7..58b9f1eb 100644
--- a/src/hb-subset-plan.cc
+++ b/src/hb-subset-plan.cc
@@ -28,6 +28,7 @@
 
 #include "hb-subset-plan.hh"
 #include "hb-ot-cmap-table.hh"
+#include "hb-ot-glyf-table.hh"
 
 static int
 _hb_codepoint_t_cmp (const void *pa, const void *pb)
@@ -94,9 +95,9 @@ _populate_codepoints (hb_set_t *input_codepoints,
 }
 
 static void
-_add_composite_gids (const OT::glyf::accelerator_t &glyf,
-                     hb_codepoint_t gid,
-                     hb_set_t *gids_to_retain)
+_add_gid_and_children (const OT::glyf::accelerator_t &glyf,
+		       hb_codepoint_t gid,
+		       hb_set_t *gids_to_retain)
 {
   if (hb_set_has (gids_to_retain, gid))
     // Already visited this gid, ignore.
@@ -109,8 +110,8 @@ _add_composite_gids (const OT::glyf::accelerator_t &glyf,
   {
     do
     {
-      _add_composite_gids (glyf, (hb_codepoint_t) composite->glyphIndex, gids_to_retain);
-    } while (glyf.next_composite (&composite);
+      _add_gid_and_children (glyf, (hb_codepoint_t) composite->glyphIndex, gids_to_retain);
+    } while (glyf.next_composite (&composite));
   }
 }
 
@@ -121,21 +122,19 @@ _populate_gids_to_retain (hb_face_t *face,
                           hb_prealloced_array_t<hb_codepoint_t>& old_gids_sorted)
 {
   OT::cmap::accelerator_t cmap;
+  OT::glyf::accelerator_t glyf;
   cmap.init (face);
+  glyf.init (face);
 
   hb_auto_array_t<unsigned int> bad_indices;
 
   old_gids.alloc (codepoints.len);
-  bool has_zero = false;
   for (unsigned int i = 0; i < codepoints.len; i++) {
     hb_codepoint_t gid;
     if (!cmap.get_nominal_glyph (codepoints[i], &gid)) {
       gid = -1;
       *(bad_indices.push ()) = i;
     }
-    if (gid == 0) {
-      has_zero = true;
-    }
     *(old_gids.push ()) = gid;
   }
 
@@ -148,19 +147,25 @@ _populate_gids_to_retain (hb_face_t *face,
     old_gids.remove (i);
   }
 
-  // Populate a second glyph id array that is sorted by glyph id
-  // and is gauranteed to contain 0.
-  old_gids_sorted.alloc (old_gids.len + (has_zero ? 0 : 1));
-  for (unsigned int i = 0; i < old_gids.len; i++) {
-    *(old_gids_sorted.push ()) = old_gids[i];
+  // Populate a full set of glyphs to retain by adding all referenced
+  // composite glyphs.
+  // TODO expand with glyphs reached by G*
+  hb_set_t * all_gids_to_retain = hb_set_create ();
+  _add_gid_and_children (glyf, 0, all_gids_to_retain);
+  for (unsigned int i = 0; i < old_gids.len; i++)
+  {
+    _add_gid_and_children (glyf, old_gids[i], all_gids_to_retain);
+  }
+
+  // Transfer to a sorted list.
+  unsigned int gid = HB_SET_VALUE_INVALID;
+  while (hb_set_next (all_gids_to_retain, &gid))
+  {
+    *(old_gids_sorted.push ()) = gid;
   }
-  if (!has_zero)
-    *(old_gids_sorted.push ()) = 0;
   old_gids_sorted.qsort (_hb_codepoint_t_cmp);
 
-  // TODO(Q1) expand with glyphs that make up complex glyphs
-  // TODO expand with glyphs reached by G*
-  //
+  glyf.fini ();
   cmap.fini ();
 }
 
diff --git a/src/hb-subset-plan.hh b/src/hb-subset-plan.hh
index 9a4308f9..b656faa6 100644
--- a/src/hb-subset-plan.hh
+++ b/src/hb-subset-plan.hh
@@ -38,8 +38,14 @@ struct hb_subset_plan_t {
   // TODO(Q1) actual map, drop this crap
   // Look at me ma, I'm a poor mans map codepoint : new gid
   // codepoints is sorted and aligned with gids_to_retain.
+
+  // These first two lists provide a mapping from cp -> gid
+  // As a result it does not list the full set of glyphs to retain.
   hb_prealloced_array_t<hb_codepoint_t> codepoints;
   hb_prealloced_array_t<hb_codepoint_t> gids_to_retain;
+
+  // This list contains the complete set of glyphs to retain and may contain
+  // more glyphs then the lists above.
   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
commit 58a54c9d4f72e228c012451c4469da730742d3d8
Author: Garret Rieger <grieger at google.com>
Date:   Fri Feb 16 11:20:38 2018 -0700

    [subset] add constant to get_composite and in_table methods in hb-ot-glyf-table

diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh
index c6873fb5..7af981e4 100644
--- a/src/hb-ot-glyf-table.hh
+++ b/src/hb-ot-glyf-table.hh
@@ -179,7 +179,7 @@ struct glyf
 
     inline bool in_glyph (const CompositeGlyphHeader *header,
                           unsigned int start_offset,
-                          unsigned int end_offset)
+                          unsigned int end_offset) const
     {
       do
       {
@@ -196,7 +196,7 @@ struct glyf
      * If true is returned a pointer to the composite glyph will be written into
      * composite.
      */
-    inline bool get_composite (hb_codepoint_t glyph, const CompositeGlyphHeader ** composite /* OUT */)
+    inline bool get_composite (hb_codepoint_t glyph, const CompositeGlyphHeader ** composite /* OUT */) const
     {
       unsigned int start_offset, end_offset;
       if (!get_offsets (glyph, &start_offset, &end_offset))
commit 73e1434814eb37005b4159babf972a2743b25700
Author: Garret Rieger <grieger at google.com>
Date:   Thu Feb 15 14:41:56 2018 -0800

    [subset] Add a DFS search to produce a closure of composite glyphs.

diff --git a/src/hb-subset-plan.cc b/src/hb-subset-plan.cc
index 034180a0..ea46e5f7 100644
--- a/src/hb-subset-plan.cc
+++ b/src/hb-subset-plan.cc
@@ -94,6 +94,27 @@ _populate_codepoints (hb_set_t *input_codepoints,
 }
 
 static void
+_add_composite_gids (const OT::glyf::accelerator_t &glyf,
+                     hb_codepoint_t gid,
+                     hb_set_t *gids_to_retain)
+{
+  if (hb_set_has (gids_to_retain, gid))
+    // Already visited this gid, ignore.
+    return;
+
+  hb_set_add (gids_to_retain, gid);
+
+  const OT::glyf::CompositeGlyphHeader *composite;
+  if (glyf.get_composite (gid, &composite))
+  {
+    do
+    {
+      _add_composite_gids (glyf, (hb_codepoint_t) composite->glyphIndex, gids_to_retain);
+    } while (glyf.next_composite (&composite);
+  }
+}
+
+static void
 _populate_gids_to_retain (hb_face_t *face,
                           hb_prealloced_array_t<hb_codepoint_t>& codepoints,
                           hb_prealloced_array_t<hb_codepoint_t>& old_gids,
commit d3684141437fad6ebf5f9945f92125c9a42ea853
Author: Garret Rieger <grieger at google.com>
Date:   Thu Feb 15 14:03:34 2018 -0800

    [subset] add helper methods to glyf accelerator for reading composite glyph information.

diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh
index e6b07a86..c6873fb5 100644
--- a/src/hb-ot-glyf-table.hh
+++ b/src/hb-ot-glyf-table.hh
@@ -81,9 +81,9 @@ struct glyf
   struct GlyphHeader
   {
     HBINT16		numberOfContours;	/* If the number of contours is
-					   * greater than or equal to zero,
-					   * this is a simple glyph; if negative,
-					   * this is a composite glyph. */
+                                                 * greater than or equal to zero,
+                                                 * this is a simple glyph; if negative,
+                                                 * this is a composite glyph. */
     FWORD		xMin;			/* Minimum x for coordinate data. */
     FWORD		yMin;			/* Minimum y for coordinate data. */
     FWORD		xMax;			/* Maximum x for coordinate data. */
@@ -92,6 +92,50 @@ struct glyf
     DEFINE_SIZE_STATIC (10);
   };
 
+  struct CompositeGlyphHeader
+  {
+    static const uint16_t ARG_1_AND_2_ARE_WORDS =      0x0001;
+    static const uint16_t ARGS_ARE_XY_VALUES =         0x0002;
+    static const uint16_t ROUND_XY_TO_GRID =           0x0004;
+    static const uint16_t WE_HAVE_A_SCALE =            0x0008;
+    static const uint16_t MORE_COMPONENTS =            0x0020;
+    static const uint16_t WE_HAVE_AN_X_AND_Y_SCALE =   0x0040;
+    static const uint16_t WE_HAVE_A_TWO_BY_TWO =       0x0080;
+    static const uint16_t WE_HAVE_INSTRUCTIONS =       0x0100;
+    static const uint16_t USE_MY_METRICS =             0x0200;
+    static const uint16_t OVERLAP_COMPOUND =           0x0400;
+    static const uint16_t SCALED_COMPONENT_OFFSET =    0x0800;
+    static const uint16_t UNSCALED_COMPONENT_OFFSET =  0x1000;
+
+    HBUINT16 flags;
+    HBUINT16 glyphIndex;
+
+    inline unsigned int get_size (void) const
+    {
+      unsigned int size = min_size;
+      if (flags & ARG_1_AND_2_ARE_WORDS) {
+        // arg1 and 2 are int16
+        size += 4;
+      } else {
+        // arg1 and 2 are int8
+        size += 2;
+      }
+      if (flags & WE_HAVE_A_SCALE) {
+        // One x 16 bit (scale)
+        size += 2;
+      } else if (flags & WE_HAVE_AN_X_AND_Y_SCALE) {
+        // Two x 16 bit (xscale, yscale)
+        size += 4;
+      } else if (flags & WE_HAVE_A_TWO_BY_TWO) {
+        // Four x 16 bit (xscale, scale01, scale10, yscale)
+        size += 8;
+      }
+      return size;
+    }
+
+    DEFINE_SIZE_MIN (4);
+  };
+
   struct accelerator_t
   {
     inline void init (hb_face_t *face)
@@ -122,6 +166,71 @@ struct glyf
       hb_blob_destroy (glyf_blob);
     }
 
+    inline bool in_table (const char *offset, unsigned int len) const
+    {
+      return (offset - (const char *) glyf_table + len) <= glyf_len;
+    }
+
+    inline bool in_table (const CompositeGlyphHeader *header) const
+    {
+      return in_table ((const char *) header, CompositeGlyphHeader::min_size)
+          && in_table ((const char *) header, header->get_size());
+    }
+
+    inline bool in_glyph (const CompositeGlyphHeader *header,
+                          unsigned int start_offset,
+                          unsigned int end_offset)
+    {
+      do
+      {
+        unsigned int offset_in_glyf = (const char *) header - (const char*) glyf_table;
+        if (offset_in_glyf < start_offset
+            || offset_in_glyf + header->get_size() > end_offset)
+          return false;
+      } while (next_composite (&header));
+      return true;
+    }
+
+    /*
+     * Returns true if the referenced glyph is a valid glyph and a composite glyph.
+     * If true is returned a pointer to the composite glyph will be written into
+     * composite.
+     */
+    inline bool get_composite (hb_codepoint_t glyph, const CompositeGlyphHeader ** composite /* OUT */)
+    {
+      unsigned int start_offset, end_offset;
+      if (!get_offsets (glyph, &start_offset, &end_offset))
+        return false; /* glyph not found */
+
+      if (end_offset - start_offset < GlyphHeader::static_size)
+	return false; /* Empty glyph; zero extents. */
+
+      const GlyphHeader &glyph_header = StructAtOffset<GlyphHeader> (glyf_table, start_offset);
+      if (glyph_header.numberOfContours < 0) {
+        const CompositeGlyphHeader *possible = &StructAfter<CompositeGlyphHeader, GlyphHeader> (glyph_header);
+        if (!in_table (possible)
+            || !in_glyph (possible, start_offset, end_offset))
+          return false;
+        *composite = possible;
+        return true;
+      }
+
+      return false;
+    }
+
+    inline bool next_composite (const CompositeGlyphHeader ** next /* IN/OUT */) const
+    {
+      if ((*next)->flags & CompositeGlyphHeader::MORE_COMPONENTS)
+      {
+        const CompositeGlyphHeader *possible = &StructAfter<CompositeGlyphHeader, CompositeGlyphHeader> (**next);
+        if (!in_table (possible))
+          return false;
+        *next = possible;
+        return true;
+      }
+      return false;
+    }
+
     inline bool get_offsets (hb_codepoint_t  glyph,
                              unsigned int   *start_offset /* OUT */,
                              unsigned int   *end_offset   /* OUT */) const


More information about the HarfBuzz mailing list