From 58fdbd8e5daa5f4a44da45b3093fa7285ec1d5bf Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Sat, 30 Jul 2022 02:05:15 +0000 Subject: [PATCH 01/27] [repacker] begin adding PairPosFormat2 splitting support. --- src/OT/Layout/GPOS/ValueFormat.hh | 9 ++++ src/graph/pairpos-graph.hh | 89 ++++++++++++++++++++++++++++++- 2 files changed, 96 insertions(+), 2 deletions(-) diff --git a/src/OT/Layout/GPOS/ValueFormat.hh b/src/OT/Layout/GPOS/ValueFormat.hh index b29f287bc..8a653e7ee 100644 --- a/src/OT/Layout/GPOS/ValueFormat.hh +++ b/src/OT/Layout/GPOS/ValueFormat.hh @@ -59,6 +59,15 @@ struct ValueFormat : HBUINT16 unsigned int get_len () const { return hb_popcount ((unsigned int) *this); } unsigned int get_size () const { return get_len () * Value::static_size; } + bool has_offsets () const { + unsigned format = *this; + return format & ( + xPlaDevice | + yPlaDevice | + xAdvDevice | + yAdvDevice ); + } + bool apply_value (hb_ot_apply_context_t *c, const void *base, const Value *values, diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index 3ca4fc701..e8dd0d954 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -70,7 +70,7 @@ struct PairPosFormat1 : public OT::Layout::GPOS_impl::PairPosFormat1_3 split_subtables (gsubgpos_graph_context_t& c, unsigned this_index) { - // TODO(garretrieger): implement me! + // TODO(garretrieger): sanitization. + hb_set_t visited; + + const unsigned base_size = OT::Layout::GPOS_impl::PairPosFormat2_4::min_size + + size_of (c, this_index, &coverage) + + size_of (c, this_index, &classDef1) + + size_of (c, this_index, &classDef2); + + const unsigned class1_count = class1Count; + const unsigned class2_count = class2Count; + const unsigned class1_record_size = + class2_count * (valueFormat1.get_size () + valueFormat2.get_size ()); + const unsigned value_1_len = valueFormat1.get_len (); + const unsigned value_2_len = valueFormat2.get_len (); + const unsigned total_value_len = value_1_len + value_2_len; + + bool has_offsets = valueFormat1.has_offsets () || valueFormat2.has_offsets (); + + unsigned accumulated = base_size; + hb_vector_t split_points; + for (unsigned i = 0; i < class1_count; i++) + { + accumulated += class1_record_size; + if (has_offsets) { + for (unsigned j = 0; j < class2_count; j++) + { + unsigned value1_index = total_value_len * (class2_count * i + j); + unsigned value2_index = value1_index + value_1_len; + + accumulated += size_of_value_record_children (c, + this_index, + valueFormat1, + value1_index, + visited); + accumulated += size_of_value_record_children (c, + this_index, + valueFormat2, + value2_index, + visited); + } + } + + // TODO(garretrieger): don't count the size of the largest pairset against the limit, since + // it will be packed last in the order and does not contribute to + // the 64kb limit. + + if (accumulated > (1 << 16)) + { + split_points.push (i); + accumulated = base_size; + visited.clear (); // node sharing isn't allowed between splits. + } + } + + return do_split (c, this_index, split_points); + } + private: + + hb_vector_t do_split (gsubgpos_graph_context_t& c, + unsigned this_index, + hb_vector_t split_points) + { + // TODO(garretrieger): implement me. return hb_vector_t (); } + + unsigned size_of_value_record_children (gsubgpos_graph_context_t& c, + unsigned this_index, + unsigned format, + unsigned value_record_index, + hb_set_t& visited) + { + // TODO(garretrieger): implement me, walk through flags and recurse for each offset + // found. + // Actually may be better to just walk the offsets on the vertex directly. ie. prescan + // all of the links and exclude those for coverage, classDef1/2 and then sort by position. + // then we know how many offsets each valueFormat1 and valueFormat2 consume, then we can just + // grab that many offsets and recurse on each iteration. + return 0; + } + + unsigned size_of (gsubgpos_graph_context_t& c, + unsigned this_index, + const void* offset) const + { + const unsigned id = c.graph.index_for_offset (this_index, offset); + return c.graph.vertices_[id].table_size (); + } }; struct PairPos : public OT::Layout::GPOS_impl::PairPos From 7f4b2037a56609173682902a793efa5607eaa310 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Tue, 2 Aug 2022 18:43:25 +0000 Subject: [PATCH 02/27] [repacker] include size of device tables when determining PairPos2 split points. --- src/OT/Layout/GPOS/ValueFormat.hh | 21 ++++++++---- src/graph/pairpos-graph.hh | 57 ++++++++++++++++++++----------- 2 files changed, 53 insertions(+), 25 deletions(-) diff --git a/src/OT/Layout/GPOS/ValueFormat.hh b/src/OT/Layout/GPOS/ValueFormat.hh index 8a653e7ee..38758a163 100644 --- a/src/OT/Layout/GPOS/ValueFormat.hh +++ b/src/OT/Layout/GPOS/ValueFormat.hh @@ -59,13 +59,22 @@ struct ValueFormat : HBUINT16 unsigned int get_len () const { return hb_popcount ((unsigned int) *this); } unsigned int get_size () const { return get_len () * Value::static_size; } - bool has_offsets () const { + hb_vector_t get_device_table_indices () const { + unsigned i = 0; + hb_vector_t result; unsigned format = *this; - return format & ( - xPlaDevice | - yPlaDevice | - xAdvDevice | - yAdvDevice ); + + if (format & xPlacement) i++; + if (format & yPlacement) i++; + if (format & xAdvance) i++; + if (format & yAdvance) i++; + + if (format & xPlaDevice) result.push (i++); + if (format & yPlaDevice) result.push (i++); + if (format & xAdvDevice) result.push (i++); + if (format & yAdvDevice) result.push (i++); + + return result; } bool apply_value (hb_ot_apply_context_t *c, diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index e8dd0d954..acd6148ea 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -250,8 +250,6 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 split_subtables (gsubgpos_graph_context_t& c, unsigned this_index) { // TODO(garretrieger): sanitization. - hb_set_t visited; - const unsigned base_size = OT::Layout::GPOS_impl::PairPosFormat2_4::min_size + size_of (c, this_index, &coverage) + size_of (c, this_index, &classDef1) @@ -261,31 +259,36 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 split_points; + + hb_hashmap_t device_tables = get_all_device_tables (c, this_index); + hb_vector_t format1_device_table_indices = valueFormat1.get_device_table_indices (); + hb_vector_t format2_device_table_indices = valueFormat2.get_device_table_indices (); + bool has_device_tables = bool(format1_device_table_indices) || bool(format2_device_table_indices); + + hb_set_t visited; for (unsigned i = 0; i < class1_count; i++) { accumulated += class1_record_size; - if (has_offsets) { + if (has_device_tables) { for (unsigned j = 0; j < class2_count; j++) { unsigned value1_index = total_value_len * (class2_count * i + j); unsigned value2_index = value1_index + value_1_len; - accumulated += size_of_value_record_children (c, - this_index, - valueFormat1, + device_tables, + format1_device_table_indices, value1_index, visited); accumulated += size_of_value_record_children (c, - this_index, - valueFormat2, + device_tables, + format2_device_table_indices, value2_index, visited); } @@ -315,19 +318,35 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 (); } + hb_hashmap_t + get_all_device_tables (gsubgpos_graph_context_t& c, + unsigned this_index) const + { + hb_hashmap_t result; + + const auto& o = c.graph.object (this_index); + for (const auto& l : o.real_links) { + result.set ((void*) (((uint8_t*)this) + l.position), l.objidx); + } + + return result; + } + unsigned size_of_value_record_children (gsubgpos_graph_context_t& c, - unsigned this_index, - unsigned format, + const hb_hashmap_t& device_tables, + const hb_vector_t device_table_indices, unsigned value_record_index, hb_set_t& visited) { - // TODO(garretrieger): implement me, walk through flags and recurse for each offset - // found. - // Actually may be better to just walk the offsets on the vertex directly. ie. prescan - // all of the links and exclude those for coverage, classDef1/2 and then sort by position. - // then we know how many offsets each valueFormat1 and valueFormat2 consume, then we can just - // grab that many offsets and recurse on each iteration. - return 0; + unsigned size = 0; + for (unsigned i : device_table_indices) + { + OT::Layout::GPOS_impl::Value* record = &values[value_record_index + i]; + unsigned* obj_idx; + if (!device_tables.has ((void*) record, &obj_idx)) continue; + size += c.graph.find_subgraph_size (*obj_idx, visited); + } + return size; } unsigned size_of (gsubgpos_graph_context_t& c, From ca0df565f73191e624dcb05e9419947ba72ecfc0 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Tue, 2 Aug 2022 20:04:46 +0000 Subject: [PATCH 03/27] [repacker] extract coverage cloning into helper. --- src/graph/coverage-graph.hh | 62 ++++++++++++++++++++ src/graph/pairpos-graph.hh | 110 ++++++++++++++++++++---------------- 2 files changed, 123 insertions(+), 49 deletions(-) diff --git a/src/graph/coverage-graph.hh b/src/graph/coverage-graph.hh index 1d9fd0eb5..c18b4f112 100644 --- a/src/graph/coverage-graph.hh +++ b/src/graph/coverage-graph.hh @@ -56,6 +56,68 @@ struct CoverageFormat2 : public OT::Layout::Common::CoverageFormat2_4sanitize (coverage_v)) + return false; + + auto new_coverage = + + hb_zip (coverage_table->iter (), hb_range ()) + | hb_filter ([&] (hb_pair_t p) { + return p.second >= start && p.second < end; + }) + | hb_map_retains_sorting (hb_first) + ; + + unsigned coverage_prime_id = c.graph.new_node (nullptr, nullptr); + auto& coverage_prime_vertex = c.graph.vertices_[coverage_prime_id]; + if (!make_coverage (c, new_coverage, coverage_prime_id, coverage_size)) + return false; + + auto* coverage_link = c.graph.vertices_[new_parent_id].obj.real_links.push (); + coverage_link->width = SmallTypes::size; + coverage_link->objidx = coverage_prime_id; + coverage_link->position = 2; + coverage_prime_vertex.parents.push (new_parent_id); + + return true; + } + + template + static bool make_coverage (gsubgpos_graph_context_t& c, + It glyphs, + unsigned dest_obj, + unsigned max_size) + { + char* buffer = (char*) hb_calloc (1, max_size); + hb_serialize_context_t serializer (buffer, max_size); + Coverage_serialize (&serializer, glyphs); + serializer.end_serialize (); + if (serializer.in_error ()) + { + hb_free (buffer); + return false; + } + + hb_bytes_t coverage_copy = serializer.copy_bytes (); + c.add_buffer ((char *) coverage_copy.arrayZ); // Give ownership to the context, it will cleanup the buffer. + + auto& obj = c.graph.vertices_[dest_obj].obj; + obj.head = (char *) coverage_copy.arrayZ; + obj.tail = obj.head + coverage_copy.length; + + hb_free (buffer); + return true; + } + bool sanitize (graph_t::vertex_t& vertex) const { int64_t vertex_len = vertex.obj.tail - vertex.obj.head; diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index acd6148ea..dce38cd6b 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -144,7 +144,7 @@ struct PairPosFormat1 : public OT::Layout::GPOS_impl::PairPosFormat1_3sanitize (coverage_v)) - return false; - - auto new_coverage = - + hb_zip (coverage_table->iter (), hb_range ()) - | hb_filter ([&] (hb_pair_t p) { - return p.second >= start && p.second < end; - }) - | hb_map_retains_sorting (hb_first) - ; - - unsigned coverage_prime_id = c.graph.new_node (nullptr, nullptr); - auto& coverage_prime_vertex = c.graph.vertices_[coverage_prime_id]; - if (!make_coverage (c, new_coverage, coverage_prime_id, coverage_size)) + if (!Coverage::clone_coverage (c, + coverage_id, + pair_pos_prime_id, + 2, + start, end)) return -1; - auto* coverage_link = c.graph.vertices_[pair_pos_prime_id].obj.real_links.push (); - coverage_link->width = SmallTypes::size; - coverage_link->objidx = coverage_prime_id; - coverage_link->position = 2; - coverage_prime_vertex.parents.push (pair_pos_prime_id); - return pair_pos_prime_id; } - template - bool make_coverage (gsubgpos_graph_context_t& c, - It glyphs, - unsigned dest_obj, - unsigned max_size) const - { - char* buffer = (char*) hb_calloc (1, max_size); - hb_serialize_context_t serializer (buffer, max_size); - Coverage_serialize (&serializer, glyphs); - serializer.end_serialize (); - if (serializer.in_error ()) - { - hb_free (buffer); - return false; - } - hb_bytes_t coverage_copy = serializer.copy_bytes (); - c.add_buffer ((char *) coverage_copy.arrayZ); // Give ownership to the context, it will cleanup the buffer. - - auto& obj = c.graph.vertices_[dest_obj].obj; - obj.head = (char *) coverage_copy.arrayZ; - obj.tail = obj.head + coverage_copy.length; - - hb_free (buffer); - return true; - } unsigned pair_set_graph_index (gsubgpos_graph_context_t& c, unsigned this_index, unsigned i) const { @@ -310,6 +267,17 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4& device_tables; + const hb_vector_t& format1_device_table_indices; + const hb_vector_t& format2_device_table_indices; + }; + hb_vector_t do_split (gsubgpos_graph_context_t& c, unsigned this_index, hb_vector_t split_points) @@ -318,6 +286,50 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 (); } + unsigned clone_range (split_context& split_context, + unsigned start, unsigned end) const + { + DEBUG_MSG (SUBSET_REPACK, nullptr, + " Cloning PairPosFormat2 (%u) range [%u, %u).", split_context.this_index, start, end); + + unsigned num_records = end - start; + unsigned prime_size = OT::Layout::GPOS_impl::PairPosFormat2_4::min_size + + num_records * split_context.class1_record_size; + + unsigned pair_pos_prime_id = split_context.c.create_node (prime_size); + if (pair_pos_prime_id == (unsigned) -1) return -1; + + PairPosFormat2* pair_pos_prime = + (PairPosFormat2*) split_context.c.graph.object (pair_pos_prime_id).head; + pair_pos_prime->format = this->format; + pair_pos_prime->valueFormat1 = this->valueFormat1; + pair_pos_prime->valueFormat2 = this->valueFormat2; + pair_pos_prime->class1Count = this->class1Count; + pair_pos_prime->class2Count = this->class2Count; + clone_class1_records (split_context, pair_pos_prime, start, end); + + + + // TODO + return -1; + } + + void clone_class1_records (split_context& split_context, + PairPosFormat2* pair_pos_prime, + unsigned start, unsigned end) const + { + // TODO: implement, in addition to copying the records, also move device tables as needed. + } + + bool shrink (gsubgpos_graph_context_t& c, + unsigned this_index, + unsigned count) + { + // TODO + return false; + } + + hb_hashmap_t get_all_device_tables (gsubgpos_graph_context_t& c, unsigned this_index) const From 68b90153eae113589d0562b726d307ba23cac8a9 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Tue, 2 Aug 2022 20:58:35 +0000 Subject: [PATCH 04/27] [repacker] Add class def sanitize and range cloning. --- src/Makefile.sources | 1 + src/graph/classdef-graph.hh | 141 ++++++++++++++++++++++++++++++++++++ src/graph/coverage-graph.hh | 2 +- src/graph/pairpos-graph.hh | 14 +++- src/meson.build | 1 + 5 files changed, 156 insertions(+), 3 deletions(-) create mode 100644 src/graph/classdef-graph.hh diff --git a/src/Makefile.sources b/src/Makefile.sources index a3fb92bac..14c6ed346 100644 --- a/src/Makefile.sources +++ b/src/Makefile.sources @@ -353,6 +353,7 @@ HB_SUBSET_sources = \ graph/gsubgpos-context.cc \ graph/pairpos-graph.hh \ graph/coverage-graph.hh \ + graph/classdef-graph.hh \ graph/pairpos-graph.hh \ graph/serialize.hh \ $(NULL) diff --git a/src/graph/classdef-graph.hh b/src/graph/classdef-graph.hh new file mode 100644 index 000000000..e81cd6ec4 --- /dev/null +++ b/src/graph/classdef-graph.hh @@ -0,0 +1,141 @@ +/* + * Copyright © 2022 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 + */ + +#include "graph.hh" +#include "../hb-ot-layout-common.hh" + +#ifndef GRAPH_CLASSDEF_GRAPH_HH +#define GRAPH_CLASSDEF_GRAPH_HH + +namespace graph { + +struct ClassDefFormat1 : public OT::ClassDefFormat1_3 +{ + bool sanitize (graph_t::vertex_t& vertex) const + { + int64_t vertex_len = vertex.obj.tail - vertex.obj.head; + constexpr unsigned min_size = OT::ClassDefFormat1_3::min_size; + if (vertex_len < min_size) return false; + return vertex_len >= min_size + classValue.get_size () - classValue.len.get_size (); + } +}; + +struct ClassDefFormat2 : public OT::ClassDefFormat2_4 +{ + bool sanitize (graph_t::vertex_t& vertex) const + { + int64_t vertex_len = vertex.obj.tail - vertex.obj.head; + constexpr unsigned min_size = OT::ClassDefFormat2_4::min_size; + if (vertex_len < min_size) return false; + return vertex_len >= min_size + rangeRecord.get_size () - rangeRecord.len.get_size (); + } +}; + +struct ClassDef : public OT::ClassDef +{ + template + static bool clone_class_def (gsubgpos_graph_context_t& c, + unsigned class_def_id, + unsigned new_parent_id, + unsigned link_position, + It glyphs) + { + unsigned class_def_size = c.graph.vertices_[class_def_id].table_size (); + auto& class_def_v = c.graph.vertices_[class_def_id]; + ClassDef* class_def_table = (ClassDef*) class_def_v.obj.head; + if (!class_def_table->sanitize (class_def_v)) + return false; + + auto new_class_def = + + glyphs + | hb_map_retains_sorting ([&] (hb_codepoint_t gid) { + return hb_pair (gid, class_def_table[gid]); + }) + ; + + unsigned class_def_prime_id = c.graph.new_node (nullptr, nullptr); + auto& class_def_prime_vertex = c.graph.vertices_[class_def_prime_id]; + if (!make_class_def (c, new_class_def, class_def_prime_id, class_def_size)) + return false; + + auto* class_def_link = c.graph.vertices_[new_parent_id].obj.real_links.push (); + class_def_link->width = SmallTypes::size; + class_def_link->objidx = class_def_prime_id; + class_def_link->position = link_position; + class_def_prime_vertex.parents.push (new_parent_id); + + return true; + } + + template + static bool make_class_def (gsubgpos_graph_context_t& c, + It glyph_and_class, + unsigned dest_obj, + unsigned max_size) + { + char* buffer = (char*) hb_calloc (1, max_size); + hb_serialize_context_t serializer (buffer, max_size); + ClassDef_serialize (&serializer, glyph_and_class); + serializer.end_serialize (); + if (serializer.in_error ()) + { + hb_free (buffer); + return false; + } + + hb_bytes_t class_def_copy = serializer.copy_bytes (); + c.add_buffer ((char *) class_def_copy.arrayZ); // Give ownership to the context, it will cleanup the buffer. + + auto& obj = c.graph.vertices_[dest_obj].obj; + obj.head = (char *) class_def_copy.arrayZ; + obj.tail = obj.head + class_def_copy.length; + + hb_free (buffer); + return true; + } + + bool sanitize (graph_t::vertex_t& vertex) const + { + int64_t vertex_len = vertex.obj.tail - vertex.obj.head; + if (vertex_len < OT::ClassDef::min_size) return false; + switch (u.format) + { + case 1: return ((ClassDefFormat1*)this)->sanitize (vertex); + case 2: return ((ClassDefFormat2*)this)->sanitize (vertex); +#ifndef HB_NO_BORING_EXPANSION + // Not currently supported + case 3: + case 4: +#endif + default: return false; + } + } +}; + + +} + +#endif // GRAPH_CLASSDEF_GRAPH_HH diff --git a/src/graph/coverage-graph.hh b/src/graph/coverage-graph.hh index c18b4f112..28ceed945 100644 --- a/src/graph/coverage-graph.hh +++ b/src/graph/coverage-graph.hh @@ -85,7 +85,7 @@ struct Coverage : public OT::Layout::Common::Coverage auto* coverage_link = c.graph.vertices_[new_parent_id].obj.real_links.push (); coverage_link->width = SmallTypes::size; coverage_link->objidx = coverage_prime_id; - coverage_link->position = 2; + coverage_link->position = link_position; coverage_prime_vertex.parents.push (new_parent_id); return true; diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index dce38cd6b..d52d3bb09 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -28,6 +28,7 @@ #define GRAPH_PAIRPOS_GRAPH_HH #include "coverage-graph.hh" +#include "classdef-graph.hh" #include "../OT/Layout/GPOS/PairPos.hh" #include "../OT/Layout/GPOS/PosLookupSubTable.hh" @@ -308,10 +309,19 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4class2Count = this->class2Count; clone_class1_records (split_context, pair_pos_prime, start, end); + unsigned coverage_id = + split_context.c.graph.index_for_offset (split_context.this_index, &coverage); + if (!Coverage::clone_coverage (split_context.c, + coverage_id, + pair_pos_prime_id, + 2, + start, end)) + return -1; + // TODO: class def 1 (clone_classdef (start, end)) + // TODO: class def 2 (just link to existing) - // TODO - return -1; + return -1; // TODO } void clone_class1_records (split_context& split_context, diff --git a/src/meson.build b/src/meson.build index 1f1105cae..2118eb57c 100644 --- a/src/meson.build +++ b/src/meson.build @@ -350,6 +350,7 @@ hb_subset_sources = files( 'graph/gsubgpos-graph.hh', 'graph/pairpos-graph.hh', 'graph/coverage-graph.hh', + 'graph/classdef-graph.hh', 'hb-subset.cc', 'hb-subset.hh', ) From 22eae32b3b5fc901929d9a27a332b03d7b4ef656 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Tue, 2 Aug 2022 21:04:38 +0000 Subject: [PATCH 05/27] [repacker] add classDef1 clone_range to PairPosFormat2 split. --- src/graph/classdef-graph.hh | 4 ++-- src/graph/coverage-graph.hh | 16 ++++++++-------- src/graph/pairpos-graph.hh | 23 +++++++++++++++++------ 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/src/graph/classdef-graph.hh b/src/graph/classdef-graph.hh index e81cd6ec4..c355b9732 100644 --- a/src/graph/classdef-graph.hh +++ b/src/graph/classdef-graph.hh @@ -72,7 +72,7 @@ struct ClassDef : public OT::ClassDef auto new_class_def = + glyphs | hb_map_retains_sorting ([&] (hb_codepoint_t gid) { - return hb_pair (gid, class_def_table[gid]); + return hb_pair (gid, class_def_table->get_class (gid)); }) ; @@ -98,7 +98,7 @@ struct ClassDef : public OT::ClassDef { char* buffer = (char*) hb_calloc (1, max_size); hb_serialize_context_t serializer (buffer, max_size); - ClassDef_serialize (&serializer, glyph_and_class); + OT::ClassDef_serialize (&serializer, glyph_and_class); serializer.end_serialize (); if (serializer.in_error ()) { diff --git a/src/graph/coverage-graph.hh b/src/graph/coverage-graph.hh index 28ceed945..a5e5e1491 100644 --- a/src/graph/coverage-graph.hh +++ b/src/graph/coverage-graph.hh @@ -56,18 +56,18 @@ struct CoverageFormat2 : public OT::Layout::Common::CoverageFormat2_4sanitize (coverage_v)) - return false; + return nullptr; auto new_coverage = + hb_zip (coverage_table->iter (), hb_range ()) @@ -80,7 +80,7 @@ struct Coverage : public OT::Layout::Common::Coverage unsigned coverage_prime_id = c.graph.new_node (nullptr, nullptr); auto& coverage_prime_vertex = c.graph.vertices_[coverage_prime_id]; if (!make_coverage (c, new_coverage, coverage_prime_id, coverage_size)) - return false; + return nullptr; auto* coverage_link = c.graph.vertices_[new_parent_id].obj.real_links.push (); coverage_link->width = SmallTypes::size; @@ -88,7 +88,7 @@ struct Coverage : public OT::Layout::Common::Coverage coverage_link->position = link_position; coverage_prime_vertex.parents.push (new_parent_id); - return true; + return (Coverage*) coverage_prime_vertex.obj.head; } template diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index d52d3bb09..245d519ec 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -311,14 +311,25 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4iter ())) return -1; - // TODO: class def 1 (clone_classdef (start, end)) // TODO: class def 2 (just link to existing) return -1; // TODO From 9f2a44640c228e6e304ea81a56f8323c6fc67cf9 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Tue, 2 Aug 2022 21:47:53 +0000 Subject: [PATCH 06/27] [repack] implement device table transfer for PairPosFormat2. --- src/graph/pairpos-graph.hh | 82 +++++++++++++++++++++++++++++++++++--- 1 file changed, 77 insertions(+), 5 deletions(-) diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index 245d519ec..ae18fbd03 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -273,6 +273,9 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4& device_tables; const hb_vector_t& format1_device_table_indices; @@ -307,7 +310,10 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4valueFormat2 = this->valueFormat2; pair_pos_prime->class1Count = this->class1Count; pair_pos_prime->class2Count = this->class2Count; - clone_class1_records (split_context, pair_pos_prime, start, end); + clone_class1_records (split_context, + pair_pos_prime_id, + start, + end); unsigned coverage_id = split_context.c.graph.index_for_offset (split_context.this_index, &coverage); @@ -330,16 +336,82 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4iter ())) return -1; - // TODO: class def 2 (just link to existing) + // classDef2 + unsigned class_def_2_id = + split_context.c.graph.index_for_offset (split_context.this_index, &classDef2); + auto* class_def_link = split_context.c.graph.vertices_[pair_pos_prime_id].obj.real_links.push (); + class_def_link->width = SmallTypes::size; + class_def_link->objidx = class_def_2_id; + class_def_link->position = 10; + split_context.c.graph.vertices_[class_def_2_id].parents.push (pair_pos_prime_id); - return -1; // TODO + return pair_pos_prime_id; } void clone_class1_records (split_context& split_context, - PairPosFormat2* pair_pos_prime, + unsigned pair_pos_prime_id, unsigned start, unsigned end) const { - // TODO: implement, in addition to copying the records, also move device tables as needed. + PairPosFormat2* pair_pos_prime = + (PairPosFormat2*) split_context.c.graph.object (pair_pos_prime_id).head; + + char* start_addr = ((char*)&values[0]) + start * split_context.class1_record_size; + unsigned num_records = end - start; + memcpy (&pair_pos_prime->values[0], + start_addr, + num_records * split_context.class1_record_size); + + if (!split_context.format1_device_table_indices + && !split_context.format2_device_table_indices) + // No device tables to move over. + return; + + unsigned class2_count = class2Count; + for (unsigned i = start; i < end; i++) + { + for (unsigned j = 0; j < class2_count; j++) + { + unsigned value1_index = split_context.value_record_len * (class2_count * i + j); + unsigned value2_index = value1_index + split_context.value1_record_len; + + unsigned new_value1_index = split_context.value_record_len * (class2_count * (i - start) + j); + unsigned new_value2_index = new_value1_index + split_context.value1_record_len; + + transfer_device_tables (split_context, + pair_pos_prime_id, + split_context.format1_device_table_indices, + value1_index, + new_value1_index); + + transfer_device_tables (split_context, + pair_pos_prime_id, + split_context.format2_device_table_indices, + value2_index, + new_value2_index); + } + } + } + + void transfer_device_tables (split_context& split_context, + unsigned pair_pos_prime_id, + const hb_vector_t& device_table_indices, + unsigned old_value_record_index, + unsigned new_value_record_index) const + { + PairPosFormat2* pair_pos_prime = + (PairPosFormat2*) split_context.c.graph.object (pair_pos_prime_id).head; + + for (unsigned i : device_table_indices) + { + OT::Offset16* record = (OT::Offset16*) &values[old_value_record_index + i]; + if (!split_context.device_tables.has ((void*) record)) continue; + + split_context.c.graph.move_child ( + split_context.this_index, + record, + pair_pos_prime_id, + (OT::Offset16*) &pair_pos_prime->values[new_value_record_index + i]); + } } bool shrink (gsubgpos_graph_context_t& c, From f43055f35ac3255589c842288dd4291d89b68e9c Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Tue, 2 Aug 2022 22:16:29 +0000 Subject: [PATCH 07/27] [repacker] Implement PairPosFormat2::shrink. --- src/graph/pairpos-graph.hh | 56 ++++++++++++++++++++++++++++++++++---- 1 file changed, 51 insertions(+), 5 deletions(-) diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index ae18fbd03..96d325f69 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -414,14 +414,60 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4= old_count) + return true; + class1Count = count; + split_context.c.graph.vertices_[split_context.this_index].obj.tail -= + (old_count - count) * split_context.class1_record_size; + + unsigned coverage_id = + split_context.c.graph.index_for_offset (split_context.this_index, &coverage); + auto& coverage_v = split_context.c.graph.vertices_[coverage_id]; + Coverage* coverage_table = (Coverage*) coverage_v.obj.head; + if (!coverage_table->sanitize (coverage_v)) + return false; + + auto new_coverage = + + hb_zip (coverage_table->iter (), hb_range ()) + | hb_filter ([&] (hb_pair_t p) { + return p.second < count; + }) + | hb_map_retains_sorting (hb_first) + ; + + if (!Coverage::make_coverage (split_context.c, new_coverage, coverage_id, coverage_v.table_size ())) + return false; + + // classDef1 + coverage_table = (Coverage*) coverage_v.obj.head; // get the new table + unsigned class_def_id = split_context.c.graph.index_for_offset (split_context.this_index, + &classDef1); + auto& class_def_v = split_context.c.graph.vertices_[class_def_id]; + ClassDef* class_def_table = (ClassDef*) class_def_v.obj.head; + if (!class_def_table->sanitize (class_def_v)) + return false; + + auto new_class_def = + + coverage_table->iter () + | hb_map_retains_sorting ([&] (hb_codepoint_t gid) { + return hb_pair (gid, class_def_table->get_class (gid)); + }) + ; + + return ClassDef::make_class_def (split_context.c, + new_class_def, + class_def_id, + class_def_v.table_size ()); + } hb_hashmap_t get_all_device_tables (gsubgpos_graph_context_t& c, From 65ed82fde5ad413ebbfb214a692b4c671f49d097 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Tue, 2 Aug 2022 22:22:42 +0000 Subject: [PATCH 08/27] [repacker] PairPosFormat2::do_split. --- src/graph/pairpos-graph.hh | 45 +++++++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index 96d325f69..2f8df0b16 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -264,7 +264,19 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4& format2_device_table_indices; }; - hb_vector_t do_split (gsubgpos_graph_context_t& c, - unsigned this_index, + hb_vector_t do_split (split_context& split_context, hb_vector_t split_points) { - // TODO(garretrieger): implement me. - return hb_vector_t (); + hb_vector_t new_objects; + if (!split_points) + return new_objects; + + for (unsigned i = 0; i < split_points.length; i++) + { + unsigned start = split_points[i]; + unsigned end = (i < split_points.length - 1) ? split_points[i + 1] : class1Count; + unsigned id = clone_range (split_context, start, end); + + if (id == (unsigned) -1) + { + new_objects.reset (); + new_objects.allocated = -1; // mark error + return new_objects; + } + new_objects.push (id); + } + + if (!shrink (split_context, split_points[0])) + { + new_objects.reset (); + new_objects.allocated = -1; // mark error + } + + return new_objects; } unsigned clone_range (split_context& split_context, From 6be152420f8db34c7442b29ce3d47b0d975dbf61 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Wed, 3 Aug 2022 19:02:20 +0000 Subject: [PATCH 09/27] [repacker] add basic test for PairPos2 splitting. --- src/graph/pairpos-graph.hh | 4 +- src/test-repacker.cc | 161 +++++++++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+), 1 deletion(-) diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index 2f8df0b16..76684d2cc 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -295,8 +295,10 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 do_split (split_context& split_context, - hb_vector_t split_points) + const hb_vector_t& split_points) { + // TODO(garretrieger): refactor into a common method shared between subtables. + // template on context which could provide the clone and shrink methods. hb_vector_t new_objects; if (!split_points) return new_objects; diff --git a/src/test-repacker.cc b/src/test-repacker.cc index 1b7e1f08b..eefb46efd 100644 --- a/src/test-repacker.cc +++ b/src/test-repacker.cc @@ -167,6 +167,29 @@ static unsigned add_coverage (char start, char end, return add_object (coverage, 10, c); } +static unsigned add_class_def (uint16_t class_count, + hb_serialize_context_t* c) +{ + uint8_t header[] = { + 0, 1, // format + 0, 5, // startGlyphID + (uint8_t) ((class_count >> 8) & 0xFF), + (uint8_t) (class_count & 0xFF), // count + }; + + start_object ((char*) header, 6, c); + for (uint16_t i = 1; i <= class_count; i++) + { + uint8_t class_value[] = { + (uint8_t) ((i >> 8) & 0xFF), + (uint8_t) (i & 0xFF), // count + }; + extend ((char*) class_value, 2, c); + } + + return c->pop_pack (false); +} + static unsigned add_pair_pos_1 (unsigned* pair_sets, char count, unsigned coverage, @@ -192,6 +215,57 @@ static unsigned add_pair_pos_1 (unsigned* pair_sets, return c->pop_pack (false); } +static unsigned add_pair_pos_2 (unsigned starting_class, + unsigned coverage, + unsigned class_def_1, uint16_t class_def_1_count, + unsigned class_def_2, uint16_t class_def_2_count, + hb_serialize_context_t* c) +{ + uint8_t format[] = { + 0, 2 + }; + + start_object ((char*) format, 2, c); + add_offset (coverage, c); + + constexpr unsigned num_values = 4; + uint8_t format1 = 0x01 | 0x02 | 0x08; + uint8_t format2 = 0x04; + uint8_t value_format[] = { + 0, format1, + 0, format2, + }; + extend ((char*) value_format, 4, c); + + add_offset (class_def_1, c); + add_offset (class_def_2, c); + + uint8_t class_counts[] = { + (uint8_t) ((class_def_1_count >> 8) & 0xFF), + (uint8_t) (class_def_1_count & 0xFF), + (uint8_t) ((class_def_2_count >> 8) & 0xFF), + (uint8_t) (class_def_2_count & 0xFF), + }; + extend ((char*) class_counts, 4, c); + + unsigned num_bytes_per_record = class_def_2_count * num_values * 2; + uint8_t* record = (uint8_t*) calloc (1, num_bytes_per_record); + for (uint16_t i = 0; i < class_def_1_count; i++) + { + + for (unsigned j = 0; j < num_bytes_per_record; j++) + { + record[j] = (uint8_t) (i + starting_class); + } + + extend((char*) record, num_bytes_per_record, c); + } + free (record); + + return c->pop_pack (false); +} + + static void run_resolve_overflow_test (const char* name, hb_serialize_context_t& overflowing, hb_serialize_context_t& expected, @@ -1108,6 +1182,67 @@ populate_serializer_with_large_pair_pos_1 (hb_serialize_context_t* c, c->end_serialize(); } +template +static void +populate_serializer_with_large_pair_pos_2 (hb_serialize_context_t* c, + bool as_extension = false) +{ + std::string large_string(60000, 'a'); + c->start_serialize (); + + unsigned coverage[num_pair_pos_2]; + unsigned class_def_1[num_pair_pos_2]; + unsigned class_def_2[num_pair_pos_2]; + unsigned pair_pos_2[num_pair_pos_2]; + + for (int i = num_pair_pos_2 - 1; i >= 0; i--) + { + if (num_class_2 >= num_class_1) + { + class_def_2[i] = add_class_def (num_class_2, c); + class_def_1[i] = add_class_def (num_class_1, c); + } else { + class_def_1[i] = add_class_def (num_class_1, c); + class_def_2[i] = add_class_def (num_class_2, c); + } + + + coverage[i] = add_coverage (5, + 5 + num_class_1 - 1, + c); + + pair_pos_2[i] = add_pair_pos_2 (i * num_class_1, + coverage[i], + class_def_1[i], num_class_1, + class_def_2[i], num_class_2, + c); + } + + unsigned pair_pos_1 = add_object (large_string.c_str(), 200, c); + + if (as_extension) { + + for (int i = num_pair_pos_2 - 1; i >= 0; i--) + pair_pos_2[i] = add_extension (pair_pos_2[i], 2, c); + pair_pos_1 = add_extension (pair_pos_1, 2, c); + } + + start_lookup (as_extension ? 9 : 2, 1 + num_pair_pos_2, c); + + add_offset (pair_pos_1, c); + for (int i = 0; i < num_pair_pos_2; i++) + add_offset (pair_pos_2[i], c); + + + unsigned lookup = finish_lookup (c); + + unsigned lookup_list = add_lookup_list (&lookup, 1, c); + + add_gsubgpos_header (lookup_list, c); + + c->end_serialize(); +} + static void test_sort_shortest () { size_t buffer_size = 100; @@ -1523,6 +1658,28 @@ static void test_resolve_with_extension_pair_pos_1_split () free (expected_buffer); } +static void test_resolve_with_basic_pair_pos_2_split () +{ + size_t buffer_size = 200000; + void* buffer = malloc (buffer_size); + assert (buffer); + hb_serialize_context_t c (buffer, buffer_size); + populate_serializer_with_large_pair_pos_2 <1, 4, 3000>(&c); + + void* expected_buffer = malloc (buffer_size); + assert (expected_buffer); + hb_serialize_context_t e (expected_buffer, buffer_size); + populate_serializer_with_large_pair_pos_2 <2, 2, 3000>(&e, true); + + run_resolve_overflow_test ("test_resolve_with_basic_pair_pos_2_split", + c, + e, + 20, + true, + HB_TAG('G', 'P', 'O', 'S')); + free (buffer); + free (expected_buffer); +} static void test_resolve_overflows_via_splitting_spaces () { @@ -1649,6 +1806,7 @@ test_shared_node_with_virtual_links () int main (int argc, char **argv) { + if (0) { test_serialize (); test_sort_shortest (); test_will_overflow_1 (); @@ -1673,6 +1831,9 @@ main (int argc, char **argv) test_resolve_with_extension_promotion (); test_resolve_with_basic_pair_pos_1_split (); test_resolve_with_extension_pair_pos_1_split (); + } + + test_resolve_with_basic_pair_pos_2_split (); // TODO(grieger): test with extensions already mixed in as well. // TODO(grieger): test two layer ext promotion setup. From 60d6ffb3758fddca681cfc28828175eb8b5aa3e6 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Wed, 3 Aug 2022 21:01:23 +0000 Subject: [PATCH 10/27] [repacker] always duplicate classDef2 when splitting a PairPos2. Splits are done in a way that it shouldn't be possible to share the classDef2 between split PairPos2's so pre-emptively duplicate it. --- src/graph/pairpos-graph.hh | 1 + src/test-repacker.cc | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index 76684d2cc..f8c315d14 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -381,6 +381,7 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4objidx = class_def_2_id; class_def_link->position = 10; split_context.c.graph.vertices_[class_def_2_id].parents.push (pair_pos_prime_id); + split_context.c.graph.duplicate (pair_pos_prime_id, class_def_2_id); return pair_pos_prime_id; } diff --git a/src/test-repacker.cc b/src/test-repacker.cc index eefb46efd..f6863ece8 100644 --- a/src/test-repacker.cc +++ b/src/test-repacker.cc @@ -1211,7 +1211,7 @@ populate_serializer_with_large_pair_pos_2 (hb_serialize_context_t* c, 5 + num_class_1 - 1, c); - pair_pos_2[i] = add_pair_pos_2 (i * num_class_1, + pair_pos_2[i] = add_pair_pos_2 (1 + i * num_class_1, coverage[i], class_def_1[i], num_class_1, class_def_2[i], num_class_2, From 54fab21cb12d5e22382cba91506a195ae2e6c63a Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Wed, 3 Aug 2022 21:57:37 +0000 Subject: [PATCH 11/27] [repacker] get basic pair pos 2 split test working. --- src/graph/pairpos-graph.hh | 2 +- src/test-repacker.cc | 31 ++++++++++++++++--------------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index f8c315d14..1099c8859 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -345,7 +345,7 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4format = this->format; pair_pos_prime->valueFormat1 = this->valueFormat1; pair_pos_prime->valueFormat2 = this->valueFormat2; - pair_pos_prime->class1Count = this->class1Count; + pair_pos_prime->class1Count = num_records; pair_pos_prime->class2Count = this->class2Count; clone_class1_records (split_context, pair_pos_prime_id, diff --git a/src/test-repacker.cc b/src/test-repacker.cc index f6863ece8..5df8bd5d8 100644 --- a/src/test-repacker.cc +++ b/src/test-repacker.cc @@ -167,12 +167,13 @@ static unsigned add_coverage (char start, char end, return add_object (coverage, 10, c); } -static unsigned add_class_def (uint16_t class_count, +static unsigned add_class_def (uint8_t start_glyph, + uint16_t class_count, hb_serialize_context_t* c) { uint8_t header[] = { 0, 1, // format - 0, 5, // startGlyphID + 0, start_glyph, // startGlyphID (uint8_t) ((class_count >> 8) & 0xFF), (uint8_t) (class_count & 0xFF), // count }; @@ -180,9 +181,10 @@ static unsigned add_class_def (uint16_t class_count, start_object ((char*) header, 6, c); for (uint16_t i = 1; i <= class_count; i++) { + unsigned klass = start_glyph + 10 + i; uint8_t class_value[] = { - (uint8_t) ((i >> 8) & 0xFF), - (uint8_t) (i & 0xFF), // count + (uint8_t) ((klass >> 8) & 0xFF), + (uint8_t) (klass & 0xFF), // count }; extend ((char*) class_value, 2, c); } @@ -1187,7 +1189,7 @@ static void populate_serializer_with_large_pair_pos_2 (hb_serialize_context_t* c, bool as_extension = false) { - std::string large_string(60000, 'a'); + std::string large_string(100000, 'a'); c->start_serialize (); unsigned coverage[num_pair_pos_2]; @@ -1199,16 +1201,15 @@ populate_serializer_with_large_pair_pos_2 (hb_serialize_context_t* c, { if (num_class_2 >= num_class_1) { - class_def_2[i] = add_class_def (num_class_2, c); - class_def_1[i] = add_class_def (num_class_1, c); + class_def_2[i] = add_class_def (10, num_class_2, c); + class_def_1[i] = add_class_def (5 + i * num_class_1, num_class_1, c); } else { - class_def_1[i] = add_class_def (num_class_1, c); - class_def_2[i] = add_class_def (num_class_2, c); + class_def_1[i] = add_class_def (5 + i * num_class_1, num_class_1, c); + class_def_2[i] = add_class_def (10, num_class_2, c); } - - coverage[i] = add_coverage (5, - 5 + num_class_1 - 1, + coverage[i] = add_coverage (5 + i * num_class_1, + 5 + (i + 1) * num_class_1 - 1, c); pair_pos_2[i] = add_pair_pos_2 (1 + i * num_class_1, @@ -1218,10 +1219,10 @@ populate_serializer_with_large_pair_pos_2 (hb_serialize_context_t* c, c); } - unsigned pair_pos_1 = add_object (large_string.c_str(), 200, c); + unsigned pair_pos_1 = add_object (large_string.c_str(), 100000, c); + if (as_extension) { - for (int i = num_pair_pos_2 - 1; i >= 0; i--) pair_pos_2[i] = add_extension (pair_pos_2[i], 2, c); pair_pos_1 = add_extension (pair_pos_1, 2, c); @@ -1660,7 +1661,7 @@ static void test_resolve_with_extension_pair_pos_1_split () static void test_resolve_with_basic_pair_pos_2_split () { - size_t buffer_size = 200000; + size_t buffer_size = 300000; void* buffer = malloc (buffer_size); assert (buffer); hb_serialize_context_t c (buffer, buffer_size); From 51a5060273a896008a7bddbe1f851bbb452da408 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Wed, 3 Aug 2022 22:30:42 +0000 Subject: [PATCH 12/27] [repacker] add test for splitting a PairPos2 w/ device tables. --- src/test-repacker.cc | 77 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 68 insertions(+), 9 deletions(-) diff --git a/src/test-repacker.cc b/src/test-repacker.cc index 5df8bd5d8..e7aee2efb 100644 --- a/src/test-repacker.cc +++ b/src/test-repacker.cc @@ -221,6 +221,7 @@ static unsigned add_pair_pos_2 (unsigned starting_class, unsigned coverage, unsigned class_def_1, uint16_t class_def_1_count, unsigned class_def_2, uint16_t class_def_2_count, + unsigned* device_tables, hb_serialize_context_t* c) { uint8_t format[] = { @@ -230,13 +231,18 @@ static unsigned add_pair_pos_2 (unsigned starting_class, start_object ((char*) format, 2, c); add_offset (coverage, c); - constexpr unsigned num_values = 4; + unsigned num_values = 4; uint8_t format1 = 0x01 | 0x02 | 0x08; uint8_t format2 = 0x04; + if (device_tables) { + format2 |= 0x20; + num_values += 1; + } uint8_t value_format[] = { 0, format1, 0, format2, }; + extend ((char*) value_format, 4, c); add_offset (class_def_1, c); @@ -252,15 +258,24 @@ static unsigned add_pair_pos_2 (unsigned starting_class, unsigned num_bytes_per_record = class_def_2_count * num_values * 2; uint8_t* record = (uint8_t*) calloc (1, num_bytes_per_record); + int device_index = 0; for (uint16_t i = 0; i < class_def_1_count; i++) { - for (unsigned j = 0; j < num_bytes_per_record; j++) + for (uint16_t j = 0; j < class_def_2_count; j++) { - record[j] = (uint8_t) (i + starting_class); - } + for (int k = 0; k < 4; k++) { + uint8_t value[] = { + (uint8_t) (i + starting_class), + (uint8_t) (i + starting_class), + }; + extend ((char*) value, 2, c); + } - extend((char*) record, num_bytes_per_record, c); + if (device_tables) { + add_offset (device_tables[device_index++], c); + } + } } free (record); @@ -1187,7 +1202,8 @@ populate_serializer_with_large_pair_pos_1 (hb_serialize_context_t* c, template static void populate_serializer_with_large_pair_pos_2 (hb_serialize_context_t* c, - bool as_extension = false) + bool as_extension = false, + bool with_device_tables = false) { std::string large_string(100000, 'a'); c->start_serialize (); @@ -1197,6 +1213,9 @@ populate_serializer_with_large_pair_pos_2 (hb_serialize_context_t* c, unsigned class_def_2[num_pair_pos_2]; unsigned pair_pos_2[num_pair_pos_2]; + unsigned* device_tables = (unsigned*) calloc (num_pair_pos_2 * num_class_1 * num_class_2, + sizeof(unsigned)); + for (int i = num_pair_pos_2 - 1; i >= 0; i--) { if (num_class_2 >= num_class_1) @@ -1212,10 +1231,27 @@ populate_serializer_with_large_pair_pos_2 (hb_serialize_context_t* c, 5 + (i + 1) * num_class_1 - 1, c); + if (with_device_tables) + { + for(int j = (i + 1) * num_class_1 * num_class_2 - 1; + j >= i * num_class_1 * num_class_2; + j--) + { + uint8_t table[] = { + (uint8_t) ((j >> 8) & 0xFF), + (uint8_t) (j & 0xFF), + }; + device_tables[j] = add_object ((char*) table, 2, c); + } + } + pair_pos_2[i] = add_pair_pos_2 (1 + i * num_class_1, coverage[i], class_def_1[i], num_class_1, class_def_2[i], num_class_2, + with_device_tables + ? &device_tables[i * num_class_1 * num_class_2] + : nullptr, c); } @@ -1242,6 +1278,8 @@ populate_serializer_with_large_pair_pos_2 (hb_serialize_context_t* c, add_gsubgpos_header (lookup_list, c); c->end_serialize(); + + free (device_tables); } static void test_sort_shortest () @@ -1682,6 +1720,29 @@ static void test_resolve_with_basic_pair_pos_2_split () free (expected_buffer); } +static void test_resolve_with_pair_pos_2_split_with_device_tables () +{ + size_t buffer_size = 300000; + void* buffer = malloc (buffer_size); + assert (buffer); + hb_serialize_context_t c (buffer, buffer_size); + populate_serializer_with_large_pair_pos_2 <1, 4, 2000>(&c, false, true); + + void* expected_buffer = malloc (buffer_size); + assert (expected_buffer); + hb_serialize_context_t e (expected_buffer, buffer_size); + populate_serializer_with_large_pair_pos_2 <2, 2, 2000>(&e, true, true); + + run_resolve_overflow_test ("test_resolve_with_pair_pos_2_split_with_device_tables", + c, + e, + 20, + true, + HB_TAG('G', 'P', 'O', 'S')); + free (buffer); + free (expected_buffer); +} + static void test_resolve_overflows_via_splitting_spaces () { size_t buffer_size = 160000; @@ -1807,7 +1868,6 @@ test_shared_node_with_virtual_links () int main (int argc, char **argv) { - if (0) { test_serialize (); test_sort_shortest (); test_will_overflow_1 (); @@ -1832,9 +1892,8 @@ main (int argc, char **argv) test_resolve_with_extension_promotion (); test_resolve_with_basic_pair_pos_1_split (); test_resolve_with_extension_pair_pos_1_split (); - } - test_resolve_with_basic_pair_pos_2_split (); + test_resolve_with_pair_pos_2_split_with_device_tables (); // TODO(grieger): test with extensions already mixed in as well. // TODO(grieger): test two layer ext promotion setup. From 88e0dd02cb728ba91e96298d6346cdabe18a95ab Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Thu, 4 Aug 2022 01:03:07 +0000 Subject: [PATCH 13/27] [repacker] add sanitization for PairPosFormat2. --- src/graph/pairpos-graph.hh | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index 1099c8859..aa504e0e1 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -201,13 +201,17 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4::min_size; + if (vertex_len < min_size) return false; + + const unsigned class1_count = class1Count; + return vertex_len >= + min_size + class1_count * get_class1_record_size (); } hb_vector_t split_subtables (gsubgpos_graph_context_t& c, unsigned this_index) { - // TODO(garretrieger): sanitization. const unsigned base_size = OT::Layout::GPOS_impl::PairPosFormat2_4::min_size + size_of (c, this_index, &coverage) + size_of (c, this_index, &classDef1) @@ -215,8 +219,7 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4& format2_device_table_indices; }; + size_t get_class1_record_size () const + { + const size_t class2_count = class2Count; + return + class2_count * (valueFormat1.get_size () + valueFormat2.get_size ()); + } + hb_vector_t do_split (split_context& split_context, const hb_vector_t& split_points) { From b154b1e4c3564bcef14f6efe9062e543808ed659 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Thu, 4 Aug 2022 01:37:21 +0000 Subject: [PATCH 14/27] [repacker] pull out PairPosFormat1,2::do_split() into a common helper method. --- src/Makefile.sources | 1 + src/graph/pairpos-graph.hh | 113 +++++++++++++++---------------------- src/graph/split-helpers.hh | 68 ++++++++++++++++++++++ src/meson.build | 1 + 4 files changed, 115 insertions(+), 68 deletions(-) create mode 100644 src/graph/split-helpers.hh diff --git a/src/Makefile.sources b/src/Makefile.sources index 14c6ed346..0a6b845ee 100644 --- a/src/Makefile.sources +++ b/src/Makefile.sources @@ -355,6 +355,7 @@ HB_SUBSET_sources = \ graph/coverage-graph.hh \ graph/classdef-graph.hh \ graph/pairpos-graph.hh \ + graph/split-helpers.hh \ graph/serialize.hh \ $(NULL) diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index aa504e0e1..6b0e4e075 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -27,6 +27,7 @@ #ifndef GRAPH_PAIRPOS_GRAPH_HH #define GRAPH_PAIRPOS_GRAPH_HH +#include "split-helpers.hh" #include "coverage-graph.hh" #include "classdef-graph.hh" #include "../OT/Layout/GPOS/PairPos.hh" @@ -75,45 +76,37 @@ struct PairPosFormat1 : public OT::Layout::GPOS_impl::PairPosFormat1_3 (split_context, split_points); } private: - // Split this PairPos into two or more PairPos's. split_points defines - // the indices (first index to include in the new table) to split at. - // Returns the object id's of the newly created PairPos subtables. - hb_vector_t do_split (gsubgpos_graph_context_t& c, - unsigned this_index, - const hb_vector_t split_points) - { - hb_vector_t new_objects; - if (!split_points) - return new_objects; + struct split_context_t { + gsubgpos_graph_context_t& c; + PairPosFormat1* thiz; + unsigned this_index; - for (unsigned i = 0; i < split_points.length; i++) + unsigned original_count () { - unsigned start = split_points[i]; - unsigned end = (i < split_points.length - 1) ? split_points[i + 1] : pairSet.len; - unsigned id = clone_range (c, this_index, start, end); - - if (id == (unsigned) -1) - { - new_objects.reset (); - new_objects.allocated = -1; // mark error - return new_objects; - } - new_objects.push (id); + return thiz->pairSet.len; } - if (!shrink (c, this_index, split_points[0])) + unsigned clone_range (unsigned start, unsigned end) { - new_objects.reset (); - new_objects.allocated = -1; // mark error + return thiz->clone_range (this->c, this->this_index, start, end); } - return new_objects; - } + bool shrink (unsigned count) + { + return thiz->shrink (this->c, this->this_index, count); + } + }; bool shrink (gsubgpos_graph_context_t& c, unsigned this_index, @@ -267,8 +260,9 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 (split_context, split_points); } private: - struct split_context + struct split_context_t { gsubgpos_graph_context_t& c; + PairPosFormat2* thiz; unsigned this_index; unsigned class1_record_size; unsigned value_record_len; @@ -295,6 +290,21 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4& device_tables; const hb_vector_t& format1_device_table_indices; const hb_vector_t& format2_device_table_indices; + + unsigned original_count () + { + return thiz->class1Count; + } + + unsigned clone_range (unsigned start, unsigned end) + { + return thiz->clone_range (*this, start, end); + } + + bool shrink (unsigned count) + { + return thiz->shrink (*this, count); + } }; size_t get_class1_record_size () const @@ -304,40 +314,7 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 do_split (split_context& split_context, - const hb_vector_t& split_points) - { - // TODO(garretrieger): refactor into a common method shared between subtables. - // template on context which could provide the clone and shrink methods. - hb_vector_t new_objects; - if (!split_points) - return new_objects; - - for (unsigned i = 0; i < split_points.length; i++) - { - unsigned start = split_points[i]; - unsigned end = (i < split_points.length - 1) ? split_points[i + 1] : class1Count; - unsigned id = clone_range (split_context, start, end); - - if (id == (unsigned) -1) - { - new_objects.reset (); - new_objects.allocated = -1; // mark error - return new_objects; - } - new_objects.push (id); - } - - if (!shrink (split_context, split_points[0])) - { - new_objects.reset (); - new_objects.allocated = -1; // mark error - } - - return new_objects; - } - - unsigned clone_range (split_context& split_context, + unsigned clone_range (split_context_t& split_context, unsigned start, unsigned end) const { DEBUG_MSG (SUBSET_REPACK, nullptr, @@ -396,7 +373,7 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4& device_table_indices, unsigned old_value_record_index, @@ -462,7 +439,7 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 +hb_vector_t actuate_subtable_split (Context& split_context, + const hb_vector_t& split_points) +{ + hb_vector_t new_objects; + if (!split_points) + return new_objects; + + for (unsigned i = 0; i < split_points.length; i++) + { + unsigned start = split_points[i]; + unsigned end = (i < split_points.length - 1) + ? split_points[i + 1] + : split_context.original_count (); + unsigned id = split_context.clone_range (start, end); + + if (id == (unsigned) -1) + { + new_objects.reset (); + new_objects.allocated = -1; // mark error + return new_objects; + } + new_objects.push (id); + } + + if (!split_context.shrink (split_points[0])) + { + new_objects.reset (); + new_objects.allocated = -1; // mark error + } + + return new_objects; +} + +} + +#endif // GRAPH_SPLIT_HELPERS_HH diff --git a/src/meson.build b/src/meson.build index 2118eb57c..5e0c172c6 100644 --- a/src/meson.build +++ b/src/meson.build @@ -351,6 +351,7 @@ hb_subset_sources = files( 'graph/pairpos-graph.hh', 'graph/coverage-graph.hh', 'graph/classdef-graph.hh', + 'graph/split-helpers.hh', 'hb-subset.cc', 'hb-subset.hh', ) From fdd1952c751960392f29162298e7884d5d5ca6c4 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Thu, 4 Aug 2022 19:21:16 +0000 Subject: [PATCH 15/27] [repacker] PairPosFormat2 splitting - fix coverage and classdef splitting. The old code was splitting based on coverage index, but should have been splitting on class value. --- src/graph/classdef-graph.hh | 29 +++------ src/graph/coverage-graph.hh | 16 ++++- src/graph/pairpos-graph.hh | 117 +++++++++++++++++++++--------------- src/test-repacker.cc | 44 +++++++++----- 4 files changed, 117 insertions(+), 89 deletions(-) diff --git a/src/graph/classdef-graph.hh b/src/graph/classdef-graph.hh index c355b9732..fd6472a0d 100644 --- a/src/graph/classdef-graph.hh +++ b/src/graph/classdef-graph.hh @@ -57,35 +57,22 @@ struct ClassDefFormat2 : public OT::ClassDefFormat2_4 struct ClassDef : public OT::ClassDef { template - static bool clone_class_def (gsubgpos_graph_context_t& c, - unsigned class_def_id, - unsigned new_parent_id, - unsigned link_position, - It glyphs) + static bool add_class_def (gsubgpos_graph_context_t& c, + unsigned parent_id, + unsigned link_position, + It glyph_and_class, + unsigned max_size) { - unsigned class_def_size = c.graph.vertices_[class_def_id].table_size (); - auto& class_def_v = c.graph.vertices_[class_def_id]; - ClassDef* class_def_table = (ClassDef*) class_def_v.obj.head; - if (!class_def_table->sanitize (class_def_v)) - return false; - - auto new_class_def = - + glyphs - | hb_map_retains_sorting ([&] (hb_codepoint_t gid) { - return hb_pair (gid, class_def_table->get_class (gid)); - }) - ; - unsigned class_def_prime_id = c.graph.new_node (nullptr, nullptr); auto& class_def_prime_vertex = c.graph.vertices_[class_def_prime_id]; - if (!make_class_def (c, new_class_def, class_def_prime_id, class_def_size)) + if (!make_class_def (c, glyph_and_class, class_def_prime_id, max_size)) return false; - auto* class_def_link = c.graph.vertices_[new_parent_id].obj.real_links.push (); + auto* class_def_link = c.graph.vertices_[parent_id].obj.real_links.push (); class_def_link->width = SmallTypes::size; class_def_link->objidx = class_def_prime_id; class_def_link->position = link_position; - class_def_prime_vertex.parents.push (new_parent_id); + class_def_prime_vertex.parents.push (parent_id); return true; } diff --git a/src/graph/coverage-graph.hh b/src/graph/coverage-graph.hh index a5e5e1491..7309029c2 100644 --- a/src/graph/coverage-graph.hh +++ b/src/graph/coverage-graph.hh @@ -77,16 +77,26 @@ struct Coverage : public OT::Layout::Common::Coverage | hb_map_retains_sorting (hb_first) ; + return add_coverage (c, new_parent_id, link_position, new_coverage, coverage_size); + } + + template + static Coverage* add_coverage (gsubgpos_graph_context_t& c, + unsigned parent_id, + unsigned link_position, + It glyphs, + unsigned max_size) + { unsigned coverage_prime_id = c.graph.new_node (nullptr, nullptr); auto& coverage_prime_vertex = c.graph.vertices_[coverage_prime_id]; - if (!make_coverage (c, new_coverage, coverage_prime_id, coverage_size)) + if (!make_coverage (c, glyphs, coverage_prime_id, max_size)) return nullptr; - auto* coverage_link = c.graph.vertices_[new_parent_id].obj.real_links.push (); + auto* coverage_link = c.graph.vertices_[parent_id].obj.real_links.push (); coverage_link->width = SmallTypes::size; coverage_link->objidx = coverage_prime_id; coverage_link->position = link_position; - coverage_prime_vertex.parents.push (new_parent_id); + coverage_prime_vertex.parents.push (parent_id); return (Coverage*) coverage_prime_vertex.obj.head; } diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index 6b0e4e075..00cae4fad 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -320,6 +320,8 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4::min_size + num_records * split_context.class1_record_size; @@ -328,7 +330,7 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4format = this->format; pair_pos_prime->valueFormat1 = this->valueFormat1; pair_pos_prime->valueFormat2 = this->valueFormat2; @@ -340,35 +342,55 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4sanitize (coverage_v) + || !class_def_1_table->sanitize (class_def_1_v)) + return false; - Coverage* new_coverage = Coverage::clone_coverage (split_context.c, - coverage_id, - pair_pos_prime_id, - 2, - start, end); - if (!new_coverage) + auto klass_map = + + coverage_table->iter () + | hb_map_retains_sorting ([&] (hb_codepoint_t gid) { + return hb_pair (gid, class_def_1_table->get_class (gid)); + }) + | hb_filter ([&] (hb_codepoint_t klass) { + return klass >= start && klass < end; + }, hb_second) + | hb_map_retains_sorting ([&] (hb_pair_t gid_and_class) { + // Classes must be from 0...N so subtract start + return hb_pair (gid_and_class.first, gid_and_class.second - start); + }) + ; + + if (!Coverage::add_coverage (split_context.c, + pair_pos_prime_id, + 2, + + klass_map | hb_map_retains_sorting (hb_first), + coverage_v.table_size ())) return -1; // classDef1 - unsigned class_def_1_id = - split_context.c.graph.index_for_offset (split_context.this_index, &classDef1); - if (!ClassDef::clone_class_def (split_context.c, - class_def_1_id, - pair_pos_prime_id, - 8, - new_coverage->iter ())) + if (!ClassDef::add_class_def (split_context.c, + pair_pos_prime_id, + 8, + + klass_map, + class_def_1_v.table_size ())) return -1; // classDef2 unsigned class_def_2_id = - split_context.c.graph.index_for_offset (split_context.this_index, &classDef2); - auto* class_def_link = split_context.c.graph.vertices_[pair_pos_prime_id].obj.real_links.push (); + graph.index_for_offset (split_context.this_index, &classDef2); + auto* class_def_link = graph.vertices_[pair_pos_prime_id].obj.real_links.push (); class_def_link->width = SmallTypes::size; class_def_link->objidx = class_def_2_id; class_def_link->position = 10; - split_context.c.graph.vertices_[class_def_2_id].parents.push (pair_pos_prime_id); - split_context.c.graph.duplicate (pair_pos_prime_id, class_def_2_id); + graph.vertices_[class_def_2_id].parents.push (pair_pos_prime_id); + graph.duplicate (pair_pos_prime_id, class_def_2_id); return pair_pos_prime_id; } @@ -450,48 +472,43 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4= old_count) return true; + graph_t& graph = split_context.c.graph; class1Count = count; - split_context.c.graph.vertices_[split_context.this_index].obj.tail -= + graph.vertices_[split_context.this_index].obj.tail -= (old_count - count) * split_context.class1_record_size; unsigned coverage_id = - split_context.c.graph.index_for_offset (split_context.this_index, &coverage); - auto& coverage_v = split_context.c.graph.vertices_[coverage_id]; + graph.index_for_offset (split_context.this_index, &coverage); + unsigned class_def_1_id = + graph.index_for_offset (split_context.this_index, &classDef1); + auto& coverage_v = graph.vertices_[coverage_id]; + auto& class_def_1_v = graph.vertices_[class_def_1_id]; Coverage* coverage_table = (Coverage*) coverage_v.obj.head; - if (!coverage_table->sanitize (coverage_v)) + ClassDef* class_def_1_table = (ClassDef*) class_def_1_v.obj.head; + if (!coverage_table->sanitize (coverage_v) + || !class_def_1_table->sanitize (class_def_1_v)) return false; - auto new_coverage = - + hb_zip (coverage_table->iter (), hb_range ()) - | hb_filter ([&] (hb_pair_t p) { - return p.second < count; - }) - | hb_map_retains_sorting (hb_first) - ; + auto klass_map = + + coverage_table->iter () + | hb_map_retains_sorting ([&] (hb_codepoint_t gid) { + return hb_pair (gid, class_def_1_table->get_class (gid)); + }) + | hb_filter ([&] (hb_codepoint_t klass) { + return klass < count; + }, hb_second) + ; - if (!Coverage::make_coverage (split_context.c, new_coverage, coverage_id, coverage_v.table_size ())) + if (!Coverage::make_coverage (split_context.c, + + klass_map | hb_map_retains_sorting (hb_first), + coverage_id, + coverage_v.table_size ())) return false; - // classDef1 - coverage_table = (Coverage*) coverage_v.obj.head; // get the new table - unsigned class_def_id = split_context.c.graph.index_for_offset (split_context.this_index, - &classDef1); - auto& class_def_v = split_context.c.graph.vertices_[class_def_id]; - ClassDef* class_def_table = (ClassDef*) class_def_v.obj.head; - if (!class_def_table->sanitize (class_def_v)) - return false; - - auto new_class_def = - + coverage_table->iter () - | hb_map_retains_sorting ([&] (hb_codepoint_t gid) { - return hb_pair (gid, class_def_table->get_class (gid)); - }) - ; - return ClassDef::make_class_def (split_context.c, - new_class_def, - class_def_id, - class_def_v.table_size ()); + + klass_map, + class_def_1_id, + class_def_1_v.table_size ()); } hb_hashmap_t diff --git a/src/test-repacker.cc b/src/test-repacker.cc index e7aee2efb..ae2a05a90 100644 --- a/src/test-repacker.cc +++ b/src/test-repacker.cc @@ -143,6 +143,7 @@ static unsigned add_extension (unsigned child, } +// Adds coverage table fro [start, end] static unsigned add_coverage (char start, char end, hb_serialize_context_t* c) { @@ -167,24 +168,29 @@ static unsigned add_coverage (char start, char end, return add_object (coverage, 10, c); } -static unsigned add_class_def (uint8_t start_glyph, - uint16_t class_count, +// Adds a class that maps glyphs from [start_glyph, end_glyph) +// to classes 1...n +static unsigned add_class_def (uint16_t start_glyph, + uint16_t end_glyph, hb_serialize_context_t* c) { + unsigned count = end_glyph - start_glyph; uint8_t header[] = { 0, 1, // format - 0, start_glyph, // startGlyphID - (uint8_t) ((class_count >> 8) & 0xFF), - (uint8_t) (class_count & 0xFF), // count + + (uint8_t) ((start_glyph >> 8) & 0xFF), + (uint8_t) (start_glyph & 0xFF), // start_glyph + + (uint8_t) ((count >> 8) & 0xFF), + (uint8_t) (count & 0xFF), // count }; start_object ((char*) header, 6, c); - for (uint16_t i = 1; i <= class_count; i++) + for (uint16_t i = 1; i <= count; i++) { - unsigned klass = start_glyph + 10 + i; uint8_t class_value[] = { - (uint8_t) ((klass >> 8) & 0xFF), - (uint8_t) (klass & 0xFF), // count + (uint8_t) ((i >> 8) & 0xFF), + (uint8_t) (i & 0xFF), // count }; extend ((char*) class_value, 2, c); } @@ -1216,19 +1222,27 @@ populate_serializer_with_large_pair_pos_2 (hb_serialize_context_t* c, unsigned* device_tables = (unsigned*) calloc (num_pair_pos_2 * num_class_1 * num_class_2, sizeof(unsigned)); + // Total glyphs = num_class_1 * num_pair_pos_2 for (int i = num_pair_pos_2 - 1; i >= 0; i--) { + unsigned start_glyph = 5 + i * num_class_1; if (num_class_2 >= num_class_1) { - class_def_2[i] = add_class_def (10, num_class_2, c); - class_def_1[i] = add_class_def (5 + i * num_class_1, num_class_1, c); + class_def_2[i] = add_class_def (11, + 10 + num_class_2, c); + class_def_1[i] = add_class_def (start_glyph + 1, + start_glyph + num_class_1, + c); } else { - class_def_1[i] = add_class_def (5 + i * num_class_1, num_class_1, c); - class_def_2[i] = add_class_def (10, num_class_2, c); + class_def_1[i] = add_class_def (start_glyph + 1, + start_glyph + num_class_1, + c); + class_def_2[i] = add_class_def (11, + 10 + num_class_2, c); } - coverage[i] = add_coverage (5 + i * num_class_1, - 5 + (i + 1) * num_class_1 - 1, + coverage[i] = add_coverage (start_glyph, + start_glyph + num_class_1 - 1, c); if (with_device_tables) From 506547c958b5e03d5b712b94b2333dffac0e6b7e Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Thu, 4 Aug 2022 21:36:21 +0000 Subject: [PATCH 16/27] [repacker] Use hb_pair_t constructor instead of hb_pair (). hb_pair was causing corrupted gid values. --- src/graph/pairpos-graph.hh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index 00cae4fad..20e54da44 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -351,19 +351,19 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4sanitize (coverage_v) || !class_def_1_table->sanitize (class_def_1_v)) - return false; + return -1; auto klass_map = + coverage_table->iter () | hb_map_retains_sorting ([&] (hb_codepoint_t gid) { - return hb_pair (gid, class_def_1_table->get_class (gid)); + return hb_pair_t (gid, class_def_1_table->get_class (gid)); }) | hb_filter ([&] (hb_codepoint_t klass) { return klass >= start && klass < end; }, hb_second) | hb_map_retains_sorting ([&] (hb_pair_t gid_and_class) { // Classes must be from 0...N so subtract start - return hb_pair (gid_and_class.first, gid_and_class.second - start); + return hb_pair_t (gid_and_class.first, gid_and_class.second - start); }) ; @@ -492,7 +492,7 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4iter () | hb_map_retains_sorting ([&] (hb_codepoint_t gid) { - return hb_pair (gid, class_def_1_table->get_class (gid)); + return hb_pair_t (gid, class_def_1_table->get_class (gid)); }) | hb_filter ([&] (hb_codepoint_t klass) { return klass < count; From a733a9afa581ba2c8bac54ba5c0fe3daaddfc30c Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Fri, 5 Aug 2022 00:32:47 +0000 Subject: [PATCH 17/27] [repacker] insert new subtables immediately after the subtable they split from in the lookup. --- src/graph/gsubgpos-graph.hh | 72 +++++++++++++++++++++++++++---------- src/hb-serialize.hh | 5 +++ src/test-repacker.cc | 6 ++-- 3 files changed, 60 insertions(+), 23 deletions(-) diff --git a/src/graph/gsubgpos-graph.hh b/src/graph/gsubgpos-graph.hh index afa1152c4..a42549655 100644 --- a/src/graph/gsubgpos-graph.hh +++ b/src/graph/gsubgpos-graph.hh @@ -124,7 +124,7 @@ struct Lookup : public OT::Lookup if (!is_ext && type != OT::Layout::GPOS_impl::PosLookupSubTable::Type::Pair) return true; - hb_vector_t all_new_subtables; + hb_vector_t>> all_new_subtables; for (unsigned i = 0; i < subTable.len; i++) { unsigned subtable_index = c.graph.index_for_offset (this_index, &subTable[i]); @@ -147,11 +147,14 @@ struct Lookup : public OT::Lookup hb_vector_t new_sub_tables = pairPos->split_subtables (c, subtable_index); if (new_sub_tables.in_error ()) return false; - + new_sub_tables.iter() | hb_sink (all_new_subtables); + hb_pair_t>* entry = all_new_subtables.push (); + entry->first = i; + entry->second = std::move (new_sub_tables); } - if (all_new_subtables) + if (all_new_subtables) { add_sub_tables (c, this_index, type, all_new_subtables); + } return true; } @@ -159,13 +162,18 @@ struct Lookup : public OT::Lookup void add_sub_tables (gsubgpos_graph_context_t& c, unsigned this_index, unsigned type, - hb_vector_t& subtable_indices) + hb_vector_t>>& subtable_ids) { bool is_ext = is_extension (c.table_tag); auto& v = c.graph.vertices_[this_index]; + fix_existing_subtable_links (c, this_index, subtable_ids); + + unsigned new_subtable_count = 0; + for (const auto& p : subtable_ids) + new_subtable_count += p.second.length; size_t new_size = v.table_size () - + subtable_indices.length * OT::Offset16::static_size; + + new_subtable_count * OT::Offset16::static_size; char* buffer = (char*) hb_calloc (1, new_size); c.add_buffer (buffer); memcpy (buffer, v.obj.head, v.table_size()); @@ -175,30 +183,56 @@ struct Lookup : public OT::Lookup Lookup* new_lookup = (Lookup*) buffer; - new_lookup->subTable.len = subTable.len + subtable_indices.length; - unsigned offset_index = subTable.len; - for (unsigned subtable_id : subtable_indices) + new_lookup->subTable.len = subTable.len + new_subtable_count; + for (const auto& p : subtable_ids) { - if (is_ext) + unsigned offset_index = p.first + 1; + for (unsigned subtable_id : p.second) { - unsigned ext_id = create_extension_subtable (c, subtable_id, type); - c.graph.vertices_[subtable_id].parents.push (ext_id); - subtable_id = ext_id; - } + if (is_ext) + { + unsigned ext_id = create_extension_subtable (c, subtable_id, type); + c.graph.vertices_[subtable_id].parents.push (ext_id); + subtable_id = ext_id; + } - auto* link = v.obj.real_links.push (); - link->width = 2; - link->objidx = subtable_id; - link->position = (char*) &new_lookup->subTable[offset_index++] - - (char*) new_lookup; - c.graph.vertices_[subtable_id].parents.push (this_index); + auto* link = v.obj.real_links.push (); + link->width = 2; + link->objidx = subtable_id; + link->position = (char*) &new_lookup->subTable[offset_index++] - + (char*) new_lookup; + c.graph.vertices_[subtable_id].parents.push (this_index); + } } + // Repacker sort order depends on link order, which we've messed up so resort it. + v.obj.real_links.qsort (); + // The head location of the lookup has changed, invalidating the lookups map entry // in the context. Update the map. c.lookups.set (this_index, new_lookup); } + void fix_existing_subtable_links (gsubgpos_graph_context_t& c, + unsigned this_index, + hb_vector_t>>& subtable_ids) + { + auto& v = c.graph.vertices_[this_index]; + Lookup* lookup = (Lookup*) v.obj.head; + + for (const auto& p : subtable_ids) + { + unsigned insert_index = p.first; + unsigned pos_offset = p.second.length * OT::Offset16::static_size; + unsigned insert_offset = (char*) &lookup->subTable[insert_index] - (char*) lookup; + + for (auto& l : v.obj.all_links_writer ()) + { + if (l.position > insert_offset) l.position += pos_offset; + } + } + } + unsigned create_extension_subtable (gsubgpos_graph_context_t& c, unsigned subtable_index, unsigned type) diff --git a/src/hb-serialize.hh b/src/hb-serialize.hh index cecdcdeb7..f42257407 100644 --- a/src/hb-serialize.hh +++ b/src/hb-serialize.hh @@ -139,6 +139,11 @@ struct hb_serialize_context_t objidx = o.objidx; } #endif + + HB_INTERNAL static int cmp (const void* a, const void* b) + { + return ((const link_t*)a)->position - ((const link_t*)b)->position; + } }; char *head; diff --git a/src/test-repacker.cc b/src/test-repacker.cc index ae2a05a90..30fe44a4e 100644 --- a/src/test-repacker.cc +++ b/src/test-repacker.cc @@ -1183,18 +1183,16 @@ populate_serializer_with_large_pair_pos_1 (hb_serialize_context_t* c, unsigned pair_pos_2 = add_object (large_string.c_str(), 200, c); if (as_extension) { - + pair_pos_2 = add_extension (pair_pos_2, 2, c); for (int i = num_pair_pos_1 - 1; i >= 0; i--) pair_pos_1[i] = add_extension (pair_pos_1[i], 2, c); - pair_pos_2 = add_extension (pair_pos_2, 2, c); } start_lookup (as_extension ? 9 : 2, 1 + num_pair_pos_1, c); - add_offset (pair_pos_2, c); for (int i = 0; i < num_pair_pos_1; i++) add_offset (pair_pos_1[i], c); - + add_offset (pair_pos_2, c); unsigned lookup = finish_lookup (c); From e1ab355056040e7f1566aef55408eb24fec4c5d4 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Fri, 5 Aug 2022 01:25:16 +0000 Subject: [PATCH 18/27] [repacker] correct lookup link insertion. --- src/graph/gsubgpos-graph.hh | 9 +++++++-- src/test-repacker.cc | 3 +++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/graph/gsubgpos-graph.hh b/src/graph/gsubgpos-graph.hh index a42549655..98302ff75 100644 --- a/src/graph/gsubgpos-graph.hh +++ b/src/graph/gsubgpos-graph.hh @@ -183,10 +183,13 @@ struct Lookup : public OT::Lookup Lookup* new_lookup = (Lookup*) buffer; + unsigned shift = 0; new_lookup->subTable.len = subTable.len + new_subtable_count; for (const auto& p : subtable_ids) { - unsigned offset_index = p.first + 1; + unsigned offset_index = p.first + shift + 1; + shift += p.second.length; + for (unsigned subtable_id : p.second) { if (is_ext) @@ -220,11 +223,13 @@ struct Lookup : public OT::Lookup auto& v = c.graph.vertices_[this_index]; Lookup* lookup = (Lookup*) v.obj.head; + unsigned shift = 0; for (const auto& p : subtable_ids) { - unsigned insert_index = p.first; + unsigned insert_index = p.first + shift; unsigned pos_offset = p.second.length * OT::Offset16::static_size; unsigned insert_offset = (char*) &lookup->subTable[insert_index] - (char*) lookup; + shift += p.second.length; for (auto& l : v.obj.all_links_writer ()) { diff --git a/src/test-repacker.cc b/src/test-repacker.cc index 30fe44a4e..e9aea87a3 100644 --- a/src/test-repacker.cc +++ b/src/test-repacker.cc @@ -1907,6 +1907,9 @@ main (int argc, char **argv) test_resolve_with_basic_pair_pos_2_split (); test_resolve_with_pair_pos_2_split_with_device_tables (); + // TODO(grieger): have run overflow tests compare graph equality not final packed binary. + // TODO(grieger): split test where multiple subtables in one lookup are split to test link ordering. + // TODO(grieger): split test where coverage table in subtable that is being split is shared. // TODO(grieger): test with extensions already mixed in as well. // TODO(grieger): test two layer ext promotion setup. // TODO(grieger): test sorting by subtables per byte in ext. promotion. From 5d824c09c0dd644b29f0a90ac2dd6bcbd119e789 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Fri, 5 Aug 2022 01:37:14 +0000 Subject: [PATCH 19/27] [repacker] during table splits don't mutate shared coverage/classdef in place. If other subtables are sharing coverage with a subtable being split we have to duplicate the coverage/classdef tables before they are modified during the shrink operation. --- src/graph/graph.hh | 22 +++++++++++++++++++++- src/graph/pairpos-graph.hh | 7 ++++--- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/graph/graph.hh b/src/graph/graph.hh index b3aef558a..0d6adcb64 100644 --- a/src/graph/graph.hh +++ b/src/graph/graph.hh @@ -345,7 +345,9 @@ struct graph_t } } - unsigned index_for_offset(unsigned node_idx, const void* offset) const + // Finds the object id of the object pointed to by the offset at 'offset' + // within object[node_idx]. + unsigned index_for_offset (unsigned node_idx, const void* offset) const { const auto& node = object (node_idx); if (offset < node.head || offset >= node.tail) return -1; @@ -360,6 +362,24 @@ struct graph_t return -1; } + // Finds the object id of the object pointed to by the offset at 'offset' + // within object[node_idx]. Ensures that the returned object is safe to mutate. + // That is, if the original child object is shared by parents other than node_idx + // it will be duplicated and the duplicate will be returned instead. + unsigned mutable_index_for_offset (unsigned node_idx, const void* offset) + { + unsigned child_idx = index_for_offset (node_idx, offset); + auto& child = vertices_[child_idx]; + for (unsigned p : child.parents) + { + if (p != node_idx) { + return duplicate (node_idx, child_idx); + } + } + + return child_idx; + } + /* * Assign unique space numbers to each connected subgraph of 24 bit and/or 32 bit offset(s). diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index 20e54da44..4cfcd89c5 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -123,9 +123,10 @@ struct PairPosFormat1 : public OT::Layout::GPOS_impl::PairPosFormat1_3sanitize (coverage_v)) return false; @@ -478,9 +479,9 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 Date: Fri, 5 Aug 2022 18:33:03 +0000 Subject: [PATCH 20/27] [repacker] add utility that can calculate the size of Coverage+ClassDef via incremental class inclusion. --- src/Makefile.am | 5 ++ src/graph/classdef-graph.hh | 82 +++++++++++++++++++++++ src/graph/test-classdef-graph.cc | 110 +++++++++++++++++++++++++++++++ src/meson.build | 1 + 4 files changed, 198 insertions(+) create mode 100644 src/graph/test-classdef-graph.cc diff --git a/src/Makefile.am b/src/Makefile.am index 83cda8f85..6f080fc8f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -371,6 +371,7 @@ COMPILED_TESTS = \ test-unicode-ranges \ test-vector \ test-repacker \ + test-classdef-graph \ $(NULL) COMPILED_TESTS_CPPFLAGS = $(HBCFLAGS) -DMAIN -UNDEBUG COMPILED_TESTS_LDADD = libharfbuzz.la $(HBLIBS) @@ -417,6 +418,10 @@ test_repacker_SOURCES = test-repacker.cc hb-static.cc graph/gsubgpos-context.cc test_repacker_CPPFLAGS = $(HBCFLAGS) test_repacker_LDADD = libharfbuzz.la libharfbuzz-subset.la $(HBLIBS) +test_classdef_graph_SOURCES = graph/test-classdef-graph.cc hb-static.cc graph/gsubgpos-context.cc +test_classdef_graph_CPPFLAGS = $(HBCFLAGS) +test_classdef_graph_LDADD = libharfbuzz.la libharfbuzz-subset.la $(HBLIBS) + test_set_SOURCES = test-set.cc hb-static.cc test_set_CPPFLAGS = $(COMPILED_TESTS_CPPFLAGS) test_set_LDADD = $(COMPILED_TESTS_LDADD) diff --git a/src/graph/classdef-graph.hh b/src/graph/classdef-graph.hh index fd6472a0d..38130952f 100644 --- a/src/graph/classdef-graph.hh +++ b/src/graph/classdef-graph.hh @@ -123,6 +123,88 @@ struct ClassDef : public OT::ClassDef }; +struct class_def_size_estimator_t +{ + template + class_def_size_estimator_t (It glyph_and_class) + : gids_consecutive (true), num_ranges_per_class (), glyphs_per_class () + { + unsigned last_gid = (unsigned) -1; + for (auto p : + glyph_and_class) + { + unsigned gid = p.first; + unsigned klass = p.second; + + if (last_gid != (unsigned) -1 && gid != last_gid + 1) + gids_consecutive = false; + last_gid = gid; + + hb_set_t* glyphs; + if (glyphs_per_class.has (klass, &glyphs) && glyphs) { + glyphs->add (gid); + continue; + } + + hb_set_t new_glyphs; + new_glyphs.add (gid); + glyphs_per_class.set (klass, std::move (new_glyphs)); + } + + if (in_error ()) return; + + for (unsigned klass : glyphs_per_class.keys ()) + { + if (!klass) continue; // class 0 doesn't get encoded. + + const hb_set_t& glyphs = glyphs_per_class.get (klass); + hb_codepoint_t start = HB_SET_VALUE_INVALID; + hb_codepoint_t end = HB_SET_VALUE_INVALID; + + unsigned count = 0; + while (glyphs.next_range (&start, &end)) + count++; + + num_ranges_per_class.set (klass, count); + } + } + + // Incremental increase in the Coverage and ClassDef table size + // (worst case) if all glyphs associated with 'klass' were added. + unsigned incremental_size_for_class (unsigned klass) const + { + // Coverage takes 2 bytes per glyph worst case, + unsigned cov_size = 2 * glyphs_per_class.get (klass).get_population (); + // ClassDef takes 6 bytes per range + unsigned class_def_2_size = 6 * num_ranges_per_class.get (klass); + if (gids_consecutive) + { + // ClassDef1 takes 2 bytes per glyph, but only can be used + // when gids are consecutive. + return cov_size + hb_min (cov_size, class_def_2_size); + } + + return cov_size + class_def_2_size; + } + + bool in_error () + { + if (num_ranges_per_class.in_error ()) return true; + if (glyphs_per_class.in_error ()) return true; + + for (const hb_set_t& s : glyphs_per_class.values ()) + { + if (s.in_error ()) return true; + } + return false; + } + + private: + bool gids_consecutive; + hb_hashmap_t num_ranges_per_class; + hb_hashmap_t glyphs_per_class; +}; + + } #endif // GRAPH_CLASSDEF_GRAPH_HH diff --git a/src/graph/test-classdef-graph.cc b/src/graph/test-classdef-graph.cc new file mode 100644 index 000000000..908658446 --- /dev/null +++ b/src/graph/test-classdef-graph.cc @@ -0,0 +1,110 @@ +/* + * Copyright © 2022 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 + */ + +#include "gsubgpos-context.hh" +#include "classdef-graph.hh" + +typedef hb_pair_t gid_and_class_t; +typedef hb_vector_t gid_and_class_list_t; + + +static bool incremental_size_is (const gid_and_class_list_t& list, unsigned klass, unsigned expected) +{ + graph::class_def_size_estimator_t estimator (list.iter ()); + unsigned result = estimator.incremental_size_for_class (klass); + if (result != expected) + { + printf ("FAIL: expected size %u but was %u\n", expected, result); + return false; + } + + return true; +} + +static void test_class_and_coverage_size_estimates () +{ + gid_and_class_list_t empty = { + }; + assert (incremental_size_is (empty, 0, 0)); + assert (incremental_size_is (empty, 1, 0)); + + gid_and_class_list_t class_zero = { + {5, 0}, + }; + assert (incremental_size_is (class_zero, 0, 2)); + + gid_and_class_list_t consecutive = { + {4, 0}, + {5, 0}, + {6, 1}, + {7, 1}, + {8, 2}, + {9, 2}, + {10, 2}, + {11, 2}, + }; + assert (incremental_size_is (consecutive, 0, 4)); + assert (incremental_size_is (consecutive, 1, 4 + 4)); + assert (incremental_size_is (consecutive, 2, 8 + 6)); + + gid_and_class_list_t non_consecutive = { + {4, 0}, + {5, 0}, + + {6, 1}, + {7, 1}, + + {9, 2}, + {10, 2}, + {11, 2}, + {12, 2}, + }; + assert (incremental_size_is (non_consecutive, 0, 4)); + assert (incremental_size_is (non_consecutive, 1, 4 + 6)); + assert (incremental_size_is (non_consecutive, 2, 8 + 6)); + + gid_and_class_list_t multiple_ranges = { + {4, 0}, + {5, 0}, + + {6, 1}, + {7, 1}, + + {9, 1}, + + {11, 1}, + {12, 1}, + {13, 1}, + }; + assert (incremental_size_is (multiple_ranges, 0, 4)); + assert (incremental_size_is (multiple_ranges, 1, 2 * 6 + 3 * 6)); +} + +int +main (int argc, char **argv) +{ + test_class_and_coverage_size_estimates (); +} diff --git a/src/meson.build b/src/meson.build index 5e0c172c6..9ee919dd1 100644 --- a/src/meson.build +++ b/src/meson.build @@ -581,6 +581,7 @@ if get_option('tests').enabled() 'test-ot-tag': ['hb-ot-tag.cc'], 'test-priority-queue': ['test-priority-queue.cc', 'hb-static.cc'], 'test-repacker': ['test-repacker.cc', 'hb-static.cc', 'graph/gsubgpos-context.cc'], + 'test-classdef-graph': ['graph/test-classdef-graph.cc', 'hb-static.cc', 'graph/gsubgpos-context.cc'], 'test-set': ['test-set.cc', 'hb-static.cc'], 'test-serialize': ['test-serialize.cc', 'hb-static.cc'], 'test-unicode-ranges': ['test-unicode-ranges.cc'], From 0e48a65d329b4ecbadc31359fc3f7f57c8f3de5a Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Fri, 5 Aug 2022 20:19:11 +0000 Subject: [PATCH 21/27] [repacker] estimate size of classDef1 and coverage during PairPos2 split point analysis. --- src/graph/classdef-graph.hh | 14 +++++-- src/graph/pairpos-graph.hh | 57 ++++++++++++++++++++++----- src/graph/test-classdef-graph.cc | 39 +++++++++++-------- src/test-repacker.cc | 66 +++++++++++++++++++++++++------- 4 files changed, 134 insertions(+), 42 deletions(-) diff --git a/src/graph/classdef-graph.hh b/src/graph/classdef-graph.hh index 38130952f..0bda76ac2 100644 --- a/src/graph/classdef-graph.hh +++ b/src/graph/classdef-graph.hh @@ -170,20 +170,26 @@ struct class_def_size_estimator_t // Incremental increase in the Coverage and ClassDef table size // (worst case) if all glyphs associated with 'klass' were added. - unsigned incremental_size_for_class (unsigned klass) const + unsigned incremental_coverage_size (unsigned klass) const { // Coverage takes 2 bytes per glyph worst case, - unsigned cov_size = 2 * glyphs_per_class.get (klass).get_population (); + return 2 * glyphs_per_class.get (klass).get_population (); + } + + // Incremental increase in the Coverage and ClassDef table size + // (worst case) if all glyphs associated with 'klass' were added. + unsigned incremental_class_def_size (unsigned klass) const + { // ClassDef takes 6 bytes per range unsigned class_def_2_size = 6 * num_ranges_per_class.get (klass); if (gids_consecutive) { // ClassDef1 takes 2 bytes per glyph, but only can be used // when gids are consecutive. - return cov_size + hb_min (cov_size, class_def_2_size); + return hb_min (2 * glyphs_per_class.get (klass).get_population (), class_def_2_size); } - return cov_size + class_def_2_size; + return class_def_2_size; } bool in_error () diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index 4cfcd89c5..0c5a096b1 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -206,10 +206,17 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 split_subtables (gsubgpos_graph_context_t& c, unsigned this_index) { - const unsigned base_size = OT::Layout::GPOS_impl::PairPosFormat2_4::min_size - + size_of (c, this_index, &coverage) - + size_of (c, this_index, &classDef1) - + size_of (c, this_index, &classDef2); + const unsigned base_size = OT::Layout::GPOS_impl::PairPosFormat2_4::min_size; + const unsigned class_def_2_size = size_of (c, this_index, &classDef2); + const Coverage* coverage = get_coverage (c, this_index); + const ClassDef* class_def_1 = get_class_def_1 (c, this_index); + auto gid_and_class = + + coverage->iter () + | hb_map_retains_sorting ([&] (hb_codepoint_t gid) { + return hb_pair_t (gid, class_def_1->get_class (gid)); + }) + ; + class_def_size_estimator_t estimator (gid_and_class); const unsigned class1_count = class1Count; const unsigned class2_count = class2Count; @@ -220,6 +227,9 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 split_points; hb_hashmap_t device_tables = get_all_device_tables (c, this_index); @@ -231,6 +241,9 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 (1 << 16)) + unsigned total = accumulated + + coverage_size + class_def_1_size + class_def_2_size + // The largest object will pack last and can exceed the size limit. + - hb_max (hb_max (coverage_size, class_def_1_size), class_def_2_size); + if (total > (1 << 16)) { split_points.push (i); accumulated = base_size; + coverage_size = 4; + class_def_1_size = 4; visited.clear (); // node sharing isn't allowed between splits. } } @@ -526,6 +541,30 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4sanitize (coverage_v)) + return &Null(Coverage); + return coverage_table; + } + + const ClassDef* get_class_def_1 (gsubgpos_graph_context_t& c, + unsigned this_index) const + { + unsigned class_def_1_id = c.graph.index_for_offset (this_index, &classDef1); + auto& class_def_1_v = c.graph.vertices_[class_def_1_id]; + + ClassDef* class_def_1_table = (ClassDef*) class_def_1_v.obj.head; + if (!class_def_1_table->sanitize (class_def_1_v)) + return &Null(ClassDef); + return class_def_1_table; + } + unsigned size_of_value_record_children (gsubgpos_graph_context_t& c, const hb_hashmap_t& device_tables, const hb_vector_t device_table_indices, diff --git a/src/graph/test-classdef-graph.cc b/src/graph/test-classdef-graph.cc index 908658446..55854ff5c 100644 --- a/src/graph/test-classdef-graph.cc +++ b/src/graph/test-classdef-graph.cc @@ -31,13 +31,22 @@ typedef hb_pair_t gid_and_class_t; typedef hb_vector_t gid_and_class_list_t; -static bool incremental_size_is (const gid_and_class_list_t& list, unsigned klass, unsigned expected) +static bool incremental_size_is (const gid_and_class_list_t& list, unsigned klass, + unsigned cov_expected, unsigned class_def_expected) { graph::class_def_size_estimator_t estimator (list.iter ()); - unsigned result = estimator.incremental_size_for_class (klass); - if (result != expected) + + unsigned result = estimator.incremental_coverage_size (klass); + if (result != cov_expected) { - printf ("FAIL: expected size %u but was %u\n", expected, result); + printf ("FAIL: coverage expected size %u but was %u\n", cov_expected, result); + return false; + } + + result = estimator.incremental_class_def_size (klass); + if (result != class_def_expected) + { + printf ("FAIL: class def expected size %u but was %u\n", class_def_expected, result); return false; } @@ -48,13 +57,13 @@ static void test_class_and_coverage_size_estimates () { gid_and_class_list_t empty = { }; - assert (incremental_size_is (empty, 0, 0)); - assert (incremental_size_is (empty, 1, 0)); + assert (incremental_size_is (empty, 0, 0, 0)); + assert (incremental_size_is (empty, 1, 0, 0)); gid_and_class_list_t class_zero = { {5, 0}, }; - assert (incremental_size_is (class_zero, 0, 2)); + assert (incremental_size_is (class_zero, 0, 2, 0)); gid_and_class_list_t consecutive = { {4, 0}, @@ -66,9 +75,9 @@ static void test_class_and_coverage_size_estimates () {10, 2}, {11, 2}, }; - assert (incremental_size_is (consecutive, 0, 4)); - assert (incremental_size_is (consecutive, 1, 4 + 4)); - assert (incremental_size_is (consecutive, 2, 8 + 6)); + assert (incremental_size_is (consecutive, 0, 4, 0)); + assert (incremental_size_is (consecutive, 1, 4, 4)); + assert (incremental_size_is (consecutive, 2, 8, 6)); gid_and_class_list_t non_consecutive = { {4, 0}, @@ -82,9 +91,9 @@ static void test_class_and_coverage_size_estimates () {11, 2}, {12, 2}, }; - assert (incremental_size_is (non_consecutive, 0, 4)); - assert (incremental_size_is (non_consecutive, 1, 4 + 6)); - assert (incremental_size_is (non_consecutive, 2, 8 + 6)); + assert (incremental_size_is (non_consecutive, 0, 4, 0)); + assert (incremental_size_is (non_consecutive, 1, 4, 6)); + assert (incremental_size_is (non_consecutive, 2, 8, 6)); gid_and_class_list_t multiple_ranges = { {4, 0}, @@ -99,8 +108,8 @@ static void test_class_and_coverage_size_estimates () {12, 1}, {13, 1}, }; - assert (incremental_size_is (multiple_ranges, 0, 4)); - assert (incremental_size_is (multiple_ranges, 1, 2 * 6 + 3 * 6)); + assert (incremental_size_is (multiple_ranges, 0, 4, 0)); + assert (incremental_size_is (multiple_ranges, 1, 2 * 6, 3 * 6)); } int diff --git a/src/test-repacker.cc b/src/test-repacker.cc index e9aea87a3..e6ac36e7a 100644 --- a/src/test-repacker.cc +++ b/src/test-repacker.cc @@ -144,28 +144,37 @@ static unsigned add_extension (unsigned child, } // Adds coverage table fro [start, end] -static unsigned add_coverage (char start, char end, +static unsigned add_coverage (unsigned start, unsigned end, hb_serialize_context_t* c) { if (end - start == 1) { - char coverage[] = { + uint8_t coverage[] = { 0, 1, // format 0, 2, // count - 0, start, // glyph[0] - 0, end, // glyph[1] + + (uint8_t) ((start >> 8) & 0xFF), + (uint8_t) (start & 0xFF), // glyph[0] + + (uint8_t) ((end >> 8) & 0xFF), + (uint8_t) (end & 0xFF), // glyph[1] }; - return add_object (coverage, 8, c); + return add_object ((char*) coverage, 8, c); } - char coverage[] = { + uint8_t coverage[] = { 0, 2, // format 0, 1, // range count - 0, start, // start - 0, end, // end + + (uint8_t) ((start >> 8) & 0xFF), + (uint8_t) (start & 0xFF), // start + + (uint8_t) ((end >> 8) & 0xFF), + (uint8_t) (end & 0xFF), // end + 0, 0, }; - return add_object (coverage, 10, c); + return add_object ((char*) coverage, 10, c); } // Adds a class that maps glyphs from [start_glyph, end_glyph) @@ -1207,7 +1216,8 @@ template static void populate_serializer_with_large_pair_pos_2 (hb_serialize_context_t* c, bool as_extension = false, - bool with_device_tables = false) + bool with_device_tables = false, + bool extra_table = true) { std::string large_string(100000, 'a'); c->start_serialize (); @@ -1267,22 +1277,26 @@ populate_serializer_with_large_pair_pos_2 (hb_serialize_context_t* c, c); } - unsigned pair_pos_1 = add_object (large_string.c_str(), 100000, c); + unsigned pair_pos_1 = 0; + if (extra_table) pair_pos_1 = add_object (large_string.c_str(), 100000, c); if (as_extension) { for (int i = num_pair_pos_2 - 1; i >= 0; i--) pair_pos_2[i] = add_extension (pair_pos_2[i], 2, c); - pair_pos_1 = add_extension (pair_pos_1, 2, c); + + if (extra_table) + pair_pos_1 = add_extension (pair_pos_1, 2, c); } start_lookup (as_extension ? 9 : 2, 1 + num_pair_pos_2, c); - add_offset (pair_pos_1, c); + if (extra_table) + add_offset (pair_pos_1, c); + for (int i = 0; i < num_pair_pos_2; i++) add_offset (pair_pos_2[i], c); - unsigned lookup = finish_lookup (c); unsigned lookup_list = add_lookup_list (&lookup, 1, c); @@ -1732,6 +1746,29 @@ static void test_resolve_with_basic_pair_pos_2_split () free (expected_buffer); } +static void test_resolve_with_close_to_limit_pair_pos_2_split () +{ + size_t buffer_size = 300000; + void* buffer = malloc (buffer_size); + assert (buffer); + hb_serialize_context_t c (buffer, buffer_size); + populate_serializer_with_large_pair_pos_2 <1, 1596, 10>(&c, true, false, false); + + void* expected_buffer = malloc (buffer_size); + assert (expected_buffer); + hb_serialize_context_t e (expected_buffer, buffer_size); + populate_serializer_with_large_pair_pos_2 <2, 798, 10>(&e, true, false, false); + + run_resolve_overflow_test ("test_resolve_with_close_to_limit_pair_pos_2_split", + c, + e, + 20, + true, + HB_TAG('G', 'P', 'O', 'S')); + free (buffer); + free (expected_buffer); +} + static void test_resolve_with_pair_pos_2_split_with_device_tables () { size_t buffer_size = 300000; @@ -1906,6 +1943,7 @@ main (int argc, char **argv) test_resolve_with_extension_pair_pos_1_split (); test_resolve_with_basic_pair_pos_2_split (); test_resolve_with_pair_pos_2_split_with_device_tables (); + test_resolve_with_close_to_limit_pair_pos_2_split (); // TODO(grieger): have run overflow tests compare graph equality not final packed binary. // TODO(grieger): split test where multiple subtables in one lookup are split to test link ordering. From b37e8bef0ec1401710e10bf83ac83da7449e3178 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Fri, 5 Aug 2022 22:16:20 +0000 Subject: [PATCH 22/27] [repacker] count size of the current class at the split point in the next segment. --- src/graph/pairpos-graph.hh | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index 0c5a096b1..ad691195b 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -240,7 +240,7 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 (1 << 16)) { split_points.push (i); - accumulated = base_size; - coverage_size = 4; - class_def_1_size = 4; + // split does not include i, so add the size for i when we reset the size counters. + accumulated = base_size + accumulated_delta; + coverage_size = 4 + estimator.incremental_coverage_size (i); + class_def_1_size = 4 + estimator.incremental_class_def_size (i); visited.clear (); // node sharing isn't allowed between splits. } } From dde0a2b0711a5922db6124f08d62fe35f3500dd5 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Fri, 5 Aug 2022 22:30:37 +0000 Subject: [PATCH 23/27] [repacker] track estimated coverage size during PairPosFormat1 split point analysis. --- src/graph/pairpos-graph.hh | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index ad691195b..1c2f8a302 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -53,25 +53,27 @@ struct PairPosFormat1 : public OT::Layout::GPOS_impl::PairPosFormat1_3::min_size - + coverage_size; + const unsigned base_size = OT::Layout::GPOS_impl::PairPosFormat1_3::min_size; + unsigned partial_coverage_size = 4; unsigned accumulated = base_size; hb_vector_t split_points; for (unsigned i = 0; i < pairSet.len; i++) { unsigned pair_set_index = pair_set_graph_index (c, this_index, i); - accumulated += c.graph.find_subgraph_size (pair_set_index, visited); - accumulated += SmallTypes::size; // for PairSet offset. + unsigned accumulated_delta = + c.graph.find_subgraph_size (pair_set_index, visited) + + SmallTypes::size; // for PairSet offset. + partial_coverage_size += OT::HBUINT16::static_size; - // TODO(garretrieger): don't count the size of the largest pairset against the limit, since - // it will be packed last in the order and does not contribute to - // the 64kb limit. + accumulated += accumulated_delta; + unsigned total = accumulated + hb_min (partial_coverage_size, coverage_size); - if (accumulated > (1 << 16)) + if (total >= (1 << 16)) { split_points.push (i); - accumulated = base_size; + accumulated = base_size + accumulated_delta; + partial_coverage_size = 6; visited.clear (); // node sharing isn't allowed between splits. } } @@ -267,7 +269,7 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 (1 << 16)) + if (total >= (1 << 16)) { split_points.push (i); // split does not include i, so add the size for i when we reset the size counters. From 13253233f77196011d0eba3a55e3481381a9a68f Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Fri, 5 Aug 2022 23:15:10 +0000 Subject: [PATCH 24/27] [repacker] in PairPosFormat2 splitting use the max estimated coverage/classdef size for sizing serialization buffers. --- src/graph/pairpos-graph.hh | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index 1c2f8a302..d9edfbb55 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -231,6 +231,8 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 split_points; @@ -245,6 +247,8 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4& device_tables; const hb_vector_t& format1_device_table_indices; @@ -391,7 +399,7 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 Date: Fri, 5 Aug 2022 23:37:11 +0000 Subject: [PATCH 25/27] [repacker] Check for nullptr's before sanitizing. --- src/graph/coverage-graph.hh | 2 +- src/graph/gsubgpos-graph.hh | 11 +++++------ src/graph/pairpos-graph.hh | 14 +++++++++----- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/graph/coverage-graph.hh b/src/graph/coverage-graph.hh index 7309029c2..da71ea6fb 100644 --- a/src/graph/coverage-graph.hh +++ b/src/graph/coverage-graph.hh @@ -66,7 +66,7 @@ struct Coverage : public OT::Layout::Common::Coverage unsigned coverage_size = c.graph.vertices_[coverage_id].table_size (); auto& coverage_v = c.graph.vertices_[coverage_id]; Coverage* coverage_table = (Coverage*) coverage_v.obj.head; - if (!coverage_table->sanitize (coverage_v)) + if (!coverage_table || !coverage_table->sanitize (coverage_v)) return nullptr; auto new_coverage = diff --git a/src/graph/gsubgpos-graph.hh b/src/graph/gsubgpos-graph.hh index 98302ff75..f963a49ad 100644 --- a/src/graph/gsubgpos-graph.hh +++ b/src/graph/gsubgpos-graph.hh @@ -133,7 +133,7 @@ struct Lookup : public OT::Lookup ExtensionFormat1* extension = (ExtensionFormat1*) c.graph.object (ext_subtable_index).head; - if (!extension->sanitize (c.graph.vertices_[ext_subtable_index])) + if (!extension || !extension->sanitize (c.graph.vertices_[ext_subtable_index])) continue; subtable_index = extension->get_subtable_index (c.graph, ext_subtable_index); @@ -143,7 +143,7 @@ struct Lookup : public OT::Lookup } PairPos* pairPos = (PairPos*) c.graph.object (subtable_index).head; - if (!pairPos->sanitize (c.graph.vertices_[subtable_index])) continue; + if (!pairPos || !pairPos->sanitize (c.graph.vertices_[subtable_index])) continue; hb_vector_t new_sub_tables = pairPos->split_subtables (c, subtable_index); if (new_sub_tables.in_error ()) return false; @@ -320,7 +320,7 @@ struct GSTAR : public OT::GSUBGPOS const auto& r = graph.root (); GSTAR* gstar = (GSTAR*) r.obj.head; - if (!gstar->sanitize (r)) + if (!gstar || !gstar->sanitize (r)) return nullptr; return gstar; @@ -366,17 +366,16 @@ struct GSTAR : public OT::GSUBGPOS hb_hashmap_t& lookups /* OUT */) { unsigned lookup_list_idx = get_lookup_list_index (graph); - const LookupList* lookupList = (const LookupList*) graph.object (lookup_list_idx).head; - if (!lookupList->sanitize (graph.vertices_[lookup_list_idx])) + if (!lookupList || !lookupList->sanitize (graph.vertices_[lookup_list_idx])) return; for (unsigned i = 0; i < lookupList->len; i++) { unsigned lookup_idx = graph.index_for_offset (lookup_list_idx, &(lookupList->arrayZ[i])); Lookup* lookup = (Lookup*) graph.object (lookup_idx).head; - if (!lookup->sanitize (graph.vertices_[lookup_idx])) continue; + if (!lookup || !lookup->sanitize (graph.vertices_[lookup_idx])) continue; lookups.set (lookup_idx, lookup); } } diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index d9edfbb55..29dee54d4 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -130,7 +130,7 @@ struct PairPosFormat1 : public OT::Layout::GPOS_impl::PairPosFormat1_3sanitize (coverage_v)) + if (!coverage_table || !coverage_table->sanitize (coverage_v)) return false; auto new_coverage = @@ -377,7 +377,9 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4sanitize (coverage_v) + if (!coverage_table + || !coverage_table->sanitize (coverage_v) + || !class_def_1_table || !class_def_1_table->sanitize (class_def_1_v)) return -1; @@ -513,7 +515,9 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4sanitize (coverage_v) + if (!coverage_table + || !coverage_table->sanitize (coverage_v) + || !class_def_1_table || !class_def_1_table->sanitize (class_def_1_v)) return false; @@ -560,7 +564,7 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4sanitize (coverage_v)) + if (!coverage_table || !coverage_table->sanitize (coverage_v)) return &Null(Coverage); return coverage_table; } @@ -572,7 +576,7 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4sanitize (class_def_1_v)) + if (!class_def_1_table || !class_def_1_table->sanitize (class_def_1_v)) return &Null(ClassDef); return class_def_1_table; } From fe15f2559f44377a40da50ff4fdbbc8438de8670 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Mon, 8 Aug 2022 16:57:28 +0000 Subject: [PATCH 26/27] [repacker] use position instead of memory address as key in device_tables map. --- src/graph/pairpos-graph.hh | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index 29dee54d4..769f29064 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -236,7 +236,7 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 split_points; - hb_hashmap_t device_tables = get_all_device_tables (c, this_index); + hb_hashmap_t device_tables = get_all_device_tables (c, this_index); hb_vector_t format1_device_table_indices = valueFormat1.get_device_table_indices (); hb_vector_t format2_device_table_indices = valueFormat2.get_device_table_indices (); bool has_device_tables = bool(format1_device_table_indices) || bool(format2_device_table_indices); @@ -315,7 +315,7 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4& device_tables; + const hb_hashmap_t& device_tables; const hb_vector_t& format1_device_table_indices; const hb_vector_t& format2_device_table_indices; @@ -481,7 +481,8 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 + hb_hashmap_t get_all_device_tables (gsubgpos_graph_context_t& c, unsigned this_index) const { - hb_hashmap_t result; + hb_hashmap_t result; const auto& o = c.graph.object (this_index); for (const auto& l : o.real_links) { - result.set ((void*) (((uint8_t*)this) + l.position), l.objidx); + result.set (l.position, l.objidx); } return result; @@ -582,7 +583,7 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4& device_tables, + const hb_hashmap_t& device_tables, const hb_vector_t device_table_indices, unsigned value_record_index, hb_set_t& visited) @@ -591,8 +592,9 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 Date: Mon, 8 Aug 2022 17:07:14 +0000 Subject: [PATCH 27/27] [repacker] Make actuate_subtable_split internal. --- src/graph/split-helpers.hh | 1 + 1 file changed, 1 insertion(+) diff --git a/src/graph/split-helpers.hh b/src/graph/split-helpers.hh index 2fbffda07..61fd7c2d2 100644 --- a/src/graph/split-helpers.hh +++ b/src/graph/split-helpers.hh @@ -30,6 +30,7 @@ namespace graph { template +HB_INTERNAL hb_vector_t actuate_subtable_split (Context& split_context, const hb_vector_t& split_points) {