Skip to content

Commit

Permalink
Migrate font fallback on Mac to Skia implementation
Browse files Browse the repository at this point in the history
The gfx GetFontFallback(...) using Skia is using the same
API under the hood than font_fallback_mac.mm.
see: https://cs.chromium.org/chromium/src/third_party/skia/src/ports/SkFontHost_mac.cpp?l=3043

This CL is doing the migration to use GetSkiaFallbackTypeface(..) like
other platforms to have an more uniform results.

Skia Implementation also supported other corner cases like decomposition.
see: ui/gfx/font_fallback_skia_impl.cc

Bug: 1026735
Change-Id: If26ce32a0a826fea688dbbf25977ffb7590e31df
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2075222
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745591}
  • Loading branch information
bergeret authored and Commit Bot committed Feb 28, 2020
1 parent 46961b1 commit 6cb2232
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 34 deletions.
2 changes: 1 addition & 1 deletion ui/gfx/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ jumbo_component("gfx") {
]
}

if (is_android || is_fuchsia || is_win) {
if (is_android || is_fuchsia || is_win || is_mac) {
sources += [
"font_fallback_skia_impl.cc",
"font_fallback_skia_impl.h",
Expand Down
46 changes: 15 additions & 31 deletions ui/gfx/font_fallback_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -11,33 +11,12 @@
#import "base/mac/mac_util.h"
#include "base/mac/scoped_cftyperef.h"
#import "base/strings/sys_string_conversions.h"
#include "base/trace_event/trace_event.h"
#include "ui/gfx/font.h"
#include "ui/gfx/font_fallback_skia_impl.h"
#include "ui/gfx/platform_font.h"

namespace gfx {
namespace {

// CTFontCreateForString() sometimes re-wraps its result in a new CTFontRef with
// identical attributes. This wastes time shaping the text run and confounds
// Skia's internal typeface cache.
bool FontsEqual(CTFontRef lhs, CTFontRef rhs) {
if (lhs == rhs)
return true;

// Compare ATSFontRef typeface IDs. These are typedef uint32_t. Typically if
// RenderText decided to hunt for a fallback in the first place, this check
// fails and FontsEqual returns here.
if (CTFontGetPlatformFont(lhs, nil) != CTFontGetPlatformFont(rhs, nil))
return false;

// Comparing addresses of descriptors seems to be sufficient for other cases.
base::ScopedCFTypeRef<CTFontDescriptorRef> lhs_descriptor(
CTFontCopyFontDescriptor(lhs));
base::ScopedCFTypeRef<CTFontDescriptorRef> rhs_descriptor(
CTFontCopyFontDescriptor(rhs));
return lhs_descriptor.get() == rhs_descriptor.get();
}

} // namespace

std::vector<Font> GetFallbackFonts(const Font& font) {
DCHECK(font.GetNativeFont());
Expand Down Expand Up @@ -76,15 +55,20 @@ bool GetFallbackFont(const Font& font,
const std::string& locale,
base::StringPiece16 text,
Font* result) {
base::ScopedCFTypeRef<CFStringRef> cf_string(CFStringCreateWithCharacters(
kCFAllocatorDefault, text.data(), text.length()));
CTFontRef ct_font = base::mac::NSToCFCast(font.GetNativeFont());
base::ScopedCFTypeRef<CTFontRef> ct_result(
CTFontCreateForString(ct_font, cf_string, {0, text.length()}));
if (FontsEqual(ct_font, ct_result))
TRACE_EVENT0("fonts", "gfx::GetFallbackFont");

sk_sp<SkTypeface> fallback_typeface =
GetSkiaFallbackTypeface(font, locale, text);

if (!fallback_typeface)
return false;

*result = Font(base::mac::CFToNSCast(ct_result.get()));
// Fallback needs to keep the exact SkTypeface, as re-matching the font using
// family name and styling information loses access to the underlying platform
// font handles and is not guaranteed to result in the correct typeface, see
// https://crbug.com/1003829
*result = Font(PlatformFont::CreateFromSkTypeface(
std::move(fallback_typeface), font.GetFontSize(), base::nullopt));
return true;
}

Expand Down
2 changes: 0 additions & 2 deletions ui/gfx/font_fallback_mac_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ TEST(FontFallbackMacTest, GetFallbackFont) {
const base::string16 emoji = base::UTF8ToUTF16("😋");

gfx::Font fallback;
EXPECT_FALSE(
GetFallbackFont(arial, kDefaultApplicationLocale, ascii, &fallback));
EXPECT_TRUE(
GetFallbackFont(arial, kDefaultApplicationLocale, hebrew, &fallback));
EXPECT_EQ("Lucida Grande", fallback.GetFontName());
Expand Down
4 changes: 4 additions & 0 deletions ui/gfx/platform_font_mac.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ class PlatformFontMac : public PlatformFont {
PlatformFontMac(const std::string& font_name,
int font_size);

PlatformFontMac(sk_sp<SkTypeface> typeface,
int font_size_pixels,
const base::Optional<FontRenderParams>& params);

// Overridden from PlatformFont:
Font DeriveFont(int size_delta,
int style,
Expand Down
24 changes: 24 additions & 0 deletions ui/gfx/platform_font_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,12 @@ int GetFontStyleFromNSFont(NSFont* font) {
return font ? font : [NSFont systemFontOfSize:[NSFont systemFontSize]];
}

std::string GetFamilyNameFromTypeface(sk_sp<SkTypeface> typeface) {
SkString family;
typeface->getFamilyName(&family);
return family.c_str();
}

} // namespace

////////////////////////////////////////////////////////////////////////////////
Expand All @@ -205,6 +211,16 @@ int GetFontStyleFromNSFont(NSFont* font) {
Font::NORMAL,
Font::Weight::NORMAL) {}

PlatformFontMac::PlatformFontMac(sk_sp<SkTypeface> typeface,
int font_size_pixels,
const base::Optional<FontRenderParams>& params)
: PlatformFontMac(
base::mac::CFToNSCast(SkTypeface_GetCTFontRef(typeface.get())),
GetFamilyNameFromTypeface(typeface),
font_size_pixels,
(typeface->isItalic() ? Font::ITALIC : Font::NORMAL),
FontWeightFromInt(typeface->fontStyle().weight())) {}

////////////////////////////////////////////////////////////////////////////////
// PlatformFontMac, PlatformFont implementation:

Expand Down Expand Up @@ -388,4 +404,12 @@ int GetFontStyleFromNSFont(NSFont* font) {
return new PlatformFontMac(font_name, font_size);
}

// static
PlatformFont* PlatformFont::CreateFromSkTypeface(
sk_sp<SkTypeface> typeface,
int font_size_pixels,
const base::Optional<FontRenderParams>& params) {
return new PlatformFontMac(typeface, font_size_pixels, params);
}

} // namespace gfx

0 comments on commit 6cb2232

Please sign in to comment.