Commit 78464805 authored by Etienne Bergeron's avatar Etienne Bergeron Committed by Commit Bot

Fix font fallback for Emoji on Mac

This CL is fixing the fallback selection for Emoji.

The problem got introduced here:
  https://chromium-review.googlesource.com/c/chromium/src/+/2075222

The original fallback implementation rely on
CTFontCreateForString(...) over the whole text of a run.
This was causing problem since the API on some versions of Mac
(see crbug/1036652) is broken with ignorable codepoints
(e.g. ZWJ and ZWNJ on arabic script).

The skia implementation is calling CTFontCreateForString(...)
but over a single codepoint. An algorithm is trying to find the
best match by taking the font with the most codepoints. While
this is working well for script, this is not working well for
emoji.

Emoji can have joiners (ZWNJ) between codepoints. The codepoints
that are joined can be on different blocks. Emoji can have a
presentation codepoint (<emoji> u+FE0E or <emoji> u+FE0F).

The fallback font was broken because the algorithm tries to find a
font for codepoints {u+2699, u+fe0f}. The codepoint u+fe0f is
an ignorable codepoint and was not taken into account while
choosing the fonts. The "Menlo" font was returned. The codepoints
u+2699 favor the textual representation and was not taking the emoji
color font.

The Apple Emoji Color font support both codepoints. We could take
into account the ignorable u+fe0f, but this is causing problem with
this example: {u+2699, u+fe0e}. There is no font that has both
of them. The selection can choose any font with {u+2699} or with
{u+fe0e}. Selecting font with only {u+fe0e} is also wrong. Same
issue can happen with the ZWNJ/ZWJ that can be found in emoji
sequence.

The proposal is to have a specific font for handling emoji.

Bug: 1099591
Change-Id: Id02952b2fa2c7b4052611c06c3b2a535e20600ae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2429503
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810900}
parent 06fe2c0f
...@@ -7,17 +7,34 @@ ...@@ -7,17 +7,34 @@
#include <CoreText/CoreText.h> #include <CoreText/CoreText.h>
#import <Foundation/Foundation.h> #import <Foundation/Foundation.h>
#include "base/i18n/char_iterator.h"
#include "base/mac/foundation_util.h" #include "base/mac/foundation_util.h"
#import "base/mac/mac_util.h" #import "base/mac/mac_util.h"
#include "base/mac/scoped_cftyperef.h" #include "base/mac/scoped_cftyperef.h"
#import "base/strings/sys_string_conversions.h" #import "base/strings/sys_string_conversions.h"
#include "base/trace_event/trace_event.h" #include "base/trace_event/trace_event.h"
#include "third_party/icu/source/common/unicode/uchar.h"
#include "ui/gfx/font.h" #include "ui/gfx/font.h"
#include "ui/gfx/font_fallback_skia_impl.h" #include "ui/gfx/font_fallback_skia_impl.h"
#include "ui/gfx/platform_font.h" #include "ui/gfx/platform_font.h"
namespace gfx { namespace gfx {
namespace {
bool TextSequenceHasEmoji(base::StringPiece16 text) {
base::i18n::UTF16CharIterator iter(text.data(), text.length());
while (!iter.end()) {
const UChar32 codepoint = iter.get();
if (u_hasBinaryProperty(codepoint, UCHAR_EMOJI))
return true;
iter.Advance();
}
return false;
}
} // namespace
std::vector<Font> GetFallbackFonts(const Font& font) { std::vector<Font> GetFallbackFonts(const Font& font) {
DCHECK(font.GetNativeFont()); DCHECK(font.GetNativeFont());
// On Mac "There is a system default cascade list (which is polymorphic, based // On Mac "There is a system default cascade list (which is polymorphic, based
...@@ -57,6 +74,11 @@ bool GetFallbackFont(const Font& font, ...@@ -57,6 +74,11 @@ bool GetFallbackFont(const Font& font,
Font* result) { Font* result) {
TRACE_EVENT0("fonts", "gfx::GetFallbackFont"); TRACE_EVENT0("fonts", "gfx::GetFallbackFont");
if (TextSequenceHasEmoji(text)) {
*result = Font("Apple Color Emoji", font.GetFontSize());
return true;
}
sk_sp<SkTypeface> fallback_typeface = sk_sp<SkTypeface> fallback_typeface =
GetSkiaFallbackTypeface(font, locale, text); GetSkiaFallbackTypeface(font, locale, text);
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "ui/gfx/font_fallback.h" #include "ui/gfx/font_fallback.h"
#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "ui/gfx/font.h" #include "ui/gfx/font.h"
...@@ -33,7 +34,7 @@ TEST(FontFallbackMacTest, GetFallbackFont) { ...@@ -33,7 +34,7 @@ TEST(FontFallbackMacTest, GetFallbackFont) {
const base::string16 hebrew = base::WideToUTF16(L"\x5d0\x5d1\x5d2"); const base::string16 hebrew = base::WideToUTF16(L"\x5d0\x5d1\x5d2");
const base::string16 emoji = base::UTF8ToUTF16("😋"); const base::string16 emoji = base::UTF8ToUTF16("😋");
gfx::Font fallback; Font fallback;
EXPECT_TRUE( EXPECT_TRUE(
GetFallbackFont(arial, kDefaultApplicationLocale, hebrew, &fallback)); GetFallbackFont(arial, kDefaultApplicationLocale, hebrew, &fallback));
EXPECT_EQ("Lucida Grande", fallback.GetFontName()); EXPECT_EQ("Lucida Grande", fallback.GetFontName());
...@@ -42,4 +43,34 @@ TEST(FontFallbackMacTest, GetFallbackFont) { ...@@ -42,4 +43,34 @@ TEST(FontFallbackMacTest, GetFallbackFont) {
EXPECT_EQ("Apple Color Emoji", fallback.GetFontName()); EXPECT_EQ("Apple Color Emoji", fallback.GetFontName());
} }
TEST(FontFallbackMacTest, GetFallbackFontForEmoji) {
static struct {
const char* test_name;
const wchar_t* text;
} kEmojiTests[] = {
{"aries", L"\u2648"},
{"candle", L"\U0001F56F"},
{"anchor", L"\u2693"},
{"grinning_face", L"\U0001F600"},
{"flag_andorra", L"\U0001F1E6\U0001F1E9"},
{"woman_man_hands_light", L"\U0001F46B\U0001F3FB"},
{"hole_text", L"\U0001F573\uFE0E"},
{"hole_emoji", L"\U0001F573\uFE0F"},
{"man_judge_medium", L"\U0001F468\U0001F3FD\u200D\u2696\uFE0F"},
{"woman_turban", L"\U0001F473\u200D\u2640\uFE0F"},
{"rainbow_flag", L"\U0001F3F3\uFE0F\u200D\U0001F308"},
{"eye_bubble", L"\U0001F441\uFE0F\u200D\U0001F5E8\uFE0F"},
};
Font font;
for (const auto& test : kEmojiTests) {
SCOPED_TRACE(
base::StringPrintf("GetFallbackFontForEmoji [%s]", test.test_name));
Font fallback;
EXPECT_TRUE(GetFallbackFont(font, kDefaultApplicationLocale,
base::WideToUTF16(test.text), &fallback));
EXPECT_EQ("Apple Color Emoji", fallback.GetFontName());
}
}
} // namespace gfx } // namespace gfx
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment