From 252a926fbfc1167c81b9a82bdb3caf2f0d90f8c8 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Fri, 8 Mar 2024 19:46:48 +0000 Subject: [PATCH 1/8] [repacker] Rework how ClassDef sizes are estimated during splitting. The old approach considered only one class at a time, which in some cases can generate the wrong answer. This change updates the estimation to consider how all classes in the current split would end up encoded in a single ClassDef table. Additionally compute whether glyphs are consecutive only for the current split (instead of the fully mapping). --- src/graph/classdef-graph.hh | 63 ++++++++++++----- src/graph/pairpos-graph.hh | 8 ++- src/graph/test-classdef-graph.cc | 115 ++++++++++++++++++++++++++----- 3 files changed, 149 insertions(+), 37 deletions(-) diff --git a/src/graph/classdef-graph.hh b/src/graph/classdef-graph.hh index 9cf845a82..685ee5d3e 100644 --- a/src/graph/classdef-graph.hh +++ b/src/graph/classdef-graph.hh @@ -134,20 +134,17 @@ struct ClassDef : public OT::ClassDef struct class_def_size_estimator_t { + // TODO(garretrieger): for coverage take the same approach as class def (compute the running total for each format). template class_def_size_estimator_t (It glyph_and_class) - : gids_consecutive (true), num_ranges_per_class (), glyphs_per_class () + : num_ranges_per_class (), glyphs_per_class () { - unsigned last_gid = (unsigned) -1; + reset(); 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); @@ -177,6 +174,13 @@ struct class_def_size_estimator_t } } + void reset() { + class_def_1_size = 4; + class_def_2_size = 4; + included_glyphs.clear(); + included_classes.clear(); + } + // Incremental increase in the Coverage and ClassDef table size // (worst case) if all glyphs associated with 'klass' were added. unsigned incremental_coverage_size (unsigned klass) const @@ -185,20 +189,40 @@ struct class_def_size_estimator_t 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 + // Compute the new size of the ClassDef table if all glyphs associated with 'klass' were added. + unsigned class_def_size (unsigned klass) { - // 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 hb_min (2 * glyphs_per_class.get (klass).get_population (), class_def_2_size); + if (!included_classes.has(klass)) { + // ClassDef 1 takes 2 bytes per glyph. + class_def_1_size += 2 * glyphs_per_class.get (klass).get_population (); + // ClassDef 2 takes 6 bytes per range. + class_def_2_size += 6 * num_ranges_per_class.get (klass); + + hb_set_t* glyphs = nullptr; + if (glyphs_per_class.has(klass, &glyphs)) { + included_glyphs.union_(*glyphs); + } + + included_classes.add(klass); } - return class_def_2_size; + if (!gids_consecutive()) + return class_def_2_size; + + // ClassDef1 can only be used when gids are consecutive. + return hb_min (class_def_1_size, class_def_2_size); + } + + bool gids_consecutive() const { + hb_codepoint_t start = HB_SET_VALUE_INVALID; + hb_codepoint_t end = HB_SET_VALUE_INVALID; + + unsigned count = 0; + while (included_glyphs.next_range (&start, &end)) { + count++; + if (count > 1) return false; + } + return true; } bool in_error () @@ -214,9 +238,12 @@ struct class_def_size_estimator_t } private: - bool gids_consecutive; hb_hashmap_t num_ranges_per_class; hb_hashmap_t glyphs_per_class; + hb_set_t included_classes; + hb_set_t included_glyphs; + unsigned class_def_1_size = 4; + unsigned class_def_2_size = 4; }; diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index f7f74b18c..5c137b12b 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -232,7 +232,7 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 Date: Fri, 8 Mar 2024 20:15:43 +0000 Subject: [PATCH 2/8] Update classdef size estimator to pick the min coverage format. Previously this just assumed a worst case format 1. --- src/graph/classdef-graph.hh | 26 ++++++++--- src/graph/pairpos-graph.hh | 8 ++-- src/graph/test-classdef-graph.cc | 78 ++++++++++++++++++-------------- 3 files changed, 68 insertions(+), 44 deletions(-) diff --git a/src/graph/classdef-graph.hh b/src/graph/classdef-graph.hh index 685ee5d3e..4dfd5d6d8 100644 --- a/src/graph/classdef-graph.hh +++ b/src/graph/classdef-graph.hh @@ -26,6 +26,7 @@ #include "graph.hh" #include "../hb-ot-layout-common.hh" +#include #ifndef GRAPH_CLASSDEF_GRAPH_HH #define GRAPH_CLASSDEF_GRAPH_HH @@ -181,16 +182,18 @@ struct class_def_size_estimator_t included_classes.clear(); } - // Incremental increase in the Coverage and ClassDef table size - // (worst case) if all glyphs associated with 'klass' were added. - unsigned incremental_coverage_size (unsigned klass) const + // Compute the size of coverage for all glyphs added via 'add_class_def_size'. + unsigned coverage_size () const { - // Coverage takes 2 bytes per glyph worst case, - return 2 * glyphs_per_class.get (klass).get_population (); + // format 1 is 2 bytes per glyph. + unsigned format1_size = 4 + 2 * included_glyphs.get_population(); + // format 2 is 6 bytes per range. + unsigned format2_size = 4 + 6 * num_glyph_ranges(); + return hb_min(format1_size, format2_size); } // Compute the new size of the ClassDef table if all glyphs associated with 'klass' were added. - unsigned class_def_size (unsigned klass) + unsigned add_class_def_size (unsigned klass) { if (!included_classes.has(klass)) { // ClassDef 1 takes 2 bytes per glyph. @@ -213,6 +216,17 @@ struct class_def_size_estimator_t return hb_min (class_def_1_size, class_def_2_size); } + unsigned num_glyph_ranges() const { + hb_codepoint_t start = HB_SET_VALUE_INVALID; + hb_codepoint_t end = HB_SET_VALUE_INVALID; + + unsigned count = 0; + while (included_glyphs.next_range (&start, &end)) { + count++; + } + return count; + } + bool gids_consecutive() const { hb_codepoint_t start = HB_SET_VALUE_INVALID; hb_codepoint_t end = HB_SET_VALUE_INVALID; diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index 5c137b12b..309a41157 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -247,8 +247,8 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 Date: Fri, 8 Mar 2024 21:53:10 +0000 Subject: [PATCH 3/8] [repacker] in classdef estimator tests compare results to actual class def serialization. Fix the estimator to actually match real serialization sizes. --- src/graph/classdef-graph.hh | 48 ++++----- src/graph/test-classdef-graph.cc | 167 ++++++++++++++++++++++--------- 2 files changed, 142 insertions(+), 73 deletions(-) diff --git a/src/graph/classdef-graph.hh b/src/graph/classdef-graph.hh index 4dfd5d6d8..a2aa8990f 100644 --- a/src/graph/classdef-graph.hh +++ b/src/graph/classdef-graph.hh @@ -135,7 +135,13 @@ struct ClassDef : public OT::ClassDef struct class_def_size_estimator_t { - // TODO(garretrieger): for coverage take the same approach as class def (compute the running total for each format). + // TODO(garretrieger): update to support beyond64k coverage/classdef tables. + constexpr static unsigned class_def_format1_base_size = 6; + constexpr static unsigned class_def_format2_base_size = 4; + constexpr static unsigned coverage_base_size = 4; + constexpr static unsigned bytes_per_range = 6; + constexpr static unsigned bytes_per_glyph = 2; + template class_def_size_estimator_t (It glyph_and_class) : num_ranges_per_class (), glyphs_per_class () @@ -176,8 +182,8 @@ struct class_def_size_estimator_t } void reset() { - class_def_1_size = 4; - class_def_2_size = 4; + class_def_1_size = class_def_format1_base_size; + class_def_2_size = class_def_format2_base_size; included_glyphs.clear(); included_classes.clear(); } @@ -185,10 +191,8 @@ struct class_def_size_estimator_t // Compute the size of coverage for all glyphs added via 'add_class_def_size'. unsigned coverage_size () const { - // format 1 is 2 bytes per glyph. - unsigned format1_size = 4 + 2 * included_glyphs.get_population(); - // format 2 is 6 bytes per range. - unsigned format2_size = 4 + 6 * num_glyph_ranges(); + unsigned format1_size = coverage_base_size + bytes_per_glyph * included_glyphs.get_population(); + unsigned format2_size = coverage_base_size + bytes_per_range * num_glyph_ranges(); return hb_min(format1_size, format2_size); } @@ -196,23 +200,23 @@ struct class_def_size_estimator_t unsigned add_class_def_size (unsigned klass) { if (!included_classes.has(klass)) { - // ClassDef 1 takes 2 bytes per glyph. - class_def_1_size += 2 * glyphs_per_class.get (klass).get_population (); - // ClassDef 2 takes 6 bytes per range. - class_def_2_size += 6 * num_ranges_per_class.get (klass); - hb_set_t* glyphs = nullptr; if (glyphs_per_class.has(klass, &glyphs)) { included_glyphs.union_(*glyphs); } + class_def_1_size = class_def_format1_base_size; + if (!included_glyphs.is_empty()) { + unsigned min_glyph = included_glyphs.get_min(); + unsigned max_glyph = included_glyphs.get_max(); + class_def_1_size += bytes_per_glyph * (max_glyph - min_glyph + 1); + } + + class_def_2_size += bytes_per_range * num_ranges_per_class.get (klass); + included_classes.add(klass); } - if (!gids_consecutive()) - return class_def_2_size; - - // ClassDef1 can only be used when gids are consecutive. return hb_min (class_def_1_size, class_def_2_size); } @@ -227,18 +231,6 @@ struct class_def_size_estimator_t return count; } - bool gids_consecutive() const { - hb_codepoint_t start = HB_SET_VALUE_INVALID; - hb_codepoint_t end = HB_SET_VALUE_INVALID; - - unsigned count = 0; - while (included_glyphs.next_range (&start, &end)) { - count++; - if (count > 1) return false; - } - return true; - } - bool in_error () { if (num_ranges_per_class.in_error ()) return true; diff --git a/src/graph/test-classdef-graph.cc b/src/graph/test-classdef-graph.cc index 4bd4793a2..bb569e1e7 100644 --- a/src/graph/test-classdef-graph.cc +++ b/src/graph/test-classdef-graph.cc @@ -26,27 +26,113 @@ #include "gsubgpos-context.hh" #include "classdef-graph.hh" +#include "hb-iter.hh" +#include "hb-serialize.hh" typedef hb_codepoint_pair_t gid_and_class_t; typedef hb_vector_t gid_and_class_list_t; +template +static unsigned actual_class_def_size(It glyph_and_class) { + char buffer[100]; + hb_serialize_context_t serializer(buffer, 100); + OT::ClassDef_serialize (&serializer, glyph_and_class); + serializer.end_serialize (); + assert(!serializer.in_error()); + hb_bytes_t class_def_copy = serializer.copy_bytes (); + return class_def_copy.get_size(); +} -static bool incremental_size_is (const gid_and_class_list_t& list, unsigned klass, - unsigned cov_expected, unsigned class_def_expected) +static unsigned actual_class_def_size(gid_and_class_list_t consecutive_map, hb_vector_t classes) { + auto filtered_it = + + consecutive_map.as_sorted_array().iter() + | hb_filter([&] (unsigned c) { + for (unsigned klass : classes) { + if (c == klass) { + return true; + } + } + return false; + }, hb_second); + return actual_class_def_size(+ filtered_it); +} + +template +static unsigned actual_coverage_size(It glyphs) { + char buffer[100]; + hb_serialize_context_t serializer(buffer, 100); + OT::Layout::Common::Coverage_serialize (&serializer, glyphs); + serializer.end_serialize (); + assert(!serializer.in_error()); + hb_bytes_t coverage_copy = serializer.copy_bytes (); + return coverage_copy.get_size(); +} + +static unsigned actual_coverage_size(gid_and_class_list_t consecutive_map, hb_vector_t classes) { + auto filtered_it = + + consecutive_map.as_sorted_array().iter() + | hb_filter([&] (unsigned c) { + for (unsigned klass : classes) { + if (c == klass) { + return true; + } + } + return false; + }, hb_second); + return actual_coverage_size(+ filtered_it | hb_map_retains_sorting(hb_first)); +} + +static bool check_coverage_size(graph::class_def_size_estimator_t& estimator, + const gid_and_class_list_t& map, + hb_vector_t klasses) +{ + unsigned result = estimator.coverage_size(); + unsigned expected = actual_coverage_size(map, klasses); + if (result != expected) { + printf ("FAIL: estimated coverage expected size %u but was %u\n", expected, result); + return false; + } + return true; +} + +static bool check_add_class_def_size(graph::class_def_size_estimator_t& estimator, + const gid_and_class_list_t& map, + unsigned klass, hb_vector_t klasses) +{ + unsigned result = estimator.add_class_def_size(klass); + unsigned expected = actual_class_def_size(map, klasses); + if (result != expected) { + printf ("FAIL: estimated class def expected size %u but was %u\n", expected, result); + return false; + } + + return check_coverage_size(estimator, map, klasses); +} + +static bool check_add_class_def_size (const gid_and_class_list_t& list, unsigned klass) { graph::class_def_size_estimator_t estimator (list.iter ()); unsigned result = estimator.add_class_def_size (klass); - if (result != class_def_expected) + auto filtered_it = + + list.as_sorted_array().iter() + | hb_filter([&] (unsigned c) { + return c == klass; + }, hb_second); + + unsigned expected = actual_class_def_size(filtered_it); + if (result != expected) { - printf ("FAIL: class def expected size %u but was %u\n", class_def_expected, result); + printf ("FAIL: class def expected size %u but was %u\n", expected, result); return false; } + auto cov_it = + filtered_it | hb_map_retains_sorting(hb_first); result = estimator.coverage_size (); - if (result != cov_expected) + expected = actual_coverage_size(cov_it); + if (result != expected) { - printf ("FAIL: coverage expected size %u but was %u\n", cov_expected, result); + printf ("FAIL: coverage expected size %u but was %u\n", expected, result); return false; } @@ -55,30 +141,31 @@ static bool incremental_size_is (const gid_and_class_list_t& list, unsigned klas static void test_class_and_coverage_size_estimates () { - // TODO(garretrieger): test against the actual serialized sizes of class def tables gid_and_class_list_t empty = { }; - assert (incremental_size_is (empty, 0, 4, 4)); - assert (incremental_size_is (empty, 1, 4, 4)); + assert (check_add_class_def_size (empty, 0)); + assert (check_add_class_def_size (empty, 1)); gid_and_class_list_t class_zero = { {5, 0}, }; - assert (incremental_size_is (class_zero, 0, 6, 4)); + assert (check_add_class_def_size (class_zero, 0)); 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, 8, 4)); - assert (incremental_size_is (consecutive, 1, 8, 8)); - assert (incremental_size_is (consecutive, 2, 10, 10)); + assert (check_add_class_def_size (consecutive, 0)); + assert (check_add_class_def_size (consecutive, 1)); + assert (check_add_class_def_size (consecutive, 2)); gid_and_class_list_t non_consecutive = { {4, 0}, @@ -92,9 +179,9 @@ static void test_class_and_coverage_size_estimates () {11, 2}, {13, 2}, }; - assert (incremental_size_is (non_consecutive, 0, 8, 4)); - assert (incremental_size_is (non_consecutive, 1, 8, 4 + 2*6)); - assert (incremental_size_is (non_consecutive, 2, 12, 4 + 2*6)); + assert (check_add_class_def_size (non_consecutive, 0)); + assert (check_add_class_def_size (non_consecutive, 1)); + assert (check_add_class_def_size (non_consecutive, 2)); gid_and_class_list_t multiple_ranges = { {4, 0}, @@ -109,8 +196,8 @@ static void test_class_and_coverage_size_estimates () {12, 1}, {13, 1}, }; - assert (incremental_size_is (multiple_ranges, 0, 8, 4)); - assert (incremental_size_is (multiple_ranges, 1, 4 + 2 * 6, 4 + 3 * 6)); + assert (check_add_class_def_size (multiple_ranges, 0)); + assert (check_add_class_def_size (multiple_ranges, 1)); } static void test_running_class_and_coverage_size_estimates () { @@ -136,18 +223,13 @@ static void test_running_class_and_coverage_size_estimates () { }; graph::class_def_size_estimator_t estimator1(consecutive_map.iter()); - assert(estimator1.add_class_def_size(1) == 4 + 6); // format 2, 1 range - assert(estimator1.coverage_size() == 4 + 6); // format 2, 1 range - assert(estimator1.add_class_def_size(2) == 4 + 10); // format 1, 5 glyphs - assert(estimator1.coverage_size() == 4 + 6); // format 2, 1 range - assert(estimator1.add_class_def_size(3) == 4 + 18); // format 2, 3 ranges - assert(estimator1.coverage_size() == 4 + 6); // format 2, 1 range + assert(check_add_class_def_size(estimator1, consecutive_map, 1, {1})); + assert(check_add_class_def_size(estimator1, consecutive_map, 2, {1, 2})); + assert(check_add_class_def_size(estimator1, consecutive_map, 3, {1, 2, 3})); estimator1.reset(); - assert(estimator1.add_class_def_size(2) == 4 + 2); // format 1, 1 glyph - assert(estimator1.coverage_size() == 4 + 2); // format 1, 1 glyph - assert(estimator1.add_class_def_size(3) == 4 + 12); // format 2, 2 ranges - assert(estimator1.coverage_size() == 4 + 6); // format 1, 1 range + assert(check_add_class_def_size(estimator1, consecutive_map, 2, {2})); + assert(check_add_class_def_size(estimator1, consecutive_map, 3, {2, 3})); // #### With non-consecutive gids: always uses format 2 ### gid_and_class_list_t non_consecutive_map = { @@ -172,35 +254,30 @@ static void test_running_class_and_coverage_size_estimates () { }; graph::class_def_size_estimator_t estimator2(non_consecutive_map.iter()); - assert(estimator2.add_class_def_size(1) == 4 + 6); // format 2, 1 range - assert(estimator2.coverage_size() == 4 + 6); // format 2, 1 range - assert(estimator2.add_class_def_size(2) == 4 + 18); // format 2, 3 ranges - assert(estimator2.coverage_size() == 4 + 2 * 6); // format 1, 6 glyphs - assert(estimator2.add_class_def_size(3) == 4 + 24); // format 2, 4 ranges - assert(estimator2.coverage_size() == 4 + 3 * 6); // format 2, 3 ranges + assert(check_add_class_def_size(estimator2, non_consecutive_map, 1, {1})); + assert(check_add_class_def_size(estimator2, non_consecutive_map, 2, {1, 2})); + assert(check_add_class_def_size(estimator2, non_consecutive_map, 3, {1, 2, 3})); estimator2.reset(); - assert(estimator2.add_class_def_size(2) == 4 + 12); // format 1, 1 range - assert(estimator2.coverage_size() == 4 + 4); // format 1, 2 glyphs - assert(estimator2.add_class_def_size(3) == 4 + 18); // format 2, 2 ranges - assert(estimator2.coverage_size() == 4 + 2 * 6); // format 2, 2 ranges + assert(check_add_class_def_size(estimator2, non_consecutive_map, 2, {2})); + assert(check_add_class_def_size(estimator2, non_consecutive_map, 3, {2, 3})); } static void test_running_class_size_estimates_with_locally_consecutive_glyphs () { - gid_and_class_list_t consecutive_map = { + gid_and_class_list_t map = { {1, 1}, {6, 2}, {7, 3}, }; - graph::class_def_size_estimator_t estimator(consecutive_map.iter()); - assert(estimator.add_class_def_size(1) == 4 + 2); // format 1, 1 glyph - assert(estimator.add_class_def_size(2) == 4 + 12); // format 2, 2 ranges - assert(estimator.add_class_def_size(3) == 4 + 18); // format 2, 3 ranges + graph::class_def_size_estimator_t estimator(map.iter()); + assert(check_add_class_def_size(estimator, map, 1, {1})); + assert(check_add_class_def_size(estimator, map, 2, {1, 2})); + assert(check_add_class_def_size(estimator, map, 3, {1, 2, 3})); estimator.reset(); - assert(estimator.add_class_def_size(2) == 4 + 2); // format 1, 1 glyphs - assert(estimator.add_class_def_size(3) == 4 + 4); // format 1, 2 glyphs + assert(check_add_class_def_size(estimator, map, 2, {2})); + assert(check_add_class_def_size(estimator, map, 3, {2, 3})); } int From 17b37f10d5a5339f85f965fca900c08762e480fd Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Fri, 8 Mar 2024 22:01:05 +0000 Subject: [PATCH 4/8] [repacker] add classdef size est. test that you can add the same class multiple times. --- src/graph/test-classdef-graph.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/graph/test-classdef-graph.cc b/src/graph/test-classdef-graph.cc index bb569e1e7..818226656 100644 --- a/src/graph/test-classdef-graph.cc +++ b/src/graph/test-classdef-graph.cc @@ -225,6 +225,7 @@ static void test_running_class_and_coverage_size_estimates () { graph::class_def_size_estimator_t estimator1(consecutive_map.iter()); assert(check_add_class_def_size(estimator1, consecutive_map, 1, {1})); assert(check_add_class_def_size(estimator1, consecutive_map, 2, {1, 2})); + assert(check_add_class_def_size(estimator1, consecutive_map, 2, {1, 2})); // check that adding the same class again works assert(check_add_class_def_size(estimator1, consecutive_map, 3, {1, 2, 3})); estimator1.reset(); From 8e1beefe0563589285921845bd1f89721f72cb86 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Fri, 8 Mar 2024 22:05:20 +0000 Subject: [PATCH 5/8] [repacker] small fixes. --- src/graph/classdef-graph.hh | 4 ++-- src/graph/pairpos-graph.hh | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/graph/classdef-graph.hh b/src/graph/classdef-graph.hh index a2aa8990f..a6396f432 100644 --- a/src/graph/classdef-graph.hh +++ b/src/graph/classdef-graph.hh @@ -248,8 +248,8 @@ struct class_def_size_estimator_t hb_hashmap_t glyphs_per_class; hb_set_t included_classes; hb_set_t included_glyphs; - unsigned class_def_1_size = 4; - unsigned class_def_2_size = 4; + unsigned class_def_1_size; + unsigned class_def_2_size; }; diff --git a/src/graph/pairpos-graph.hh b/src/graph/pairpos-graph.hh index 309a41157..fd46861de 100644 --- a/src/graph/pairpos-graph.hh +++ b/src/graph/pairpos-graph.hh @@ -232,7 +232,7 @@ struct PairPosFormat2 : public OT::Layout::GPOS_impl::PairPosFormat2_4 Date: Sat, 9 Mar 2024 00:06:37 +0000 Subject: [PATCH 6/8] [repacker] Fix repacker test. With class def size estimation changes this test is now able to pack closer to the limit. --- src/test-repacker.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test-repacker.cc b/src/test-repacker.cc index e227999a4..7f57d8eac 100644 --- a/src/test-repacker.cc +++ b/src/test-repacker.cc @@ -1986,12 +1986,12 @@ static void test_resolve_with_close_to_limit_pair_pos_2_split () 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); + populate_serializer_with_large_pair_pos_2 <1, 1636, 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); + populate_serializer_with_large_pair_pos_2 <2, 818, 10>(&e, true, false, false); run_resolve_overflow_test ("test_resolve_with_close_to_limit_pair_pos_2_split", c, From 025f52769f637b23fa14f3229a4be86451570f52 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Sat, 9 Mar 2024 00:32:36 +0000 Subject: [PATCH 7/8] [repacker] fix mem leak in test-classdef-graph test. --- src/graph/test-classdef-graph.cc | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/graph/test-classdef-graph.cc b/src/graph/test-classdef-graph.cc index 818226656..2da934811 100644 --- a/src/graph/test-classdef-graph.cc +++ b/src/graph/test-classdef-graph.cc @@ -39,8 +39,11 @@ static unsigned actual_class_def_size(It glyph_and_class) { OT::ClassDef_serialize (&serializer, glyph_and_class); serializer.end_serialize (); assert(!serializer.in_error()); - hb_bytes_t class_def_copy = serializer.copy_bytes (); - return class_def_copy.get_size(); + + hb_blob_t* blob = serializer.copy_blob(); + unsigned size = hb_blob_get_length(blob); + hb_blob_destroy(blob); + return size; } static unsigned actual_class_def_size(gid_and_class_list_t consecutive_map, hb_vector_t classes) { @@ -64,8 +67,11 @@ static unsigned actual_coverage_size(It glyphs) { OT::Layout::Common::Coverage_serialize (&serializer, glyphs); serializer.end_serialize (); assert(!serializer.in_error()); - hb_bytes_t coverage_copy = serializer.copy_bytes (); - return coverage_copy.get_size(); + + hb_blob_t* blob = serializer.copy_blob(); + unsigned size = hb_blob_get_length(blob); + hb_blob_destroy(blob); + return size; } static unsigned actual_coverage_size(gid_and_class_list_t consecutive_map, hb_vector_t classes) { From 79eaa217ac65d7564e1fadd871de95c5206896ac Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Thu, 14 Mar 2024 21:22:22 +0000 Subject: [PATCH 8/8] [repacker] remove unused include. --- src/graph/classdef-graph.hh | 1 - 1 file changed, 1 deletion(-) diff --git a/src/graph/classdef-graph.hh b/src/graph/classdef-graph.hh index a6396f432..da6378820 100644 --- a/src/graph/classdef-graph.hh +++ b/src/graph/classdef-graph.hh @@ -26,7 +26,6 @@ #include "graph.hh" #include "../hb-ot-layout-common.hh" -#include #ifndef GRAPH_CLASSDEF_GRAPH_HH #define GRAPH_CLASSDEF_GRAPH_HH