From 6b9c9dec1fbd2f0440ea2a8de06a9827afb2e47e Mon Sep 17 00:00:00 2001 From: Seigo Nonaka Date: Tue, 15 Nov 2016 19:02:52 +0900 Subject: [PATCH] Implement word spacing Add a wordSpacing paint parameter, which will be used for justification. Bug: 31707212 Test: ran minikin_tests Change-Id: I91224ab8ef882ac0c87425c28ab731fead283612 --- engine/src/flutter/include/minikin/Layout.h | 2 +- .../src/flutter/include/minikin/MinikinFont.h | 5 +- engine/src/flutter/libs/minikin/Layout.cpp | 29 +- .../src/flutter/libs/minikin/LayoutUtils.cpp | 15 +- engine/src/flutter/libs/minikin/LayoutUtils.h | 5 + engine/src/flutter/tests/unittest/Android.mk | 2 + .../src/flutter/tests/unittest/LayoutTest.cpp | 343 ++++++++++++++++++ .../flutter/tests/util/MinikinFontForTest.cpp | 12 +- 8 files changed, 395 insertions(+), 18 deletions(-) create mode 100644 engine/src/flutter/tests/unittest/LayoutTest.cpp diff --git a/engine/src/flutter/include/minikin/Layout.h b/engine/src/flutter/include/minikin/Layout.h index 87d5e0534b..625e05c433 100644 --- a/engine/src/flutter/include/minikin/Layout.h +++ b/engine/src/flutter/include/minikin/Layout.h @@ -145,7 +145,7 @@ private: bool isRtl, LayoutContext* ctx); // Append another layout (for example, cached value) into this one - void appendLayout(Layout* src, size_t start); + void appendLayout(Layout* src, size_t start, float extraAdvance); std::vector mGlyphs; std::vector mAdvances; diff --git a/engine/src/flutter/include/minikin/MinikinFont.h b/engine/src/flutter/include/minikin/MinikinFont.h index ac9235be8d..18848535d2 100644 --- a/engine/src/flutter/include/minikin/MinikinFont.h +++ b/engine/src/flutter/include/minikin/MinikinFont.h @@ -45,8 +45,8 @@ class MinikinFont; // Possibly move into own .h file? // Note: if you add a field here, either add it to LayoutCacheKey or to skipCache() struct MinikinPaint { - MinikinPaint() : font(0), size(0), scaleX(0), skewX(0), letterSpacing(0), paintFlags(0), - fakery(), fontFeatureSettings() { } + MinikinPaint() : font(0), size(0), scaleX(0), skewX(0), letterSpacing(0), wordSpacing(0), + paintFlags(0), fakery(), hyphenEdit(), fontFeatureSettings() { } bool skipCache() const { return !fontFeatureSettings.empty(); @@ -57,6 +57,7 @@ struct MinikinPaint { float scaleX; float skewX; float letterSpacing; + float wordSpacing; uint32_t paintFlags; FontFakery fakery; HyphenEdit hyphenEdit; diff --git a/engine/src/flutter/libs/minikin/Layout.cpp b/engine/src/flutter/libs/minikin/Layout.cpp index 452718217f..a365411d8f 100644 --- a/engine/src/flutter/libs/minikin/Layout.cpp +++ b/engine/src/flutter/libs/minikin/Layout.cpp @@ -653,27 +653,38 @@ float Layout::doLayoutWord(const uint16_t* buf, size_t start, size_t count, size Layout* layout, float* advances) { LayoutCache& cache = LayoutEngine::getInstance().layoutCache; LayoutCacheKey key(collection, ctx->paint, ctx->style, buf, start, count, bufSize, isRtl); - bool skipCache = ctx->paint.skipCache(); - if (skipCache) { + + float wordSpacing = count == 1 && isWordSpace(buf[start]) ? ctx->paint.wordSpacing : 0; + + float advance; + if (ctx->paint.skipCache()) { Layout layoutForWord; key.doLayout(&layoutForWord, ctx, collection); if (layout) { - layout->appendLayout(&layoutForWord, bufStart); + layout->appendLayout(&layoutForWord, bufStart, wordSpacing); } if (advances) { layoutForWord.getAdvances(advances); } - return layoutForWord.getAdvance(); + advance = layoutForWord.getAdvance(); } else { Layout* layoutForWord = cache.get(key, ctx, collection); if (layout) { - layout->appendLayout(layoutForWord, bufStart); + layout->appendLayout(layoutForWord, bufStart, wordSpacing); } if (advances) { layoutForWord->getAdvances(advances); } - return layoutForWord->getAdvance(); + advance = layoutForWord->getAdvance(); } + + if (wordSpacing != 0) { + advance += wordSpacing; + if (advances) { + advances[0] += wordSpacing; + } + } + return advance; } static void addFeatures(const string &str, vector* features) { @@ -855,7 +866,7 @@ void Layout::doLayoutRun(const uint16_t* buf, size_t start, size_t count, size_t mAdvance = x; } -void Layout::appendLayout(Layout* src, size_t start) { +void Layout::appendLayout(Layout* src, size_t start, float extraAdvance) { int fontMapStack[16]; int* fontMap; if (src->mFaces.size() < sizeof(fontMapStack) / sizeof(fontMapStack[0])) { @@ -879,11 +890,13 @@ void Layout::appendLayout(Layout* src, size_t start) { } for (size_t i = 0; i < src->mAdvances.size(); i++) { mAdvances[i + start] = src->mAdvances[i]; + if (i == 0) + mAdvances[i + start] += extraAdvance; } MinikinRect srcBounds(src->mBounds); srcBounds.offset(x0, 0); mBounds.join(srcBounds); - mAdvance += src->mAdvance; + mAdvance += src->mAdvance + extraAdvance; if (fontMap != fontMapStack) { delete[] fontMap; diff --git a/engine/src/flutter/libs/minikin/LayoutUtils.cpp b/engine/src/flutter/libs/minikin/LayoutUtils.cpp index 4e59afd6b2..a3238d448d 100644 --- a/engine/src/flutter/libs/minikin/LayoutUtils.cpp +++ b/engine/src/flutter/libs/minikin/LayoutUtils.cpp @@ -20,13 +20,22 @@ namespace minikin { +const uint16_t CHAR_NBSP = 0x00A0; + +/* + * Determine whether the code unit is a word space for the purposes of justification. + */ +bool isWordSpace(uint16_t code_unit) { + return code_unit == ' ' || code_unit == CHAR_NBSP; +} + /** * For the purpose of layout, a word break is a boundary with no * kerning or complex script processing. This is necessarily a * heuristic, but should be accurate most of the time. */ -static bool isWordBreakAfter(int c) { - if (c == ' ' || (c >= 0x2000 && c <= 0x200a) || c == 0x3000) { +static bool isWordBreakAfter(uint16_t c) { + if (isWordSpace(c) || (c >= 0x2000 && c <= 0x200a) || c == 0x3000) { // spaces return true; } @@ -34,7 +43,7 @@ static bool isWordBreakAfter(int c) { return false; } -static bool isWordBreakBefore(int c) { +static bool isWordBreakBefore(uint16_t c) { // CJK ideographs (and yijing hexagram symbols) return isWordBreakAfter(c) || (c >= 0x3400 && c <= 0x9fff); } diff --git a/engine/src/flutter/libs/minikin/LayoutUtils.h b/engine/src/flutter/libs/minikin/LayoutUtils.h index f35e843cfa..b89004cf19 100644 --- a/engine/src/flutter/libs/minikin/LayoutUtils.h +++ b/engine/src/flutter/libs/minikin/LayoutUtils.h @@ -21,6 +21,11 @@ namespace minikin { +/* + * Determine whether the code unit is a word space for the purposes of justification. + */ +bool isWordSpace(uint16_t code_unit); + /** * Return offset of previous word break. It is either < offset or == 0. * diff --git a/engine/src/flutter/tests/unittest/Android.mk b/engine/src/flutter/tests/unittest/Android.mk index a81d17cca7..61d845b01b 100644 --- a/engine/src/flutter/tests/unittest/Android.mk +++ b/engine/src/flutter/tests/unittest/Android.mk @@ -81,6 +81,7 @@ LOCAL_SRC_FILES += \ HbFontCacheTest.cpp \ MinikinInternalTest.cpp \ GraphemeBreakTests.cpp \ + LayoutTest.cpp \ LayoutUtilsTest.cpp \ UnicodeUtilsTest.cpp \ WordBreakerTests.cpp @@ -88,6 +89,7 @@ LOCAL_SRC_FILES += \ LOCAL_C_INCLUDES := \ $(LOCAL_PATH)/../../libs/minikin/ \ $(LOCAL_PATH)/../util \ + external/freetype/include \ external/harfbuzz_ng/src \ external/libxml2/include \ external/skia/src/core diff --git a/engine/src/flutter/tests/unittest/LayoutTest.cpp b/engine/src/flutter/tests/unittest/LayoutTest.cpp new file mode 100644 index 0000000000..c023625b16 --- /dev/null +++ b/engine/src/flutter/tests/unittest/LayoutTest.cpp @@ -0,0 +1,343 @@ +/* + * Copyright (C) 2016 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 + +#include "ICUTestBase.h" +#include "minikin/FontCollection.h" +#include "minikin/Layout.h" +#include "../util/FontTestUtils.h" +#include "../util/UnicodeUtils.h" + +const char* SYSTEM_FONT_PATH = "/system/fonts/"; +const char* SYSTEM_FONT_XML = "/system/etc/fonts.xml"; + +namespace minikin { + +const float UNTOUCHED_MARKER = 1e+38; + +static void expectAdvances(std::vector expected, float* advances, size_t length) { + EXPECT_LE(expected.size(), length); + for (size_t i = 0; i < expected.size(); ++i) { + EXPECT_EQ(expected[i], advances[i]) + << i << "th element is different. Expected: " << expected[i] + << ", Actual: " << advances[i]; + } + EXPECT_EQ(UNTOUCHED_MARKER, advances[expected.size()]); +} + +static void resetAdvances(float* advances, size_t length) { + for (size_t i = 0; i < length; ++i) { + advances[i] = UNTOUCHED_MARKER; + } +} + +class LayoutTest : public ICUTestBase { +protected: + LayoutTest() : mCollection(nullptr) { + } + + virtual ~LayoutTest() {} + + virtual void SetUp() override { + mCollection = getFontCollection(SYSTEM_FONT_PATH, SYSTEM_FONT_XML); + } + + virtual void TearDown() override { + mCollection->Unref(); + } + + FontCollection* mCollection; +}; + +TEST_F(LayoutTest, doLayoutTest) { + MinikinPaint paint; + MinikinRect rect; + const size_t kMaxAdvanceLength = 32; + float advances[kMaxAdvanceLength]; + std::vector expectedValues; + + Layout layout; + layout.setFontCollection(mCollection); + std::vector text; + + // The mock implementation returns 10.0f advance and 0,0-10x10 bounds for all glyph. + { + SCOPED_TRACE("one word"); + text = utf8ToUtf16("oneword"); + layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint); + EXPECT_EQ(70.0f, layout.getAdvance()); + layout.getBounds(&rect); + EXPECT_EQ(0.0f, rect.mLeft); + EXPECT_EQ(0.0f, rect.mTop); + EXPECT_EQ(70.0f, rect.mRight); + EXPECT_EQ(10.0f, rect.mBottom); + resetAdvances(advances, kMaxAdvanceLength); + layout.getAdvances(advances); + expectedValues.resize(text.size()); + for (size_t i = 0; i < expectedValues.size(); ++i) { + expectedValues[i] = 10.0f; + } + expectAdvances(expectedValues, advances, kMaxAdvanceLength); + } + { + SCOPED_TRACE("two words"); + text = utf8ToUtf16("two words"); + layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint); + EXPECT_EQ(90.0f, layout.getAdvance()); + layout.getBounds(&rect); + EXPECT_EQ(0.0f, rect.mLeft); + EXPECT_EQ(0.0f, rect.mTop); + EXPECT_EQ(90.0f, rect.mRight); + EXPECT_EQ(10.0f, rect.mBottom); + resetAdvances(advances, kMaxAdvanceLength); + layout.getAdvances(advances); + expectedValues.resize(text.size()); + for (size_t i = 0; i < expectedValues.size(); ++i) { + expectedValues[i] = 10.0f; + } + expectAdvances(expectedValues, advances, kMaxAdvanceLength); + } + { + SCOPED_TRACE("three words"); + text = utf8ToUtf16("three words test"); + layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint); + EXPECT_EQ(160.0f, layout.getAdvance()); + layout.getBounds(&rect); + EXPECT_EQ(0.0f, rect.mLeft); + EXPECT_EQ(0.0f, rect.mTop); + EXPECT_EQ(160.0f, rect.mRight); + EXPECT_EQ(10.0f, rect.mBottom); + resetAdvances(advances, kMaxAdvanceLength); + layout.getAdvances(advances); + expectedValues.resize(text.size()); + for (size_t i = 0; i < expectedValues.size(); ++i) { + expectedValues[i] = 10.0f; + } + expectAdvances(expectedValues, advances, kMaxAdvanceLength); + } + { + SCOPED_TRACE("two spaces"); + text = utf8ToUtf16("two spaces"); + layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint); + EXPECT_EQ(110.0f, layout.getAdvance()); + layout.getBounds(&rect); + EXPECT_EQ(0.0f, rect.mLeft); + EXPECT_EQ(0.0f, rect.mTop); + EXPECT_EQ(110.0f, rect.mRight); + EXPECT_EQ(10.0f, rect.mBottom); + resetAdvances(advances, kMaxAdvanceLength); + layout.getAdvances(advances); + expectedValues.resize(text.size()); + for (size_t i = 0; i < expectedValues.size(); ++i) { + expectedValues[i] = 10.0f; + } + expectAdvances(expectedValues, advances, kMaxAdvanceLength); + } +} + +TEST_F(LayoutTest, doLayoutTest_wordSpacing) { + MinikinPaint paint; + MinikinRect rect; + const size_t kMaxAdvanceLength = 32; + float advances[kMaxAdvanceLength]; + std::vector expectedValues; + std::vector text; + + Layout layout; + layout.setFontCollection(mCollection); + + paint.wordSpacing = 5.0f; + + // The mock implementation returns 10.0f advance and 0,0-10x10 bounds for all glyph. + { + SCOPED_TRACE("one word"); + text = utf8ToUtf16("oneword"); + layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint); + EXPECT_EQ(70.0f, layout.getAdvance()); + layout.getBounds(&rect); + EXPECT_EQ(0.0f, rect.mLeft); + EXPECT_EQ(0.0f, rect.mTop); + EXPECT_EQ(70.0f, rect.mRight); + EXPECT_EQ(10.0f, rect.mBottom); + resetAdvances(advances, kMaxAdvanceLength); + layout.getAdvances(advances); + expectedValues.resize(text.size()); + for (size_t i = 0; i < expectedValues.size(); ++i) { + expectedValues[i] = 10.0f; + } + expectAdvances(expectedValues, advances, kMaxAdvanceLength); + } + { + SCOPED_TRACE("two words"); + text = utf8ToUtf16("two words"); + layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint); + EXPECT_EQ(95.0f, layout.getAdvance()); + layout.getBounds(&rect); + EXPECT_EQ(0.0f, rect.mLeft); + EXPECT_EQ(0.0f, rect.mTop); + EXPECT_EQ(95.0f, rect.mRight); + EXPECT_EQ(10.0f, rect.mBottom); + resetAdvances(advances, kMaxAdvanceLength); + layout.getAdvances(advances); + EXPECT_EQ(UNTOUCHED_MARKER, advances[text.size()]); + resetAdvances(advances, kMaxAdvanceLength); + layout.getAdvances(advances); + expectedValues.resize(text.size()); + for (size_t i = 0; i < expectedValues.size(); ++i) { + expectedValues[i] = 10.0f; + } + expectedValues[3] = 15.0f; + expectAdvances(expectedValues, advances, kMaxAdvanceLength); + } + { + SCOPED_TRACE("three words test"); + text = utf8ToUtf16("three words test"); + layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint); + EXPECT_EQ(170.0f, layout.getAdvance()); + layout.getBounds(&rect); + EXPECT_EQ(0.0f, rect.mLeft); + EXPECT_EQ(0.0f, rect.mTop); + EXPECT_EQ(170.0f, rect.mRight); + EXPECT_EQ(10.0f, rect.mBottom); + resetAdvances(advances, kMaxAdvanceLength); + layout.getAdvances(advances); + expectedValues.resize(text.size()); + for (size_t i = 0; i < expectedValues.size(); ++i) { + expectedValues[i] = 10.0f; + } + expectedValues[5] = 15.0f; + expectedValues[11] = 15.0f; + expectAdvances(expectedValues, advances, kMaxAdvanceLength); + } + { + SCOPED_TRACE("two spaces"); + text = utf8ToUtf16("two spaces"); + layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint); + EXPECT_EQ(120.0f, layout.getAdvance()); + layout.getBounds(&rect); + EXPECT_EQ(0.0f, rect.mLeft); + EXPECT_EQ(0.0f, rect.mTop); + EXPECT_EQ(120.0f, rect.mRight); + EXPECT_EQ(10.0f, rect.mBottom); + resetAdvances(advances, kMaxAdvanceLength); + layout.getAdvances(advances); + expectedValues.resize(text.size()); + for (size_t i = 0; i < expectedValues.size(); ++i) { + expectedValues[i] = 10.0f; + } + expectedValues[3] = 15.0f; + expectedValues[4] = 15.0f; + expectAdvances(expectedValues, advances, kMaxAdvanceLength); + } +} + +TEST_F(LayoutTest, doLayoutTest_negativeWordSpacing) { + MinikinPaint paint; + MinikinRect rect; + const size_t kMaxAdvanceLength = 32; + float advances[kMaxAdvanceLength]; + std::vector expectedValues; + + Layout layout; + layout.setFontCollection(mCollection); + std::vector text; + + // Negative word spacing also should work. + paint.wordSpacing = -5.0f; + + { + SCOPED_TRACE("one word"); + text = utf8ToUtf16("oneword"); + layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint); + EXPECT_EQ(70.0f, layout.getAdvance()); + layout.getBounds(&rect); + EXPECT_EQ(0.0f, rect.mLeft); + EXPECT_EQ(0.0f, rect.mTop); + EXPECT_EQ(70.0f, rect.mRight); + EXPECT_EQ(10.0f, rect.mBottom); + resetAdvances(advances, kMaxAdvanceLength); + layout.getAdvances(advances); + expectedValues.resize(text.size()); + for (size_t i = 0; i < expectedValues.size(); ++i) { + expectedValues[i] = 10.0f; + } + expectAdvances(expectedValues, advances, kMaxAdvanceLength); + } + { + SCOPED_TRACE("two words"); + text = utf8ToUtf16("two words"); + layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint); + EXPECT_EQ(85.0f, layout.getAdvance()); + layout.getBounds(&rect); + EXPECT_EQ(0.0f, rect.mLeft); + EXPECT_EQ(0.0f, rect.mTop); + EXPECT_EQ(85.0f, rect.mRight); + EXPECT_EQ(10.0f, rect.mBottom); + resetAdvances(advances, kMaxAdvanceLength); + layout.getAdvances(advances); + expectedValues.resize(text.size()); + for (size_t i = 0; i < expectedValues.size(); ++i) { + expectedValues[i] = 10.0f; + } + expectedValues[3] = 5.0f; + expectAdvances(expectedValues, advances, kMaxAdvanceLength); + } + { + SCOPED_TRACE("three words"); + text = utf8ToUtf16("three word test"); + layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint); + EXPECT_EQ(140.0f, layout.getAdvance()); + layout.getBounds(&rect); + EXPECT_EQ(0.0f, rect.mLeft); + EXPECT_EQ(0.0f, rect.mTop); + EXPECT_EQ(140.0f, rect.mRight); + EXPECT_EQ(10.0f, rect.mBottom); + resetAdvances(advances, kMaxAdvanceLength); + layout.getAdvances(advances); + expectedValues.resize(text.size()); + for (size_t i = 0; i < expectedValues.size(); ++i) { + expectedValues[i] = 10.0f; + } + expectedValues[5] = 5.0f; + expectedValues[10] = 5.0f; + expectAdvances(expectedValues, advances, kMaxAdvanceLength); + } + { + SCOPED_TRACE("two spaces"); + text = utf8ToUtf16("two spaces"); + layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint); + EXPECT_EQ(100.0f, layout.getAdvance()); + layout.getBounds(&rect); + EXPECT_EQ(0.0f, rect.mLeft); + EXPECT_EQ(0.0f, rect.mTop); + EXPECT_EQ(100.0f, rect.mRight); + EXPECT_EQ(10.0f, rect.mBottom); + resetAdvances(advances, kMaxAdvanceLength); + layout.getAdvances(advances); + expectedValues.resize(text.size()); + for (size_t i = 0; i < expectedValues.size(); ++i) { + expectedValues[i] = 10.0f; + } + expectedValues[3] = 5.0f; + expectedValues[4] = 5.0f; + expectAdvances(expectedValues, advances, kMaxAdvanceLength); + } +} + +// TODO: Add more test cases, e.g. measure text, letter spacing. + +} // namespace minikin diff --git a/engine/src/flutter/tests/util/MinikinFontForTest.cpp b/engine/src/flutter/tests/util/MinikinFontForTest.cpp index 807b234296..fd5f564506 100644 --- a/engine/src/flutter/tests/util/MinikinFontForTest.cpp +++ b/engine/src/flutter/tests/util/MinikinFontForTest.cpp @@ -47,13 +47,17 @@ MinikinFontForTest::MinikinFontForTest(const std::string& font_path, sk_spmLeft = 0.0f; + bounds->mTop = 0.0f; + bounds->mRight = 10.0f; + bounds->mBottom = 10.0f; } const void* MinikinFontForTest::GetTable(uint32_t tag, size_t* size,