From f9b089b695edc89023e3d62700ae68d5648f8494 Mon Sep 17 00:00:00 2001 From: rsheeter Date: Wed, 8 May 2019 14:43:18 -0700 Subject: [PATCH 01/23] [subset] Starting to sketch glyf as iter --- src/hb-ot-glyf-table.hh | 58 ++++++++++++++++++++++++++++++----------- src/hb-subset-plan.hh | 2 ++ src/hb-subset.cc | 2 +- 3 files changed, 46 insertions(+), 16 deletions(-) diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh index f7629eca3..e30bdf799 100644 --- a/src/hb-ot-glyf-table.hh +++ b/src/hb-ot-glyf-table.hh @@ -81,24 +81,52 @@ struct glyf return_trace (true); } - bool subset (hb_subset_plan_t *plan) const + template + bool serialize(hb_serialize_context_t *c, + Iterator it) { - hb_blob_t *glyf_prime = nullptr; - hb_blob_t *loca_prime = nullptr; + TRACE_SERIALIZE (this); - 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; - } - hb_blob_destroy (loca_prime); - hb_blob_destroy (glyf_prime); + // TODO actually copy glyph + // TODO worry about instructions + // TODO capture dest locations for loca - return success; + return_trace (true); + } + + bool subset (hb_subset_context_t *c) const + { + TRACE_SUBSET (this); + + glyf *glyf_prime = c->serializer->start_embed (); + if (unlikely (!glyf_prime)) return_trace (false); + + OT::glyf::accelerator_t glyf; + glyf.init (c->plan->source); + + // stream new-gids => pair of start/end offsets + // can now copy glyph from =>end + // TODO(instructions) + auto it = + + hb_iota (c->plan->num_output_glyphs ()) + | hb_map ([&] (hb_codepoint_t new_gid) { + unsigned int start_offset = 0, end_offset = 0; + // simple case: empty glyph + hb_codepoint_t old_gid; + if (!c->plan->old_gid_for_new_gid (new_gid, &old_gid)) return hb_pair (start_offset, end_offset); + + if (unlikely (!(glyf.get_offsets (old_gid, &start_offset, &end_offset) && + glyf.remove_padding (start_offset, &end_offset)))) + { + // TODO signal irreversible error + return hb_pair (start_offset, end_offset); + } + return hb_pair (start_offset, end_offset); + }); + + glyf_prime->serialize (c->serializer, it); + + return_trace (true); } static bool diff --git a/src/hb-subset-plan.hh b/src/hb-subset-plan.hh index abbab5e22..bdc710101 100644 --- a/src/hb-subset-plan.hh +++ b/src/hb-subset-plan.hh @@ -68,6 +68,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 diff --git a/src/hb-subset.cc b/src/hb-subset.cc index 800bd35c8..99a8747d7 100644 --- a/src/hb-subset.cc +++ b/src/hb-subset.cc @@ -156,7 +156,7 @@ _subset_table (hb_subset_plan_t *plan, bool result = true; switch (tag) { case HB_OT_TAG_glyf: - result = _subset (plan); + result = _subset2 (plan); break; case HB_OT_TAG_hdmx: result = _subset (plan); From fb9bff955a9356b053c5c9bcd7aa9101edb55767 Mon Sep 17 00:00:00 2001 From: rsheeter Date: Wed, 8 May 2019 14:59:03 -0700 Subject: [PATCH 02/23] [subset] Remove subset-glyf; want everything to point to new iter-based edition. Some of the code will resurface as impl builds out. --- src/Makefile.sources | 2 - src/hb-ot-glyf-table.hh | 9 +- src/hb-subset-glyf.cc | 346 ---------------------------------------- src/hb-subset-glyf.hh | 40 ----- src/hb-subset.cc | 1 - 5 files changed, 6 insertions(+), 392 deletions(-) delete mode 100644 src/hb-subset-glyf.cc delete mode 100644 src/hb-subset-glyf.hh diff --git a/src/Makefile.sources b/src/Makefile.sources index e61315be6..e345ffbf3 100644 --- a/src/Makefile.sources +++ b/src/Makefile.sources @@ -241,8 +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 \ diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh index e30bdf799..b54c11fa7 100644 --- a/src/hb-ot-glyf-table.hh +++ b/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 Reiger, 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 { @@ -111,10 +110,14 @@ struct glyf + hb_iota (c->plan->num_output_glyphs ()) | hb_map ([&] (hb_codepoint_t new_gid) { unsigned int start_offset = 0, end_offset = 0; - // simple case: empty glyph + hb_codepoint_t old_gid; + // should never happen, ALL old gids should be mapped if (!c->plan->old_gid_for_new_gid (new_gid, &old_gid)) return hb_pair (start_offset, end_offset); + // being empty is perfectly normal + if (c->plan->is_empty_glyph (old_gid)) return hb_pair (start_offset, end_offset); + if (unlikely (!(glyf.get_offsets (old_gid, &start_offset, &end_offset) && glyf.remove_padding (start_offset, &end_offset)))) { diff --git a/src/hb-subset-glyf.cc b/src/hb-subset-glyf.cc deleted file mode 100644 index 0647ee4e7..000000000 --- a/src/hb-subset-glyf.cc +++ /dev/null @@ -1,346 +0,0 @@ -/* - * 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 *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 *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 (&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 &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 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 (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; -} diff --git a/src/hb-subset-glyf.hh b/src/hb-subset-glyf.hh deleted file mode 100644 index 99cf8f071..000000000 --- a/src/hb-subset-glyf.hh +++ /dev/null @@ -1,40 +0,0 @@ -/* - * 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 */ diff --git a/src/hb-subset.cc b/src/hb-subset.cc index 99a8747d7..c42d97856 100644 --- a/src/hb-subset.cc +++ b/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" From 8f174870e9eed4c47463fdb0d4fe3e22a6f5fdc8 Mon Sep 17 00:00:00 2001 From: rsheeter Date: Wed, 8 May 2019 16:52:00 -0700 Subject: [PATCH 03/23] [subset] Dinner time, checkpoint --- src/hb-ot-glyf-table.hh | 97 ++++++++++++++++++++++++----------------- 1 file changed, 58 insertions(+), 39 deletions(-) diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh index b54c11fa7..2867d0f23 100644 --- a/src/hb-ot-glyf-table.hh +++ b/src/hb-ot-glyf-table.hh @@ -86,9 +86,13 @@ struct glyf { TRACE_SERIALIZE (this); - // TODO actually copy glyph - // TODO worry about instructions - // TODO capture dest locations for loca + // pad glyphs to 2-byte boundaries to permit short loca + HBUINT8 pad; + + it + | hb_apply ( [&] (hb_bytes_t glyph) { + glyph.copy(c); + if (glyph.length % 2) c->extend(pad); + }); return_trace (true); } @@ -103,32 +107,54 @@ struct glyf OT::glyf::accelerator_t glyf; glyf.init (c->plan->source); - // stream new-gids => pair of start/end offsets - // can now copy glyph from =>end - // TODO(instructions) + // stream new-gids => glyph to keep + // if dropping hints the glyph to keep doesn't have them anymore auto it = + hb_iota (c->plan->num_output_glyphs ()) | hb_map ([&] (hb_codepoint_t new_gid) { - unsigned int start_offset = 0, end_offset = 0; - hb_codepoint_t old_gid; - // should never happen, ALL old gids should be mapped - if (!c->plan->old_gid_for_new_gid (new_gid, &old_gid)) return hb_pair (start_offset, end_offset); - - // being empty is perfectly normal - if (c->plan->is_empty_glyph (old_gid)) return hb_pair (start_offset, end_offset); + // should never happen fail, ALL old gids should be mapped + if (!c->plan->old_gid_for_new_gid (new_gid, &old_gid)) return hb_bytes_t (); + unsigned int start_offset, end_offset; if (unlikely (!(glyf.get_offsets (old_gid, &start_offset, &end_offset) && glyf.remove_padding (start_offset, &end_offset)))) { // TODO signal irreversible error - return hb_pair (start_offset, end_offset); + return hb_bytes_t (); } - return hb_pair (start_offset, end_offset); + hb_bytes_t glyph ((const char *) this, end_offset - start_offset); + + // if dropping hints, find hints region and subtract it + if (c->plan->drop_hints) { + unsigned int instruction_length; + if (!glyf.get_instruction_length (glyph, &instruction_length)) + { + // TODO signal irreversible failure + return hb_bytes_t (); + } + glyph = hb_bytes_t (&glyph, glyph.length - instruction_length); + } + + return glyph; }); glyf_prime->serialize (c->serializer, it); + // we know enough to write loca + // TODO calculate size, do we need two or four byte loca entries + unsigned int offset = 0; + HBUINT16 loca_entry; + loca *loca_prime = c->serializer->start_embed (); + + hb_enumerate (it) + | hb_apply ([&] (hb_pair_t p) { + offset += p.second.length; + loca_entry = offset / 2; + c->serializer->embed(loca_entry); + }); + // one for the road + c->serializer->embed(loca_entry); + return_trace (true); } @@ -412,45 +438,40 @@ 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.get_size() < GlyphHeader::static_size) { - *instruction_start = 0; - *instruction_end = 0; - return true; /* Empty glyph; no instructions. */ + *length = 0; + return true; } - const GlyphHeader &glyph_header = StructAtOffset (glyf_table, start_offset); + unsigned int start = glyph.length; + unsigned int end = glyph.length; + const GlyphHeader &glyph_header = StructAtOffset (&glyph, 0); int16_t num_contours = (int16_t) glyph_header.numberOfContours; if (num_contours < 0) { 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 (); + 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; } } 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; @@ -459,15 +480,13 @@ struct glyf const HBUINT16 &instruction_length = StructAtOffset (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 + if (unlikely (end > 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 = end - start; return true; } From 02d4d4f3e67dcc37915bc386d506bb272455ff1e Mon Sep 17 00:00:00 2001 From: rsheeter Date: Wed, 8 May 2019 14:43:18 -0700 Subject: [PATCH 04/23] [subset] Starting to sketch glyf as iter --- src/hb-ot-glyf-table.hh | 58 ++++++++++++++++++++++++++++++----------- src/hb-subset-plan.hh | 2 ++ src/hb-subset.cc | 2 +- 3 files changed, 46 insertions(+), 16 deletions(-) diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh index f7629eca3..e30bdf799 100644 --- a/src/hb-ot-glyf-table.hh +++ b/src/hb-ot-glyf-table.hh @@ -81,24 +81,52 @@ struct glyf return_trace (true); } - bool subset (hb_subset_plan_t *plan) const + template + bool serialize(hb_serialize_context_t *c, + Iterator it) { - hb_blob_t *glyf_prime = nullptr; - hb_blob_t *loca_prime = nullptr; + TRACE_SERIALIZE (this); - 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; - } - hb_blob_destroy (loca_prime); - hb_blob_destroy (glyf_prime); + // TODO actually copy glyph + // TODO worry about instructions + // TODO capture dest locations for loca - return success; + return_trace (true); + } + + bool subset (hb_subset_context_t *c) const + { + TRACE_SUBSET (this); + + glyf *glyf_prime = c->serializer->start_embed (); + if (unlikely (!glyf_prime)) return_trace (false); + + OT::glyf::accelerator_t glyf; + glyf.init (c->plan->source); + + // stream new-gids => pair of start/end offsets + // can now copy glyph from =>end + // TODO(instructions) + auto it = + + hb_iota (c->plan->num_output_glyphs ()) + | hb_map ([&] (hb_codepoint_t new_gid) { + unsigned int start_offset = 0, end_offset = 0; + // simple case: empty glyph + hb_codepoint_t old_gid; + if (!c->plan->old_gid_for_new_gid (new_gid, &old_gid)) return hb_pair (start_offset, end_offset); + + if (unlikely (!(glyf.get_offsets (old_gid, &start_offset, &end_offset) && + glyf.remove_padding (start_offset, &end_offset)))) + { + // TODO signal irreversible error + return hb_pair (start_offset, end_offset); + } + return hb_pair (start_offset, end_offset); + }); + + glyf_prime->serialize (c->serializer, it); + + return_trace (true); } static bool diff --git a/src/hb-subset-plan.hh b/src/hb-subset-plan.hh index abbab5e22..bdc710101 100644 --- a/src/hb-subset-plan.hh +++ b/src/hb-subset-plan.hh @@ -68,6 +68,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 diff --git a/src/hb-subset.cc b/src/hb-subset.cc index f4fc77172..ab9b4d885 100644 --- a/src/hb-subset.cc +++ b/src/hb-subset.cc @@ -156,7 +156,7 @@ _subset_table (hb_subset_plan_t *plan, bool result = true; switch (tag) { case HB_OT_TAG_glyf: - result = _subset (plan); + result = _subset2 (plan); break; case HB_OT_TAG_hdmx: result = _subset2 (plan); From 240bc86e3a0b177ee84ec9c60723304a0cf4c405 Mon Sep 17 00:00:00 2001 From: rsheeter Date: Wed, 8 May 2019 14:59:03 -0700 Subject: [PATCH 05/23] [subset] Remove subset-glyf; want everything to point to new iter-based edition. Some of the code will resurface as impl builds out. --- src/Makefile.sources | 2 - src/hb-ot-glyf-table.hh | 9 +- src/hb-subset-glyf.cc | 346 ---------------------------------------- src/hb-subset-glyf.hh | 40 ----- src/hb-subset.cc | 1 - 5 files changed, 6 insertions(+), 392 deletions(-) delete mode 100644 src/hb-subset-glyf.cc delete mode 100644 src/hb-subset-glyf.hh diff --git a/src/Makefile.sources b/src/Makefile.sources index e61315be6..e345ffbf3 100644 --- a/src/Makefile.sources +++ b/src/Makefile.sources @@ -241,8 +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 \ diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh index e30bdf799..b54c11fa7 100644 --- a/src/hb-ot-glyf-table.hh +++ b/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 Reiger, 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 { @@ -111,10 +110,14 @@ struct glyf + hb_iota (c->plan->num_output_glyphs ()) | hb_map ([&] (hb_codepoint_t new_gid) { unsigned int start_offset = 0, end_offset = 0; - // simple case: empty glyph + hb_codepoint_t old_gid; + // should never happen, ALL old gids should be mapped if (!c->plan->old_gid_for_new_gid (new_gid, &old_gid)) return hb_pair (start_offset, end_offset); + // being empty is perfectly normal + if (c->plan->is_empty_glyph (old_gid)) return hb_pair (start_offset, end_offset); + if (unlikely (!(glyf.get_offsets (old_gid, &start_offset, &end_offset) && glyf.remove_padding (start_offset, &end_offset)))) { diff --git a/src/hb-subset-glyf.cc b/src/hb-subset-glyf.cc deleted file mode 100644 index 0647ee4e7..000000000 --- a/src/hb-subset-glyf.cc +++ /dev/null @@ -1,346 +0,0 @@ -/* - * 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 *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 *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 (&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 &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 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 (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; -} diff --git a/src/hb-subset-glyf.hh b/src/hb-subset-glyf.hh deleted file mode 100644 index 99cf8f071..000000000 --- a/src/hb-subset-glyf.hh +++ /dev/null @@ -1,40 +0,0 @@ -/* - * 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 */ diff --git a/src/hb-subset.cc b/src/hb-subset.cc index ab9b4d885..792a1f6cc 100644 --- a/src/hb-subset.cc +++ b/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" From 0d7fef2d50bba714815c0c13f3b3dd6464710a1d Mon Sep 17 00:00:00 2001 From: rsheeter Date: Wed, 8 May 2019 16:52:00 -0700 Subject: [PATCH 06/23] [subset] Dinner time, checkpoint --- src/hb-ot-glyf-table.hh | 97 ++++++++++++++++++++++++----------------- 1 file changed, 58 insertions(+), 39 deletions(-) diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh index b54c11fa7..2867d0f23 100644 --- a/src/hb-ot-glyf-table.hh +++ b/src/hb-ot-glyf-table.hh @@ -86,9 +86,13 @@ struct glyf { TRACE_SERIALIZE (this); - // TODO actually copy glyph - // TODO worry about instructions - // TODO capture dest locations for loca + // pad glyphs to 2-byte boundaries to permit short loca + HBUINT8 pad; + + it + | hb_apply ( [&] (hb_bytes_t glyph) { + glyph.copy(c); + if (glyph.length % 2) c->extend(pad); + }); return_trace (true); } @@ -103,32 +107,54 @@ struct glyf OT::glyf::accelerator_t glyf; glyf.init (c->plan->source); - // stream new-gids => pair of start/end offsets - // can now copy glyph from =>end - // TODO(instructions) + // stream new-gids => glyph to keep + // if dropping hints the glyph to keep doesn't have them anymore auto it = + hb_iota (c->plan->num_output_glyphs ()) | hb_map ([&] (hb_codepoint_t new_gid) { - unsigned int start_offset = 0, end_offset = 0; - hb_codepoint_t old_gid; - // should never happen, ALL old gids should be mapped - if (!c->plan->old_gid_for_new_gid (new_gid, &old_gid)) return hb_pair (start_offset, end_offset); - - // being empty is perfectly normal - if (c->plan->is_empty_glyph (old_gid)) return hb_pair (start_offset, end_offset); + // should never happen fail, ALL old gids should be mapped + if (!c->plan->old_gid_for_new_gid (new_gid, &old_gid)) return hb_bytes_t (); + unsigned int start_offset, end_offset; if (unlikely (!(glyf.get_offsets (old_gid, &start_offset, &end_offset) && glyf.remove_padding (start_offset, &end_offset)))) { // TODO signal irreversible error - return hb_pair (start_offset, end_offset); + return hb_bytes_t (); } - return hb_pair (start_offset, end_offset); + hb_bytes_t glyph ((const char *) this, end_offset - start_offset); + + // if dropping hints, find hints region and subtract it + if (c->plan->drop_hints) { + unsigned int instruction_length; + if (!glyf.get_instruction_length (glyph, &instruction_length)) + { + // TODO signal irreversible failure + return hb_bytes_t (); + } + glyph = hb_bytes_t (&glyph, glyph.length - instruction_length); + } + + return glyph; }); glyf_prime->serialize (c->serializer, it); + // we know enough to write loca + // TODO calculate size, do we need two or four byte loca entries + unsigned int offset = 0; + HBUINT16 loca_entry; + loca *loca_prime = c->serializer->start_embed (); + + hb_enumerate (it) + | hb_apply ([&] (hb_pair_t p) { + offset += p.second.length; + loca_entry = offset / 2; + c->serializer->embed(loca_entry); + }); + // one for the road + c->serializer->embed(loca_entry); + return_trace (true); } @@ -412,45 +438,40 @@ 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.get_size() < GlyphHeader::static_size) { - *instruction_start = 0; - *instruction_end = 0; - return true; /* Empty glyph; no instructions. */ + *length = 0; + return true; } - const GlyphHeader &glyph_header = StructAtOffset (glyf_table, start_offset); + unsigned int start = glyph.length; + unsigned int end = glyph.length; + const GlyphHeader &glyph_header = StructAtOffset (&glyph, 0); int16_t num_contours = (int16_t) glyph_header.numberOfContours; if (num_contours < 0) { 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 (); + 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; } } 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; @@ -459,15 +480,13 @@ struct glyf const HBUINT16 &instruction_length = StructAtOffset (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 + if (unlikely (end > 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 = end - start; return true; } From ab3fe5de2bbe10fdc13711537f824b62d091f995 Mon Sep 17 00:00:00 2001 From: Rod Sheeter Date: Thu, 9 May 2019 22:12:20 -0700 Subject: [PATCH 07/23] [subset] Glyf by iter now runs but fails tests --- src/hb-ot-glyf-table.hh | 88 +++++++++++++++++++++++++++-------------- 1 file changed, 59 insertions(+), 29 deletions(-) diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh index 2867d0f23..39b712d27 100644 --- a/src/hb-ot-glyf-table.hh +++ b/src/hb-ot-glyf-table.hh @@ -80,20 +80,35 @@ struct glyf return_trace (true); } + template + static void + _write_loca (Iterator it, unsigned size_denom, char * loca_data) + { + unsigned int offset = 0; + + it + | hb_apply ([&] (hb_bytes_t _) { + offset += _.length / size_denom; + EntryType *entry = (EntryType *) loca_data; + *entry = offset; + loca_data += entry->get_size(); + }); + EntryType *entry = (EntryType *) loca_data; + *entry = offset; + } + template bool serialize(hb_serialize_context_t *c, Iterator it) { TRACE_SERIALIZE (this); - // pad glyphs to 2-byte boundaries to permit short loca HBUINT8 pad; + + it | hb_apply ( [&] (hb_bytes_t glyph) { glyph.copy(c); - if (glyph.length % 2) c->extend(pad); + if (glyph.length % 2) c->embed(pad); }); - return_trace (true); } @@ -107,53 +122,68 @@ struct glyf OT::glyf::accelerator_t glyf; glyf.init (c->plan->source); - // stream new-gids => glyph to keep - // if dropping hints the glyph to keep doesn't have them anymore + // make an iterator of per-glyph hb_bytes_t. + // unpadded, hints removed if that was requested. + unsigned int glyf_padded_size = 0; // auto it = - + hb_iota (c->plan->num_output_glyphs ()) + + hb_range (c->plan->num_output_glyphs ()) | hb_map ([&] (hb_codepoint_t new_gid) { hb_codepoint_t old_gid; - // should never happen fail, ALL old gids should be mapped + + // should never fail, ALL old gids should be mapped if (!c->plan->old_gid_for_new_gid (new_gid, &old_gid)) return hb_bytes_t (); unsigned int start_offset, end_offset; if (unlikely (!(glyf.get_offsets (old_gid, &start_offset, &end_offset) && glyf.remove_padding (start_offset, &end_offset)))) { - // TODO signal irreversible error + // TODO signal fatal error return hb_bytes_t (); } hb_bytes_t glyph ((const char *) this, end_offset - start_offset); // if dropping hints, find hints region and subtract it if (c->plan->drop_hints) { - unsigned int instruction_length; - if (!glyf.get_instruction_length (glyph, &instruction_length)) - { - // TODO signal irreversible failure - return hb_bytes_t (); - } - glyph = hb_bytes_t (&glyph, glyph.length - instruction_length); + unsigned int instruction_length; + if (!glyf.get_instruction_length (glyph, &instruction_length)) + { + // TODO signal fatal error + return hb_bytes_t (); + } + glyph = hb_bytes_t (&glyph, glyph.length - instruction_length); } - + glyf_padded_size += glyph.length + glyph.length % 2; return glyph; }); glyf_prime->serialize (c->serializer, it); - // we know enough to write loca - // TODO calculate size, do we need two or four byte loca entries - unsigned int offset = 0; - HBUINT16 loca_entry; - loca *loca_prime = c->serializer->start_embed (); - + hb_enumerate (it) - | hb_apply ([&] (hb_pair_t p) { - offset += p.second.length; - loca_entry = offset / 2; - c->serializer->embed(loca_entry); - }); - // one for the road - c->serializer->embed(loca_entry); + // TODO whats the right way to serialize loca? + // _subset2 will think these bytes are part of glyf if we write to serializer + bool use_short_loca = glyf_padded_size > 131070; + unsigned int loca_prime_size = (c->plan->num_output_glyphs () + 1) * (use_short_loca ? 2 : 4); + char *loca_prime_data = (char *) calloc(1, loca_prime_size); + + if (use_short_loca) + { + _write_loca (it, 2, loca_prime_data); + } + else + { + _write_loca (it, 1, loca_prime_data); + } + + hb_blob_t * loca_blob = hb_blob_create (loca_prime_data, + loca_prime_size, + HB_MEMORY_MODE_READONLY, + loca_prime_data, + free); + if (unlikely (! (c->plan->add_table (HB_OT_TAG_loca, loca_blob) + && _add_head_and_set_loca_version(c->plan, use_short_loca)))) + { + // TODO signal fatal error + return false; + } return_trace (true); } From 14db6512f8dca80a575f468708949346b005834a Mon Sep 17 00:00:00 2001 From: Rod Sheeter Date: Fri, 10 May 2019 09:32:43 -0700 Subject: [PATCH 08/23] [subset] Correct flipped use short computation --- src/hb-ot-glyf-table.hh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh index 39b712d27..6b2a2be7d 100644 --- a/src/hb-ot-glyf-table.hh +++ b/src/hb-ot-glyf-table.hh @@ -103,7 +103,7 @@ struct glyf TRACE_SERIALIZE (this); // pad glyphs to 2-byte boundaries to permit short loca HBUINT8 pad; - + pad = 0; + it | hb_apply ( [&] (hb_bytes_t glyph) { glyph.copy(c); @@ -160,7 +160,7 @@ struct glyf // TODO whats the right way to serialize loca? // _subset2 will think these bytes are part of glyf if we write to serializer - bool use_short_loca = glyf_padded_size > 131070; + bool use_short_loca = glyf_padded_size <= 131070; unsigned int loca_prime_size = (c->plan->num_output_glyphs () + 1) * (use_short_loca ? 2 : 4); char *loca_prime_data = (char *) calloc(1, loca_prime_size); From b77dde8f138442935e5ca99460a520a4117d6dd2 Mon Sep 17 00:00:00 2001 From: Rod Sheeter Date: Fri, 10 May 2019 16:52:19 -0700 Subject: [PATCH 09/23] [subset] Destroy blob --- src/hb-ot-glyf-table.hh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh index 6b2a2be7d..86c43cd4e 100644 --- a/src/hb-ot-glyf-table.hh +++ b/src/hb-ot-glyf-table.hh @@ -182,9 +182,11 @@ struct glyf && _add_head_and_set_loca_version(c->plan, use_short_loca)))) { // TODO signal fatal error + hb_blob_destroy (loca_blob); return false; } + hb_blob_destroy (loca_blob); return_trace (true); } From 3a4c928fcfce5a8c7a56907b9945e87b0ce8e327 Mon Sep 17 00:00:00 2001 From: rsheeter Date: Sat, 11 May 2019 22:06:46 -0700 Subject: [PATCH 10/23] [subset] Fix glyf tests except hint stripping & local test asan --- src/hb-ot-glyf-table.hh | 93 +++++++++++++++++++++++++++---------- test/api/hb-subset-test.h | 2 +- test/api/hb-test.h | 13 ++++++ test/api/test-subset-glyf.c | 8 ++-- 4 files changed, 86 insertions(+), 30 deletions(-) diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh index 86c43cd4e..5087f457b 100644 --- a/src/hb-ot-glyf-table.hh +++ b/src/hb-ot-glyf-table.hh @@ -82,33 +82,54 @@ struct glyf template static void - _write_loca (Iterator it, unsigned size_denom, char * loca_data) + _write_loca (Iterator it, unsigned size_denom, char * dest) { + // write loca[0] through loca[numGlyphs-1] + EntryType * loca_start = (EntryType *) dest; + EntryType * loca_current = loca_start; unsigned int offset = 0; + it - | hb_apply ([&] (hb_bytes_t _) { - offset += _.length / size_denom; - EntryType *entry = (EntryType *) loca_data; - *entry = offset; - loca_data += entry->get_size(); + | hb_apply ([&] (unsigned int padded_size) { + DEBUG_MSG(SUBSET, nullptr, "loca entry %ld offset %d", loca_current - loca_start, offset); + *loca_current = offset / size_denom; + offset += padded_size; + loca_current++; }); - EntryType *entry = (EntryType *) loca_data; - *entry = offset; + // one bonus element so loca[numGlyphs] - loca[numGlyphs -1] is size of last glyph + DEBUG_MSG(SUBSET, nullptr, "loca entry %ld offset %d", loca_current - loca_start, offset); + *loca_current = offset / size_denom; } + // TODO don't pass in plan template bool serialize(hb_serialize_context_t *c, - Iterator it) + Iterator it, + const hb_subset_plan_t *plan) { TRACE_SERIALIZE (this); // pad glyphs to 2-byte boundaries to permit short loca HBUINT8 pad; pad = 0; + it - | hb_apply ( [&] (hb_bytes_t glyph) { - glyph.copy(c); - if (glyph.length % 2) c->embed(pad); + | hb_apply ( [&] (hb_pair_t _) { + const hb_bytes_t& src_glyph = _.first; + unsigned int padded_size = _.second; + hb_bytes_t dest_glyph = src_glyph.copy(c); + unsigned int padding = padded_size - src_glyph.length; + DEBUG_MSG(SUBSET, nullptr, "serialize %d byte glyph, width %d pad %d", src_glyph.length, padded_size, padding); + while (padding > 0) + { + c->embed(pad); + padding--; + } + + _fix_component_gids (plan, dest_glyph); }); + + // Things old impl did we now don't: + // TODO set instruction length to 0 where appropriate + // TODO _remove_composite_instruction_flag + return_trace (true); } @@ -124,8 +145,7 @@ struct glyf // make an iterator of per-glyph hb_bytes_t. // unpadded, hints removed if that was requested. - unsigned int glyf_padded_size = 0; // - auto it = + auto glyphs = + hb_range (c->plan->num_output_glyphs ()) | hb_map ([&] (hb_codepoint_t new_gid) { hb_codepoint_t old_gid; @@ -138,40 +158,43 @@ struct glyf glyf.remove_padding (start_offset, &end_offset)))) { // TODO signal fatal error + DEBUG_MSG(SUBSET, nullptr, "Unable to get offset or remove padding for new_gid %d", new_gid); return hb_bytes_t (); } - hb_bytes_t glyph ((const char *) this, end_offset - start_offset); + hb_bytes_t glyph (((const char *) this) + start_offset, end_offset - start_offset); // if dropping hints, find hints region and subtract it + unsigned int instruction_length = 0; if (c->plan->drop_hints) { - unsigned int instruction_length; if (!glyf.get_instruction_length (glyph, &instruction_length)) { // TODO signal fatal error + DEBUG_MSG(SUBSET, nullptr, "Unable to read instruction length for new_gid %d", new_gid); return hb_bytes_t (); } glyph = hb_bytes_t (&glyph, glyph.length - instruction_length); } - glyf_padded_size += glyph.length + glyph.length % 2; + return glyph; }); - glyf_prime->serialize (c->serializer, it); + auto padded_offsets = + + glyphs + | hb_map ([&] (hb_bytes_t _) { return _.length + _.length % 2; }); + + glyf_prime->serialize (c->serializer, hb_zip (glyphs, padded_offsets), c->plan); // TODO whats the right way to serialize loca? // _subset2 will think these bytes are part of glyf if we write to serializer - bool use_short_loca = glyf_padded_size <= 131070; + unsigned int max_offset = + padded_offsets | hb_reduce (hb_max, 0); + bool use_short_loca = max_offset <= 131070; unsigned int loca_prime_size = (c->plan->num_output_glyphs () + 1) * (use_short_loca ? 2 : 4); char *loca_prime_data = (char *) calloc(1, loca_prime_size); if (use_short_loca) - { - _write_loca (it, 2, loca_prime_data); - } + _write_loca (padded_offsets, 2, loca_prime_data); else - { - _write_loca (it, 1, loca_prime_data); - } + _write_loca (padded_offsets, 1, loca_prime_data); hb_blob_t * loca_blob = hb_blob_create (loca_prime_data, loca_prime_size, @@ -190,6 +213,26 @@ struct glyf return_trace (true); } + static void + _fix_component_gids (const hb_subset_plan_t *plan, + hb_bytes_t glyph) + { + 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 ()); + } + } + static bool _add_head_and_set_loca_version (hb_subset_plan_t *plan, bool use_short_loca) { diff --git a/test/api/hb-subset-test.h b/test/api/hb-subset-test.h index 8f32aee67..de86a6a5b 100644 --- a/test/api/hb-subset-test.h +++ b/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); diff --git a/test/api/hb-test.h b/test/api/hb-test.h index 872f45c4b..fc7dbbb4b 100644 --- a/test/api/hb-test.h +++ b/test/api/hb-test.h @@ -173,6 +173,19 @@ 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 (int i = 0; i < expected_length; i++) + { + int expected = *(raw_expected + i); + int actual = *(raw_actual + i); + if (expected != actual) + { + fprintf(stderr, "+%d %02x != %02x\n", i, expected, actual); + } + + } + } g_assert_cmpint(0, ==, memcmp(raw_expected, raw_actual, expected_length)); } diff --git a/test/api/test-subset-glyf.c b/test/api/test-subset-glyf.c index 4671156f9..bb8746105 100644 --- a/test/api/test-subset-glyf.c +++ b/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); @@ -315,8 +315,8 @@ main (int argc, char **argv) hb_test_add (test_subset_glyf_noop); hb_test_add (test_subset_glyf); hb_test_add (test_subset_glyf_with_input_glyphs); - hb_test_add (test_subset_glyf_strip_hints_simple); - hb_test_add (test_subset_glyf_strip_hints_composite); + //hb_test_add (test_subset_glyf_strip_hints_simple); + //hb_test_add (test_subset_glyf_strip_hints_composite); hb_test_add (test_subset_glyf_strip_hints_invalid); hb_test_add (test_subset_glyf_with_components); hb_test_add (test_subset_glyf_with_gsub); From 9d09ac13a114967576284d0b006a0ac7965d928a Mon Sep 17 00:00:00 2001 From: rsheeter Date: Sat, 11 May 2019 23:16:40 -0700 Subject: [PATCH 11/23] [subset] Tweak hint stripping --- src/hb-ot-glyf-table.hh | 46 +++++++++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh index 5087f457b..c1192f8cd 100644 --- a/src/hb-ot-glyf-table.hh +++ b/src/hb-ot-glyf-table.hh @@ -107,7 +107,7 @@ struct glyf const hb_subset_plan_t *plan) { TRACE_SERIALIZE (this); - // pad glyphs to 2-byte boundaries to permit short loca + HBUINT8 pad; pad = 0; + it @@ -124,10 +124,14 @@ struct glyf } _fix_component_gids (plan, dest_glyph); + if (plan->drop_hints) + { + // we copied the glyph w/o instructions, just need to zero instruction length + _zero_instruction_length (dest_glyph); + } }); // Things old impl did we now don't: - // TODO set instruction length to 0 where appropriate // TODO _remove_composite_instruction_flag return_trace (true); @@ -145,6 +149,9 @@ struct glyf // make an iterator of per-glyph hb_bytes_t. // unpadded, hints removed if that was requested. + + // TODO log shows we redo a bunch of the work here; should sink this at end? + auto glyphs = + hb_range (c->plan->num_output_glyphs ()) | hb_map ([&] (hb_codepoint_t new_gid) { @@ -163,15 +170,16 @@ struct glyf } hb_bytes_t glyph (((const char *) this) + start_offset, end_offset - start_offset); - // if dropping hints, find hints region and subtract it - unsigned int instruction_length = 0; + // if dropping hints, find hints region and chop it off the end if (c->plan->drop_hints) { + unsigned int instruction_length = 0; if (!glyf.get_instruction_length (glyph, &instruction_length)) { // TODO signal fatal error DEBUG_MSG(SUBSET, nullptr, "Unable to read instruction length for new_gid %d", new_gid); return hb_bytes_t (); } + DEBUG_MSG(SUBSET, nullptr, "new_gid %d drop %d instruction bytes from %d byte glyph", new_gid, instruction_length, glyph.length); glyph = hb_bytes_t (&glyph, glyph.length - instruction_length); } @@ -233,6 +241,18 @@ struct glyf } } + static void + _zero_instruction_length (hb_bytes_t glyph) + { + const GlyphHeader &glyph_header = StructAtOffset (&glyph, 0); + int16_t num_contours = (int16_t) glyph_header.numberOfContours; + if (num_contours > 0) + { + const HBUINT16 &instruction_length = StructAtOffset (&glyph, GlyphHeader::static_size + 2 * num_contours); + (HBUINT16 &) instruction_length = 0; + } + } + static bool _add_head_and_set_loca_version (hb_subset_plan_t *plan, bool use_short_loca) { @@ -517,17 +537,18 @@ struct glyf unsigned int * length /* OUT */) const { /* Empty glyph; no instructions. */ - if (glyph.get_size() < GlyphHeader::static_size) + if (glyph.length < GlyphHeader::static_size) { *length = 0; - return true; + // only 0 byte glyphs are healthy when missing GlyphHeader + return glyph.length == 0; } - unsigned int start = glyph.length; - unsigned int end = glyph.length; const GlyphHeader &glyph_header = StructAtOffset (&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; CompositeGlyphHeader::Iterator composite_it; if (unlikely (!CompositeGlyphHeader::get_iterator (&glyph, glyph.length, &composite_it))) return false; const CompositeGlyphHeader *last; @@ -542,6 +563,7 @@ struct glyf DEBUG_MSG(SUBSET, nullptr, "Invalid instruction offset, %d is outside %d byte buffer", start, glyph.length); return false; } + *length = end - start; } else { @@ -552,16 +574,14 @@ struct glyf return false; } - const HBUINT16 &instruction_length = StructAtOffset (glyf_table, instruction_length_offset); - unsigned int start = instruction_length_offset + 2; - unsigned int end = start + (uint16_t) instruction_length; - if (unlikely (end > glyph.length)) // Out of bounds of the current glyph + const HBUINT16 &instruction_length = StructAtOffset (&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; } + *length = (uint16_t) instruction_length; } - *length = end - start; return true; } From 8a84b540c7b850c1fb30d5bc1ffdeb43033be173 Mon Sep 17 00:00:00 2001 From: Rod Sheeter Date: Thu, 16 May 2019 19:14:16 -0700 Subject: [PATCH 12/23] [subset] Tests passing using iterator based glyf --- src/hb-ot-glyf-table.hh | 119 +++++++++++++++++++++++++----------- test/api/test-subset-glyf.c | 4 +- 2 files changed, 87 insertions(+), 36 deletions(-) diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh index c1192f8cd..0dd042744 100644 --- a/src/hb-ot-glyf-table.hh +++ b/src/hb-ot-glyf-table.hh @@ -111,12 +111,14 @@ struct glyf HBUINT8 pad; pad = 0; + it - | hb_apply ( [&] (hb_pair_t _) { - const hb_bytes_t& src_glyph = _.first; + | hb_apply ( [&] (hb_pair_t _) { + const SubsetGlyph& src_glyph = _.first; unsigned int padded_size = _.second; - hb_bytes_t dest_glyph = src_glyph.copy(c); - unsigned int padding = padded_size - src_glyph.length; - DEBUG_MSG(SUBSET, nullptr, "serialize %d byte glyph, width %d pad %d", src_glyph.length, padded_size, padding); + hb_bytes_t dest_glyph = src_glyph.start.copy(c); + src_glyph.end.copy(c); + dest_glyph.length += src_glyph.end.length; + unsigned int padding = padded_size - dest_glyph.length; + DEBUG_MSG(SUBSET, nullptr, "serialize %d byte glyph, width %d pad %d", dest_glyph.length, padded_size, padding); while (padding > 0) { c->embed(pad); @@ -126,14 +128,14 @@ struct glyf _fix_component_gids (plan, dest_glyph); if (plan->drop_hints) { - // we copied the glyph w/o instructions, just need to zero instruction length _zero_instruction_length (dest_glyph); + if (unlikely (!_remove_composite_instruction_flag (dest_glyph))) + { + // TODO signal irreversible failure + } } }); - // Things old impl did we now don't: - // TODO _remove_composite_instruction_flag - return_trace (true); } @@ -150,15 +152,16 @@ struct glyf // make an iterator of per-glyph hb_bytes_t. // unpadded, hints removed if that was requested. - // TODO log shows we redo a bunch of the work here; should sink this at end? - + // TODO hb_sink so we don't redo this work for every + glyphs | ... use. auto glyphs = + hb_range (c->plan->num_output_glyphs ()) | hb_map ([&] (hb_codepoint_t new_gid) { hb_codepoint_t old_gid; + SubsetGlyph subset_glyph; + // should never fail, ALL old gids should be mapped - if (!c->plan->old_gid_for_new_gid (new_gid, &old_gid)) return hb_bytes_t (); + if (!c->plan->old_gid_for_new_gid (new_gid, &old_gid)) return subset_glyph; unsigned int start_offset, end_offset; if (unlikely (!(glyf.get_offsets (old_gid, &start_offset, &end_offset) && @@ -166,29 +169,52 @@ struct glyf { // TODO signal fatal error DEBUG_MSG(SUBSET, nullptr, "Unable to get offset or remove padding for new_gid %d", new_gid); - return hb_bytes_t (); + return subset_glyph; } - hb_bytes_t glyph (((const char *) this) + start_offset, end_offset - start_offset); - - // if dropping hints, find hints region and chop it off the end - if (c->plan->drop_hints) { - unsigned int instruction_length = 0; - if (!glyf.get_instruction_length (glyph, &instruction_length)) - { - // TODO signal fatal error - DEBUG_MSG(SUBSET, nullptr, "Unable to read instruction length for new_gid %d", new_gid); - return hb_bytes_t (); - } - DEBUG_MSG(SUBSET, nullptr, "new_gid %d drop %d instruction bytes from %d byte glyph", new_gid, instruction_length, glyph.length); - glyph = hb_bytes_t (&glyph, glyph.length - instruction_length); + subset_glyph.start = hb_bytes_t (((const char *) this) + start_offset, end_offset - start_offset); + if (subset_glyph.start.length == 0) return subset_glyph; + if (unlikely (subset_glyph.start.length < GlyphHeader::static_size)) + { + // TODO signal fatal error, invalid glyph + DEBUG_MSG(SUBSET, nullptr, "Glyph size smaller than minimum header %d", new_gid); + return subset_glyph; } - return glyph; + if (!c->plan->drop_hints) return subset_glyph; + + unsigned int instruction_length = 0; + if (!glyf.get_instruction_length (subset_glyph.start, &instruction_length)) + { + // TODO signal fatal error + DEBUG_MSG(SUBSET, nullptr, "Unable to read instruction length for new_gid %d", new_gid); + return subset_glyph; + } + DEBUG_MSG(SUBSET, nullptr, "new_gid %d drop %d instruction bytes from %d byte glyph", new_gid, instruction_length, subset_glyph.start.length); + + const GlyphHeader& header = StructAtOffset (&subset_glyph.start, 0); + if (header.numberOfContours < 0) + { + // composite, just chop instructions off the end + subset_glyph.start = hb_bytes_t (&subset_glyph.start, subset_glyph.start.length - instruction_length); + } + else + { + // simple glyph + unsigned start_length = GlyphHeader::static_size + 2 * header.numberOfContours + 2; + subset_glyph.end = hb_bytes_t (&subset_glyph.start + start_length + instruction_length, + subset_glyph.start.length - start_length - instruction_length); + subset_glyph.start = hb_bytes_t (&subset_glyph.start, start_length); + } + + return subset_glyph; }); auto padded_offsets = + glyphs - | hb_map ([&] (hb_bytes_t _) { return _.length + _.length % 2; }); + | hb_map ([&] (SubsetGlyph _) { + unsigned length = _.start.length + _.end.length; + return length + length % 2; + }); glyf_prime->serialize (c->serializer, hb_zip (glyphs, padded_offsets), c->plan); @@ -198,6 +224,7 @@ struct glyf bool use_short_loca = max_offset <= 131070; unsigned int loca_prime_size = (c->plan->num_output_glyphs () + 1) * (use_short_loca ? 2 : 4); char *loca_prime_data = (char *) calloc(1, loca_prime_size); +DEBUG_MSG(SUBSET, nullptr, "calloc %u for loca", loca_prime_size); // TEMPORARY if (use_short_loca) _write_loca (padded_offsets, 2, loca_prime_data); @@ -246,13 +273,29 @@ struct glyf { const GlyphHeader &glyph_header = StructAtOffset (&glyph, 0); int16_t num_contours = (int16_t) glyph_header.numberOfContours; - if (num_contours > 0) - { - const HBUINT16 &instruction_length = StructAtOffset (&glyph, GlyphHeader::static_size + 2 * num_contours); - (HBUINT16 &) instruction_length = 0; - } + if (num_contours <= 0) return; // only for simple glyphs + + const HBUINT16 &instruction_length = StructAtOffset (&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 (&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 (&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) { @@ -271,6 +314,13 @@ struct glyf return success; } + struct SubsetGlyph + { + hb_bytes_t start; + hb_bytes_t end; + }; + + struct GlyphHeader { HBINT16 numberOfContours; /* If the number of contours is @@ -549,6 +599,7 @@ struct glyf { 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 (&glyph, glyph.length, &composite_it))) return false; const CompositeGlyphHeader *last; @@ -557,7 +608,7 @@ struct glyf } while (composite_it.move_to_next ()); if ((uint16_t) last->flags & CompositeGlyphHeader::WE_HAVE_INSTRUCTIONS) - start = ((char *) last - (char *) glyf_table->dataZ.arrayZ) + last->get_size (); + 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 byte buffer", start, glyph.length); diff --git a/test/api/test-subset-glyf.c b/test/api/test-subset-glyf.c index b2d5c2ebe..0bdb1bc33 100644 --- a/test/api/test-subset-glyf.c +++ b/test/api/test-subset-glyf.c @@ -339,8 +339,8 @@ main (int argc, char **argv) hb_test_add (test_subset_glyf_noop); hb_test_add (test_subset_glyf); hb_test_add (test_subset_glyf_with_input_glyphs); - //hb_test_add (test_subset_glyf_strip_hints_simple); - //hb_test_add (test_subset_glyf_strip_hints_composite); + hb_test_add (test_subset_glyf_strip_hints_simple); + hb_test_add (test_subset_glyf_strip_hints_composite); hb_test_add (test_subset_glyf_strip_hints_invalid); hb_test_add (test_subset_glyf_with_components); hb_test_add (test_subset_glyf_with_gsub); From 5cedda5e4a3f726168b87d357aee723e6fd919cd Mon Sep 17 00:00:00 2001 From: Rod Sheeter Date: Thu, 16 May 2019 19:16:52 -0700 Subject: [PATCH 13/23] [subset] Fix null pointer deref, tidy up a bit --- src/hb-ot-glyf-table.hh | 282 ++++++++++++++++++++---------------- test/api/test-subset-glyf.c | 18 +-- 2 files changed, 167 insertions(+), 133 deletions(-) diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh index 0dd042744..c9b9cf499 100644 --- a/src/hb-ot-glyf-table.hh +++ b/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, Garret Reiger, Roderick Sheeter + * Google Author(s): Behdad Esfahbod, Garret Rieger, Roderick Sheeter */ #ifndef HB_OT_GLYF_TABLE_HH @@ -80,6 +80,34 @@ struct glyf return_trace (true); } + template + static void + _add_loca_and_head (hb_subset_plan_t * plan, Iterator padded_offsets) + { + unsigned int max_offset = + + padded_offsets + | hb_reduce (hb_max, 0); + bool use_short_loca = max_offset <= 131070; + unsigned int loca_prime_size = (padded_offsets.len () + 1) * (use_short_loca ? 2 : 4); + char *loca_prime_data = (char *) calloc(1, loca_prime_size); + + if (use_short_loca) + _write_loca (padded_offsets, 2, loca_prime_data); + else + _write_loca (padded_offsets, 1, loca_prime_data); + + hb_blob_t * loca_blob = hb_blob_create (loca_prime_data, + loca_prime_size, + HB_MEMORY_MODE_READONLY, + loca_prime_data, + free); + + plan->add_table (HB_OT_TAG_loca, loca_blob); + _add_head_and_set_loca_version(plan, use_short_loca); + + hb_blob_destroy (loca_blob); + } + template static void _write_loca (Iterator it, unsigned size_denom, char * dest) @@ -100,7 +128,6 @@ struct glyf *loca_current = offset / size_denom; } - // TODO don't pass in plan template bool serialize(hb_serialize_context_t *c, Iterator it, @@ -108,33 +135,8 @@ struct glyf { TRACE_SERIALIZE (this); - HBUINT8 pad; - pad = 0; + it - | hb_apply ( [&] (hb_pair_t _) { - const SubsetGlyph& src_glyph = _.first; - unsigned int padded_size = _.second; - hb_bytes_t dest_glyph = src_glyph.start.copy(c); - src_glyph.end.copy(c); - dest_glyph.length += src_glyph.end.length; - unsigned int padding = padded_size - dest_glyph.length; - DEBUG_MSG(SUBSET, nullptr, "serialize %d byte glyph, width %d pad %d", dest_glyph.length, padded_size, padding); - while (padding > 0) - { - c->embed(pad); - padding--; - } - - _fix_component_gids (plan, dest_glyph); - if (plan->drop_hints) - { - _zero_instruction_length (dest_glyph); - if (unlikely (!_remove_composite_instruction_flag (dest_glyph))) - { - // TODO signal irreversible failure - } - } - }); + | hb_apply ( [&] (const SubsetGlyph& _) { _.serialize (c, plan); }); return_trace (true); } @@ -149,102 +151,35 @@ struct glyf OT::glyf::accelerator_t glyf; glyf.init (c->plan->source); - // make an iterator of per-glyph hb_bytes_t. - // unpadded, hints removed if that was requested. - - // TODO hb_sink so we don't redo this work for every + glyphs | ... use. - auto glyphs = + // 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 glyphs; + hb_range (c->plan->num_output_glyphs ()) | hb_map ([&] (hb_codepoint_t new_gid) { - hb_codepoint_t old_gid; - SubsetGlyph subset_glyph; + subset_glyph.new_gid = new_gid; - // should never fail, ALL old gids should be mapped - if (!c->plan->old_gid_for_new_gid (new_gid, &old_gid)) return subset_glyph; + // should never fail: all old gids should be mapped + if (!c->plan->old_gid_for_new_gid (new_gid, &subset_glyph.old_gid)) return subset_glyph; - unsigned int start_offset, end_offset; - if (unlikely (!(glyf.get_offsets (old_gid, &start_offset, &end_offset) && - glyf.remove_padding (start_offset, &end_offset)))) - { - // TODO signal fatal error - DEBUG_MSG(SUBSET, nullptr, "Unable to get offset or remove padding for new_gid %d", new_gid); - return subset_glyph; - } - subset_glyph.start = hb_bytes_t (((const char *) this) + start_offset, end_offset - start_offset); - if (subset_glyph.start.length == 0) return subset_glyph; - if (unlikely (subset_glyph.start.length < GlyphHeader::static_size)) - { - // TODO signal fatal error, invalid glyph - DEBUG_MSG(SUBSET, nullptr, "Glyph size smaller than minimum header %d", new_gid); - return subset_glyph; - } - - if (!c->plan->drop_hints) return subset_glyph; - - unsigned int instruction_length = 0; - if (!glyf.get_instruction_length (subset_glyph.start, &instruction_length)) - { - // TODO signal fatal error - DEBUG_MSG(SUBSET, nullptr, "Unable to read instruction length for new_gid %d", new_gid); - return subset_glyph; - } - DEBUG_MSG(SUBSET, nullptr, "new_gid %d drop %d instruction bytes from %d byte glyph", new_gid, instruction_length, subset_glyph.start.length); - - const GlyphHeader& header = StructAtOffset (&subset_glyph.start, 0); - if (header.numberOfContours < 0) - { - // composite, just chop instructions off the end - subset_glyph.start = hb_bytes_t (&subset_glyph.start, subset_glyph.start.length - instruction_length); - } - else - { - // simple glyph - unsigned start_length = GlyphHeader::static_size + 2 * header.numberOfContours + 2; - subset_glyph.end = hb_bytes_t (&subset_glyph.start + start_length + instruction_length, - subset_glyph.start.length - start_length - instruction_length); - subset_glyph.start = hb_bytes_t (&subset_glyph.start, start_length); - } + subset_glyph.source_glyph = glyf.bytes_for_glyph ((const char *) this, subset_glyph.old_gid); + if (c->plan->drop_hints) subset_glyph.drop_hints (glyf); + else subset_glyph.dest_start = subset_glyph.source_glyph; return subset_glyph; - }); + }) + | hb_sink (glyphs); - auto padded_offsets = - + glyphs - | hb_map ([&] (SubsetGlyph _) { - unsigned length = _.start.length + _.end.length; - return length + length % 2; - }); + glyf_prime->serialize (c->serializer, hb_iter (glyphs), c->plan); - glyf_prime->serialize (c->serializer, hb_zip (glyphs, padded_offsets), c->plan); + hb_vector_t padded_offsets; + + hb_iter (glyphs) + | hb_map ([&] (const SubsetGlyph& _) { return _.padded_size(); }) + | hb_sink (padded_offsets); - // TODO whats the right way to serialize loca? - // _subset2 will think these bytes are part of glyf if we write to serializer - unsigned int max_offset = + padded_offsets | hb_reduce (hb_max, 0); - bool use_short_loca = max_offset <= 131070; - unsigned int loca_prime_size = (c->plan->num_output_glyphs () + 1) * (use_short_loca ? 2 : 4); - char *loca_prime_data = (char *) calloc(1, loca_prime_size); -DEBUG_MSG(SUBSET, nullptr, "calloc %u for loca", loca_prime_size); // TEMPORARY + _add_loca_and_head (c->plan, hb_iter (padded_offsets)); - if (use_short_loca) - _write_loca (padded_offsets, 2, loca_prime_data); - else - _write_loca (padded_offsets, 1, loca_prime_data); - - hb_blob_t * loca_blob = hb_blob_create (loca_prime_data, - loca_prime_size, - HB_MEMORY_MODE_READONLY, - loca_prime_data, - free); - if (unlikely (! (c->plan->add_table (HB_OT_TAG_loca, loca_blob) - && _add_head_and_set_loca_version(c->plan, use_short_loca)))) - { - // TODO signal fatal error - hb_blob_destroy (loca_blob); - return false; - } - - hb_blob_destroy (loca_blob); return_trace (true); } @@ -259,11 +194,11 @@ DEBUG_MSG(SUBSET, nullptr, "calloc %u for loca", loca_prime_size); // TEMPORARY { do { - hb_codepoint_t new_gid; - if (!plan->new_gid_for_old_gid (iterator.current->glyphIndex, - &new_gid)) + 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; + ((OT::glyf::CompositeGlyphHeader *) iterator.current)->glyphIndex = new_gid; } while (iterator.move_to_next ()); } } @@ -314,13 +249,6 @@ DEBUG_MSG(SUBSET, nullptr, "calloc %u for loca", loca_prime_size); // TEMPORARY return success; } - struct SubsetGlyph - { - hb_bytes_t start; - hb_bytes_t end; - }; - - struct GlyphHeader { HBINT16 numberOfContours; /* If the number of contours is @@ -655,6 +583,25 @@ DEBUG_MSG(SUBSET, nullptr, "calloc %u for loca", loca_prime_size); // TEMPORARY 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; @@ -662,6 +609,93 @@ DEBUG_MSG(SUBSET, nullptr, "calloc %u for loca", loca_prime_size); // TEMPORARY hb_blob_ptr_t 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 (&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 dataZ; /* Glyphs data. */ public: diff --git a/test/api/test-subset-glyf.c b/test/api/test-subset-glyf.c index 0bdb1bc33..af71f27e3 100644 --- a/test/api/test-subset-glyf.c +++ b/test/api/test-subset-glyf.c @@ -190,9 +190,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); @@ -214,9 +214,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); @@ -239,9 +239,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); @@ -296,9 +296,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); @@ -320,9 +320,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); From 3a43603ecea2c349f58396e103a52948776681e0 Mon Sep 17 00:00:00 2001 From: Rod Sheeter Date: Mon, 20 May 2019 20:40:55 -0700 Subject: [PATCH 14/23] [subset] Fix memory leak caused by failure to cleanup glyf accelerator --- src/hb-ot-glyf-table.hh | 46 +++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh index c9b9cf499..4d45a93c7 100644 --- a/src/hb-ot-glyf-table.hh +++ b/src/hb-ot-glyf-table.hh @@ -148,28 +148,11 @@ struct glyf glyf *glyf_prime = c->serializer->start_embed (); if (unlikely (!glyf_prime)) return_trace (false); - OT::glyf::accelerator_t glyf; - glyf.init (c->plan->source); - // 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 glyphs; - + hb_range (c->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 (!c->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 (c->plan->drop_hints) subset_glyph.drop_hints (glyf); - else subset_glyph.dest_start = subset_glyph.source_glyph; - - return subset_glyph; - }) - | hb_sink (glyphs); + _populate_subset_glyphs (c->plan, &glyphs); glyf_prime->serialize (c->serializer, hb_iter (glyphs), c->plan); @@ -183,6 +166,33 @@ struct glyf return_trace (true); } + template + void + _populate_subset_glyphs (const hb_subset_plan_t * plan, + hb_vector_t * 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) From 95445d79be0a79e6e2d384d46819730146d397d8 Mon Sep 17 00:00:00 2001 From: Rod Sheeter Date: Tue, 21 May 2019 11:14:31 -0700 Subject: [PATCH 15/23] [subset] Write loca using more idiomatic harfbuzzese --- src/hb-ot-glyf-table.hh | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh index 4d45a93c7..3addeb63b 100644 --- a/src/hb-ot-glyf-table.hh +++ b/src/hb-ot-glyf-table.hh @@ -80,7 +80,8 @@ struct glyf return_trace (true); } - template + template static void _add_loca_and_head (hb_subset_plan_t * plan, Iterator padded_offsets) { @@ -92,9 +93,9 @@ struct glyf char *loca_prime_data = (char *) calloc(1, loca_prime_size); if (use_short_loca) - _write_loca (padded_offsets, 2, loca_prime_data); + _write_loca (padded_offsets, 1, loca_prime_data); else - _write_loca (padded_offsets, 1, loca_prime_data); + _write_loca (padded_offsets, 0, loca_prime_data); hb_blob_t * loca_blob = hb_blob_create (loca_prime_data, loca_prime_size, @@ -108,26 +109,27 @@ struct glyf hb_blob_destroy (loca_blob); } - template + template static void - _write_loca (Iterator it, unsigned size_denom, char * dest) + _write_loca (Iterator it, unsigned right_shift, char * dest) { - // write loca[0] through loca[numGlyphs-1] - EntryType * loca_start = (EntryType *) dest; - EntryType * loca_current = loca_start; + hb_array_t entries ((EntryType *) dest, it.len () + 1); unsigned int offset = 0; + it - | hb_apply ([&] (unsigned int padded_size) { - DEBUG_MSG(SUBSET, nullptr, "loca entry %ld offset %d", loca_current - loca_start, offset); - *loca_current = offset / size_denom; + | hb_map ([&] (unsigned int padded_size) { + unsigned int result = offset >> right_shift; + DEBUG_MSG(SUBSET, nullptr, "loca entry offset %d shifted %d", offset, result); offset += padded_size; - loca_current++; - }); + return result; + }) + | hb_sink ( hb_iter (entries)); // one bonus element so loca[numGlyphs] - loca[numGlyphs -1] is size of last glyph - DEBUG_MSG(SUBSET, nullptr, "loca entry %ld offset %d", loca_current - loca_start, offset); - *loca_current = offset / size_denom; + entries.arrayZ[entries.length - 1] = offset >> right_shift; + DEBUG_MSG(SUBSET, nullptr, "loca entry offset %d", (int16_t) entries.arrayZ[entries.length - 1]); } + // requires source of SubsetGlyph complains the identifier isn't declared template bool serialize(hb_serialize_context_t *c, Iterator it, @@ -169,7 +171,7 @@ struct glyf template void _populate_subset_glyphs (const hb_subset_plan_t * plan, - hb_vector_t * glyphs /* OUT */) const + hb_vector_t * glyphs /* OUT */) const { OT::glyf::accelerator_t glyf; glyf.init (plan->source); @@ -311,6 +313,7 @@ struct glyf return size; } + // TODO rewrite using new iterator framework if possible struct Iterator { const char *glyph_start; From 349d692b0ee45330220fd3ec9267979d73acd149 Mon Sep 17 00:00:00 2001 From: Rod Sheeter Date: Tue, 21 May 2019 12:38:53 -0700 Subject: [PATCH 16/23] [subset] Iter in and out for loca --- src/hb-ot-glyf-table.hh | 31 ++++++++++++++++--------------- test/api/hb-test.h | 7 ++----- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh index 3addeb63b..a5a81ad35 100644 --- a/src/hb-ot-glyf-table.hh +++ b/src/hb-ot-glyf-table.hh @@ -81,21 +81,23 @@ struct glyf } template - static void + 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); bool use_short_loca = max_offset <= 131070; - unsigned int loca_prime_size = (padded_offsets.len () + 1) * (use_short_loca ? 2 : 4); + unsigned int loca_prime_size = padded_offsets.len () * (use_short_loca ? 2 : 4); char *loca_prime_data = (char *) calloc(1, loca_prime_size); + if (unlikely (!loca_prime_data)) return false; + if (use_short_loca) - _write_loca (padded_offsets, 1, loca_prime_data); + _write_loca (padded_offsets, 1, hb_array_t ((HBUINT16*) loca_prime_data, padded_offsets.len ())); else - _write_loca (padded_offsets, 0, loca_prime_data); + _write_loca (padded_offsets, 0, hb_array_t ((HBUINT32*) loca_prime_data, padded_offsets.len ())); hb_blob_t * loca_blob = hb_blob_create (loca_prime_data, loca_prime_size, @@ -107,14 +109,15 @@ struct glyf _add_head_and_set_loca_version(plan, use_short_loca); hb_blob_destroy (loca_blob); + return true; } - template + template static void - _write_loca (Iterator it, unsigned right_shift, char * dest) + _write_loca (IteratorIn it, unsigned right_shift, IteratorOut dest) { - hb_array_t entries ((EntryType *) dest, it.len () + 1); unsigned int offset = 0; + it | hb_map ([&] (unsigned int padded_size) { @@ -123,10 +126,7 @@ struct glyf offset += padded_size; return result; }) - | hb_sink ( hb_iter (entries)); - // one bonus element so loca[numGlyphs] - loca[numGlyphs -1] is size of last glyph - entries.arrayZ[entries.length - 1] = offset >> right_shift; - DEBUG_MSG(SUBSET, nullptr, "loca entry offset %d", (int16_t) entries.arrayZ[entries.length - 1]); + | hb_sink (dest); } // requires source of SubsetGlyph complains the identifier isn't declared @@ -163,9 +163,10 @@ struct glyf | hb_map ([&] (const SubsetGlyph& _) { return _.padded_size(); }) | hb_sink (padded_offsets); - _add_loca_and_head (c->plan, hb_iter (padded_offsets)); + // loca ends with a final entry == last offset (+0) + padded_offsets << 0; - return_trace (true); + return_trace (c->serializer->check_success (_add_loca_and_head (c->plan, hb_iter (padded_offsets)))); } template diff --git a/test/api/hb-test.h b/test/api/hb-test.h index fc7dbbb4b..59e0a9b36 100644 --- a/test/api/hb-test.h +++ b/test/api/hb-test.h @@ -179,11 +179,8 @@ static inline void hb_test_assert_blobs_equal (hb_blob_t *expected_blob, hb_blob { int expected = *(raw_expected + i); int actual = *(raw_actual + i); - if (expected != actual) - { - fprintf(stderr, "+%d %02x != %02x\n", i, expected, actual); - } - + if (expected != actual) fprintf(stderr, "+%d %02x != %02x\n", i, expected, actual); + else fprintf(stderr, "+%d %02x\n", i, expected); } } g_assert_cmpint(0, ==, memcmp(raw_expected, raw_actual, expected_length)); From 4ea44112b5163591ce0b086e0d13ec368f4f6ddc Mon Sep 17 00:00:00 2001 From: Rod Sheeter Date: Tue, 21 May 2019 13:07:43 -0700 Subject: [PATCH 17/23] [subset] Remove missed reference to hb-subset-glyf, was deleted --- src/Makefile.sources | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Makefile.sources b/src/Makefile.sources index b175d2004..46fbc56f6 100644 --- a/src/Makefile.sources +++ b/src/Makefile.sources @@ -242,7 +242,6 @@ HB_SUBSET_sources = \ hb-subset-cff1.hh \ hb-subset-cff2.cc \ hb-subset-cff2.hh \ - hb-subset-glyf.hh \ hb-subset-input.cc \ hb-subset-input.hh \ hb-subset-plan.cc \ From 14e3b0cf41d9657c39f1f921f7e09a1418fa3278 Mon Sep 17 00:00:00 2001 From: Rod Sheeter Date: Tue, 21 May 2019 20:09:36 -0700 Subject: [PATCH 18/23] [subset] Code review feedback --- src/hb-ot-glyf-table.hh | 53 +++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh index a5a81ad35..2ef0952a5 100644 --- a/src/hb-ot-glyf-table.hh +++ b/src/hb-ot-glyf-table.hh @@ -85,23 +85,22 @@ struct glyf 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); - bool use_short_loca = max_offset <= 131070; - unsigned int loca_prime_size = padded_offsets.len () * (use_short_loca ? 2 : 4); - char *loca_prime_data = (char *) calloc(1, loca_prime_size); + 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_t ((HBUINT16*) loca_prime_data, padded_offsets.len ())); + _write_loca (padded_offsets, 1, hb_array_t ((HBUINT16*) loca_prime_data, num_offsets)); else - _write_loca (padded_offsets, 0, hb_array_t ((HBUINT32*) loca_prime_data, padded_offsets.len ())); + _write_loca (padded_offsets, 0, hb_array_t ((HBUINT32*) loca_prime_data, num_offsets)); hb_blob_t * loca_blob = hb_blob_create (loca_prime_data, - loca_prime_size, - HB_MEMORY_MODE_READONLY, + entry_size * num_offsets, + HB_MEMORY_MODE_WRITABLE, loca_prime_data, free); @@ -112,21 +111,24 @@ struct glyf return true; } - template + hb_requires (hb_is_sink_of (IteratorOut, EntryType))> static void _write_loca (IteratorIn it, unsigned right_shift, IteratorOut dest) { unsigned int offset = 0; + it - | hb_map ([&] (unsigned int padded_size) { - unsigned int result = offset >> right_shift; - DEBUG_MSG(SUBSET, nullptr, "loca entry offset %d shifted %d", offset, result); + | hb_map ([=, &offset] (unsigned int padded_size) { + unsigned result = offset >> right_shift; + DEBUG_MSG(SUBSET, nullptr, "loca entry offset %d", offset); offset += padded_size; return result; }) - | hb_sink (dest); + | hb_sink (dest) + ; + DEBUG_MSG(SUBSET, nullptr, "loca entry offset %d", offset); + dest << (offset >> right_shift); } // requires source of SubsetGlyph complains the identifier isn't declared @@ -138,7 +140,8 @@ struct glyf TRACE_SERIALIZE (this); + it - | hb_apply ( [&] (const SubsetGlyph& _) { _.serialize (c, plan); }); + | hb_apply ([=] (const SubsetGlyph& _) { _.serialize (c, plan); }) + ; return_trace (true); } @@ -148,7 +151,7 @@ struct glyf TRACE_SUBSET (this); glyf *glyf_prime = c->serializer->start_embed (); - if (unlikely (!glyf_prime)) return_trace (false); + if (unlikely (!c->serializer->check_success (glyf_prime))) return_trace (false); // Byte region(s) per glyph to output // unpadded, hints removed if so requested @@ -158,15 +161,12 @@ struct glyf glyf_prime->serialize (c->serializer, hb_iter (glyphs), c->plan); - hb_vector_t padded_offsets; + auto padded_offsets = + hb_iter (glyphs) - | hb_map ([&] (const SubsetGlyph& _) { return _.padded_size(); }) - | hb_sink (padded_offsets); + | hb_map (&SubsetGlyph::padded_size) + ; - // loca ends with a final entry == last offset (+0) - padded_offsets << 0; - - return_trace (c->serializer->check_success (_add_loca_and_head (c->plan, hb_iter (padded_offsets)))); + return_trace (c->serializer->check_success (_add_loca_and_head (c->plan, padded_offsets))); } template @@ -191,7 +191,8 @@ struct glyf return subset_glyph; }) - | hb_sink (glyphs); + | hb_sink (glyphs) + ; glyf.fini(); } From 51a0129f7322e97825455df4eb6eecfea14980f5 Mon Sep 17 00:00:00 2001 From: Rod Sheeter Date: Tue, 21 May 2019 20:12:19 -0700 Subject: [PATCH 19/23] [subset] Thar be comparison of integers of different signs --- test/api/hb-test.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/api/hb-test.h b/test/api/hb-test.h index 59e0a9b36..d44e8da41 100644 --- a/test/api/hb-test.h +++ b/test/api/hb-test.h @@ -175,7 +175,7 @@ static inline void hb_test_assert_blobs_equal (hb_blob_t *expected_blob, hb_blob g_assert_cmpint(expected_length, ==, actual_length); if (memcmp (raw_expected, raw_actual, expected_length) != 0) { - for (int i = 0; i < expected_length; i++) + for (unsigned int i = 0; i < expected_length; i++) { int expected = *(raw_expected + i); int actual = *(raw_actual + i); From 58ce477ac170969430310750b78dcb5f9e3b06a3 Mon Sep 17 00:00:00 2001 From: Rod Sheeter Date: Tue, 21 May 2019 20:22:40 -0700 Subject: [PATCH 20/23] [subset] Report failure more often --- src/hb-ot-glyf-table.hh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh index 2ef0952a5..92382defc 100644 --- a/src/hb-ot-glyf-table.hh +++ b/src/hb-ot-glyf-table.hh @@ -104,11 +104,11 @@ struct glyf loca_prime_data, free); - plan->add_table (HB_OT_TAG_loca, loca_blob); - _add_head_and_set_loca_version(plan, use_short_loca); + 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 true; + return result; } template Date: Fri, 24 May 2019 10:10:12 -0700 Subject: [PATCH 21/23] [subset] Address @behdad review feedback --- src/hb-ot-glyf-table.hh | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh index 92382defc..ee5246ecd 100644 --- a/src/hb-ot-glyf-table.hh +++ b/src/hb-ot-glyf-table.hh @@ -89,14 +89,14 @@ struct glyf 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); + 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_t ((HBUINT16*) loca_prime_data, num_offsets)); + _write_loca (padded_offsets, 1, hb_array_t ((HBUINT16*) loca_prime_data, num_offsets)); else - _write_loca (padded_offsets, 0, hb_array_t ((HBUINT32*) loca_prime_data, num_offsets)); + _write_loca (padded_offsets, 0, hb_array_t ((HBUINT32*) loca_prime_data, num_offsets)); hb_blob_t * loca_blob = hb_blob_create (loca_prime_data, entry_size * num_offsets, @@ -111,24 +111,22 @@ struct glyf return result; } - template + 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) { - unsigned result = offset >> right_shift; - DEBUG_MSG(SUBSET, nullptr, "loca entry offset %d", offset); offset += padded_size; - return result; + DEBUG_MSG(SUBSET, nullptr, "loca entry offset %d", offset); + return offset >> right_shift; }) | hb_sink (dest) ; - DEBUG_MSG(SUBSET, nullptr, "loca entry offset %d", offset); - dest << (offset >> right_shift); } // requires source of SubsetGlyph complains the identifier isn't declared From e66253283385aa67eb9c5ab627139a56f9ae5a71 Mon Sep 17 00:00:00 2001 From: Rod Sheeter Date: Fri, 24 May 2019 10:39:56 -0700 Subject: [PATCH 22/23] [subset] Cppcheck complaints --- test/api/hb-test.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/api/hb-test.h b/test/api/hb-test.h index d44e8da41..b866e442a 100644 --- a/test/api/hb-test.h +++ b/test/api/hb-test.h @@ -179,8 +179,8 @@ static inline void hb_test_assert_blobs_equal (hb_blob_t *expected_blob, hb_blob { int expected = *(raw_expected + i); int actual = *(raw_actual + i); - if (expected != actual) fprintf(stderr, "+%d %02x != %02x\n", i, expected, actual); - else fprintf(stderr, "+%d %02x\n", i, expected); + 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)); From 1197bef26c63ee896bea3fab5788635cb0fc9d18 Mon Sep 17 00:00:00 2001 From: Rod Sheeter Date: Fri, 24 May 2019 10:52:49 -0700 Subject: [PATCH 23/23] [subset] Per code review, use hb_array to avoid duplicated type name --- src/hb-ot-glyf-table.hh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh index ee5246ecd..6113dbcf8 100644 --- a/src/hb-ot-glyf-table.hh +++ b/src/hb-ot-glyf-table.hh @@ -94,9 +94,9 @@ struct glyf if (unlikely (!loca_prime_data)) return false; if (use_short_loca) - _write_loca (padded_offsets, 1, hb_array_t ((HBUINT16*) loca_prime_data, num_offsets)); + _write_loca (padded_offsets, 1, hb_array ((HBUINT16*) loca_prime_data, num_offsets)); else - _write_loca (padded_offsets, 0, hb_array_t ((HBUINT32*) loca_prime_data, num_offsets)); + _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,