From d01462f7a1affe4d863e97d9d1f0986cf50af626 Mon Sep 17 00:00:00 2001 From: Roozbeh Pournader Date: Thu, 30 Mar 2017 19:54:00 -0700 Subject: [PATCH] Override the bidi properties of new emojis Test: new Minikin tests are added, and pass Bug: 32952475 Change-Id: Ibcae60d18d0cd5efd7556aaf58a716b6b59c8ee0 --- engine/src/flutter/include/minikin/Emoji.h | 34 ++++++++ engine/src/flutter/libs/minikin/Android.mk | 1 + engine/src/flutter/libs/minikin/Emoji.cpp | 77 +++++++++++++++++++ .../flutter/libs/minikin/FontCollection.cpp | 1 + .../flutter/libs/minikin/GraphemeBreak.cpp | 1 + engine/src/flutter/libs/minikin/Layout.cpp | 8 ++ .../flutter/libs/minikin/MinikinInternal.cpp | 42 ---------- .../flutter/libs/minikin/MinikinInternal.h | 9 --- .../src/flutter/libs/minikin/WordBreaker.cpp | 3 +- engine/src/flutter/tests/unittest/Android.mk | 2 +- ...{MinikinInternalTest.cpp => EmojiTest.cpp} | 16 +++- 11 files changed, 137 insertions(+), 57 deletions(-) create mode 100644 engine/src/flutter/include/minikin/Emoji.h create mode 100644 engine/src/flutter/libs/minikin/Emoji.cpp rename engine/src/flutter/tests/unittest/{MinikinInternalTest.cpp => EmojiTest.cpp} (85%) diff --git a/engine/src/flutter/include/minikin/Emoji.h b/engine/src/flutter/include/minikin/Emoji.h new file mode 100644 index 0000000000..28261735aa --- /dev/null +++ b/engine/src/flutter/include/minikin/Emoji.h @@ -0,0 +1,34 @@ +/* + * Copyright (C) 2014 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +namespace minikin { + +// Returns true if c is emoji. +bool isEmoji(uint32_t c); + +// Returns true if c is emoji modifier base. +bool isEmojiBase(uint32_t c); + +// Returns true if c is emoji modifier. +bool isEmojiModifier(uint32_t c); + +// Bidi override for ICU that knows about new emoji. +UCharDirection emojiBidiOverride(const void* context, UChar32 c); + +} // namespace minikin + diff --git a/engine/src/flutter/libs/minikin/Android.mk b/engine/src/flutter/libs/minikin/Android.mk index 67a478a7c7..bb6234a123 100644 --- a/engine/src/flutter/libs/minikin/Android.mk +++ b/engine/src/flutter/libs/minikin/Android.mk @@ -19,6 +19,7 @@ include $(CLEAR_VARS) include $(CLEAR_VARS) minikin_src_files := \ CmapCoverage.cpp \ + Emoji.cpp \ FontCollection.cpp \ FontFamily.cpp \ FontLanguage.cpp \ diff --git a/engine/src/flutter/libs/minikin/Emoji.cpp b/engine/src/flutter/libs/minikin/Emoji.cpp new file mode 100644 index 0000000000..fbe68ca843 --- /dev/null +++ b/engine/src/flutter/libs/minikin/Emoji.cpp @@ -0,0 +1,77 @@ +/* + * Copyright (C) 2014 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +namespace minikin { + +bool isNewEmoji(uint32_t c) { + // Emoji characters new in Unicode emoji 5.0. + // From http://www.unicode.org/Public/emoji/5.0/emoji-data.txt + // TODO: Remove once emoji-data.text 5.0 is in ICU or update to 6.0. + if (c < 0x1F6F7 || c > 0x1F9E6) { + // Optimization for characters outside the new emoji range. + return false; + } + return (0x1F6F7 <= c && c <= 0x1F6F8) + || c == 0x1F91F + || (0x1F928 <= c && c <= 0x1F92F) + || (0x1F931 <= c && c <= 0x1F932) + || c == 0x1F94C + || (0x1F95F <= c && c <= 0x1F96B) + || (0x1F992 <= c && c <= 0x1F997) + || (0x1F9D0 <= c && c <= 0x1F9E6); +} + +bool isEmoji(uint32_t c) { + return isNewEmoji(c) || u_hasBinaryProperty(c, UCHAR_EMOJI); +} + +bool isEmojiModifier(uint32_t c) { + // Emoji modifier are not expected to change, so there's a small change we need to customize + // this. + return u_hasBinaryProperty(c, UCHAR_EMOJI_MODIFIER); +} + +bool isEmojiBase(uint32_t c) { + // These two characters were removed from Emoji_Modifier_Base in Emoji 4.0, but we need to keep + // them as emoji modifier bases since there are fonts and user-generated text out there that + // treats these as potential emoji bases. + if (c == 0x1F91D || c == 0x1F93C) { + return true; + } + // Emoji Modifier Base characters new in Unicode emoji 5.0. + // From http://www.unicode.org/Public/emoji/5.0/emoji-data.txt + // TODO: Remove once emoji-data.text 5.0 is in ICU or update to 6.0. + if (c == 0x1F91F + || (0x1F931 <= c && c <= 0x1F932) + || (0x1F9D1 <= c && c <= 0x1F9DD)) { + return true; + } + return u_hasBinaryProperty(c, UCHAR_EMOJI_MODIFIER_BASE); +} + +UCharDirection emojiBidiOverride(const void* /* context */, UChar32 c) { + if (isNewEmoji(c)) { + // All new emoji characters in Unicode 10.0 are of the bidi class ON. + return U_OTHER_NEUTRAL; + } else { + return u_charDirection(c); + } +} + +} // namespace minikin + diff --git a/engine/src/flutter/libs/minikin/FontCollection.cpp b/engine/src/flutter/libs/minikin/FontCollection.cpp index 962b95b4af..02ed9dc623 100644 --- a/engine/src/flutter/libs/minikin/FontCollection.cpp +++ b/engine/src/flutter/libs/minikin/FontCollection.cpp @@ -27,6 +27,7 @@ #include "FontLanguage.h" #include "FontLanguageListCache.h" #include "MinikinInternal.h" +#include #include using std::vector; diff --git a/engine/src/flutter/libs/minikin/GraphemeBreak.cpp b/engine/src/flutter/libs/minikin/GraphemeBreak.cpp index b1188e8d88..87de4213f9 100644 --- a/engine/src/flutter/libs/minikin/GraphemeBreak.cpp +++ b/engine/src/flutter/libs/minikin/GraphemeBreak.cpp @@ -20,6 +20,7 @@ #include #include +#include #include "MinikinInternal.h" namespace minikin { diff --git a/engine/src/flutter/libs/minikin/Layout.cpp b/engine/src/flutter/libs/minikin/Layout.cpp index 1f2fbc4457..568e03876b 100644 --- a/engine/src/flutter/libs/minikin/Layout.cpp +++ b/engine/src/flutter/libs/minikin/Layout.cpp @@ -39,6 +39,7 @@ #include "HbFontCache.h" #include "LayoutUtils.h" #include "MinikinInternal.h" +#include #include using std::string; @@ -522,6 +523,13 @@ BidiText::BidiText(const uint16_t* buf, size_t start, size_t count, size_t bufSi return; } UErrorCode status = U_ZERO_ERROR; + // Set callbacks to override bidi classes of new emoji + ubidi_setClassCallback(mBidi, emojiBidiOverride, nullptr, nullptr, nullptr, &status); + if (!U_SUCCESS(status)) { + ALOGE("error setting bidi callback function, status = %d", status); + return; + } + UBiDiLevel bidiReq = bidiFlags; if (bidiFlags == kBidi_Default_LTR) { bidiReq = UBIDI_DEFAULT_LTR; diff --git a/engine/src/flutter/libs/minikin/MinikinInternal.cpp b/engine/src/flutter/libs/minikin/MinikinInternal.cpp index 212ee26c0f..88acc5061b 100644 --- a/engine/src/flutter/libs/minikin/MinikinInternal.cpp +++ b/engine/src/flutter/libs/minikin/MinikinInternal.cpp @@ -20,7 +20,6 @@ #include "MinikinInternal.h" #include "HbFontCache.h" -#include #include namespace minikin { @@ -33,47 +32,6 @@ void assertMinikinLocked() { #endif } -bool isEmoji(uint32_t c) { - // Emoji characters new in Unicode emoji 5.0. - // From http://www.unicode.org/Public/emoji/5.0/emoji-data.txt - // TODO: Remove once emoji-data.text 5.0 is in ICU or update to 6.0. - if ((0x1F6F7 <= c && c <= 0x1F6F8) - || c == 0x1F91F - || (0x1F928 <= c && c <= 0x1F92F) - || (0x1F931 <= c && c <= 0x1F932) - || c == 0x1F94C - || (0x1F95F <= c && c <= 0x1F96B) - || (0x1F992 <= c && c <= 0x1F997) - || (0x1F9D0 <= c && c <= 0x1F9E6)) { - return true; - } - return u_hasBinaryProperty(c, UCHAR_EMOJI); -} - -bool isEmojiModifier(uint32_t c) { - // Emoji modifier are not expected to change, so there's a small change we need to customize - // this. - return u_hasBinaryProperty(c, UCHAR_EMOJI_MODIFIER); -} - -bool isEmojiBase(uint32_t c) { - // These two characters were removed from Emoji_Modifier_Base in Emoji 4.0, but we need to keep - // them as emoji modifier bases since there are fonts and user-generated text out there that - // treats these as potential emoji bases. - if (c == 0x1F91D || c == 0x1F93C) { - return true; - } - // Emoji Modifier Base characters new in Unicode emoji 5.0. - // From http://www.unicode.org/Public/emoji/5.0/emoji-data.txt - // TODO: Remove once emoji-data.text 5.0 is in ICU or update to 6.0. - if (c == 0x1F91F - || (0x1F931 <= c && c <= 0x1F932) - || (0x1F9D1 <= c && c <= 0x1F9DD)) { - return true; - } - return u_hasBinaryProperty(c, UCHAR_EMOJI_MODIFIER_BASE); -} - hb_blob_t* getFontTable(const MinikinFont* minikinFont, uint32_t tag) { assertMinikinLocked(); hb_font_t* font = getHbFontLocked(minikinFont); diff --git a/engine/src/flutter/libs/minikin/MinikinInternal.h b/engine/src/flutter/libs/minikin/MinikinInternal.h index 365f20cc0d..250f63d74b 100644 --- a/engine/src/flutter/libs/minikin/MinikinInternal.h +++ b/engine/src/flutter/libs/minikin/MinikinInternal.h @@ -36,15 +36,6 @@ extern android::Mutex gMinikinLock; // Aborts if gMinikinLock is not acquired. Do nothing on the release build. void assertMinikinLocked(); -// Returns true if c is emoji. -bool isEmoji(uint32_t c); - -// Returns true if c is emoji modifier base. -bool isEmojiBase(uint32_t c); - -// Returns true if c is emoji modifier. -bool isEmojiModifier(uint32_t c); - hb_blob_t* getFontTable(const MinikinFont* minikinFont, uint32_t tag); // An RAII wrapper for hb_blob_t diff --git a/engine/src/flutter/libs/minikin/WordBreaker.cpp b/engine/src/flutter/libs/minikin/WordBreaker.cpp index 3b9e956fe1..16edca7d6e 100644 --- a/engine/src/flutter/libs/minikin/WordBreaker.cpp +++ b/engine/src/flutter/libs/minikin/WordBreaker.cpp @@ -18,8 +18,9 @@ #include -#include +#include #include +#include #include "MinikinInternal.h" #include diff --git a/engine/src/flutter/tests/unittest/Android.mk b/engine/src/flutter/tests/unittest/Android.mk index eca0107397..d30c1062ce 100644 --- a/engine/src/flutter/tests/unittest/Android.mk +++ b/engine/src/flutter/tests/unittest/Android.mk @@ -66,13 +66,13 @@ LOCAL_SRC_FILES += \ ../util/FontTestUtils.cpp \ ../util/MinikinFontForTest.cpp \ ../util/UnicodeUtils.cpp \ + EmojiTest.cpp \ FontCollectionTest.cpp \ FontCollectionItemizeTest.cpp \ FontFamilyTest.cpp \ FontLanguageListCacheTest.cpp \ HbFontCacheTest.cpp \ HyphenatorTest.cpp \ - MinikinInternalTest.cpp \ GraphemeBreakTests.cpp \ LayoutTest.cpp \ LayoutUtilsTest.cpp \ diff --git a/engine/src/flutter/tests/unittest/MinikinInternalTest.cpp b/engine/src/flutter/tests/unittest/EmojiTest.cpp similarity index 85% rename from engine/src/flutter/tests/unittest/MinikinInternalTest.cpp rename to engine/src/flutter/tests/unittest/EmojiTest.cpp index 1d3ecd76b8..e7d0f56f49 100644 --- a/engine/src/flutter/tests/unittest/MinikinInternalTest.cpp +++ b/engine/src/flutter/tests/unittest/EmojiTest.cpp @@ -18,11 +18,11 @@ #include -#include "MinikinInternal.h" +#include namespace minikin { -TEST(MinikinInternalTest, isEmojiTest) { +TEST(EmojiTest, isEmojiTest) { EXPECT_TRUE(isEmoji(0x0023)); // NUMBER SIGN EXPECT_TRUE(isEmoji(0x0035)); // DIGIT FIVE EXPECT_TRUE(isEmoji(0x2640)); // FEMALE SIGN @@ -40,7 +40,7 @@ TEST(MinikinInternalTest, isEmojiTest) { EXPECT_FALSE(isEmoji(0x29E3D)); // A han character. } -TEST(MinikinInternalTest, isEmojiModifierTest) { +TEST(EmojiTest, isEmojiModifierTest) { EXPECT_TRUE(isEmojiModifier(0x1F3FB)); // EMOJI MODIFIER FITZPATRICK TYPE-1-2 EXPECT_TRUE(isEmojiModifier(0x1F3FC)); // EMOJI MODIFIER FITZPATRICK TYPE-3 EXPECT_TRUE(isEmojiModifier(0x1F3FD)); // EMOJI MODIFIER FITZPATRICK TYPE-4 @@ -53,7 +53,7 @@ TEST(MinikinInternalTest, isEmojiModifierTest) { EXPECT_FALSE(isEmojiModifier(0x29E3D)); // A han character. } -TEST(MinikinInternalTest, isEmojiBaseTest) { +TEST(EmojiTest, isEmojiBaseTest) { EXPECT_TRUE(isEmojiBase(0x261D)); // WHITE UP POINTING INDEX EXPECT_TRUE(isEmojiBase(0x270D)); // WRITING HAND EXPECT_TRUE(isEmojiBase(0x1F385)); // FATHER CHRISTMAS @@ -77,4 +77,12 @@ TEST(MinikinInternalTest, isEmojiBaseTest) { EXPECT_FALSE(isEmojiBase(0x29E3D)); // A han character. } +TEST(EmojiTest, emojiBidiOverrideTest) { + EXPECT_EQ(U_RIGHT_TO_LEFT, emojiBidiOverride(nullptr, 0x05D0)); // HEBREW LETTER ALEF + EXPECT_EQ(U_LEFT_TO_RIGHT, + emojiBidiOverride(nullptr, 0x1F170)); // NEGATIVE SQUARED LATIN CAPITAL LETTER A + EXPECT_EQ(U_OTHER_NEUTRAL, emojiBidiOverride(nullptr, 0x1F6F7)); // SLED + EXPECT_EQ(U_OTHER_NEUTRAL, emojiBidiOverride(nullptr, 0x1F9E6)); // SOCKS +} + } // namespace minikin