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

@@ -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 <unicode/uchar.h>
+
+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
+

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 \

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 <minikin/Emoji.h>
+
+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
+

engine/src/flutter/libs/minikin/FontCollection.cpp

@@ -27,6 +27,7 @@
 #include "FontLanguage.h"
 #include "FontLanguageListCache.h"
 #include "MinikinInternal.h"
+#include <minikin/Emoji.h>
 #include <minikin/FontCollection.h>
 
 using std::vector;

engine/src/flutter/libs/minikin/GraphemeBreak.cpp

@@ -20,6 +20,7 @@
 #include <unicode/utf16.h>
 
 #include <minikin/GraphemeBreak.h>
+#include <minikin/Emoji.h>
 #include "MinikinInternal.h"
 
 namespace minikin {

engine/src/flutter/libs/minikin/Layout.cpp

@@ -39,6 +39,7 @@
 #include "HbFontCache.h"
 #include "LayoutUtils.h"
 #include "MinikinInternal.h"
+#include <minikin/Emoji.h>
 #include <minikin/Layout.h>
 
 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;

engine/src/flutter/libs/minikin/MinikinInternal.cpp

@@ -20,7 +20,6 @@
 #include "MinikinInternal.h"
 #include "HbFontCache.h"
 
-#include <unicode/uchar.h>
 #include <log/log.h>
 
 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);

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

engine/src/flutter/libs/minikin/WordBreaker.cpp

@@ -18,8 +18,9 @@
 
 #include <android/log.h>
 
-#include <minikin/WordBreaker.h>
+#include <minikin/Emoji.h>
 #include <minikin/Hyphenator.h>
+#include <minikin/WordBreaker.h>
 #include "MinikinInternal.h"
 
 #include <unicode/uchar.h>

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 \

engine/src/flutter/tests/unittest/EmojiTest.cpp

@@ -18,11 +18,11 @@
 
 #include <unicode/uchar.h>
 
-#include "MinikinInternal.h"
+#include <minikin/Emoji.h>
 
 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