From dda1d95af2294c01e534524b8137d465e5a8f5b8 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 27 Mar 2025 14:32:13 -0600 Subject: [PATCH 1/3] Revert "[ot-font/trak] Move trak application to ot-font instead of ot-shape" This reverts commits ffae5b040d50d602284ef7426340ff332b811df5 and 17c11ec5238ef7efdcc340a2f578feb668c4252a. Plus manual modifications. See https://github.com/harfbuzz/harfbuzz/issues/5221 --- src/hb-aat-layout-trak-table.hh | 28 ++++++++++++++++++++ src/hb-aat-layout.cc | 13 +++++++++- src/hb-aat-layout.hh | 5 ++++ src/hb-ft.cc | 35 ------------------------- src/hb-ot-font.cc | 45 +-------------------------------- src/hb-ot-shape.cc | 13 ++++++++++ src/hb-ot-shape.hh | 2 ++ 7 files changed, 61 insertions(+), 80 deletions(-) diff --git a/src/hb-aat-layout-trak-table.hh b/src/hb-aat-layout-trak-table.hh index 24c212c2c..5a4372dd3 100644 --- a/src/hb-aat-layout-trak-table.hh +++ b/src/hb-aat-layout-trak-table.hh @@ -199,6 +199,34 @@ struct trak { float ptem = font->ptem > 0.f ? font->ptem : HB_CORETEXT_DEFAULT_FONT_SIZE; return font->em_scalef_y ((this+vertData).get_tracking (this, ptem, track)); + } + + bool apply (hb_aat_apply_context_t *c, float track = 0.f) const + { + TRACE_APPLY (this); + + float ptem = c->font->ptem; + if (unlikely (ptem <= 0.f)) + { + /* https://developer.apple.com/documentation/coretext/1508745-ctfontcreatewithgraphicsfont */ + ptem = HB_CORETEXT_DEFAULT_FONT_SIZE; + } + + hb_buffer_t *buffer = c->buffer; + if (HB_DIRECTION_IS_HORIZONTAL (buffer->props.direction)) + { + hb_position_t advance_to_add = get_h_tracking (c->font, track); + foreach_grapheme (buffer, start, end) + buffer->pos[start].x_advance += advance_to_add; + } + else + { + hb_position_t advance_to_add = get_v_tracking (c->font, track); + foreach_grapheme (buffer, start, end) + buffer->pos[start].y_advance += advance_to_add; + } + + return_trace (true); } bool sanitize (hb_sanitize_context_t *c) const diff --git a/src/hb-aat-layout.cc b/src/hb-aat-layout.cc index 374554466..a48c4e0bc 100644 --- a/src/hb-aat-layout.cc +++ b/src/hb-aat-layout.cc @@ -34,7 +34,7 @@ #include "hb-aat-layout-just-table.hh" // Just so we compile it; unused otherwise. #include "hb-aat-layout-kerx-table.hh" #include "hb-aat-layout-morx-table.hh" -#include "hb-aat-layout-trak-table.hh" // Just so we compile it; unused otherwise. +#include "hb-aat-layout-trak-table.hh" #include "hb-aat-ltag-table.hh" #include "hb-ot-layout-gsub-table.hh" @@ -394,6 +394,17 @@ hb_aat_layout_has_tracking (hb_face_t *face) return face->table.trak->has_data (); } +void +hb_aat_layout_track (const hb_ot_shape_plan_t *plan, + hb_font_t *font, + hb_buffer_t *buffer) +{ + const AAT::trak& trak = *font->face->table.trak; + + AAT::hb_aat_apply_context_t c (plan, font, buffer); + trak.apply (&c); +} + /** * hb_aat_layout_get_feature_types: * @face: #hb_face_t to work upon diff --git a/src/hb-aat-layout.hh b/src/hb-aat-layout.hh index 0e4ae6e7b..8b80fd797 100644 --- a/src/hb-aat-layout.hh +++ b/src/hb-aat-layout.hh @@ -68,5 +68,10 @@ hb_aat_layout_position (const hb_ot_shape_plan_t *plan, hb_font_t *font, hb_buffer_t *buffer); +HB_INTERNAL void +hb_aat_layout_track (const hb_ot_shape_plan_t *plan, + hb_font_t *font, + hb_buffer_t *buffer); + #endif /* HB_AAT_LAYOUT_HH */ diff --git a/src/hb-ft.cc b/src/hb-ft.cc index 756ba8a54..47b0484e0 100644 --- a/src/hb-ft.cc +++ b/src/hb-ft.cc @@ -37,11 +37,7 @@ #include "hb-draw.hh" #include "hb-font.hh" #include "hb-machinery.hh" -#ifndef HB_NO_AAT -#include "hb-aat-layout-trak-table.hh" -#endif #include "hb-ot-os2-table.hh" -#include "hb-ot-stat-table.hh" #include "hb-ot-shaper-arabic-pua.hh" #include "hb-paint.hh" @@ -534,26 +530,6 @@ hb_ft_get_glyph_h_advances (hb_font_t* font, void* font_data, first_advance = &StructAtOffsetUnaligned (first_advance, advance_stride); } } - -#ifndef HB_NO_AAT - /* According to Ned, trak is applied by default for "modern fonts", as detected by presence of STAT table. */ -#ifndef HB_NO_STYLE - bool apply_trak = font->face->table.STAT->has_data () && font->face->table.trak->has_data (); -#else - bool apply_trak = false; -#endif - if (apply_trak) - { - hb_position_t tracking = font->face->table.trak->get_h_tracking (font); - first_advance = orig_first_advance; - for (unsigned int i = 0; i < count; i++) - { - *first_advance += tracking; - first_glyph = &StructAtOffsetUnaligned (first_glyph, glyph_stride); - first_advance = &StructAtOffsetUnaligned (first_advance, advance_stride); - } - } -#endif } #ifndef HB_NO_VERTICAL @@ -594,17 +570,6 @@ hb_ft_get_glyph_v_advance (hb_font_t *font, hb_position_t y_strength = font->y_scale >= 0 ? font->y_strength : -font->y_strength; v = ((-v + (1<<9)) >> 10) + (font->embolden_in_place ? 0 : y_strength); -#ifndef HB_NO_AAT - /* According to Ned, trak is applied by default for "modern fonts", as detected by presence of STAT table. */ -#ifndef HB_NO_STYLE - bool apply_trak = font->face->table.STAT->has_data () && font->face->table.trak->has_data (); -#else - bool apply_trak = false; -#endif - if (apply_trak) - v += font->face->table.trak->get_v_tracking (font); -#endif - return v; } #endif diff --git a/src/hb-ot-font.cc b/src/hb-ot-font.cc index 9bd0db4b8..78c8a6236 100644 --- a/src/hb-ot-font.cc +++ b/src/hb-ot-font.cc @@ -36,9 +36,6 @@ #include "hb-ot-face.hh" #include "hb-outline.hh" -#ifndef HB_NO_AAT -#include "hb-aat-layout-trak-table.hh" -#endif #include "hb-ot-cmap-table.hh" #include "hb-ot-glyf-table.hh" #include "hb-ot-cff2-table.hh" @@ -76,10 +73,6 @@ struct hb_ot_font_t { const hb_ot_face_t *ot_face; -#ifndef HB_NO_AAT - bool apply_trak; -#endif - #ifndef HB_NO_OT_FONT_CMAP_CACHE hb_ot_font_cmap_cache_t *cmap_cache; #endif @@ -98,15 +91,6 @@ _hb_ot_font_create (hb_font_t *font) ot_font->ot_face = &font->face->table; -#ifndef HB_NO_AAT - /* According to Ned, trak is applied by default for "modern fonts", as detected by presence of STAT table. */ -#ifndef HB_NO_STYLE - ot_font->apply_trak = font->face->table.STAT->has_data () && font->face->table.trak->has_data (); -#else - ot_font->apply_trak = false; -#endif -#endif - #ifndef HB_NO_OT_FONT_CMAP_CACHE // retry: auto *cmap_cache = (hb_ot_font_cmap_cache_t *) hb_face_get_user_data (font->face, @@ -216,6 +200,7 @@ hb_ot_get_glyph_h_advances (hb_font_t* font, void* font_data, unsigned advance_stride, void *user_data HB_UNUSED) { + 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; @@ -307,20 +292,6 @@ hb_ot_get_glyph_h_advances (hb_font_t* font, void* font_data, first_advance = &StructAtOffsetUnaligned (first_advance, advance_stride); } } - -#ifndef HB_NO_AAT - if (ot_font->apply_trak) - { - hb_position_t tracking = font->face->table.trak->get_h_tracking (font); - first_advance = orig_first_advance; - for (unsigned int i = 0; i < count; i++) - { - *first_advance += tracking; - first_glyph = &StructAtOffsetUnaligned (first_glyph, glyph_stride); - first_advance = &StructAtOffsetUnaligned (first_advance, advance_stride); - } - } -#endif } #ifndef HB_NO_VERTICAL @@ -385,20 +356,6 @@ hb_ot_get_glyph_v_advances (hb_font_t* font, void* font_data, first_advance = &StructAtOffsetUnaligned (first_advance, advance_stride); } } - -#ifndef HB_NO_AAT - if (ot_font->apply_trak) - { - hb_position_t tracking = font->face->table.trak->get_v_tracking (font); - first_advance = orig_first_advance; - for (unsigned int i = 0; i < count; i++) - { - *first_advance += tracking; - first_glyph = &StructAtOffsetUnaligned (first_glyph, glyph_stride); - first_advance = &StructAtOffsetUnaligned (first_advance, advance_stride); - } - } -#endif } #endif diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index dfc6be0fb..47db5a0f3 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -210,6 +210,14 @@ hb_ot_shape_planner_t::compile (hb_ot_shape_plan_t &plan, https://github.com/harfbuzz/harfbuzz/issues/2967. */ if (plan.apply_morx) plan.adjust_mark_positioning_when_zeroing = false; + + /* According to Ned, trak is applied by default for "modern fonts", as detected by presence of STAT table. */ +#ifndef HB_NO_STYLE + plan.apply_trak = hb_aat_layout_has_tracking (face) && face->table.STAT->has_data (); +#else + plan.apply_trak = false; +#endif + #endif } @@ -274,6 +282,11 @@ hb_ot_shape_plan_t::position (hb_font_t *font, #endif else if (this->apply_fallback_kern) _hb_ot_shape_fallback_kern (this, font, buffer); + +#ifndef HB_NO_AAT_SHAPE + if (this->apply_trak) + hb_aat_layout_track (this, font, buffer); +#endif } diff --git a/src/hb-ot-shape.hh b/src/hb-ot-shape.hh index 493988185..af71fb45b 100644 --- a/src/hb-ot-shape.hh +++ b/src/hb-ot-shape.hh @@ -109,9 +109,11 @@ struct hb_ot_shape_plan_t #ifndef HB_NO_AAT_SHAPE bool apply_kerx : 1; bool apply_morx : 1; + bool apply_trak : 1; #else static constexpr bool apply_kerx = false; static constexpr bool apply_morx = false; + static constexpr bool apply_trak = false; #endif void collect_lookups (hb_tag_t table_tag, hb_set_t *lookups) const From 208ffb3f1f6ebeab9c0f603338bdf3b1f03e0089 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 27 Mar 2025 15:01:39 -0600 Subject: [PATCH 2/3] [coretext-font] Undo tracking in get_[hv]_advances() Fixes https://github.com/harfbuzz/harfbuzz/issues/5221 --- docs/harfbuzz-sections.txt | 1 + src/hb-aat-layout-trak-table.hh | 17 +++++++++++++++-- src/hb-aat-layout.cc | 24 ++++++++++++++++++++++++ src/hb-aat-layout.h | 4 ++++ src/hb-coretext-font.cc | 7 +++++-- 5 files changed, 49 insertions(+), 4 deletions(-) diff --git a/docs/harfbuzz-sections.txt b/docs/harfbuzz-sections.txt index 650c07a98..bd79c106b 100644 --- a/docs/harfbuzz-sections.txt +++ b/docs/harfbuzz-sections.txt @@ -10,6 +10,7 @@ hb_aat_layout_get_feature_types hb_aat_layout_has_positioning hb_aat_layout_has_substitution hb_aat_layout_has_tracking +hb_aat_layout_get_tracking
diff --git a/src/hb-aat-layout-trak-table.hh b/src/hb-aat-layout-trak-table.hh index 5a4372dd3..14a7d5794 100644 --- a/src/hb-aat-layout-trak-table.hh +++ b/src/hb-aat-layout-trak-table.hh @@ -31,6 +31,7 @@ #include "hb-aat-layout-common.hh" #include "hb-ot-layout.hh" #include "hb-open-type.hh" +#include "hb-ot-stat-table.hh" /* * trak -- Tracking @@ -115,7 +116,7 @@ struct TrackTableEntry protected: F16DOT16 track; /* Track value for this record. */ - NameID trackNameID; /* The 'name' table index for this track. + OT::NameID trackNameID; /* The 'name' table index for this track. * (a short word or phrase like "loose" * or "very tight") */ NNOffset16To> @@ -199,7 +200,19 @@ struct trak { float ptem = font->ptem > 0.f ? font->ptem : HB_CORETEXT_DEFAULT_FONT_SIZE; return font->em_scalef_y ((this+vertData).get_tracking (this, ptem, track)); - } + } + hb_position_t get_tracking (hb_font_t *font, hb_direction_t dir, float track = 0.f) const + { +#ifndef HB_NO_STYLE + if (!font->face->table.STAT->has_data ()) + return 0; + return HB_DIRECTION_IS_HORIZONTAL (dir) ? + get_h_tracking (font, track) : + get_v_tracking (font, track); +#else + return 0; +#endif + } bool apply (hb_aat_apply_context_t *c, float track = 0.f) const { diff --git a/src/hb-aat-layout.cc b/src/hb-aat-layout.cc index a48c4e0bc..f65f117b8 100644 --- a/src/hb-aat-layout.cc +++ b/src/hb-aat-layout.cc @@ -394,6 +394,30 @@ hb_aat_layout_has_tracking (hb_face_t *face) return face->table.trak->has_data (); } +/** + * hb_aat_layout_get_tracking: + * @font: #hb_font_t to work upon + * @direction: The direction of the text + * @tracking: The tracking value to apply + * + * Fetches the tracking value for the specified font and direction. + * The tracking value is a floating-point number that specifies the + * amount of tracking value to add between characters in the text. + * Most clients want to use 0. here. + * + * Return value: The tracking value for the specified font and direction + * XSince: REPLACEME + */ +hb_position_t +hb_aat_layout_get_tracking (hb_font_t *font, + hb_direction_t direction, + float tracking) +{ + const AAT::trak& trak = *font->face->table.trak; + + return trak.get_tracking (font, direction, tracking); +} + void hb_aat_layout_track (const hb_ot_shape_plan_t *plan, hb_font_t *font, diff --git a/src/hb-aat-layout.h b/src/hb-aat-layout.h index c682a2f6d..6fc66d7c8 100644 --- a/src/hb-aat-layout.h +++ b/src/hb-aat-layout.h @@ -789,6 +789,10 @@ hb_aat_layout_has_positioning (hb_face_t *face); HB_EXTERN hb_bool_t hb_aat_layout_has_tracking (hb_face_t *face); +HB_EXTERN hb_position_t +hb_aat_layout_get_tracking (hb_font_t *font, + hb_direction_t direction, + float tracking); HB_END_DECLS diff --git a/src/hb-coretext-font.cc b/src/hb-coretext-font.cc index 76c1fa02d..9a6bf0bf9 100644 --- a/src/hb-coretext-font.cc +++ b/src/hb-coretext-font.cc @@ -29,6 +29,7 @@ #ifdef HAVE_CORETEXT #include "hb-coretext.hh" +#include "hb-aat-layout-trak-table.hh" #include "hb-draw.hh" #include "hb-font.hh" @@ -209,6 +210,7 @@ hb_coretext_get_glyph_h_advances (hb_font_t* font, CGFloat ct_font_size = CTFontGetSize (ct_font); CGFloat x_mult = (CGFloat) font->x_scale / ct_font_size; + hb_position_t tracking = font->face->table.trak->get_tracking (font, HB_DIRECTION_LTR, 0.f); CGGlyph cg_glyph[MAX_GLYPHS]; CGSize advances[MAX_GLYPHS]; @@ -223,7 +225,7 @@ hb_coretext_get_glyph_h_advances (hb_font_t* font, CTFontGetAdvancesForGlyphs (ct_font, kCTFontOrientationHorizontal, cg_glyph, advances, c); for (unsigned j = 0; j < c; j++) { - *first_advance = round (advances[j].width * x_mult); + *first_advance = round (advances[j].width * x_mult) - tracking; first_advance = &StructAtOffset (first_advance, advance_stride); } } @@ -244,6 +246,7 @@ hb_coretext_get_glyph_v_advances (hb_font_t* font, CGFloat ct_font_size = CTFontGetSize (ct_font); CGFloat y_mult = (CGFloat) -font->y_scale / ct_font_size; + hb_position_t tracking = font->face->table.trak->get_tracking (font, HB_DIRECTION_TTB, 0.f); CGGlyph cg_glyph[MAX_GLYPHS]; CGSize advances[MAX_GLYPHS]; @@ -258,7 +261,7 @@ hb_coretext_get_glyph_v_advances (hb_font_t* font, CTFontGetAdvancesForGlyphs (ct_font, kCTFontOrientationVertical, cg_glyph, advances, c); for (unsigned j = 0; j < c; j++) { - *first_advance = round (advances[j].width * y_mult); + *first_advance = round (advances[j].width * y_mult) - tracking; first_advance = &StructAtOffset (first_advance, advance_stride); } } From b4fd777c23f6bc9762d1917dc25250f9b00843f2 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 27 Mar 2025 15:37:01 -0600 Subject: [PATCH 3/3] [aat] Remove hb_aat_layout_get_tracking() again https://github.com/harfbuzz/harfbuzz/pull/5223 --- docs/harfbuzz-sections.txt | 1 - src/hb-aat-layout.cc | 24 ------------------------ src/hb-aat-layout.h | 4 ---- 3 files changed, 29 deletions(-) diff --git a/docs/harfbuzz-sections.txt b/docs/harfbuzz-sections.txt index bd79c106b..650c07a98 100644 --- a/docs/harfbuzz-sections.txt +++ b/docs/harfbuzz-sections.txt @@ -10,7 +10,6 @@ hb_aat_layout_get_feature_types hb_aat_layout_has_positioning hb_aat_layout_has_substitution hb_aat_layout_has_tracking -hb_aat_layout_get_tracking
diff --git a/src/hb-aat-layout.cc b/src/hb-aat-layout.cc index f65f117b8..a48c4e0bc 100644 --- a/src/hb-aat-layout.cc +++ b/src/hb-aat-layout.cc @@ -394,30 +394,6 @@ hb_aat_layout_has_tracking (hb_face_t *face) return face->table.trak->has_data (); } -/** - * hb_aat_layout_get_tracking: - * @font: #hb_font_t to work upon - * @direction: The direction of the text - * @tracking: The tracking value to apply - * - * Fetches the tracking value for the specified font and direction. - * The tracking value is a floating-point number that specifies the - * amount of tracking value to add between characters in the text. - * Most clients want to use 0. here. - * - * Return value: The tracking value for the specified font and direction - * XSince: REPLACEME - */ -hb_position_t -hb_aat_layout_get_tracking (hb_font_t *font, - hb_direction_t direction, - float tracking) -{ - const AAT::trak& trak = *font->face->table.trak; - - return trak.get_tracking (font, direction, tracking); -} - void hb_aat_layout_track (const hb_ot_shape_plan_t *plan, hb_font_t *font, diff --git a/src/hb-aat-layout.h b/src/hb-aat-layout.h index 6fc66d7c8..c682a2f6d 100644 --- a/src/hb-aat-layout.h +++ b/src/hb-aat-layout.h @@ -789,10 +789,6 @@ hb_aat_layout_has_positioning (hb_face_t *face); HB_EXTERN hb_bool_t hb_aat_layout_has_tracking (hb_face_t *face); -HB_EXTERN hb_position_t -hb_aat_layout_get_tracking (hb_font_t *font, - hb_direction_t direction, - float tracking); HB_END_DECLS