Break grapheme clusters after viramas if they end a cluster

Previously, we stayed on the conservative side and disallowed any
grapheme breaks (and thus cursoring) where a virama was followed by a
letter, since we did not know if the virama would be forming a
cluster with the letter or not. This created problems with Indic
languages with infrequent conjuncts, such as Tamil.

Now we use the information in calculated advances to find if a
cluster is formed. If there is no cluster, we break the grapheme and
allow cursoring after the virama.

Test: Unit tests added to GraphemeBreakTests and MeasurementTests.
Test: Also manually tested Tamil sequences.
Bug: 35721792
Change-Id: Ib159edb94b3ad6f693f0d3dad016b332b2cef447
This commit is contained in:
Roozbeh Pournader 2017-03-13 21:52:52 -07:00
parent fd4124c533
commit b01028d1d7
7 changed files with 131 additions and 36 deletions

View File

@ -31,15 +31,15 @@ public:
};
// Determine whether the given offset is a grapheme break.
// This implementation generally follows Unicode TR29 extended
// grapheme break, but with some tweaks to more closely match
// existing implementations.
static bool isGraphemeBreak(const uint16_t* buf, size_t start, size_t count, size_t offset);
// This implementation generally follows Unicode's UTR #29 extended
// grapheme break, with various tweaks.
static bool isGraphemeBreak(const float* advances, const uint16_t* buf, size_t start,
size_t count, size_t offset);
// Matches Android's Java API. Note, return (size_t)-1 for AT to
// signal non-break because unsigned return type.
static size_t getTextRunCursor(const uint16_t* buf, size_t start, size_t count,
size_t offset, MoveOpt opt);
static size_t getTextRunCursor(const float* advances, const uint16_t* buf, size_t start,
size_t count, size_t offset, MoveOpt opt);
};
} // namespace minikin

View File

@ -55,8 +55,8 @@ bool isPureKiller(uint32_t c) {
|| c == 0xA953 || c == 0xABED || c == 0x11134 || c == 0x112EA || c == 0x1172B);
}
bool GraphemeBreak::isGraphemeBreak(const uint16_t* buf, size_t start, size_t count,
size_t offset) {
bool GraphemeBreak::isGraphemeBreak(const float* advances, const uint16_t* buf, size_t start,
size_t count, const size_t offset) {
// This implementation closely follows Unicode Standard Annex #29 on
// Unicode Text Segmentation (http://www.unicode.org/reports/tr29/),
// implementing a tailored version of extended grapheme clusters.
@ -73,8 +73,9 @@ bool GraphemeBreak::isGraphemeBreak(const uint16_t* buf, size_t start, size_t co
uint32_t c1 = 0;
uint32_t c2 = 0;
size_t offset_back = offset;
size_t offset_forward = offset;
U16_PREV(buf, start, offset_back, c1);
U16_NEXT(buf, offset, start + count, c2);
U16_NEXT(buf, offset_forward, start + count, c2);
int32_t p1 = tailoredGraphemeClusterBreak(c1);
int32_t p2 = tailoredGraphemeClusterBreak(c2);
// Rule GB3, CR x LF
@ -117,25 +118,23 @@ bool GraphemeBreak::isGraphemeBreak(const uint16_t* buf, size_t start, size_t co
}
}
// Note that the offset has moved forwared 2 code units by U16_NEXT.
// The number 4 comes from the number of code units in a whole flag.
return (offset - 2 - offset_back) % 4 == 0;
return (offset - offset_back) % 4 == 0;
}
// Rule GB9, x (Extend | ZWJ); Rule GB9a, x SpacingMark; Rule GB9b, Prepend x
if (p2 == U_GCB_EXTEND || p2 == U_GCB_ZWJ || p2 == U_GCB_SPACING_MARK || p1 == U_GCB_PREPEND) {
return false;
}
// Cluster indic syllables together (tailoring of UAX #29)
// Known limitation: this is overly conservative, and assumes that the virama may form a
// conjunct with the following letter, which doesn't always happen.
//
// There is no easy solution to do this correctly. Even querying the font does not help (with
// the current font technoloies), since the font may be creating the conjunct using multiple
// glyphs, while the user may be perceiving that sequence of glyphs as one conjunct or one
// letter.
// Cluster Indic syllables together (tailoring of UAX #29).
// Immediately after each virama (that is not just a pure killer) followed by a letter, we
// check to see if the next character has a non-zero width assigned to it in the advances
// array. A zero width means a cluster is formed with the virama (so there is no grapheme
// break), while a non-zero width means a new cluster is started (so there may be a grapheme
// break).
if (u_getIntPropertyValue(c1, UCHAR_CANONICAL_COMBINING_CLASS) == 9 // virama
&& !isPureKiller(c1)
&& u_getIntPropertyValue(c2, UCHAR_GENERAL_CATEGORY) == U_OTHER_LETTER) {
&& u_getIntPropertyValue(c2, UCHAR_GENERAL_CATEGORY) == U_OTHER_LETTER
&& (advances == nullptr || advances[offset - start] == 0)) {
return false;
}
// Tailoring: make emoji sequences with ZWJ a single grapheme cluster
@ -170,8 +169,8 @@ bool GraphemeBreak::isGraphemeBreak(const uint16_t* buf, size_t start, size_t co
return true;
}
size_t GraphemeBreak::getTextRunCursor(const uint16_t* buf, size_t start, size_t count,
size_t offset, MoveOpt opt) {
size_t GraphemeBreak::getTextRunCursor(const float* advances, const uint16_t* buf, size_t start,
size_t count, size_t offset, MoveOpt opt) {
switch (opt) {
case AFTER:
if (offset < start + count) {
@ -179,7 +178,7 @@ size_t GraphemeBreak::getTextRunCursor(const uint16_t* buf, size_t start, size_t
}
// fall through
case AT_OR_AFTER:
while (!isGraphemeBreak(buf, start, count, offset)) {
while (!isGraphemeBreak(advances, buf, start, count, offset)) {
offset++;
}
break;
@ -189,12 +188,12 @@ size_t GraphemeBreak::getTextRunCursor(const uint16_t* buf, size_t start, size_t
}
// fall through
case AT_OR_BEFORE:
while (!isGraphemeBreak(buf, start, count, offset)) {
while (!isGraphemeBreak(advances, buf, start, count, offset)) {
offset--;
}
break;
case AT:
if (!isGraphemeBreak(buf, start, count, offset)) {
if (!isGraphemeBreak(advances, buf, start, count, offset)) {
offset = (size_t)-1;
}
break;

View File

@ -54,7 +54,8 @@ static float getRunAdvance(const float* advances, const uint16_t* buf, size_t la
int numGraphemeClustersAfter = 0;
for (size_t i = lastCluster; i < nextCluster; i++) {
bool isAfter = i >= offset;
if (GraphemeBreak::isGraphemeBreak(buf, start, count, i)) {
if (GraphemeBreak::isGraphemeBreak(
advances + (start - layoutStart), buf, start, count, i)) {
numGraphemeClusters++;
if (isAfter) {
numGraphemeClustersAfter++;
@ -86,7 +87,7 @@ size_t getOffsetForAdvance(const float* advances, const uint16_t* buf, size_t st
float x = 0.0f, xLastClusterStart = 0.0f, xSearchStart = 0.0f;
size_t lastClusterStart = start, searchStart = start;
for (size_t i = start; i < start + count; i++) {
if (GraphemeBreak::isGraphemeBreak(buf, start, count, i)) {
if (GraphemeBreak::isGraphemeBreak(advances, buf, start, count, i)) {
searchStart = lastClusterStart;
xSearchStart = xLastClusterStart;
}
@ -103,7 +104,7 @@ size_t getOffsetForAdvance(const float* advances, const uint16_t* buf, size_t st
size_t best = searchStart;
float bestDist = FLT_MAX;
for (size_t i = searchStart; i <= start + count; i++) {
if (GraphemeBreak::isGraphemeBreak(buf, start, count, i)) {
if (GraphemeBreak::isGraphemeBreak(advances, buf, start, count, i)) {
// "getRunAdvance(layout, buf, start, count, i) - advance" but more efficient
float delta = getRunAdvance(advances, buf, start, searchStart, count - searchStart, i)

View File

@ -38,7 +38,7 @@ static void BM_GraphemeBreak_Ascii(benchmark::State& state) {
LOG_ALWAYS_FATAL_IF(result_size != 12);
const size_t testIndex = state.range(0);
while (state.KeepRunning()) {
GraphemeBreak::isGraphemeBreak(buffer, 0, result_size, testIndex);
GraphemeBreak::isGraphemeBreak(nullptr, buffer, 0, result_size, testIndex);
}
}
BENCHMARK(BM_GraphemeBreak_Ascii)
@ -53,7 +53,7 @@ static void BM_GraphemeBreak_Emoji(benchmark::State& state) {
LOG_ALWAYS_FATAL_IF(result_size != 12);
const size_t testIndex = state.range(0);
while (state.KeepRunning()) {
GraphemeBreak::isGraphemeBreak(buffer, 0, result_size, testIndex);
GraphemeBreak::isGraphemeBreak(nullptr, buffer, 0, result_size, testIndex);
}
}
BENCHMARK(BM_GraphemeBreak_Emoji)
@ -68,7 +68,7 @@ static void BM_GraphemeBreak_Emoji_Flags(benchmark::State& state) {
LOG_ALWAYS_FATAL_IF(result_size != 12);
const size_t testIndex = state.range(0);
while (state.KeepRunning()) {
GraphemeBreak::isGraphemeBreak(buffer, 0, result_size, testIndex);
GraphemeBreak::isGraphemeBreak(nullptr, buffer, 0, result_size, testIndex);
}
}
BENCHMARK(BM_GraphemeBreak_Emoji_Flags)

View File

@ -76,6 +76,7 @@ LOCAL_SRC_FILES += \
GraphemeBreakTests.cpp \
LayoutTest.cpp \
LayoutUtilsTest.cpp \
MeasurementTests.cpp \
SparseBitSetTest.cpp \
UnicodeUtilsTest.cpp \
WordBreakerTests.cpp

View File

@ -26,7 +26,16 @@ bool IsBreak(const char* src) {
size_t offset;
size_t size;
ParseUnicode(buf, BUF_SIZE, src, &size, &offset);
return GraphemeBreak::isGraphemeBreak(buf, 0, size, offset);
return GraphemeBreak::isGraphemeBreak(nullptr, buf, 0, size, offset);
}
bool IsBreakWithAdvances(const float* advances, const char* src) {
const size_t BUF_SIZE = 256;
uint16_t buf[BUF_SIZE];
size_t offset;
size_t size;
ParseUnicode(buf, BUF_SIZE, src, &size, &offset);
return GraphemeBreak::isGraphemeBreak(advances, buf, 0, size, offset);
}
TEST(GraphemeBreak, utf16) {
@ -164,6 +173,31 @@ TEST(GraphemeBreak, tailoring) {
EXPECT_FALSE(IsBreak("U+0E01 | U+0E3A U+0E01")); // thai phinthu = pure killer
EXPECT_TRUE(IsBreak("U+0E01 U+0E3A | U+0E01")); // thai phinthu = pure killer
// Repetition of above tests, but with a given advances array that implies everything
// became just one cluster.
const float conjoined[] = {1.0, 0.0, 0.0};
EXPECT_FALSE(IsBreakWithAdvances(conjoined,
"U+0915 | U+094D U+0915")); // Devanagari ka+virama+ka
EXPECT_FALSE(IsBreakWithAdvances(conjoined,
"U+0915 U+094D | U+0915")); // Devanagari ka+virama+ka
EXPECT_FALSE(IsBreakWithAdvances(conjoined,
"U+0E01 | U+0E3A U+0E01")); // thai phinthu = pure killer
EXPECT_TRUE(IsBreakWithAdvances(conjoined,
"U+0E01 U+0E3A | U+0E01")); // thai phinthu = pure killer
// Repetition of above tests, but with a given advances array that the virama did not
// form a cluster with the following consonant. The difference is that there is now
// a grapheme break after the virama in ka+virama+ka.
const float separate[] = {1.0, 0.0, 1.0};
EXPECT_FALSE(IsBreakWithAdvances(separate,
"U+0915 | U+094D U+0915")); // Devanagari ka+virama+ka
EXPECT_TRUE(IsBreakWithAdvances(separate,
"U+0915 U+094D | U+0915")); // Devanagari ka+virama+ka
EXPECT_FALSE(IsBreakWithAdvances(separate,
"U+0E01 | U+0E3A U+0E01")); // thai phinthu = pure killer
EXPECT_TRUE(IsBreakWithAdvances(separate,
"U+0E01 U+0E3A | U+0E01")); // thai phinthu = pure killer
// suppress grapheme breaks in zwj emoji sequences, see
// http://www.unicode.org/emoji/charts/emoji-zwj-sequences.html
EXPECT_FALSE(IsBreak("U+1F469 U+200D | U+2764 U+FE0F U+200D U+1F48B U+200D U+1F468"));
@ -222,10 +256,10 @@ TEST(GraphemeBreak, genderBalancedEmoji) {
TEST(GraphemeBreak, offsets) {
uint16_t string[] = { 0x0041, 0x06DD, 0x0045, 0x0301, 0x0049, 0x0301 };
EXPECT_TRUE(GraphemeBreak::isGraphemeBreak(string, 2, 3, 2));
EXPECT_FALSE(GraphemeBreak::isGraphemeBreak(string, 2, 3, 3));
EXPECT_TRUE(GraphemeBreak::isGraphemeBreak(string, 2, 3, 4));
EXPECT_TRUE(GraphemeBreak::isGraphemeBreak(string, 2, 3, 5));
EXPECT_TRUE(GraphemeBreak::isGraphemeBreak(nullptr, string, 2, 3, 2));
EXPECT_FALSE(GraphemeBreak::isGraphemeBreak(nullptr, string, 2, 3, 3));
EXPECT_TRUE(GraphemeBreak::isGraphemeBreak(nullptr, string, 2, 3, 4));
EXPECT_TRUE(GraphemeBreak::isGraphemeBreak(nullptr, string, 2, 3, 5));
}
} // namespace minikin

View File

@ -0,0 +1,60 @@
/*
* Copyright (C) 2017 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 <gtest/gtest.h>
#include <UnicodeUtils.h>
#include <minikin/Measurement.h>
namespace minikin {
float getAdvance(const float* advances, const char* src) {
const size_t BUF_SIZE = 256;
uint16_t buf[BUF_SIZE];
size_t offset;
size_t size;
ParseUnicode(buf, BUF_SIZE, src, &size, &offset);
return getRunAdvance(advances, buf, 0, size, offset);
}
// Latin fi
TEST(Measurement, getRunAdvance_fi) {
const float unligated[] = {30.0, 20.0};
EXPECT_EQ(0.0, getAdvance(unligated, "| 'f' 'i'"));
EXPECT_EQ(30.0, getAdvance(unligated, "'f' | 'i'"));
EXPECT_EQ(50.0, getAdvance(unligated, "'f' 'i' |"));
const float ligated[] = {40.0, 0.0};
EXPECT_EQ(0.0, getAdvance(ligated, "| 'f' 'i'"));
EXPECT_EQ(20.0, getAdvance(ligated, "'f' | 'i'"));
EXPECT_EQ(40.0, getAdvance(ligated, "'f' 'i' |"));
}
// Devanagari ka+virama+ka
TEST(Measurement, getRunAdvance_kka) {
const float unligated[] = {30.0, 0.0, 30.0};
EXPECT_EQ(0.0, getAdvance(unligated, "| U+0915 U+094D U+0915"));
EXPECT_EQ(30.0, getAdvance(unligated, "U+0915 | U+094D U+0915"));
EXPECT_EQ(30.0, getAdvance(unligated, "U+0915 U+094D | U+0915"));
EXPECT_EQ(60.0, getAdvance(unligated, "U+0915 U+094D U+0915 |"));
const float ligated[] = {30.0, 0.0, 0.0};
EXPECT_EQ(0.0, getAdvance(ligated, "| U+0915 U+094D U+0915"));
EXPECT_EQ(30.0, getAdvance(ligated, "U+0915 | U+094D U+0915"));
EXPECT_EQ(30.0, getAdvance(ligated, "U+0915 U+094D | U+0915"));
EXPECT_EQ(30.0, getAdvance(ligated, "U+0915 U+094D U+0915 |"));
}
} // namespace minikin