From 6c4c78baac53cf4ddb7e5db7ac89138d9efae359 Mon Sep 17 00:00:00 2001 From: reuk Date: Mon, 4 Mar 2024 20:45:41 +0000 Subject: [PATCH] Typeface: Fix advances for Apple Color Emoji The glyph spacing for this font does not scale linearly with the font size; at smaller font sizes, the spacing is relatively larger. Typeface::getStringWidth, Typeface::getGlyphPositions, and the equivalent Font member functions now take the font size and horizontal scale into account when computing advances on Apple platforms. --- modules/juce_graphics/fonts/juce_Font.cpp | 155 +---------------- modules/juce_graphics/fonts/juce_Typeface.cpp | 158 ++++++++++++++++-- modules/juce_graphics/fonts/juce_Typeface.h | 24 +-- .../juce_graphics/native/juce_Fonts_mac.mm | 12 -- 4 files changed, 159 insertions(+), 190 deletions(-) diff --git a/modules/juce_graphics/fonts/juce_Font.cpp b/modules/juce_graphics/fonts/juce_Font.cpp index 5668e3448f..f63682a7b8 100644 --- a/modules/juce_graphics/fonts/juce_Font.cpp +++ b/modules/juce_graphics/fonts/juce_Font.cpp @@ -52,130 +52,12 @@ namespace FontValues float minimumHorizontalScale = 0.7f; } -class HbScale -{ - static constexpr float factor = 1 << 16; - -public: - HbScale() = delete; - - static constexpr hb_position_t juceToHb (float pos) - { - return (hb_position_t) (pos * factor); - } - - static constexpr float hbToJuce (hb_position_t pos) - { - return (float) pos / (float) factor; - } -}; - using GetTypefaceForFont = Typeface::Ptr (*)(const Font&); GetTypefaceForFont juce_getTypefaceForFont = nullptr; float Font::getDefaultMinimumHorizontalScaleFactor() noexcept { return FontValues::minimumHorizontalScale; } void Font::setDefaultMinimumHorizontalScaleFactor (float newValue) noexcept { FontValues::minimumHorizontalScale = newValue; } -//============================================================================== -#if JUCE_MAC || JUCE_IOS -template -void getAdvancesForGlyphs (hb_font_t* hbFont, CTFontRef ctFont, Span glyphs, Span advances) -{ - jassert (glyphs.size() == advances.size()); - - int x, y; - hb_font_get_scale (hbFont, &x, &y); - const auto scaleAdjustment = HbScale::hbToJuce (orientation == kCTFontOrientationHorizontal ? x : y) / CTFontGetSize (ctFont); - - CTFontGetAdvancesForGlyphs (ctFont, orientation, std::data (glyphs), std::data (advances), (CFIndex) std::size (glyphs)); - - for (auto& advance : advances) - (orientation == kCTFontOrientationHorizontal ? advance.width : advance.height) *= scaleAdjustment; -} - -template -static auto getAdvanceFn() -{ - return [] (hb_font_t* f, void*, hb_codepoint_t glyph, void* voidFontRef) -> hb_position_t - { - auto* fontRef = static_cast (voidFontRef); - - const CGGlyph glyphs[] { (CGGlyph) glyph }; - CGSize advances[std::size (glyphs)]{}; - getAdvancesForGlyphs (f, fontRef, glyphs, advances); - - return HbScale::juceToHb ((float) (orientation == kCTFontOrientationHorizontal ? advances->width : advances->height)); - }; -} - -template -static auto getAdvancesFn() -{ - return [] (hb_font_t* f, - void*, - unsigned int count, - const hb_codepoint_t* firstGlyph, - unsigned int glyphStride, - hb_position_t* firstAdvance, - unsigned int advanceStride, - void* voidFontRef) - { - auto* fontRef = static_cast (voidFontRef); - - std::vector glyphs (count); - - for (auto [index, glyph] : enumerate (glyphs)) - glyph = (CGGlyph) *addBytesToPointer (firstGlyph, glyphStride * index); - - std::vector advances (count); - - getAdvancesForGlyphs (f, fontRef, glyphs, advances); - - for (auto [index, advance] : enumerate (advances)) - *addBytesToPointer (firstAdvance, advanceStride * index) = HbScale::juceToHb ((float) (orientation == kCTFontOrientationHorizontal ? advance.width : advance.height)); - }; -} - -/* This function overrides the callbacks that fetch glyph advances for fonts on macOS. - The built-in OpenType glyph metric callbacks that HarfBuzz uses by default for fonts such as - "Apple Color Emoji" don't always return correct advances, resulting in emoji that may overlap - with subsequent characters. This may be to do with ignoring the 'trak' table, but I'm not an - expert, so I'm not sure! - - In any case, using CTFontGetAdvancesForGlyphs produces much nicer advances for emoji on Apple - platforms, as long as the CTFont is set to the size that will eventually be rendered. - - This might need a bit of testing to make sure that it correctly handles advances for - custom (non-Apple?) fonts. - - @param hb a hb_font_t to update with Apple-specific advances - @param fontRef the CTFontRef (normally with a custom point size) that will be queried when computing advances -*/ -static void overrideCTFontAdvances (hb_font_t* hb, CTFontRef fontRef) -{ - using HbFontFuncs = std::unique_ptr>; - const HbFontFuncs funcs { hb_font_funcs_create() }; - - // We pass the CTFontRef as user data to each of these functions. - // We don't pass a custom destructor for the user data, as that will be handled by the custom - // destructor for the hb_font_funcs_t. - hb_font_funcs_set_glyph_h_advance_func (funcs.get(), getAdvanceFn (), (void*) fontRef, nullptr); - hb_font_funcs_set_glyph_v_advance_func (funcs.get(), getAdvanceFn (), (void*) fontRef, nullptr); - hb_font_funcs_set_glyph_h_advances_func (funcs.get(), getAdvancesFn(), (void*) fontRef, nullptr); - hb_font_funcs_set_glyph_v_advances_func (funcs.get(), getAdvancesFn(), (void*) fontRef, nullptr); - - // We want to keep a copy of the font around so that all of our custom callbacks can query it, - // so retain it here and release it once the custom functions are no longer in use. - jassert (fontRef != nullptr); - CFRetain (fontRef); - - hb_font_set_funcs (hb, funcs.get(), (void*) fontRef, [] (void* ptr) - { - CFRelease ((CTFontRef) ptr); - }); -} -#endif - //============================================================================== class TypefaceCache final : private DeletedAtShutdown { @@ -410,24 +292,8 @@ public: HbFont getFontPtr (const Font& f) { - const ScopedLock lock (mutex); - if (auto ptr = getTypefacePtr (f)) - { - if (HbFont subFont { hb_font_create_sub_font (ptr->getNativeDetails().getFont()) }) - { - const auto points = legacyHeightToPoints (ptr, height); - - hb_font_set_ptem (subFont.get(), points); - hb_font_set_scale (subFont.get(), HbScale::juceToHb (points * horizontalScale), HbScale::juceToHb (points)); - - #if JUCE_MAC || JUCE_IOS - overrideCTFontAdvances (subFont.get(), hb_coretext_font_get_ct_font (subFont.get())); - #endif - - return subFont; - } - } + return ptr->getNativeDetails().getFontAtSizeAndScale (f.getHeight(), f.getHorizontalScale()); return {}; } @@ -913,17 +779,13 @@ int Font::getStringWidth (const String& text) const float Font::getStringWidthFloat (const String& text) const { - auto w = getTypefacePtr()->getStringWidth (text); - - if (! approximatelyEqual (font->getKerning(), 0.0f)) - w += font->getKerning() * (float) text.length(); - - return w * font->getHeight() * font->getHorizontalScale(); + const auto w = getTypefacePtr()->getStringWidth (text, getHeight(), getHorizontalScale()); + return w + (font->getHeight() * font->getHorizontalScale() * font->getKerning() * (float) text.length()); } void Font::getGlyphPositions (const String& text, Array& glyphs, Array& xOffsets) const { - getTypefacePtr()->getGlyphPositions (text, glyphs, xOffsets); + getTypefacePtr()->getGlyphPositions (text, glyphs, xOffsets, getHeight(), getHorizontalScale()); if (auto num = xOffsets.size()) { @@ -931,15 +793,8 @@ void Font::getGlyphPositions (const String& text, Array& glyphs, ArraygetKerning(), 0.0f)) - { for (int i = 0; i < num; ++i) - x[i] = (x[i] + (float) i * font->getKerning()) * scale; - } - else - { - for (int i = 0; i < num; ++i) - x[i] *= scale; - } + x[i] += ((float) i * font->getKerning() * scale); } } diff --git a/modules/juce_graphics/fonts/juce_Typeface.cpp b/modules/juce_graphics/fonts/juce_Typeface.cpp index 9aacdae335..174c2df20f 100644 --- a/modules/juce_graphics/fonts/juce_Typeface.cpp +++ b/modules/juce_graphics/fonts/juce_Typeface.cpp @@ -35,6 +35,124 @@ namespace juce { +class HbScale +{ + static constexpr float factor = 1 << 16; + +public: + HbScale() = delete; + + static constexpr hb_position_t juceToHb (float pos) + { + return (hb_position_t) (pos * factor); + } + + static constexpr float hbToJuce (hb_position_t pos) + { + return (float) pos / (float) factor; + } +}; + +//============================================================================== +#if JUCE_MAC || JUCE_IOS +template +void getAdvancesForGlyphs (hb_font_t* hbFont, CTFontRef ctFont, Span glyphs, Span advances) +{ + jassert (glyphs.size() == advances.size()); + + int x, y; + hb_font_get_scale (hbFont, &x, &y); + const auto scaleAdjustment = HbScale::hbToJuce (orientation == kCTFontOrientationHorizontal ? x : y) / CTFontGetSize (ctFont); + + CTFontGetAdvancesForGlyphs (ctFont, orientation, std::data (glyphs), std::data (advances), (CFIndex) std::size (glyphs)); + + for (auto& advance : advances) + (orientation == kCTFontOrientationHorizontal ? advance.width : advance.height) *= scaleAdjustment; +} + +template +static auto getAdvanceFn() +{ + return [] (hb_font_t* f, void*, hb_codepoint_t glyph, void* voidFontRef) -> hb_position_t + { + auto* fontRef = static_cast (voidFontRef); + + const CGGlyph glyphs[] { (CGGlyph) glyph }; + CGSize advances[std::size (glyphs)]{}; + getAdvancesForGlyphs (f, fontRef, glyphs, advances); + + return HbScale::juceToHb ((float) (orientation == kCTFontOrientationHorizontal ? advances->width : advances->height)); + }; +} + +template +static auto getAdvancesFn() +{ + return [] (hb_font_t* f, + void*, + unsigned int count, + const hb_codepoint_t* firstGlyph, + unsigned int glyphStride, + hb_position_t* firstAdvance, + unsigned int advanceStride, + void* voidFontRef) + { + auto* fontRef = static_cast (voidFontRef); + + std::vector glyphs (count); + + for (auto [index, glyph] : enumerate (glyphs)) + glyph = (CGGlyph) *addBytesToPointer (firstGlyph, glyphStride * index); + + std::vector advances (count); + + getAdvancesForGlyphs (f, fontRef, glyphs, advances); + + for (auto [index, advance] : enumerate (advances)) + *addBytesToPointer (firstAdvance, advanceStride * index) = HbScale::juceToHb ((float) (orientation == kCTFontOrientationHorizontal ? advance.width : advance.height)); + }; +} + +/* This function overrides the callbacks that fetch glyph advances for fonts on macOS. + The built-in OpenType glyph metric callbacks that HarfBuzz uses by default for fonts such as + "Apple Color Emoji" don't always return correct advances, resulting in emoji that may overlap + with subsequent characters. This may be to do with ignoring the 'trak' table, but I'm not an + expert, so I'm not sure! + + In any case, using CTFontGetAdvancesForGlyphs produces much nicer advances for emoji on Apple + platforms, as long as the CTFont is set to the size that will eventually be rendered. + + This might need a bit of testing to make sure that it correctly handles advances for + custom (non-Apple?) fonts. + + @param hb a hb_font_t to update with Apple-specific advances + @param fontRef the CTFontRef (normally with a custom point size) that will be queried when computing advances +*/ +static void overrideCTFontAdvances (hb_font_t* hb, CTFontRef fontRef) +{ + using HbFontFuncs = std::unique_ptr>; + const HbFontFuncs funcs { hb_font_funcs_create() }; + + // We pass the CTFontRef as user data to each of these functions. + // We don't pass a custom destructor for the user data, as that will be handled by the custom + // destructor for the hb_font_funcs_t. + hb_font_funcs_set_glyph_h_advance_func (funcs.get(), getAdvanceFn (), (void*) fontRef, nullptr); + hb_font_funcs_set_glyph_v_advance_func (funcs.get(), getAdvanceFn (), (void*) fontRef, nullptr); + hb_font_funcs_set_glyph_h_advances_func (funcs.get(), getAdvancesFn(), (void*) fontRef, nullptr); + hb_font_funcs_set_glyph_v_advances_func (funcs.get(), getAdvancesFn(), (void*) fontRef, nullptr); + + // We want to keep a copy of the font around so that all of our custom callbacks can query it, + // so retain it here and release it once the custom functions are no longer in use. + jassert (fontRef != nullptr); + CFRetain (fontRef); + + hb_font_set_funcs (hb, funcs.get(), (void*) fontRef, [] (void* ptr) + { + CFRelease ((CTFontRef) ptr); + }); +} +#endif + struct TypefaceLegacyMetrics { float ascent{}; // in em units @@ -64,6 +182,21 @@ public: auto getLegacyMetrics() const { return legacyMetrics; } + HbFont getFontAtSizeAndScale (float height, float horizontalScale) const + { + HbFont subFont { hb_font_create_sub_font (font) }; + const auto points = height * getLegacyMetrics().getHeightToPointsFactor(); + + hb_font_set_ptem (subFont.get(), points); + hb_font_set_scale (subFont.get(), HbScale::juceToHb (points * horizontalScale), HbScale::juceToHb (points)); + + #if JUCE_MAC || JUCE_IOS + overrideCTFontAdvances (subFont.get(), hb_coretext_font_get_ct_font (subFont.get())); + #endif + + return subFont; + } + private: static TypefaceLegacyMetrics findLegacyMetrics (hb_font_t* f) { @@ -705,7 +838,11 @@ static constexpr auto hbTag (const char (&arr)[5]) } template -static void doSimpleShape (const Typeface& typeface, const String& text, Consumer&& consumer) +static void doSimpleShape (const Typeface& typeface, + const String& text, + float height, + float horizontalScale, + Consumer&& consumer) { HbBuffer buffer { hb_buffer_create() }; hb_buffer_add_utf8 (buffer.get(), text.toRawUTF8(), -1, 0, -1); @@ -713,7 +850,8 @@ static void doSimpleShape (const Typeface& typeface, const String& text, Consume hb_buffer_guess_segment_properties (buffer.get()); const auto& native = typeface.getNativeDetails(); - auto* font = native.getFont(); + const auto sized = native.getFontAtSizeAndScale (height, horizontalScale); + auto* font = sized.get(); // Disable ligatures, because TextEditor requires a 1:1 codepoint/glyph mapping for caret // positioning to work as expected. @@ -734,36 +872,32 @@ static void doSimpleShape (const Typeface& typeface, const String& text, Consume auto* infos = hb_buffer_get_glyph_infos (buffer.get(), &numGlyphs); auto* positions = hb_buffer_get_glyph_positions (buffer.get(), &numGlyphs); - const auto heightToPoints = native.getLegacyMetrics().getHeightToPointsFactor(); - const auto upem = hb_face_get_upem (hb_font_get_face (font)); - Point cursor{}; for (auto i = decltype (numGlyphs){}; i < numGlyphs; ++i) { const auto& info = infos[i]; const auto& position = positions[i]; - consumer (std::make_optional (info.codepoint), - heightToPoints * ((float) cursor.x + (float) position.x_offset) / (float) upem); + consumer (std::make_optional (info.codepoint), HbScale::hbToJuce (cursor.x + position.x_offset)); cursor += Point { position.x_advance, position.y_advance }; } - consumer (std::optional{}, heightToPoints * (float) cursor.x / (float) upem); + consumer (std::optional{}, HbScale::hbToJuce (cursor.x)); } -float Typeface::getStringWidth (const String& text) +float Typeface::getStringWidth (const String& text, float height, float horizontalScale) { float x{}; - doSimpleShape (*this, text, [&] (auto, auto xOffset) + doSimpleShape (*this, text, height, horizontalScale, [&] (auto, auto xOffset) { x = xOffset; }); return x; } -void Typeface::getGlyphPositions (const String& text, Array& glyphs, Array& xOffsets) +void Typeface::getGlyphPositions (const String& text, Array& glyphs, Array& xOffsets, float height, float horizontalScale) { - doSimpleShape (*this, text, [&] (auto codepoint, auto xOffset) + doSimpleShape (*this, text, height, horizontalScale, [&] (auto codepoint, auto xOffset) { if (codepoint.has_value()) glyphs.add ((int) *codepoint); diff --git a/modules/juce_graphics/fonts/juce_Typeface.h b/modules/juce_graphics/fonts/juce_Typeface.h index c8a2f575d7..6e1a7f8f48 100644 --- a/modules/juce_graphics/fonts/juce_Typeface.h +++ b/modules/juce_graphics/fonts/juce_Typeface.h @@ -142,12 +142,7 @@ public: /** @deprecated This function has several shortcomings: - - The returned value is based on a font with a normalised JUCE height of 1.0, - which will normally differ from the value that would be expected for a font - with a height of 1 pt. - - This function is unsuitable for measuring text with spacing that doesn't - scale linearly with point size, which might be the case for fonts that - implement optical sizing. + - The height parameter is specified in JUCE units rather than in points. - The result is computed assuming that ligatures and other font features will not be used when rendering the string. There's also no way of specifying a language used for the string, which may affect the widths of CJK text. @@ -157,19 +152,13 @@ public: glyphs. Measures the width of a line of text. - The distance returned is based on the font having an normalised height of 1.0. You should never need to call this! */ - float getStringWidth (const String& text); + float getStringWidth (const String& text, float normalisedHeight = 1.0f, float horizontalScale = 1.0f); /** @deprecated This function has several shortcomings: - - The returned values are based on a font with a normalised JUCE height of 1.0, - which will normally differ from the value that would be expected for a font - with a height of 1 pt. - - This function is unsuitable for measuring text with spacing that doesn't - scale linearly with point size, which might be the case for fonts that - implement optical sizing. + - The height parameter is specified in JUCE units rather than in points. - Ligatures are deliberately ignored, which will lead to ugly results if the positions are used to paint text using latin scripts, and potentially illegible results for other scripts. There's also no way of specifying a @@ -180,10 +169,13 @@ public: glyphs. Converts a line of text into its glyph numbers and their positions. - The distances returned are based on the font having an normalised height of 1.0. You should never need to call this! */ - void getGlyphPositions (const String& text, Array& glyphs, Array& xOffsets); + void getGlyphPositions (const String& text, + Array& glyphs, + Array& xOffsets, + float normalisedHeight = 1.0f, + float horizontalScale = 1.0f); /** Returns the outline for a glyph. The path returned will be normalised to a font height of 1.0. diff --git a/modules/juce_graphics/native/juce_Fonts_mac.mm b/modules/juce_graphics/native/juce_Fonts_mac.mm index edf08a14d0..26cae3b501 100644 --- a/modules/juce_graphics/native/juce_Fonts_mac.mm +++ b/modules/juce_graphics/native/juce_Fonts_mac.mm @@ -668,18 +668,6 @@ public: } private: - CFUniquePtr getAttributedStringAtts() const - { - const short zero = 0; - CFUniquePtr numberRef (CFNumberCreate (nullptr, kCFNumberShortType, &zero)); - - CFStringRef keys[] = { kCTFontAttributeName, kCTLigatureAttributeName }; - CFTypeRef values[] = { ctFont.get(), numberRef.get() }; - return CFUniquePtr (CFDictionaryCreate (nullptr, (const void**) &keys, - (const void**) &values, numElementsInArray (keys), - &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks)); - } - CoreTextTypeface (CFUniquePtr nativeFont, HbFont fontIn, const String& name,