From 48db1c958323cd1739a1e6fe8f6dfd625db7ad5d Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 20 May 2022 12:03:32 -0600 Subject: [PATCH 01/13] [font] Add serial API New API: + hb_font_get_serial() + hb_font_changed() Fixes https://github.com/harfbuzz/harfbuzz/issues/2426 Unused internally as of now. --- docs/harfbuzz-sections.txt | 2 + src/hb-font.cc | 88 ++++++++++++++++++++++++++++++++++++++ src/hb-font.h | 6 +++ src/hb-font.hh | 1 + 4 files changed, 97 insertions(+) diff --git a/docs/harfbuzz-sections.txt b/docs/harfbuzz-sections.txt index d02909784..1851a8392 100644 --- a/docs/harfbuzz-sections.txt +++ b/docs/harfbuzz-sections.txt @@ -373,6 +373,8 @@ hb_font_glyph_from_string hb_font_glyph_to_string hb_font_is_immutable hb_font_make_immutable +hb_font_get_serial +hb_font_changed hb_font_reference hb_font_set_face hb_font_set_funcs diff --git a/src/hb-font.cc b/src/hb-font.cc index 5700e0627..15b7e6add 100644 --- a/src/hb-font.cc +++ b/src/hb-font.cc @@ -1623,6 +1623,8 @@ DEFINE_NULL_INSTANCE (hb_font_t) = { HB_OBJECT_HEADER_STATIC, + 0, /* serial */ + nullptr, /* parent */ const_cast (&_hb_Null_hb_face_t), @@ -1852,6 +1854,11 @@ hb_font_set_user_data (hb_font_t *font, hb_destroy_func_t destroy /* May be NULL. */, hb_bool_t replace) { + if (hb_object_is_immutable (font)) + return false; + + font->serial++; + return hb_object_set_user_data (font, key, data, destroy, replace); } @@ -1910,6 +1917,45 @@ hb_font_is_immutable (hb_font_t *font) return hb_object_is_immutable (font); } +/** + * hb_font_get_serial: + * @font: #hb_font_t to work upon + * + * Returns the internal serial number of the font. The serial + * number is increased every time a setting on the font is + * changed, using a setter function. + * + * Return value: serial number + * + * Since: REPLACEME. + **/ +unsigned long +hb_font_get_serial (hb_font_t *font) +{ + return font->serial; +} + +/** + * hb_font_changed: + * @font: #hb_font_t to work upon + * + * Notifies the @font that underlying font data has changed. + * This has the effect of increasing the serial as returned + * by hb_font_get_serial(), which invalidates internal caches. + * + * Since: REPLACEME. + **/ +void +hb_font_changed (hb_font_t *font) +{ + if (hb_object_is_immutable (font)) + return; + + font->serial++; + + font->mults_changed (); +} + /** * hb_font_set_parent: * @font: #hb_font_t to work upon @@ -1926,6 +1972,11 @@ hb_font_set_parent (hb_font_t *font, if (hb_object_is_immutable (font)) return; + if (parent == font->parent) + return; + + font->serial++; + if (!parent) parent = hb_font_get_empty (); @@ -1968,6 +2019,11 @@ hb_font_set_face (hb_font_t *font, if (hb_object_is_immutable (font)) return; + if (face == font->face) + return; + + font->serial++; + if (unlikely (!face)) face = hb_face_get_empty (); @@ -2022,6 +2078,8 @@ hb_font_set_funcs (hb_font_t *font, return; } + font->serial++; + if (font->destroy) font->destroy (font->user_data); @@ -2059,6 +2117,8 @@ hb_font_set_funcs_data (hb_font_t *font, return; } + font->serial++; + if (font->destroy) font->destroy (font->user_data); @@ -2085,6 +2145,11 @@ hb_font_set_scale (hb_font_t *font, if (hb_object_is_immutable (font)) return; + if (font->x_scale == x_scale && font->y_scale == y_scale) + return; + + font->serial++; + font->x_scale = x_scale; font->y_scale = y_scale; font->mults_changed (); @@ -2127,6 +2192,11 @@ hb_font_set_ppem (hb_font_t *font, if (hb_object_is_immutable (font)) return; + if (font->x_ppem == x_ppem && font->y_ppem == y_ppem) + return; + + font->serial++; + font->x_ppem = x_ppem; font->y_ppem = y_ppem; } @@ -2169,6 +2239,11 @@ hb_font_set_ptem (hb_font_t *font, if (hb_object_is_immutable (font)) return; + if (font->ptem == ptem) + return; + + font->serial++; + font->ptem = ptem; } @@ -2216,6 +2291,11 @@ hb_font_set_synthetic_slant (hb_font_t *font, float slant) if (hb_object_is_immutable (font)) return; + if (font->slant == slant) + return; + + font->serial++; + font->slant = slant; font->mults_changed (); } @@ -2263,6 +2343,8 @@ hb_font_set_variations (hb_font_t *font, if (hb_object_is_immutable (font)) return; + font->serial++; + if (!variations_length) { hb_font_set_var_coords_normalized (font, nullptr, 0); @@ -2322,6 +2404,8 @@ hb_font_set_var_coords_design (hb_font_t *font, if (hb_object_is_immutable (font)) return; + font->serial++; + int *normalized = coords_length ? (int *) hb_calloc (coords_length, sizeof (int)) : nullptr; float *design_coords = coords_length ? (float *) hb_calloc (coords_length, sizeof (float)) : nullptr; @@ -2355,6 +2439,8 @@ hb_font_set_var_named_instance (hb_font_t *font, if (hb_object_is_immutable (font)) return; + font->serial++; + unsigned int coords_length = hb_ot_var_named_instance_get_design_coords (font->face, instance_index, nullptr, nullptr); float *coords = coords_length ? (float *) hb_calloc (coords_length, sizeof (float)) : nullptr; @@ -2391,6 +2477,8 @@ hb_font_set_var_coords_normalized (hb_font_t *font, if (hb_object_is_immutable (font)) return; + font->serial++; + int *copy = coords_length ? (int *) hb_calloc (coords_length, sizeof (coords[0])) : nullptr; int *unmapped = coords_length ? (int *) hb_calloc (coords_length, sizeof (coords[0])) : nullptr; float *design_coords = coords_length ? (float *) hb_calloc (coords_length, sizeof (design_coords[0])) : nullptr; diff --git a/src/hb-font.h b/src/hb-font.h index 954885753..9eedf6469 100644 --- a/src/hb-font.h +++ b/src/hb-font.h @@ -1002,6 +1002,12 @@ hb_font_make_immutable (hb_font_t *font); HB_EXTERN hb_bool_t hb_font_is_immutable (hb_font_t *font); +HB_EXTERN unsigned long +hb_font_get_serial (hb_font_t *font); + +HB_EXTERN void +hb_font_changed (hb_font_t *font); + HB_EXTERN void hb_font_set_parent (hb_font_t *font, hb_font_t *parent); diff --git a/src/hb-font.hh b/src/hb-font.hh index 70311b4a8..91dd0c968 100644 --- a/src/hb-font.hh +++ b/src/hb-font.hh @@ -104,6 +104,7 @@ DECLARE_NULL_INSTANCE (hb_font_funcs_t); struct hb_font_t { hb_object_header_t header; + unsigned long serial; hb_font_t *parent; hb_face_t *face; From 8629df188ad1a8563c2118de2cde983bdac4ecdd Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 20 May 2022 12:06:22 -0600 Subject: [PATCH 02/13] [ft] Discard advance cache if font changed Uses newly added font serial API. Part of https://github.com/harfbuzz/harfbuzz/issues/2270 But doesn't set new coords on the FT_Face. That's a lot more work :(. --- src/hb-ft.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/hb-ft.cc b/src/hb-ft.cc index 4537a8e6d..31d8ed281 100644 --- a/src/hb-ft.cc +++ b/src/hb-ft.cc @@ -86,7 +86,7 @@ struct hb_ft_font_t mutable hb_mutex_t lock; FT_Face ft_face; - mutable int cached_x_scale; + mutable unsigned long cached_serial; mutable hb_advance_cache_t advance_cache; }; @@ -103,7 +103,7 @@ _hb_ft_font_create (FT_Face ft_face, bool symbol, bool unref) ft_font->load_flags = FT_LOAD_DEFAULT | FT_LOAD_NO_HINTING; - ft_font->cached_x_scale = 0; + ft_font->cached_serial = (unsigned long) -1; ft_font->advance_cache.init (); return ft_font; @@ -337,10 +337,10 @@ hb_ft_get_glyph_h_advances (hb_font_t* font, void* font_data, int load_flags = ft_font->load_flags; int mult = font->x_scale < 0 ? -1 : +1; - if (font->x_scale != ft_font->cached_x_scale) + if (font->serial != ft_font->cached_serial) { ft_font->advance_cache.clear (); - ft_font->cached_x_scale = font->x_scale; + ft_font->cached_serial = font->serial; } for (unsigned int i = 0; i < count; i++) From a2015cd300282b05d7082fbdbdf1c0a93a8993fb Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 20 May 2022 12:15:00 -0600 Subject: [PATCH 03/13] [font] Add a separate serial_coords --- src/hb-font.cc | 11 ++++++----- src/hb-font.h | 2 +- src/hb-font.hh | 3 ++- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/hb-font.cc b/src/hb-font.cc index 15b7e6add..f1649ecd3 100644 --- a/src/hb-font.cc +++ b/src/hb-font.cc @@ -1624,6 +1624,7 @@ DEFINE_NULL_INSTANCE (hb_font_t) = HB_OBJECT_HEADER_STATIC, 0, /* serial */ + 0, /* serial_coords */ nullptr, /* parent */ const_cast (&_hb_Null_hb_face_t), @@ -1929,7 +1930,7 @@ hb_font_is_immutable (hb_font_t *font) * * Since: REPLACEME. **/ -unsigned long +unsigned int hb_font_get_serial (hb_font_t *font) { return font->serial; @@ -2343,7 +2344,7 @@ hb_font_set_variations (hb_font_t *font, if (hb_object_is_immutable (font)) return; - font->serial++; + font->serial_coords = ++font->serial; if (!variations_length) { @@ -2404,7 +2405,7 @@ hb_font_set_var_coords_design (hb_font_t *font, if (hb_object_is_immutable (font)) return; - font->serial++; + font->serial_coords = ++font->serial; int *normalized = coords_length ? (int *) hb_calloc (coords_length, sizeof (int)) : nullptr; float *design_coords = coords_length ? (float *) hb_calloc (coords_length, sizeof (float)) : nullptr; @@ -2439,7 +2440,7 @@ hb_font_set_var_named_instance (hb_font_t *font, if (hb_object_is_immutable (font)) return; - font->serial++; + font->serial_coords = ++font->serial; unsigned int coords_length = hb_ot_var_named_instance_get_design_coords (font->face, instance_index, nullptr, nullptr); @@ -2477,7 +2478,7 @@ hb_font_set_var_coords_normalized (hb_font_t *font, if (hb_object_is_immutable (font)) return; - font->serial++; + font->serial_coords = ++font->serial; int *copy = coords_length ? (int *) hb_calloc (coords_length, sizeof (coords[0])) : nullptr; int *unmapped = coords_length ? (int *) hb_calloc (coords_length, sizeof (coords[0])) : nullptr; diff --git a/src/hb-font.h b/src/hb-font.h index 9eedf6469..ca6ecf796 100644 --- a/src/hb-font.h +++ b/src/hb-font.h @@ -1002,7 +1002,7 @@ hb_font_make_immutable (hb_font_t *font); HB_EXTERN hb_bool_t hb_font_is_immutable (hb_font_t *font); -HB_EXTERN unsigned long +HB_EXTERN unsigned int hb_font_get_serial (hb_font_t *font); HB_EXTERN void diff --git a/src/hb-font.hh b/src/hb-font.hh index 91dd0c968..952beabe1 100644 --- a/src/hb-font.hh +++ b/src/hb-font.hh @@ -104,7 +104,8 @@ DECLARE_NULL_INSTANCE (hb_font_funcs_t); struct hb_font_t { hb_object_header_t header; - unsigned long serial; + unsigned int serial; + unsigned int serial_coords; hb_font_t *parent; hb_face_t *face; From d0de389de8f65f39ae97bb8b359d4b05cabd12b4 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 20 May 2022 12:18:43 -0600 Subject: [PATCH 04/13] [font] Fix test --- src/hb-font.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/hb-font.cc b/src/hb-font.cc index f1649ecd3..62a06d918 100644 --- a/src/hb-font.cc +++ b/src/hb-font.cc @@ -1855,10 +1855,8 @@ hb_font_set_user_data (hb_font_t *font, hb_destroy_func_t destroy /* May be NULL. */, hb_bool_t replace) { - if (hb_object_is_immutable (font)) - return false; - - font->serial++; + if (!hb_object_is_immutable (font)) + font->serial++; return hb_object_set_user_data (font, key, data, destroy, replace); } From 56e0ff9ea129aa91dfcc746cd61f8cbbc427dba7 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 20 May 2022 12:30:46 -0600 Subject: [PATCH 05/13] [ft] If hb_font changed, update FT_Face Fixes https://github.com/harfbuzz/harfbuzz/issues/2270 Rather untested. --- src/hb-ft.cc | 100 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 67 insertions(+), 33 deletions(-) diff --git a/src/hb-ft.cc b/src/hb-ft.cc index 31d8ed281..abfad4f67 100644 --- a/src/hb-ft.cc +++ b/src/hb-ft.cc @@ -130,6 +130,56 @@ _hb_ft_font_destroy (void *data) hb_free (ft_font); } + +/* hb_font changed, update FT_Face. */ +static void _hb_ft_changed (hb_font_t *font, FT_Face ft_face) +{ + FT_Set_Char_Size (ft_face, + abs (font->x_scale), abs (font->y_scale), + 0, 0); +#if 0 + font->x_ppem * 72 * 64 / font->x_scale, + font->y_ppem * 72 * 64 / font->y_scale); +#endif + if (font->x_scale < 0 || font->y_scale < 0) + { + FT_Matrix matrix = { font->x_scale < 0 ? -1 : +1, 0, + 0, font->y_scale < 0 ? -1 : +1}; + FT_Set_Transform (ft_face, &matrix, nullptr); + } + +#if defined(HAVE_FT_GET_VAR_BLEND_COORDINATES) && !defined(HB_NO_VAR) + unsigned int num_coords; + const int *coords = hb_font_get_var_coords_normalized (font, &num_coords); + if (num_coords) + { + FT_Fixed *ft_coords = (FT_Fixed *) hb_calloc (num_coords, sizeof (FT_Fixed)); + if (ft_coords) + { + for (unsigned int i = 0; i < num_coords; i++) + ft_coords[i] = coords[i] * 4; + FT_Set_Var_Blend_Coordinates (ft_face, num_coords, ft_coords); + hb_free (ft_coords); + } + } +#endif +} + +/* Check if hb_font changed, update FT_Face. */ +static inline bool +_hb_ft_check_changed (hb_font_t *font, + const hb_ft_font_t *ft_font) +{ + if (font->serial != ft_font->cached_serial) + { + _hb_ft_changed (font, ft_font->ft_face); + ft_font->cached_serial = font->serial; + return true; + } + return false; +} + + /** * hb_ft_font_set_load_flags: * @font: #hb_font_t to work upon @@ -337,11 +387,8 @@ hb_ft_get_glyph_h_advances (hb_font_t* font, void* font_data, int load_flags = ft_font->load_flags; int mult = font->x_scale < 0 ? -1 : +1; - if (font->serial != ft_font->cached_serial) - { + if (_hb_ft_check_changed (font, ft_font)) ft_font->advance_cache.clear (); - ft_font->cached_serial = font->serial; - } for (unsigned int i = 0; i < count; i++) { @@ -374,6 +421,8 @@ hb_ft_get_glyph_v_advance (hb_font_t *font, hb_lock_t lock (ft_font->lock); FT_Fixed v; + _hb_ft_check_changed (font, ft_font); + if (unlikely (FT_Get_Advance (ft_font->ft_face, glyph, ft_font->load_flags | FT_LOAD_VERTICAL_LAYOUT, &v))) return 0; @@ -400,6 +449,8 @@ hb_ft_get_glyph_v_origin (hb_font_t *font, hb_lock_t lock (ft_font->lock); FT_Face ft_face = ft_font->ft_face; + _hb_ft_check_changed (font, ft_font); + if (unlikely (FT_Load_Glyph (ft_face, glyph, ft_font->load_flags))) return false; @@ -429,6 +480,8 @@ hb_ft_get_glyph_h_kerning (hb_font_t *font, hb_lock_t lock (ft_font->lock); FT_Vector kerningv; + _hb_ft_check_changed (font, ft_font); + FT_Kerning_Mode mode = font->x_ppem ? FT_KERNING_DEFAULT : FT_KERNING_UNFITTED; if (FT_Get_Kerning (ft_font->ft_face, left_glyph, right_glyph, mode, &kerningv)) return 0; @@ -448,6 +501,8 @@ hb_ft_get_glyph_extents (hb_font_t *font, hb_lock_t lock (ft_font->lock); FT_Face ft_face = ft_font->ft_face; + _hb_ft_check_changed (font, ft_font); + if (unlikely (FT_Load_Glyph (ft_face, glyph, ft_font->load_flags))) return false; @@ -481,6 +536,8 @@ hb_ft_get_glyph_contour_point (hb_font_t *font HB_UNUSED, hb_lock_t lock (ft_font->lock); FT_Face ft_face = ft_font->ft_face; + _hb_ft_check_changed (font, ft_font); + if (unlikely (FT_Load_Glyph (ft_face, glyph, ft_font->load_flags))) return false; @@ -557,6 +614,9 @@ hb_ft_get_font_h_extents (hb_font_t *font HB_UNUSED, const hb_ft_font_t *ft_font = (const hb_ft_font_t *) font_data; hb_lock_t lock (ft_font->lock); FT_Face ft_face = ft_font->ft_face; + + _hb_ft_check_changed (font, ft_font); + metrics->ascender = FT_MulFix(ft_face->ascender, ft_face->size->metrics.y_scale); metrics->descender = FT_MulFix(ft_face->descender, ft_face->size->metrics.y_scale); metrics->line_gap = FT_MulFix( ft_face->height, ft_face->size->metrics.y_scale ) - (metrics->ascender - metrics->descender); @@ -620,6 +680,8 @@ hb_ft_get_glyph_shape (hb_font_t *font HB_UNUSED, hb_lock_t lock (ft_font->lock); FT_Face ft_face = ft_font->ft_face; + _hb_ft_check_changed (font, ft_font); + if (unlikely (FT_Load_Glyph (ft_face, glyph, FT_LOAD_NO_BITMAP | ft_font->load_flags))) return; @@ -1082,35 +1144,7 @@ hb_ft_font_set_funcs (hb_font_t *font) if (FT_Select_Charmap (ft_face, FT_ENCODING_MS_SYMBOL)) FT_Select_Charmap (ft_face, FT_ENCODING_UNICODE); - FT_Set_Char_Size (ft_face, - abs (font->x_scale), abs (font->y_scale), - 0, 0); -#if 0 - font->x_ppem * 72 * 64 / font->x_scale, - font->y_ppem * 72 * 64 / font->y_scale); -#endif - if (font->x_scale < 0 || font->y_scale < 0) - { - FT_Matrix matrix = { font->x_scale < 0 ? -1 : +1, 0, - 0, font->y_scale < 0 ? -1 : +1}; - FT_Set_Transform (ft_face, &matrix, nullptr); - } - -#if defined(HAVE_FT_GET_VAR_BLEND_COORDINATES) && !defined(HB_NO_VAR) - unsigned int num_coords; - const int *coords = hb_font_get_var_coords_normalized (font, &num_coords); - if (num_coords) - { - FT_Fixed *ft_coords = (FT_Fixed *) hb_calloc (num_coords, sizeof (FT_Fixed)); - if (ft_coords) - { - for (unsigned int i = 0; i < num_coords; i++) - ft_coords[i] = coords[i] * 4; - FT_Set_Var_Blend_Coordinates (ft_face, num_coords, ft_coords); - hb_free (ft_coords); - } - } -#endif + _hb_ft_changed (font, ft_face); ft_face->generic.data = blob; ft_face->generic.finalizer = (FT_Generic_Finalizer) _release_blob; From 80c49933c6dead0ebb6678eece7520e22552e6c8 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Mon, 23 May 2022 19:02:27 -0600 Subject: [PATCH 06/13] [hb-ft] Adjust serial signature --- src/hb-ft.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hb-ft.cc b/src/hb-ft.cc index abfad4f67..57daa8280 100644 --- a/src/hb-ft.cc +++ b/src/hb-ft.cc @@ -86,7 +86,7 @@ struct hb_ft_font_t mutable hb_mutex_t lock; FT_Face ft_face; - mutable unsigned long cached_serial; + mutable unsigned cached_serial; mutable hb_advance_cache_t advance_cache; }; @@ -103,7 +103,7 @@ _hb_ft_font_create (FT_Face ft_face, bool symbol, bool unref) ft_font->load_flags = FT_LOAD_DEFAULT | FT_LOAD_NO_HINTING; - ft_font->cached_serial = (unsigned long) -1; + ft_font->cached_serial = (unsigned) -1; ft_font->advance_cache.init (); return ft_font; From 970e03ecaebdc5cf5120ec80cb6716dd9bd40e52 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Mon, 23 May 2022 19:02:36 -0600 Subject: [PATCH 07/13] [ot-font] Add a hb_ot_font_t struct --- src/hb-ot-font.cc | 41 +++++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/src/hb-ot-font.cc b/src/hb-ot-font.cc index 521b3ee92..55a20605d 100644 --- a/src/hb-ot-font.cc +++ b/src/hb-ot-font.cc @@ -58,6 +58,10 @@ * never need to call these functions directly. **/ +struct hb_ot_font_t +{ + const hb_ot_face_t *ot_face; +}; static hb_bool_t hb_ot_get_nominal_glyph (hb_font_t *font HB_UNUSED, @@ -66,7 +70,8 @@ hb_ot_get_nominal_glyph (hb_font_t *font HB_UNUSED, hb_codepoint_t *glyph, void *user_data HB_UNUSED) { - const hb_ot_face_t *ot_face = (const hb_ot_face_t *) font_data; + const hb_ot_font_t *ot_font = (const hb_ot_font_t *) font_data; + const hb_ot_face_t *ot_face = ot_font->ot_face; return ot_face->cmap->get_nominal_glyph (unicode, glyph); } @@ -80,7 +85,8 @@ hb_ot_get_nominal_glyphs (hb_font_t *font HB_UNUSED, unsigned int glyph_stride, void *user_data HB_UNUSED) { - const hb_ot_face_t *ot_face = (const hb_ot_face_t *) font_data; + const hb_ot_font_t *ot_font = (const hb_ot_font_t *) font_data; + const hb_ot_face_t *ot_face = ot_font->ot_face; return ot_face->cmap->get_nominal_glyphs (count, first_unicode, unicode_stride, first_glyph, glyph_stride); @@ -94,7 +100,8 @@ hb_ot_get_variation_glyph (hb_font_t *font HB_UNUSED, hb_codepoint_t *glyph, void *user_data HB_UNUSED) { - const hb_ot_face_t *ot_face = (const hb_ot_face_t *) font_data; + const hb_ot_font_t *ot_font = (const hb_ot_font_t *) font_data; + const hb_ot_face_t *ot_face = ot_font->ot_face; return ot_face->cmap->get_variation_glyph (unicode, variation_selector, glyph); } @@ -107,7 +114,8 @@ hb_ot_get_glyph_h_advances (hb_font_t* font, void* font_data, unsigned advance_stride, void *user_data HB_UNUSED) { - const hb_ot_face_t *ot_face = (const hb_ot_face_t *) font_data; + const hb_ot_font_t *ot_font = (const hb_ot_font_t *) font_data; + const hb_ot_face_t *ot_face = ot_font->ot_face; const OT::hmtx_accelerator_t &hmtx = *ot_face->hmtx; #ifndef HB_NO_VAR @@ -140,7 +148,8 @@ hb_ot_get_glyph_v_advances (hb_font_t* font, void* font_data, unsigned advance_stride, void *user_data HB_UNUSED) { - const hb_ot_face_t *ot_face = (const hb_ot_face_t *) font_data; + const hb_ot_font_t *ot_font = (const hb_ot_font_t *) font_data; + const hb_ot_face_t *ot_face = ot_font->ot_face; const OT::vmtx_accelerator_t &vmtx = *ot_face->vmtx; if (vmtx.has_data ()) @@ -189,7 +198,8 @@ hb_ot_get_glyph_v_origin (hb_font_t *font, hb_position_t *y, void *user_data HB_UNUSED) { - const hb_ot_face_t *ot_face = (const hb_ot_face_t *) font_data; + const hb_ot_font_t *ot_font = (const hb_ot_font_t *) font_data; + const hb_ot_face_t *ot_face = ot_font->ot_face; *x = font->get_glyph_h_advance (glyph) / 2; @@ -234,7 +244,8 @@ hb_ot_get_glyph_extents (hb_font_t *font, hb_glyph_extents_t *extents, void *user_data HB_UNUSED) { - const hb_ot_face_t *ot_face = (const hb_ot_face_t *) font_data; + const hb_ot_font_t *ot_font = (const hb_ot_font_t *) font_data; + const hb_ot_face_t *ot_face = ot_font->ot_face; #if !defined(HB_NO_OT_FONT_BITMAP) && !defined(HB_NO_COLOR) if (ot_face->sbix->get_extents (font, glyph, extents)) return true; @@ -260,7 +271,9 @@ hb_ot_get_glyph_name (hb_font_t *font HB_UNUSED, char *name, unsigned int size, void *user_data HB_UNUSED) { - const hb_ot_face_t *ot_face = (const hb_ot_face_t *) font_data; + const hb_ot_font_t *ot_font = (const hb_ot_font_t *) font_data; + const hb_ot_face_t *ot_face = ot_font->ot_face; + if (ot_face->post->get_glyph_name (glyph, name, size)) return true; #ifndef HB_NO_OT_FONT_CFF if (ot_face->cff1->get_glyph_name (glyph, name, size)) return true; @@ -274,7 +287,9 @@ hb_ot_get_glyph_from_name (hb_font_t *font HB_UNUSED, hb_codepoint_t *glyph, void *user_data HB_UNUSED) { - const hb_ot_face_t *ot_face = (const hb_ot_face_t *) font_data; + const hb_ot_font_t *ot_font = (const hb_ot_font_t *) font_data; + const hb_ot_face_t *ot_face = ot_font->ot_face; + if (ot_face->post->get_glyph_from_name (name, len, glyph)) return true; #ifndef HB_NO_OT_FONT_CFF if (ot_face->cff1->get_glyph_from_name (name, len, glyph)) return true; @@ -390,9 +405,15 @@ _hb_ot_get_font_funcs () void hb_ot_font_set_funcs (hb_font_t *font) { + hb_ot_font_t *ot_font = (hb_ot_font_t *) calloc (1, sizeof (hb_ot_font_t)); + if (unlikely (!ot_font)) + return; + + ot_font->ot_face = &font->face->table; + hb_font_set_funcs (font, _hb_ot_get_font_funcs (), - &font->face->table, + ot_font, nullptr); } From 39a07bf3eba74ab91827cb43b98127ae85f781e2 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Mon, 23 May 2022 19:13:05 -0600 Subject: [PATCH 08/13] [ot-font] Rename cache to varStore_cache --- src/hb-ot-font.cc | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/hb-ot-font.cc b/src/hb-ot-font.cc index 55a20605d..76fe3fc55 100644 --- a/src/hb-ot-font.cc +++ b/src/hb-ot-font.cc @@ -121,20 +121,20 @@ hb_ot_get_glyph_h_advances (hb_font_t* font, void* font_data, #ifndef HB_NO_VAR const OT::HVARVVAR &HVAR = *hmtx.var_table; const OT::VariationStore &varStore = &HVAR + HVAR.varStore; - OT::VariationStore::cache_t *cache = font->num_coords ? varStore.create_cache () : nullptr; + OT::VariationStore::cache_t *varStore_cache = font->num_coords ? varStore.create_cache () : nullptr; #else - OT::VariationStore::cache_t *cache = nullptr; + OT::VariationStore::cache_t *varStore_cache = nullptr; #endif for (unsigned int i = 0; i < count; i++) { - *first_advance = font->em_scale_x (hmtx.get_advance (*first_glyph, font, cache)); + *first_advance = font->em_scale_x (hmtx.get_advance (*first_glyph, font, varStore_cache)); first_glyph = &StructAtOffsetUnaligned (first_glyph, glyph_stride); first_advance = &StructAtOffsetUnaligned (first_advance, advance_stride); } #ifndef HB_NO_VAR - OT::VariationStore::destroy_cache (cache); + OT::VariationStore::destroy_cache (varStore_cache); #endif } @@ -157,20 +157,20 @@ hb_ot_get_glyph_v_advances (hb_font_t* font, void* font_data, #ifndef HB_NO_VAR const OT::HVARVVAR &VVAR = *vmtx.var_table; const OT::VariationStore &varStore = &VVAR + VVAR.varStore; - OT::VariationStore::cache_t *cache = font->num_coords ? varStore.create_cache () : nullptr; + OT::VariationStore::cache_t *varStore_cache = font->num_coords ? varStore.create_cache () : nullptr; #else - OT::VariationStore::cache_t *cache = nullptr; + OT::VariationStore::cache_t *varStore_cache = nullptr; #endif for (unsigned int i = 0; i < count; i++) { - *first_advance = font->em_scale_y (-(int) vmtx.get_advance (*first_glyph, font, cache)); + *first_advance = font->em_scale_y (-(int) vmtx.get_advance (*first_glyph, font, varStore_cache)); first_glyph = &StructAtOffsetUnaligned (first_glyph, glyph_stride); first_advance = &StructAtOffsetUnaligned (first_advance, advance_stride); } #ifndef HB_NO_VAR - OT::VariationStore::destroy_cache (cache); + OT::VariationStore::destroy_cache (varStore_cache); #endif } else @@ -405,7 +405,7 @@ _hb_ot_get_font_funcs () void hb_ot_font_set_funcs (hb_font_t *font) { - hb_ot_font_t *ot_font = (hb_ot_font_t *) calloc (1, sizeof (hb_ot_font_t)); + hb_ot_font_t *ot_font = (hb_ot_font_t *) hb_calloc (1, sizeof (hb_ot_font_t)); if (unlikely (!ot_font)) return; @@ -414,7 +414,7 @@ hb_ot_font_set_funcs (hb_font_t *font) hb_font_set_funcs (font, _hb_ot_get_font_funcs (), ot_font, - nullptr); + hb_free); } #ifndef HB_NO_VAR From 3548b6025fb53fd287910642cd52b12991e82d2d Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Mon, 23 May 2022 19:39:52 -0600 Subject: [PATCH 09/13] [ot-font] Cache h-advances for variable fonts --- src/hb-ot-font.cc | 97 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 89 insertions(+), 8 deletions(-) diff --git a/src/hb-ot-font.cc b/src/hb-ot-font.cc index 76fe3fc55..db35965fb 100644 --- a/src/hb-ot-font.cc +++ b/src/hb-ot-font.cc @@ -30,6 +30,7 @@ #include "hb-ot.h" +#include "hb-cache.hh" #include "hb-font.hh" #include "hb-machinery.hh" #include "hb-ot-face.hh" @@ -61,8 +62,42 @@ struct hb_ot_font_t { const hb_ot_face_t *ot_face; + + /* h_advance caching */ + mutable hb_mutex_t lock; + mutable unsigned cached_coords_serial; + mutable hb_advance_cache_t *advance_cache; }; +static hb_ot_font_t * +_hb_ot_font_create (hb_font_t *font) +{ + hb_ot_font_t *ot_font = (hb_ot_font_t *) hb_calloc (1, sizeof (hb_ot_font_t)); + if (unlikely (!ot_font)) + return nullptr; + + ot_font->ot_face = &font->face->table; + ot_font->lock.init (); + + return ot_font; +} + +static void +_hb_ot_font_destroy (void *font_data) +{ + hb_ot_font_t *ot_font = (hb_ot_font_t *) font_data; + + ot_font->lock.fini (); + + if (ot_font->advance_cache) + { + ot_font->advance_cache->fini (); + hb_free (ot_font->advance_cache); + } + + hb_free (ot_font); +} + static hb_bool_t hb_ot_get_nominal_glyph (hb_font_t *font HB_UNUSED, void *font_data, @@ -122,15 +157,63 @@ hb_ot_get_glyph_h_advances (hb_font_t* font, void* font_data, const OT::HVARVVAR &HVAR = *hmtx.var_table; const OT::VariationStore &varStore = &HVAR + HVAR.varStore; OT::VariationStore::cache_t *varStore_cache = font->num_coords ? varStore.create_cache () : nullptr; + + bool use_cache = font->num_coords; #else OT::VariationStore::cache_t *varStore_cache = nullptr; + bool use_cache = false; #endif - for (unsigned int i = 0; i < count; i++) + if (use_cache && !ot_font->advance_cache) { - *first_advance = font->em_scale_x (hmtx.get_advance (*first_glyph, font, varStore_cache)); - first_glyph = &StructAtOffsetUnaligned (first_glyph, glyph_stride); - first_advance = &StructAtOffsetUnaligned (first_advance, advance_stride); + ot_font->lock.lock (); + ot_font->advance_cache = (hb_advance_cache_t *) hb_malloc (sizeof (*ot_font->advance_cache)); + if (unlikely (!ot_font->advance_cache)) + { + ot_font->lock.unlock (); + use_cache = false; + } + else + { + ot_font->advance_cache->init (); + ot_font->cached_coords_serial = font->serial_coords; + } + } + + if (!use_cache) + { + for (unsigned int i = 0; i < count; i++) + { + *first_advance = font->em_scale_x (hmtx.get_advance (*first_glyph, font, varStore_cache)); + first_glyph = &StructAtOffsetUnaligned (first_glyph, glyph_stride); + first_advance = &StructAtOffsetUnaligned (first_advance, advance_stride); + } + } + else + { /* Use cache; lock already held and cache initialized. */ + if (ot_font->cached_coords_serial != font->serial_coords) + { + ot_font->advance_cache->init (); + ot_font->cached_coords_serial = font->serial_coords; + } + + for (unsigned int i = 0; i < count; i++) + { + hb_position_t v; + unsigned cv; + if (ot_font->advance_cache->get (*first_glyph, &cv)) + v = cv; + else + { + v = hmtx.get_advance (*first_glyph, font, varStore_cache); + ot_font->advance_cache->set (*first_glyph, v); + } + *first_advance = font->em_scale_x (v); + first_glyph = &StructAtOffsetUnaligned (first_glyph, glyph_stride); + first_advance = &StructAtOffsetUnaligned (first_advance, advance_stride); + } + + ot_font->lock.unlock (); } #ifndef HB_NO_VAR @@ -405,16 +488,14 @@ _hb_ot_get_font_funcs () void hb_ot_font_set_funcs (hb_font_t *font) { - hb_ot_font_t *ot_font = (hb_ot_font_t *) hb_calloc (1, sizeof (hb_ot_font_t)); + hb_ot_font_t *ot_font = _hb_ot_font_create (font); if (unlikely (!ot_font)) return; - ot_font->ot_face = &font->face->table; - hb_font_set_funcs (font, _hb_ot_get_font_funcs (), ot_font, - hb_free); + _hb_ot_font_destroy); } #ifndef HB_NO_VAR From 0919eaa6e84c4de9d9fbaab8938474295a480892 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 24 May 2022 13:07:24 -0600 Subject: [PATCH 10/13] [ot-font] Remove lock around cache Not needed. --- src/hb-ot-cff1-table.hh | 6 +++--- src/hb-ot-font.cc | 46 ++++++++++++++++++++++------------------- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/hb-ot-cff1-table.hh b/src/hb-ot-cff1-table.hh index f441f6a4d..43046d829 100644 --- a/src/hb-ot-cff1-table.hh +++ b/src/hb-ot-cff1-table.hh @@ -1333,7 +1333,7 @@ struct cff1 if (names) { names->fini (); - free (names); + hb_free (names); } SUPER::fini (); @@ -1379,7 +1379,7 @@ struct cff1 hb_sorted_vector_t *names = glyph_names.get (); if (unlikely (!names)) { - names = (hb_sorted_vector_t *) calloc (sizeof (hb_sorted_vector_t), 1); + names = (hb_sorted_vector_t *) hb_calloc (sizeof (hb_sorted_vector_t), 1); if (likely (names)) { names->init (); @@ -1409,7 +1409,7 @@ struct cff1 if (names) { names->fini (); - free (names); + hb_free (names); } goto retry; } diff --git a/src/hb-ot-font.cc b/src/hb-ot-font.cc index db35965fb..3fa522a19 100644 --- a/src/hb-ot-font.cc +++ b/src/hb-ot-font.cc @@ -64,9 +64,8 @@ struct hb_ot_font_t const hb_ot_face_t *ot_face; /* h_advance caching */ - mutable hb_mutex_t lock; mutable unsigned cached_coords_serial; - mutable hb_advance_cache_t *advance_cache; + mutable hb_atomic_ptr_t advance_cache; }; static hb_ot_font_t * @@ -77,7 +76,6 @@ _hb_ot_font_create (hb_font_t *font) return nullptr; ot_font->ot_face = &font->face->table; - ot_font->lock.init (); return ot_font; } @@ -87,12 +85,11 @@ _hb_ot_font_destroy (void *font_data) { hb_ot_font_t *ot_font = (hb_ot_font_t *) font_data; - ot_font->lock.fini (); - - if (ot_font->advance_cache) + auto *cache = ot_font->advance_cache.get_relaxed (); + if (cache) { - ot_font->advance_cache->fini (); - hb_free (ot_font->advance_cache); + cache->fini (); + hb_free (cache); } hb_free (ot_font); @@ -164,21 +161,30 @@ hb_ot_get_glyph_h_advances (hb_font_t* font, void* font_data, bool use_cache = false; #endif - if (use_cache && !ot_font->advance_cache) + hb_advance_cache_t *cache = nullptr; + if (use_cache) { - ot_font->lock.lock (); - ot_font->advance_cache = (hb_advance_cache_t *) hb_malloc (sizeof (*ot_font->advance_cache)); - if (unlikely (!ot_font->advance_cache)) - { - ot_font->lock.unlock (); - use_cache = false; - } - else + retry: + cache = ot_font->advance_cache.get (); + if (unlikely (!cache)) { - ot_font->advance_cache->init (); + cache = (hb_advance_cache_t *) hb_malloc (sizeof (hb_advance_cache_t)); + if (unlikely (!cache)) + { + use_cache = false; + goto out; + } + + cache->init (); + if (unlikely (!ot_font->advance_cache.cmpexch (nullptr, cache))) + { + hb_free (cache); + goto retry; + } ot_font->cached_coords_serial = font->serial_coords; } } + out: if (!use_cache) { @@ -190,7 +196,7 @@ hb_ot_get_glyph_h_advances (hb_font_t* font, void* font_data, } } else - { /* Use cache; lock already held and cache initialized. */ + { /* Use cache. */ if (ot_font->cached_coords_serial != font->serial_coords) { ot_font->advance_cache->init (); @@ -212,8 +218,6 @@ hb_ot_get_glyph_h_advances (hb_font_t* font, void* font_data, first_glyph = &StructAtOffsetUnaligned (first_glyph, glyph_stride); first_advance = &StructAtOffsetUnaligned (first_advance, advance_stride); } - - ot_font->lock.unlock (); } #ifndef HB_NO_VAR From 12fff976b6cc4433dd3ed6aa7cf852031f7bd289 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 24 May 2022 13:42:25 -0600 Subject: [PATCH 11/13] [ot-var] Use atomic int for cached-serial --- src/hb-ot-font.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/hb-ot-font.cc b/src/hb-ot-font.cc index 3fa522a19..4b25db64a 100644 --- a/src/hb-ot-font.cc +++ b/src/hb-ot-font.cc @@ -64,7 +64,7 @@ struct hb_ot_font_t const hb_ot_face_t *ot_face; /* h_advance caching */ - mutable unsigned cached_coords_serial; + mutable hb_atomic_int_t cached_coords_serial; mutable hb_atomic_ptr_t advance_cache; }; @@ -181,7 +181,7 @@ hb_ot_get_glyph_h_advances (hb_font_t* font, void* font_data, hb_free (cache); goto retry; } - ot_font->cached_coords_serial = font->serial_coords; + ot_font->cached_coords_serial.set_relaxed (font->serial_coords); } } out: @@ -197,10 +197,10 @@ hb_ot_get_glyph_h_advances (hb_font_t* font, void* font_data, } else { /* Use cache. */ - if (ot_font->cached_coords_serial != font->serial_coords) + if (ot_font->cached_coords_serial.get_relaxed () != (int) font->serial_coords) { ot_font->advance_cache->init (); - ot_font->cached_coords_serial = font->serial_coords; + ot_font->cached_coords_serial.set_relaxed (font->serial_coords); } for (unsigned int i = 0; i < count; i++) From 5248b2567b9f627097ad25afd9671da9c9997224 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 24 May 2022 13:55:17 -0600 Subject: [PATCH 12/13] [ot-font/h-advance] Adjust varStore cache condition This gives best performance for short strings, now that we have a h-advance cache as well. The en-words benchmark in particular, now ot-font is faster than ft. Second to last line is of interest: Before: ----------------------------------------------------------------------------------------------------- Benchmark Time CPU Iterations ----------------------------------------------------------------------------------------------------- BM_Shape/en-words.txt/Roboto-Regular.ttf/hb 29.8 ms 29.8 ms 23 BM_Shape/en-words.txt/Roboto-Regular.ttf/ft 30.4 ms 30.4 ms 23 BM_Shape/en-words.txt/SourceSerifVariable-Roman.ttf/hb 16.3 ms 16.3 ms 43 BM_Shape/en-words.txt/SourceSerifVariable-Roman.ttf/ft 16.5 ms 16.5 ms 42 BM_Shape/en-words.txt/SourceSerifVariable-Roman.ttf/var/hb 18.0 ms 18.0 ms 39 BM_Shape/en-words.txt/SourceSerifVariable-Roman.ttf/var/ft 17.8 ms 17.8 ms 39 After: behdad@Behdads-MacBook-Pro harfbuzz % ninja -Cbuild && build/perf/benchmark-shape --benchmark_filter=en-words ----------------------------------------------------------------------------------------------------- Benchmark Time CPU Iterations ----------------------------------------------------------------------------------------------------- BM_Shape/en-words.txt/Roboto-Regular.ttf/hb 30.0 ms 30.0 ms 23 BM_Shape/en-words.txt/Roboto-Regular.ttf/ft 30.3 ms 30.3 ms 23 BM_Shape/en-words.txt/SourceSerifVariable-Roman.ttf/hb 16.3 ms 16.3 ms 43 BM_Shape/en-words.txt/SourceSerifVariable-Roman.ttf/ft 16.4 ms 16.4 ms 43 BM_Shape/en-words.txt/SourceSerifVariable-Roman.ttf/var/hb 17.6 ms 17.6 ms 40 BM_Shape/en-words.txt/SourceSerifVariable-Roman.ttf/var/ft 17.8 ms 17.8 ms 39 --- src/hb-ot-font.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hb-ot-font.cc b/src/hb-ot-font.cc index 4b25db64a..d1032068b 100644 --- a/src/hb-ot-font.cc +++ b/src/hb-ot-font.cc @@ -153,7 +153,7 @@ hb_ot_get_glyph_h_advances (hb_font_t* font, void* font_data, #ifndef HB_NO_VAR const OT::HVARVVAR &HVAR = *hmtx.var_table; const OT::VariationStore &varStore = &HVAR + HVAR.varStore; - OT::VariationStore::cache_t *varStore_cache = font->num_coords ? varStore.create_cache () : nullptr; + OT::VariationStore::cache_t *varStore_cache = font->num_coords * count >= 128 ? varStore.create_cache () : nullptr; bool use_cache = font->num_coords; #else From a719e67887ccd5659aab0ba1fc6ff819795f7aa7 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 24 May 2022 17:51:24 -0600 Subject: [PATCH 13/13] [ot-font] Use atomic ops for cache serial number This guarantees the cache is coherent. --- src/hb-ot-font.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hb-ot-font.cc b/src/hb-ot-font.cc index d1032068b..af1bc86d4 100644 --- a/src/hb-ot-font.cc +++ b/src/hb-ot-font.cc @@ -181,7 +181,7 @@ hb_ot_get_glyph_h_advances (hb_font_t* font, void* font_data, hb_free (cache); goto retry; } - ot_font->cached_coords_serial.set_relaxed (font->serial_coords); + ot_font->cached_coords_serial.set (font->serial_coords); } } out: @@ -197,10 +197,10 @@ hb_ot_get_glyph_h_advances (hb_font_t* font, void* font_data, } else { /* Use cache. */ - if (ot_font->cached_coords_serial.get_relaxed () != (int) font->serial_coords) + if (ot_font->cached_coords_serial.get () != (int) font->serial_coords) { ot_font->advance_cache->init (); - ot_font->cached_coords_serial.set_relaxed (font->serial_coords); + ot_font->cached_coords_serial.set (font->serial_coords); } for (unsigned int i = 0; i < count; i++)