From ed6d287d1105219b246a2810685097918f974497 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 2 Feb 2022 14:10:16 -0600 Subject: [PATCH 01/12] [ot-face] Load num-glyphs from `loca` table before `maxp` Implements [boring-expansion] [maxp] Relax https://github.com/be-fonts/boring-expansion-spec/issues/6 --- src/hb-ot-hmtx-table.hh | 2 +- src/hb-static.cc | 54 ++++++++++++++++++++++++--- test/api/Makefile.am | 1 + test/api/hb-test.h | 7 ++++ test/api/meson.build | 1 + test/api/test-be-num-glyphs.c | 69 +++++++++++++++++++++++++++++++++++ 6 files changed, 127 insertions(+), 7 deletions(-) create mode 100644 test/api/test-be-num-glyphs.c diff --git a/src/hb-ot-hmtx-table.hh b/src/hb-ot-hmtx-table.hh index 739474f2f..fdf82ee60 100644 --- a/src/hb-ot-hmtx-table.hh +++ b/src/hb-ot-hmtx-table.hh @@ -185,7 +185,7 @@ struct hmtxvmtx table = hb_sanitize_context_t ().reference_table (face, T::tableTag); - /* Cap num_metrics() and num_advances() based on table length. */ + /* Cap num_metrics and num_advances based on table length. */ unsigned int len = table.get_length (); if (unlikely (num_advances * 4 > len)) num_advances = len / 4; diff --git a/src/hb-static.cc b/src/hb-static.cc index ec4b24147..5d0dd07ba 100644 --- a/src/hb-static.cc +++ b/src/hb-static.cc @@ -33,6 +33,7 @@ #include "hb-aat-layout-feat-table.hh" #include "hb-ot-layout-common.hh" #include "hb-ot-cmap-table.hh" +#include "hb-ot-glyf-table.hh" #include "hb-ot-head-table.hh" #include "hb-ot-maxp-table.hh" @@ -55,17 +56,58 @@ const unsigned char _hb_Null_AAT_Lookup[2] = {0xFF, 0xFF}; /* hb_face_t */ +static inline unsigned +load_num_glyphs_from_loca (const hb_face_t *face) +{ + unsigned ret = 0; + hb_sanitize_context_t c = hb_sanitize_context_t (); + c.set_num_glyphs (0); /* So we don't recurse ad infinitum. */ + + /* We cannot use table.head because that would use sanitizer, + * which would try accessing face.num_glyphs, which would + * recurse here again... */ + hb_blob_t *head_blob = c.reference_table (face); + const OT::head *head_table = head_blob->as (); + unsigned indexToLocFormat = head_table->indexToLocFormat; + hb_blob_destroy (head_blob); + + if (indexToLocFormat <= 1) + { + bool short_offset = 0 == indexToLocFormat; + hb_blob_t *loca_blob = c.reference_table (face); + ret = hb_max (1u, loca_blob->length / (short_offset ? 2 : 4)) - 1; + hb_blob_destroy (loca_blob); + } + + return ret; +} + +static inline unsigned +load_num_glyphs_from_maxp (const hb_face_t *face) +{ + unsigned ret = 0; + hb_sanitize_context_t c = hb_sanitize_context_t (); + c.set_num_glyphs (0); /* So we don't recurse ad infinitum. */ + + hb_blob_t *maxp_blob = c.reference_table (face); + const OT::maxp *maxp_table = maxp_blob->as (); + ret = maxp_table->get_num_glyphs (); + hb_blob_destroy (maxp_blob); + + return ret; +} + unsigned int hb_face_t::load_num_glyphs () const { - hb_sanitize_context_t c = hb_sanitize_context_t (); - c.set_num_glyphs (0); /* So we don't recurse ad infinitum. */ - hb_blob_t *maxp_blob = c.reference_table (this); - const OT::maxp *maxp_table = maxp_blob->as (); + unsigned ret = 0; + + ret = load_num_glyphs_from_loca (this); + + if (!ret) + ret = load_num_glyphs_from_maxp (this); - unsigned int ret = maxp_table->get_num_glyphs (); num_glyphs.set_relaxed (ret); - hb_blob_destroy (maxp_blob); return ret; } diff --git a/test/api/Makefile.am b/test/api/Makefile.am index 883464cea..8226cf019 100644 --- a/test/api/Makefile.am +++ b/test/api/Makefile.am @@ -30,6 +30,7 @@ noinst_PROGRAMS = $(TEST_PROGS) TEST_PROGS = \ test-aat-layout \ test-baseline \ + test-be-num-glyphs \ test-blob \ test-buffer \ test-c \ diff --git a/test/api/hb-test.h b/test/api/hb-test.h index 7390e57a7..8e3cc1e88 100644 --- a/test/api/hb-test.h +++ b/test/api/hb-test.h @@ -52,6 +52,13 @@ HB_BEGIN_DECLS ((const char *) s)[2], \ ((const char *) s)[3])) +#define HB_FACE_ADD_TABLE(face, tag, data) \ + hb_face_builder_add_table ((face), \ + HB_TAG_CHAR4(tag), \ + hb_blob_create_or_fail ((data), \ + sizeof (data), \ + HB_MEMORY_MODE_READONLY, \ + NULL, NULL)) static inline const char * srcdir (void) diff --git a/test/api/meson.build b/test/api/meson.build index 7ea295420..5a1d4a7ab 100644 --- a/test/api/meson.build +++ b/test/api/meson.build @@ -6,6 +6,7 @@ endif tests = [ 'test-aat-layout.c', 'test-baseline.c', + 'test-be-num-glyphs.c', 'test-blob.c', 'test-buffer.c', 'test-c.c', diff --git a/test/api/test-be-num-glyphs.c b/test/api/test-be-num-glyphs.c new file mode 100644 index 000000000..9ba42eb7f --- /dev/null +++ b/test/api/test-be-num-glyphs.c @@ -0,0 +1,69 @@ +/* + * Copyright © 2022 Behdad Esfahbod + * + * 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. + */ + +#include "hb-test.h" + +#include + +static void +test_maxp_and_loca (void) +{ + hb_face_t *face; + + const char maxp_data[] = "\x00\x00\x50\x00" // version + "\x00\x05" // numGlyphs + ; + const char loca_data[18] = ""; + + face = hb_face_builder_create (); + g_assert_cmpuint (hb_face_get_glyph_count (face), ==, 0); + hb_face_destroy (face); + + face = hb_face_builder_create (); + HB_FACE_ADD_TABLE (face, "maxp", maxp_data); + g_assert_cmpuint (hb_face_get_glyph_count (face), ==, 5); + hb_face_destroy (face); + + face = hb_face_builder_create (); + HB_FACE_ADD_TABLE (face, "maxp", maxp_data); + HB_FACE_ADD_TABLE (face, "loca", loca_data); + g_assert_cmpuint (hb_face_get_glyph_count (face), ==, 8); + hb_face_destroy (face); + + face = hb_face_builder_create (); + HB_FACE_ADD_TABLE (face, "loca", loca_data); + g_assert_cmpuint (hb_face_get_glyph_count (face), ==, 8); + hb_face_destroy (face); +} + + +int +main (int argc, char **argv) +{ + hb_test_init (&argc, &argv); + + hb_test_add (test_maxp_and_loca); + + return hb_test_run(); +} From 622cbc485f286770ee816524c0b12aef7e81d510 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Mon, 14 Feb 2022 14:09:40 -0600 Subject: [PATCH 02/12] [hmtx] Internal rename num_metrics to num_bearings --- src/hb-ot-hmtx-table.hh | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/hb-ot-hmtx-table.hh b/src/hb-ot-hmtx-table.hh index fdf82ee60..07922ba0d 100644 --- a/src/hb-ot-hmtx-table.hh +++ b/src/hb-ot-hmtx-table.hh @@ -185,17 +185,17 @@ struct hmtxvmtx table = hb_sanitize_context_t ().reference_table (face, T::tableTag); - /* Cap num_metrics and num_advances based on table length. */ + /* Cap num_bearings and num_advances based on table length. */ unsigned int len = table.get_length (); if (unlikely (num_advances * 4 > len)) num_advances = len / 4; - num_metrics = num_advances + (len - 4 * num_advances) / 2; + num_bearings = num_advances + (len - 4 * num_advances) / 2; - /* We MUST set num_metrics to zero if num_advances is zero. + /* We MUST set num_bearings to zero if num_advances is zero. * Our get_advance() depends on that. */ if (unlikely (!num_advances)) { - num_metrics = num_advances = 0; + num_bearings = num_advances = 0; table.destroy (); table = hb_blob_get_empty (); } @@ -213,7 +213,7 @@ struct hmtxvmtx if (glyph < num_advances) return table->longMetricZ[glyph].sb; - if (unlikely (glyph >= num_metrics)) + if (unlikely (glyph >= num_bearings)) return 0; const FWORD *bearings = (const FWORD *) &table->longMetricZ[num_advances]; @@ -225,7 +225,7 @@ struct hmtxvmtx int side_bearing = get_side_bearing (glyph); #ifndef HB_NO_VAR - if (unlikely (glyph >= num_metrics) || !font->num_coords) + if (unlikely (glyph >= num_bearings) || !font->num_coords) return side_bearing; if (var_table.get_length ()) @@ -239,12 +239,12 @@ struct hmtxvmtx unsigned int get_advance (hb_codepoint_t glyph) const { - if (unlikely (glyph >= num_metrics)) + if (unlikely (glyph >= num_bearings)) { - /* If num_metrics is zero, it means we don't have the metrics table + /* If num_bearings is zero, it means we don't have the metrics table * for this direction: return default advance. Otherwise, it means that the * glyph index is out of bound: return zero. */ - if (num_metrics) + if (num_bearings) return 0; else return default_advance; @@ -259,7 +259,7 @@ struct hmtxvmtx unsigned int advance = get_advance (glyph); #ifndef HB_NO_VAR - if (unlikely (glyph >= num_metrics) || !font->num_coords) + if (unlikely (glyph >= num_bearings) || !font->num_coords) return advance; if (var_table.get_length ()) @@ -272,8 +272,8 @@ struct hmtxvmtx } protected: - unsigned int num_metrics; unsigned int num_advances; + unsigned int num_bearings; unsigned int default_advance; private: From be4ddcc30b8c1932def1b9a5beee9ead90f8928f Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Mon, 14 Feb 2022 14:12:07 -0600 Subject: [PATCH 03/12] [hmtx] Rename internal num_advances to num_long_metrics --- src/hb-ot-hmtx-table.hh | 56 ++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/src/hb-ot-hmtx-table.hh b/src/hb-ot-hmtx-table.hh index 07922ba0d..7c2223c64 100644 --- a/src/hb-ot-hmtx-table.hh +++ b/src/hb-ot-hmtx-table.hh @@ -98,12 +98,12 @@ struct hmtxvmtx hb_requires (hb_is_iterator (Iterator))> void serialize (hb_serialize_context_t *c, Iterator it, - unsigned num_advances) + unsigned num_long_metrics) { unsigned idx = 0; for (auto _ : it) { - if (idx < num_advances) + if (idx < num_long_metrics) { LongMetric lm; lm.advance = _.first; @@ -128,17 +128,17 @@ struct hmtxvmtx if (unlikely (!table_prime)) return_trace (false); accelerator_t _mtx (c->plan->source); - unsigned num_advances; + unsigned num_long_metrics; { - /* Determine num_advances to encode. */ + /* Determine num_long_metrics to encode. */ auto& plan = c->plan; - num_advances = plan->num_output_glyphs (); + num_long_metrics = plan->num_output_glyphs (); hb_codepoint_t old_gid = 0; - unsigned int last_advance = plan->old_gid_for_new_gid (num_advances - 1, &old_gid) ? _mtx.get_advance (old_gid) : 0; - while (num_advances > 1 && - last_advance == (plan->old_gid_for_new_gid (num_advances - 2, &old_gid) ? _mtx.get_advance (old_gid) : 0)) + unsigned int last_advance = plan->old_gid_for_new_gid (num_long_metrics - 1, &old_gid) ? _mtx.get_advance (old_gid) : 0; + while (num_long_metrics > 1 && + last_advance == (plan->old_gid_for_new_gid (num_long_metrics - 2, &old_gid) ? _mtx.get_advance (old_gid) : 0)) { - num_advances--; + num_long_metrics--; } } @@ -153,13 +153,13 @@ struct hmtxvmtx }) ; - table_prime->serialize (c->serializer, it, num_advances); + table_prime->serialize (c->serializer, it, num_long_metrics); if (unlikely (c->serializer->in_error ())) return_trace (false); // Amend header num hmetrics - if (unlikely (!subset_update_header (c->plan, num_advances))) + if (unlikely (!subset_update_header (c->plan, num_long_metrics))) return_trace (false); return_trace (true); @@ -174,28 +174,28 @@ struct hmtxvmtx { default_advance = default_advance_ ? default_advance_ : hb_face_get_upem (face); - num_advances = T::is_horizontal ? - face->table.hhea->numberOfLongMetrics : + num_long_metrics = T::is_horizontal ? + face->table.hhea->numberOfLongMetrics : #ifndef HB_NO_VERTICAL - face->table.vhea->numberOfLongMetrics + face->table.vhea->numberOfLongMetrics #else - 0 + 0 #endif - ; + ; table = hb_sanitize_context_t ().reference_table (face, T::tableTag); - /* Cap num_bearings and num_advances based on table length. */ + /* Cap num_bearings and num_long_metrics based on table length. */ unsigned int len = table.get_length (); - if (unlikely (num_advances * 4 > len)) - num_advances = len / 4; - num_bearings = num_advances + (len - 4 * num_advances) / 2; + if (unlikely (num_long_metrics * 4 > len)) + num_long_metrics = len / 4; + num_bearings = num_long_metrics + (len - 4 * num_long_metrics) / 2; - /* We MUST set num_bearings to zero if num_advances is zero. + /* We MUST set num_bearings to zero if num_long_metrics is zero. * Our get_advance() depends on that. */ - if (unlikely (!num_advances)) + if (unlikely (!num_long_metrics)) { - num_bearings = num_advances = 0; + num_bearings = num_long_metrics = 0; table.destroy (); table = hb_blob_get_empty (); } @@ -210,14 +210,14 @@ struct hmtxvmtx int get_side_bearing (hb_codepoint_t glyph) const { - if (glyph < num_advances) + if (glyph < num_long_metrics) return table->longMetricZ[glyph].sb; if (unlikely (glyph >= num_bearings)) return 0; - const FWORD *bearings = (const FWORD *) &table->longMetricZ[num_advances]; - return bearings[glyph - num_advances]; + const FWORD *bearings = (const FWORD *) &table->longMetricZ[num_long_metrics]; + return bearings[glyph - num_long_metrics]; } int get_side_bearing (hb_font_t *font, hb_codepoint_t glyph) const @@ -250,7 +250,7 @@ struct hmtxvmtx return default_advance; } - return table->longMetricZ[hb_min (glyph, (uint32_t) num_advances - 1)].advance; + return table->longMetricZ[hb_min (glyph, (uint32_t) num_long_metrics - 1)].advance; } unsigned int get_advance (hb_codepoint_t glyph, @@ -272,7 +272,7 @@ struct hmtxvmtx } protected: - unsigned int num_advances; + unsigned int num_long_metrics; unsigned int num_bearings; unsigned int default_advance; From 431c948ed742c936623a340e046e0e708ee0736f Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Mon, 14 Feb 2022 14:13:04 -0600 Subject: [PATCH 04/12] [hmtx] Document --- src/hb-ot-hmtx-table.hh | 1 + 1 file changed, 1 insertion(+) diff --git a/src/hb-ot-hmtx-table.hh b/src/hb-ot-hmtx-table.hh index 7c2223c64..6035a33fb 100644 --- a/src/hb-ot-hmtx-table.hh +++ b/src/hb-ot-hmtx-table.hh @@ -272,6 +272,7 @@ struct hmtxvmtx } protected: + // 0 <= num_long_metrics <= num_bearings unsigned int num_long_metrics; unsigned int num_bearings; unsigned int default_advance; From 379e526aa41b216fa1dd19cf4345e55ec3c8a8c9 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Mon, 14 Feb 2022 15:02:31 -0600 Subject: [PATCH 05/12] [test] Add test for current hmtx logic --- test/api/Makefile.am | 1 + test/api/meson.build | 1 + test/api/test-be-glyph-advance.c | 99 ++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+) create mode 100644 test/api/test-be-glyph-advance.c diff --git a/test/api/Makefile.am b/test/api/Makefile.am index 8226cf019..a6c9a6562 100644 --- a/test/api/Makefile.am +++ b/test/api/Makefile.am @@ -30,6 +30,7 @@ noinst_PROGRAMS = $(TEST_PROGS) TEST_PROGS = \ test-aat-layout \ test-baseline \ + test-be-glyph-advance.c \ test-be-num-glyphs \ test-blob \ test-buffer \ diff --git a/test/api/meson.build b/test/api/meson.build index 5a1d4a7ab..ab6433860 100644 --- a/test/api/meson.build +++ b/test/api/meson.build @@ -6,6 +6,7 @@ endif tests = [ 'test-aat-layout.c', 'test-baseline.c', + 'test-be-glyph-advance.c', 'test-be-num-glyphs.c', 'test-blob.c', 'test-buffer.c', diff --git a/test/api/test-be-glyph-advance.c b/test/api/test-be-glyph-advance.c new file mode 100644 index 000000000..c880b9267 --- /dev/null +++ b/test/api/test-be-glyph-advance.c @@ -0,0 +1,99 @@ +/* + * Copyright © 2022 Behdad Esfahbod + * + * 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. + */ + +#include "hb-test.h" + +#include + +static void +test_maxp_and_hmtx (void) +{ + hb_face_t *face; + hb_font_t *font; + + const char maxp_data[] = "\x00\x00\x50\x00" // version + "\x00\x05" // numGlyphs + ; + const char loca_data[18] = ""; + const char hhea_data[36] = + "\x00\x01\x00\x00" /* FixedVersion<>version; * 0x00010000u for version 1.0. */ + "\x02\x00" /* FWORD ascender; * Typographic ascent. */ + "\x00\x10" /* FWORD descender; * Typographic descent. */ + "\x00\x00" /* FWORD lineGap; * Typographic line gap. */ + "\x00\x00" /* UFWORD advanceMax; * Maximum advance width/height value in metrics table. */ + "\x00\x00" /* FWORD minLeadingBearing; * Minimum left/top sidebearing value in metrics table. */ + "\x00\x00" /* FWORD minTrailingBearing; * Minimum right/bottom sidebearing value; */ + "\x01\x00" /* FWORD maxExtent; * horizontal: Max(lsb + (xMax - xMin)), */ + "\x00\x00" /* HBINT16 caretSlopeRise; * Used to calculate the slope of the,*/ + "\x00\x00" /* HBINT16 caretSlopeRun; * 0 for vertical caret, 1 for horizontal. */ + "\x00\x00" /* HBINT16 caretOffset; * The amount by which a slanted */ + "\x00\x00" /* HBINT16 reserved1; * Set to 0. */ + "\x00\x00" /* HBINT16 reserved2; * Set to 0. */ + "\x00\x00" /* HBINT16 reserved3; * Set to 0. */ + "\x00\x00" /* HBINT16 reserved4; * Set to 0. */ + "\x00\x00" /* HBINT16 metricDataFormat; * 0 for current format. */ + "\x00\x02" /* HBUINT16 numberOfLongMetrics; * Number of LongMetric entries in metric table. */ + ; + const char hmtx_data[18] = + "\x00\x01\x00\x02" /* glyph 0 advance lsb */ + "\x00\x03\x00\x04" /* glyph 1 advance lsb */ + "\x00\x05" /* glyph 2 lsb */ + "\x00\x06" /* glyph 3 lsb */ + "\x00\x07" /* glyph 4 lsb */ + "\x00\x08" /* glyph 5 advance */ + "\x00\x09" /* glyph 6 advance */ + ; + + face = hb_face_builder_create (); + HB_FACE_ADD_TABLE (face, "maxp", maxp_data); + HB_FACE_ADD_TABLE (face, "loca", loca_data); + HB_FACE_ADD_TABLE (face, "hhea", hhea_data); + HB_FACE_ADD_TABLE (face, "hmtx", hmtx_data); + font = hb_font_create (face); + hb_face_destroy (face); + g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 0), ==, 1); + g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 1), ==, 3); + g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 2), ==, 3); + g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 3), ==, 3); + g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 4), ==, 3); + g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 5), ==, 3); + g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 6), ==, 3); + g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 7), ==, 0); + g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 8), ==, 0); + g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 9), ==, 0); + g_assert_cmpuint (hb_font_get_glyph_h_advance (font,10), ==, 0); + g_assert_cmpuint (hb_font_get_glyph_h_advance (font,11), ==, 0); + hb_font_destroy (font); +} + + +int +main (int argc, char **argv) +{ + hb_test_init (&argc, &argv); + + hb_test_add (test_maxp_and_hmtx); + + return hb_test_run(); +} From 8b7ccc41c4b06ad93927849c54288a9ad1816dba Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 15 Feb 2022 14:15:12 -0600 Subject: [PATCH 06/12] [hmtx] Implement [boring-expansion] >64k expansion This implements https://github.com/be-fonts/boring-expansion-spec/issues/7 --- src/hb-ot-face-table-list.hh | 1 + src/hb-ot-hmtx-table.hh | 80 ++++++++++++++++++++++---------- test/api/test-be-glyph-advance.c | 6 +-- 3 files changed, 59 insertions(+), 28 deletions(-) diff --git a/src/hb-ot-face-table-list.hh b/src/hb-ot-face-table-list.hh index eff09838a..65cb4e644 100644 --- a/src/hb-ot-face-table-list.hh +++ b/src/hb-ot-face-table-list.hh @@ -47,6 +47,7 @@ /* OpenType fundamentals. */ HB_OT_TABLE (OT, head) +HB_OT_TABLE (OT, maxp) #if !defined(HB_NO_FACE_COLLECT_UNICODES) || !defined(HB_NO_OT_FONT) HB_OT_ACCELERATOR (OT, cmap) #endif diff --git a/src/hb-ot-hmtx-table.hh b/src/hb-ot-hmtx-table.hh index 6035a33fb..0e7927b4b 100644 --- a/src/hb-ot-hmtx-table.hh +++ b/src/hb-ot-hmtx-table.hh @@ -28,6 +28,7 @@ #define HB_OT_HMTX_TABLE_HH #include "hb-open-type.hh" +#include "hb-ot-maxp-table.hh" #include "hb-ot-hhea-table.hh" #include "hb-ot-var-hvar-table.hh" #include "hb-ot-metrics.hh" @@ -172,8 +173,17 @@ struct hmtxvmtx accelerator_t (hb_face_t *face, unsigned int default_advance_ = 0) { + table = hb_sanitize_context_t ().reference_table (face, T::tableTag); + var_table = hb_sanitize_context_t ().reference_table (face, T::variationsTag); + default_advance = default_advance_ ? default_advance_ : hb_face_get_upem (face); + /* Populate count variables and sort them out as we go */ + + unsigned int len = table.get_length (); + if (len & 1) + len--; + num_long_metrics = T::is_horizontal ? face->table.hhea->numberOfLongMetrics : #ifndef HB_NO_VERTICAL @@ -182,25 +192,27 @@ struct hmtxvmtx 0 #endif ; - - table = hb_sanitize_context_t ().reference_table (face, T::tableTag); - - /* Cap num_bearings and num_long_metrics based on table length. */ - unsigned int len = table.get_length (); if (unlikely (num_long_metrics * 4 > len)) num_long_metrics = len / 4; - num_bearings = num_long_metrics + (len - 4 * num_long_metrics) / 2; + len -= num_long_metrics * 4; + + num_bearings = face->table.maxp->get_num_glyphs (); + + if (unlikely (num_bearings < num_long_metrics)) + num_bearings = num_long_metrics; + if (unlikely ((num_bearings - num_long_metrics) * 2 > len)) + num_bearings = num_long_metrics + len / 2; + len -= (num_bearings - num_long_metrics) * 2; /* We MUST set num_bearings to zero if num_long_metrics is zero. * Our get_advance() depends on that. */ if (unlikely (!num_long_metrics)) - { num_bearings = num_long_metrics = 0; - table.destroy (); - table = hb_blob_get_empty (); - } - var_table = hb_sanitize_context_t ().reference_table (face, T::variationsTag); + num_advances = num_bearings + len / 2; + num_glyphs = face->get_num_glyphs (); + if (num_glyphs < num_advances) + num_glyphs = num_advances; } ~accelerator_t () { @@ -239,18 +251,31 @@ struct hmtxvmtx unsigned int get_advance (hb_codepoint_t glyph) const { - if (unlikely (glyph >= num_bearings)) - { - /* If num_bearings is zero, it means we don't have the metrics table - * for this direction: return default advance. Otherwise, it means that the - * glyph index is out of bound: return zero. */ - if (num_bearings) - return 0; - else - return default_advance; - } + /* OpenType case. */ + if (glyph < num_bearings) + return table->longMetricZ[hb_min (glyph, (uint32_t) num_long_metrics - 1)].advance; - return table->longMetricZ[hb_min (glyph, (uint32_t) num_long_metrics - 1)].advance; + /* If num_advances is zero, it means we don't have the metrics table + * for this direction: return default advance. Otherwise, there's a + * well-defined answer. */ + if (unlikely (!num_advances)) + return default_advance; + + if (unlikely (glyph >= num_glyphs)) + return 0; + + /* num_bearings <= glyph < num_glyphs; + * num_bearings <= num_advances */ + + /* TODO Optimize */ + + if (num_bearings == num_advances) + return get_advance (num_bearings - 1); + + const FWORD *bearings = (const FWORD *) &table->longMetricZ[num_long_metrics]; + const UFWORD *advances = (const UFWORD *) &bearings[num_bearings - num_long_metrics]; + + return advances[hb_min (glyph - num_bearings, num_advances - num_bearings - 1)]; } unsigned int get_advance (hb_codepoint_t glyph, @@ -272,9 +297,12 @@ struct hmtxvmtx } protected: - // 0 <= num_long_metrics <= num_bearings - unsigned int num_long_metrics; - unsigned int num_bearings; + // 0 <= num_long_metrics <= num_bearings <= num_advances <= num_glyphs + unsigned num_long_metrics; + unsigned num_bearings; + unsigned num_advances; + unsigned num_glyphs; + unsigned int default_advance; private: @@ -306,6 +334,8 @@ struct hmtxvmtx * the end. This allows a monospaced * font to vary the side bearing * values for each glyph. */ +/*UnsizedArrayOfadvancesX;*/ + /* TODO Document. */ public: DEFINE_SIZE_ARRAY (0, longMetricZ); }; diff --git a/test/api/test-be-glyph-advance.c b/test/api/test-be-glyph-advance.c index c880b9267..eb0a04bf1 100644 --- a/test/api/test-be-glyph-advance.c +++ b/test/api/test-be-glyph-advance.c @@ -77,9 +77,9 @@ test_maxp_and_hmtx (void) g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 2), ==, 3); g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 3), ==, 3); g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 4), ==, 3); - g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 5), ==, 3); - g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 6), ==, 3); - g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 7), ==, 0); + g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 5), ==, 8); + g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 6), ==, 9); + g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 7), ==, 9); g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 8), ==, 0); g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 9), ==, 0); g_assert_cmpuint (hb_font_get_glyph_h_advance (font,10), ==, 0); From 531c27d199be9413523ae1f48703931d0ebf922f Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 15 Feb 2022 14:20:54 -0600 Subject: [PATCH 07/12] Fix build --- test/api/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/api/Makefile.am b/test/api/Makefile.am index a6c9a6562..907ec498d 100644 --- a/test/api/Makefile.am +++ b/test/api/Makefile.am @@ -30,7 +30,7 @@ noinst_PROGRAMS = $(TEST_PROGS) TEST_PROGS = \ test-aat-layout \ test-baseline \ - test-be-glyph-advance.c \ + test-be-glyph-advance \ test-be-num-glyphs \ test-blob \ test-buffer \ From 197ed8f5923b04cfd843942428814ea14b88632e Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 15 Feb 2022 14:30:12 -0600 Subject: [PATCH 08/12] [test/api] Fix leaks --- test/api/hb-test.h | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/test/api/hb-test.h b/test/api/hb-test.h index 8e3cc1e88..840637aac 100644 --- a/test/api/hb-test.h +++ b/test/api/hb-test.h @@ -53,12 +53,16 @@ HB_BEGIN_DECLS ((const char *) s)[3])) #define HB_FACE_ADD_TABLE(face, tag, data) \ - hb_face_builder_add_table ((face), \ - HB_TAG_CHAR4(tag), \ - hb_blob_create_or_fail ((data), \ - sizeof (data), \ - HB_MEMORY_MODE_READONLY, \ - NULL, NULL)) + do { \ + hb_blob_t *blob = hb_blob_create_or_fail ((data), \ + sizeof (data), \ + HB_MEMORY_MODE_READONLY, \ + NULL, NULL); \ + hb_face_builder_add_table ((face), \ + HB_TAG_CHAR4(tag), \ + blob); \ + hb_blob_destroy (blob); \ + } while (0) static inline const char * srcdir (void) From 67eb9acf792a63e4d6a8447c23cbb5d4b97891dc Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 15 Feb 2022 17:17:49 -0600 Subject: [PATCH 09/12] [config] Add HB_NO_BORING_EXPANSION --- src/hb-config.hh | 1 + src/hb-ot-hmtx-table.hh | 4 ++++ src/hb-static.cc | 5 ++++- test/api/test-be-glyph-advance.c | 2 ++ test/api/test-be-num-glyphs.c | 2 ++ 5 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/hb-config.hh b/src/hb-config.hh index 5141ad834..4b46dea93 100644 --- a/src/hb-config.hh +++ b/src/hb-config.hh @@ -85,6 +85,7 @@ #ifdef HB_MINI #define HB_NO_AAT #define HB_NO_LEGACY +#define HB_NO_BORING_EXPANSION #endif #if defined(HAVE_CONFIG_OVERRIDE_H) || defined(HB_CONFIG_OVERRIDE_H) diff --git a/src/hb-ot-hmtx-table.hh b/src/hb-ot-hmtx-table.hh index 0e7927b4b..7487e40e6 100644 --- a/src/hb-ot-hmtx-table.hh +++ b/src/hb-ot-hmtx-table.hh @@ -261,6 +261,10 @@ struct hmtxvmtx if (unlikely (!num_advances)) return default_advance; +#ifdef HB_NO_BORING_EXPANSION + return 0; +#endif + if (unlikely (glyph >= num_glyphs)) return 0; diff --git a/src/hb-static.cc b/src/hb-static.cc index 5d0dd07ba..0fd8fe9e8 100644 --- a/src/hb-static.cc +++ b/src/hb-static.cc @@ -102,7 +102,10 @@ hb_face_t::load_num_glyphs () const { unsigned ret = 0; - ret = load_num_glyphs_from_loca (this); +#ifndef HB_NO_BORING_EXPANSION + if (!ret) + ret = load_num_glyphs_from_loca (this); +#endif if (!ret) ret = load_num_glyphs_from_maxp (this); diff --git a/test/api/test-be-glyph-advance.c b/test/api/test-be-glyph-advance.c index eb0a04bf1..77fdee136 100644 --- a/test/api/test-be-glyph-advance.c +++ b/test/api/test-be-glyph-advance.c @@ -77,9 +77,11 @@ test_maxp_and_hmtx (void) g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 2), ==, 3); g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 3), ==, 3); g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 4), ==, 3); +#ifndef HB_NO_BORING_EXPANSION g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 5), ==, 8); g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 6), ==, 9); g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 7), ==, 9); +#endif g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 8), ==, 0); g_assert_cmpuint (hb_font_get_glyph_h_advance (font, 9), ==, 0); g_assert_cmpuint (hb_font_get_glyph_h_advance (font,10), ==, 0); diff --git a/test/api/test-be-num-glyphs.c b/test/api/test-be-num-glyphs.c index 9ba42eb7f..ab8d11043 100644 --- a/test/api/test-be-num-glyphs.c +++ b/test/api/test-be-num-glyphs.c @@ -45,6 +45,7 @@ test_maxp_and_loca (void) g_assert_cmpuint (hb_face_get_glyph_count (face), ==, 5); hb_face_destroy (face); +#ifndef HB_NO_BORING_EXPANSION face = hb_face_builder_create (); HB_FACE_ADD_TABLE (face, "maxp", maxp_data); HB_FACE_ADD_TABLE (face, "loca", loca_data); @@ -55,6 +56,7 @@ test_maxp_and_loca (void) HB_FACE_ADD_TABLE (face, "loca", loca_data); g_assert_cmpuint (hb_face_get_glyph_count (face), ==, 8); hb_face_destroy (face); +#endif } From 2a430790adfac00a1280c0ebfcf579be1b557ffb Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 15 Feb 2022 17:33:52 -0600 Subject: [PATCH 10/12] [machinery] Add "core table" machinery To be used in subsequent commit; or tried anyway. --- src/hb-machinery.hh | 11 ++++++++--- src/hb-ot-face-table-list.hh | 13 +++++++++++-- src/hb-ot-face.hh | 3 +++ 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/hb-machinery.hh b/src/hb-machinery.hh index b529173fc..e52a6a412 100644 --- a/src/hb-machinery.hh +++ b/src/hb-machinery.hh @@ -273,14 +273,19 @@ struct hb_face_lazy_loader_t : hb_lazy_loader_t, hb_face_t, WheresFace> {}; -template +template struct hb_table_lazy_loader_t : hb_lazy_loader_t, + hb_table_lazy_loader_t, hb_face_t, WheresFace, hb_blob_t> { static hb_blob_t *create (hb_face_t *face) - { return hb_sanitize_context_t ().reference_table (face); } + { + auto c = hb_sanitize_context_t (); + if (core) + c.set_num_glyphs (0); // So we don't recurse ad infinitum... + return c.reference_table (face); + } static void destroy (hb_blob_t *p) { hb_blob_destroy (p); } static const hb_blob_t *get_null () diff --git a/src/hb-ot-face-table-list.hh b/src/hb-ot-face-table-list.hh index 65cb4e644..99a9c22fa 100644 --- a/src/hb-ot-face-table-list.hh +++ b/src/hb-ot-face-table-list.hh @@ -32,6 +32,11 @@ #define HB_OT_FACE_TABLE_LIST_HH #endif /* HB_OT_FACE_TABLE_LIST_HH */ /* Dummy header guards */ +#ifndef HB_OT_CORE_TABLE +#define HB_OT_CORE_TABLE(Namespace, Type) HB_OT_TABLE (Namespace, Type) +#define _HB_OT_CORE_TABLE_UNDEF +#endif + #ifndef HB_OT_ACCELERATOR #define HB_OT_ACCELERATOR(Namespace, Type) HB_OT_TABLE (Namespace, Type) #define _HB_OT_ACCELERATOR_UNDEF @@ -46,8 +51,8 @@ /* OpenType fundamentals. */ -HB_OT_TABLE (OT, head) -HB_OT_TABLE (OT, maxp) +HB_OT_CORE_TABLE (OT, head) +HB_OT_CORE_TABLE (OT, maxp) #if !defined(HB_NO_FACE_COLLECT_UNICODES) || !defined(HB_NO_OT_FONT) HB_OT_ACCELERATOR (OT, cmap) #endif @@ -139,3 +144,7 @@ HB_OT_TABLE (OT, MATH) #ifdef _HB_OT_ACCELERATOR_UNDEF #undef HB_OT_ACCELERATOR #endif + +#ifdef _HB_OT_CORE_TABLE_UNDEF +#undef HB_OT_CORE_TABLE +#endif diff --git a/src/hb-ot-face.hh b/src/hb-ot-face.hh index e24d380bc..415dae8e2 100644 --- a/src/hb-ot-face.hh +++ b/src/hb-ot-face.hh @@ -63,10 +63,13 @@ struct hb_ot_face_t hb_face_t *face; /* MUST be JUST before the lazy loaders. */ #define HB_OT_TABLE(Namespace, Type) \ hb_table_lazy_loader_t Type; +#define HB_OT_CORE_TABLE(Namespace, Type) \ + hb_table_lazy_loader_t Type; #define HB_OT_ACCELERATOR(Namespace, Type) \ hb_face_lazy_loader_t Type; #include "hb-ot-face-table-list.hh" #undef HB_OT_ACCELERATOR +#undef HB_OT_CORE_TABLE #undef HB_OT_TABLE }; From c8fd8c133755fd9c62efc67033ca0193bd0dfc76 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 15 Feb 2022 18:02:53 -0600 Subject: [PATCH 11/12] [ot-face] Use core tables --- src/hb-ot-face-table-list.hh | 1 + src/hb-ot-glyf-table.hh | 3 +-- src/hb-static.cc | 24 +++--------------------- 3 files changed, 5 insertions(+), 23 deletions(-) diff --git a/src/hb-ot-face-table-list.hh b/src/hb-ot-face-table-list.hh index 99a9c22fa..c05034b3b 100644 --- a/src/hb-ot-face-table-list.hh +++ b/src/hb-ot-face-table-list.hh @@ -80,6 +80,7 @@ HB_OT_TABLE (OT, VORG) #endif /* TrueType outlines. */ +HB_OT_CORE_TABLE (OT, loca) // Also used to determine number of glyphs HB_OT_ACCELERATOR (OT, glyf) /* CFF outlines. */ diff --git a/src/hb-ot-glyf-table.hh b/src/hb-ot-glyf-table.hh index 87a7d800c..0f0db4791 100644 --- a/src/hb-ot-glyf-table.hh +++ b/src/hb-ot-glyf-table.hh @@ -936,7 +936,7 @@ struct glyf return; short_offset = 0 == head.indexToLocFormat; - loca_table = hb_sanitize_context_t ().reference_table (face); + loca_table = face->table.loca.get_blob (); // Needs no destruct! glyf_table = hb_sanitize_context_t ().reference_table (face); #ifndef HB_NO_VAR gvar = face->table.gvar; @@ -951,7 +951,6 @@ struct glyf } ~accelerator_t () { - loca_table.destroy (); glyf_table.destroy (); } diff --git a/src/hb-static.cc b/src/hb-static.cc index 0fd8fe9e8..4d486241a 100644 --- a/src/hb-static.cc +++ b/src/hb-static.cc @@ -60,23 +60,14 @@ static inline unsigned load_num_glyphs_from_loca (const hb_face_t *face) { unsigned ret = 0; - hb_sanitize_context_t c = hb_sanitize_context_t (); - c.set_num_glyphs (0); /* So we don't recurse ad infinitum. */ - /* We cannot use table.head because that would use sanitizer, - * which would try accessing face.num_glyphs, which would - * recurse here again... */ - hb_blob_t *head_blob = c.reference_table (face); - const OT::head *head_table = head_blob->as (); - unsigned indexToLocFormat = head_table->indexToLocFormat; - hb_blob_destroy (head_blob); + unsigned indexToLocFormat = face->table.head->indexToLocFormat; if (indexToLocFormat <= 1) { bool short_offset = 0 == indexToLocFormat; - hb_blob_t *loca_blob = c.reference_table (face); + hb_blob_t *loca_blob = face->table.loca.get_blob (); ret = hb_max (1u, loca_blob->length / (short_offset ? 2 : 4)) - 1; - hb_blob_destroy (loca_blob); } return ret; @@ -85,16 +76,7 @@ load_num_glyphs_from_loca (const hb_face_t *face) static inline unsigned load_num_glyphs_from_maxp (const hb_face_t *face) { - unsigned ret = 0; - hb_sanitize_context_t c = hb_sanitize_context_t (); - c.set_num_glyphs (0); /* So we don't recurse ad infinitum. */ - - hb_blob_t *maxp_blob = c.reference_table (face); - const OT::maxp *maxp_table = maxp_blob->as (); - ret = maxp_table->get_num_glyphs (); - hb_blob_destroy (maxp_blob); - - return ret; + return face->table.maxp->get_num_glyphs (); } unsigned int From f567b5561928e713737edc4655c6532ea6138a1d Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 15 Feb 2022 18:26:43 -0600 Subject: [PATCH 12/12] [face] Use max numGlyphs of maxp and loca --- src/hb-static.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/hb-static.cc b/src/hb-static.cc index 4d486241a..bd698814e 100644 --- a/src/hb-static.cc +++ b/src/hb-static.cc @@ -85,12 +85,10 @@ hb_face_t::load_num_glyphs () const unsigned ret = 0; #ifndef HB_NO_BORING_EXPANSION - if (!ret) - ret = load_num_glyphs_from_loca (this); + ret = hb_max (ret, load_num_glyphs_from_loca (this)); #endif - if (!ret) - ret = load_num_glyphs_from_maxp (this); + ret = hb_max (ret, load_num_glyphs_from_maxp (this)); num_glyphs.set_relaxed (ret); return ret;