Merge "Refactoring: pull out script splitter with unit test" into main
diff --git a/include/minikin/Debug.h b/include/minikin/Debug.h
index 4308c90..20d8d8d 100644
--- a/include/minikin/Debug.h
+++ b/include/minikin/Debug.h
@@ -25,6 +25,7 @@
struct MinikinRect;
struct MinikinExtent;
struct MinikinPaint;
+struct FontFeature;
class Range;
class U16StringPiece;
class LayoutPiece;
@@ -40,6 +41,8 @@
std::string toString(const MinikinExtent& extent);
std::string toString(const LayoutPiece& layout);
std::string toString(const MinikinPaint& paint);
+std::string toString(const FontFeature& feature);
+std::string toString(const std::vector<FontFeature>& features);
} // namespace debug
diff --git a/include/minikin/FontCollection.h b/include/minikin/FontCollection.h
index 7ad644a..ee0315e 100644
--- a/include/minikin/FontCollection.h
+++ b/include/minikin/FontCollection.h
@@ -34,6 +34,8 @@
// The maximum number of font families.
constexpr uint32_t MAX_FAMILY_COUNT = 254;
+class LocaleList;
+
class FontCollection {
public:
static std::shared_ptr<FontCollection> create(
@@ -225,6 +227,9 @@
bool isPrimaryFamily(const std::shared_ptr<FontFamily>& fontFamily) const;
+ void filterFamilyByLocale(const LocaleList& localeList,
+ const std::function<void(const FontFamily& family)>& callback) const;
+
static uint32_t calcLocaleMatchingScore(uint32_t userLocaleListId,
const FontFamily& fontFamily);
diff --git a/include/minikin/FontFeature.h b/include/minikin/FontFeature.h
new file mode 100644
index 0000000..c47594b
--- /dev/null
+++ b/include/minikin/FontFeature.h
@@ -0,0 +1,77 @@
+/*
+ * Copyright (C) 2021 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.
+ */
+
+#ifndef MINIKIN_FONT_FEATURE_H
+#define MINIKIN_FONT_FEATURE_H
+
+#include <hb.h>
+
+#include <ostream>
+#include <string_view>
+#include <vector>
+
+namespace minikin {
+
+struct MinikinPaint;
+
+// Subset of the hb_feature_t since we don't allow setting features on ranges.
+struct FontFeature {
+ hb_tag_t tag;
+ uint32_t value;
+
+ static std::vector<FontFeature> parse(std::string_view fontFeatureString);
+};
+
+/**
+ * Returns the final set of font features based on the features requested by this paint object and
+ * extra defaults or implied font features.
+ *
+ * Features are included from the paint object if they are:
+ * 1) in a supported range
+ *
+ * Default features are added based if they are:
+ * 1) implied due to Paint settings such as letterSpacing
+ * 2) default features that do not conflict with requested features
+ */
+std::vector<hb_feature_t> cleanAndAddDefaultFontFeatures(const MinikinPaint& features);
+
+// For gtest output
+inline std::ostream& operator<<(std::ostream& os, const FontFeature& feature) {
+ return os << static_cast<char>(feature.tag >> 24) << static_cast<char>(feature.tag >> 16)
+ << static_cast<char>(feature.tag >> 8) << static_cast<char>(feature.tag) << " "
+ << feature.value;
+}
+
+inline std::ostream& operator<<(std::ostream& os, const std::vector<FontFeature>& features) {
+ for (size_t i = 0; i < features.size(); ++i) {
+ if (i != 0) {
+ os << ", ";
+ }
+ os << features;
+ }
+ return os;
+}
+
+constexpr bool operator==(const FontFeature& l, const FontFeature& r) {
+ return l.tag == r.tag && l.value == r.value;
+}
+
+constexpr bool operator!=(const FontFeature& l, const FontFeature& r) {
+ return !(l == r);
+}
+
+} // namespace minikin
+#endif // MINIKIN_LAYOUT_H
diff --git a/include/minikin/Hasher.h b/include/minikin/Hasher.h
index 4a76b29..4e20195 100644
--- a/include/minikin/Hasher.h
+++ b/include/minikin/Hasher.h
@@ -18,9 +18,9 @@
#define MINIKIN_HASHER_H
#include <cstdint>
-
#include <string>
+#include "minikin/FontFeature.h"
#include "minikin/Macros.h"
namespace minikin {
@@ -57,6 +57,20 @@
return update(bits.i);
}
+ inline Hasher& update(const std::vector<FontFeature>& features) {
+ update(features.size());
+ for (const FontFeature& feature : features) {
+ update(feature);
+ }
+ return *this;
+ }
+
+ inline Hasher& update(const FontFeature& feature) {
+ update(feature.tag);
+ update(feature.value);
+ return *this;
+ }
+
inline Hasher& updateShorts(const uint16_t* data, uint32_t length) {
update(length);
uint32_t i;
diff --git a/include/minikin/LayoutCache.h b/include/minikin/LayoutCache.h
index 74300a2..fa572fb 100644
--- a/include/minikin/LayoutCache.h
+++ b/include/minikin/LayoutCache.h
@@ -55,6 +55,7 @@
mStartHyphen(startHyphen),
mEndHyphen(endHyphen),
mIsRtl(dir),
+ mFontFeatureSettings(paint.fontFeatureSettings),
mHash(computeHash()) {}
bool operator==(const LayoutCacheKey& o) const {
@@ -64,6 +65,7 @@
mFontFlags == o.mFontFlags && mLocaleListId == o.mLocaleListId &&
mFamilyVariant == o.mFamilyVariant && mStartHyphen == o.mStartHyphen &&
mEndHyphen == o.mEndHyphen && mIsRtl == o.mIsRtl && mNchars == o.mNchars &&
+ mFontFeatureSettings == o.mFontFeatureSettings &&
!memcmp(mChars, o.mChars, mNchars * sizeof(uint16_t));
}
@@ -77,6 +79,7 @@
void freeText() {
delete[] mChars;
mChars = NULL;
+ mFontFeatureSettings.clear();
}
uint32_t getMemoryUsage() const { return sizeof(LayoutCacheKey) + sizeof(uint16_t) * mNchars; }
@@ -99,6 +102,7 @@
StartHyphenEdit mStartHyphen;
EndHyphenEdit mEndHyphen;
bool mIsRtl;
+ std::vector<FontFeature> mFontFeatureSettings;
// Note: any fields added to MinikinPaint must also be reflected here.
// TODO: language matching (possibly integrate into style)
android::hash_t mHash;
@@ -120,6 +124,7 @@
.update(packHyphenEdit(mStartHyphen, mEndHyphen))
.update(mIsRtl)
.updateShorts(mChars, mNchars)
+ .update(mFontFeatureSettings)
.hash();
}
};
diff --git a/include/minikin/MinikinPaint.h b/include/minikin/MinikinPaint.h
index 54b476f..04cab1d 100644
--- a/include/minikin/MinikinPaint.h
+++ b/include/minikin/MinikinPaint.h
@@ -23,6 +23,7 @@
#include "minikin/FamilyVariant.h"
#include "minikin/FontCollection.h"
#include "minikin/FontFamily.h"
+#include "minikin/FontFeature.h"
#include "minikin/Hasher.h"
namespace minikin {
@@ -45,7 +46,7 @@
};
// Possibly move into own .h file?
-// Note: if you add a field here, either add it to LayoutCacheKey or to skipCache()
+// Note: if you add a field here, either add it to LayoutCacheKey
struct MinikinPaint {
MinikinPaint(const std::shared_ptr<FontCollection>& font)
: size(0),
@@ -59,7 +60,7 @@
fontFeatureSettings(),
font(font) {}
- bool skipCache() const { return !fontFeatureSettings.empty(); }
+ bool skipCache() const;
float size;
float scaleX;
@@ -70,7 +71,7 @@
uint32_t localeListId;
FontStyle fontStyle;
FamilyVariant familyVariant;
- std::string fontFeatureSettings;
+ std::vector<FontFeature> fontFeatureSettings;
std::shared_ptr<FontCollection> font;
void copyFrom(const MinikinPaint& paint) { *this = paint; }
@@ -100,7 +101,7 @@
.update(localeListId)
.update(fontStyle.identifier())
.update(static_cast<uint8_t>(familyVariant))
- .updateString(fontFeatureSettings)
+ .update(fontFeatureSettings)
.update(font->getId())
.hash();
}
diff --git a/libs/minikin/Debug.cpp b/libs/minikin/Debug.cpp
index 42d6d8e..8168a77 100644
--- a/libs/minikin/Debug.cpp
+++ b/libs/minikin/Debug.cpp
@@ -21,6 +21,7 @@
#include <sstream>
+#include "minikin/FontFeature.h"
#include "minikin/FontFileParser.h"
#include "minikin/LayoutCore.h"
#include "minikin/LocaleList.h"
@@ -79,6 +80,18 @@
return ss.str();
}
+std::string toString(const FontFeature& feature) {
+ std::stringstream ss;
+ ss << feature;
+ return ss.str();
+}
+
+std::string toString(const std::vector<FontFeature>& features) {
+ std::stringstream ss;
+ ss << features;
+ return ss.str();
+}
+
std::string toString(const LayoutPiece& layout) {
std::stringstream ss;
ss << "{advance=" << layout.advance() << ", extent=" << toString(layout.extent())
diff --git a/libs/minikin/FeatureFlags.h b/libs/minikin/FeatureFlags.h
index 3c2e455..9cfcf9d 100644
--- a/libs/minikin/FeatureFlags.h
+++ b/libs/minikin/FeatureFlags.h
@@ -23,21 +23,22 @@
namespace features {
-inline bool phrase_strict_fallback() {
#ifdef __ANDROID__
- return com_android_text_flags_phrase_strict_fallback();
-#else
- return true;
-#endif // __ANDROID__
-}
+#define DEFINE_FEATURE_FLAG_ACCESSOROR(feature_name) \
+ inline bool feature_name() { \
+ static bool flag = com_android_text_flags_##feature_name(); \
+ return flag; \
+ }
+#else // __ANDROID__
+#define DEFINE_FEATURE_FLAG_ACCESSOROR(feature_name) \
+ inline bool feature_name() { \
+ return true; \
+ }
+#endif // __ANDROID__
-inline bool word_style_auto() {
-#ifdef __ANDROID__
- return com_android_text_flags_word_style_auto();
-#else
- return true;
-#endif // __ANDROID__
-}
+DEFINE_FEATURE_FLAG_ACCESSOROR(phrase_strict_fallback)
+DEFINE_FEATURE_FLAG_ACCESSOROR(word_style_auto)
+DEFINE_FEATURE_FLAG_ACCESSOROR(inter_character_justification)
} // namespace features
diff --git a/libs/minikin/FontCollection.cpp b/libs/minikin/FontCollection.cpp
index 440eab2..84e5dd9 100644
--- a/libs/minikin/FontCollection.cpp
+++ b/libs/minikin/FontCollection.cpp
@@ -608,6 +608,25 @@
return b.build();
}
+void FontCollection::filterFamilyByLocale(
+ const LocaleList& localeList,
+ const std::function<void(const FontFamily& family)>& callback) const {
+ for (uint8_t i = 0; i < mFamilyCount; ++i) {
+ const auto& family = getFamilyAt(i);
+
+ uint32_t fontLocaleId = family->localeListId();
+ if (fontLocaleId == LocaleListCache::kInvalidListId) {
+ continue;
+ }
+ const LocaleList& fontLocaleList = LocaleListCache::getById(fontLocaleId);
+ if (!localeList.atLeastOneScriptMatch(fontLocaleList)) {
+ continue;
+ }
+
+ callback(*family.get());
+ }
+}
+
MinikinExtent FontCollection::getReferenceExtentForLocale(const MinikinPaint& paint) const {
uint32_t localeId = paint.localeListId;
MinikinExtent result(0, 0);
@@ -635,34 +654,35 @@
const LocaleList& requestedLocaleList = LocaleListCache::getById(localeId);
bool familyFound = false;
- for (uint8_t i = 0; i < mFamilyCount; ++i) {
- const auto& family = getFamilyAt(i);
- const FamilyVariant familyVariant = family->variant() == FamilyVariant::DEFAULT
+ filterFamilyByLocale(requestedLocaleList, [&](const FontFamily& family) {
+ const FamilyVariant familyVariant = family.variant() == FamilyVariant::DEFAULT
? FamilyVariant::COMPACT
- : family->variant();
+ : family.variant();
if (familyVariant != requestVariant) {
- continue;
+ return;
}
- uint32_t fontLocaleId = family->localeListId();
- if (fontLocaleId == LocaleListCache::kInvalidListId) {
- continue;
- }
- const LocaleList& fontLocaleList = LocaleListCache::getById(fontLocaleId);
- if (!requestedLocaleList.atLeastOneScriptMatch(fontLocaleList)) {
- continue;
- }
-
- // Use this family
MinikinExtent extent(0, 0);
- FakedFont font = getFamilyAt(i)->getClosestMatch(paint.fontStyle);
+ FakedFont font = family.getClosestMatch(paint.fontStyle);
font.typeface()->GetFontExtent(&extent, paint, font.fakery);
result.extendBy(extent);
familyFound = true;
- }
+ });
+ // If nothing matches, try non-variant match cases since it is used for fallback.
+ filterFamilyByLocale(requestedLocaleList, [&](const FontFamily& family) {
+ // Use this family
+ MinikinExtent extent(0, 0);
+ FakedFont font = family.getClosestMatch(paint.fontStyle);
+ font.typeface()->GetFontExtent(&extent, paint, font.fakery);
+ result.extendBy(extent);
+
+ familyFound = true;
+ });
+
+ // If nothing matches, use default font.
if (!familyFound) {
FakedFont font = getFamilyAt(0)->getClosestMatch(paint.fontStyle);
font.typeface()->GetFontExtent(&result, paint, font.fakery);
diff --git a/libs/minikin/FontFeatureUtils.cpp b/libs/minikin/FontFeatureUtils.cpp
index e1ec065..cc0749a 100644
--- a/libs/minikin/FontFeatureUtils.cpp
+++ b/libs/minikin/FontFeatureUtils.cpp
@@ -14,12 +14,29 @@
* limitations under the License.
*/
-#include "FontFeatureUtils.h"
-
#include "StringPiece.h"
+#include "minikin/FontFeature.h"
+#include "minikin/MinikinPaint.h"
namespace minikin {
+std::vector<FontFeature> FontFeature::parse(std::string_view fontFeatureSettings) {
+ std::vector<FontFeature> features;
+
+ SplitIterator it(StringPiece(fontFeatureSettings), ',');
+ while (it.hasNext()) {
+ StringPiece featureStr = it.next();
+ static hb_feature_t feature;
+ // We do not allow setting features on ranges. As such, reject any setting that has
+ // non-universal range.
+ if (hb_feature_from_string(featureStr.data(), featureStr.size(), &feature) &&
+ feature.start == 0 && feature.end == (unsigned int)-1) {
+ features.push_back({feature.tag, feature.value});
+ }
+ }
+ return features;
+}
+
std::vector<hb_feature_t> cleanAndAddDefaultFontFeatures(const MinikinPaint& paint) {
std::vector<hb_feature_t> features;
// Disable default-on non-required ligature features if letter-spacing
@@ -40,27 +57,17 @@
static constexpr hb_tag_t halt_tag = HB_TAG('h', 'a', 'l', 't');
static constexpr hb_tag_t palt_tag = HB_TAG('p', 'a', 'l', 't');
- SplitIterator it(paint.fontFeatureSettings, ',');
- while (it.hasNext()) {
- StringPiece featureStr = it.next();
- static hb_feature_t feature;
- // We do not allow setting features on ranges. As such, reject any setting that has
- // non-universal range.
- if (hb_feature_from_string(featureStr.data(), featureStr.size(), &feature) &&
- feature.start == 0 && feature.end == (unsigned int)-1) {
- // OpenType requires disabling default `chws` feature if glyph-width features.
- // https://docs.microsoft.com/en-us/typography/opentype/spec/features_ae#tag-chws
- // Here, we follow Chrome's impl: not enabling default `chws` feature if `palt` or
- // `halt` is enabled.
- // https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/fonts/shaping/font_features.cc;drc=77a9a09de0688ca449f5333a305ceaf3f36b6daf;l=215
- if (default_enable_chws &&
- (feature.tag == chws_tag ||
- (feature.value && (feature.tag == halt_tag || feature.tag == palt_tag)))) {
- default_enable_chws = false;
- }
-
- features.push_back(feature);
+ for (const FontFeature& feature : paint.fontFeatureSettings) {
+ // OpenType requires disabling default `chws` feature if glyph-width features.
+ // https://docs.microsoft.com/en-us/typography/opentype/spec/features_ae#tag-chws
+ // Here, we follow Chrome's impl: not enabling default `chws` feature if `palt` or
+ // `halt` is enabled.
+ // https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/fonts/shaping/font_features.cc;drc=77a9a09de0688ca449f5333a305ceaf3f36b6daf;l=215
+ if (feature.tag == chws_tag ||
+ (feature.value && (feature.tag == halt_tag || feature.tag == palt_tag))) {
+ default_enable_chws = false;
}
+ features.push_back({feature.tag, feature.value, 0, ~0u});
}
if (default_enable_chws) {
diff --git a/libs/minikin/FontFeatureUtils.h b/libs/minikin/FontFeatureUtils.h
deleted file mode 100644
index 1a02173..0000000
--- a/libs/minikin/FontFeatureUtils.h
+++ /dev/null
@@ -1,40 +0,0 @@
-/*
- * Copyright (C) 2021 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.
- */
-
-#ifndef MINIKIN_FONT_FEATURE_UTILS_H
-#define MINIKIN_FONT_FEATURE_UTILS_H
-
-#include <hb.h>
-
-#include "minikin/MinikinPaint.h"
-
-namespace minikin {
-
-/**
- * Returns the final set of font features based on the features requested by this paint object and
- * extra defaults or implied font features.
- *
- * Features are included from the paint object if they are:
- * 1) in a supported range
- *
- * Default features are added based if they are:
- * 1) implied due to Paint settings such as letterSpacing
- * 2) default features that do not conflict with requested features
- */
-std::vector<hb_feature_t> cleanAndAddDefaultFontFeatures(const MinikinPaint& paint);
-
-} // namespace minikin
-#endif // MINIKIN_LAYOUT_UTILS_H
diff --git a/libs/minikin/LayoutCore.cpp b/libs/minikin/LayoutCore.cpp
index 5a19f89..b079e85 100644
--- a/libs/minikin/LayoutCore.cpp
+++ b/libs/minikin/LayoutCore.cpp
@@ -34,12 +34,12 @@
#include <vector>
#include "BidiUtils.h"
-#include "FontFeatureUtils.h"
#include "LayoutUtils.h"
#include "LocaleListCache.h"
#include "MinikinInternal.h"
#include "ScriptUtils.h"
#include "minikin/Emoji.h"
+#include "minikin/FontFeature.h"
#include "minikin/HbUtils.h"
#include "minikin/LayoutCache.h"
#include "minikin/LayoutPieces.h"
diff --git a/libs/minikin/MinikinInternal.cpp b/libs/minikin/MinikinInternal.cpp
index d02f71f..5b81406 100644
--- a/libs/minikin/MinikinInternal.cpp
+++ b/libs/minikin/MinikinInternal.cpp
@@ -17,9 +17,12 @@
#define LOG_TAG "Minikin"
+#include "MinikinInternal.h"
+
#include <log/log.h>
-#include "MinikinInternal.h"
+#include "FeatureFlags.h"
+#include "minikin/MinikinPaint.h"
namespace minikin {
@@ -45,4 +48,12 @@
return isBMPVariationSelector(codePoint) || isVariationSelectorSupplement(codePoint);
}
+bool MinikinPaint::skipCache() const {
+ if (features::inter_character_justification()) {
+ return false; // if the flag is on, do not skip the cache.
+ } else {
+ return !fontFeatureSettings.empty();
+ }
+}
+
} // namespace minikin
diff --git a/libs/minikin/StringPiece.h b/libs/minikin/StringPiece.h
index befb312..84c7d17 100644
--- a/libs/minikin/StringPiece.h
+++ b/libs/minikin/StringPiece.h
@@ -29,6 +29,7 @@
StringPiece(const char* data) : mData(data), mLength(data == nullptr ? 0 : strlen(data)) {}
StringPiece(const char* data, size_t length) : mData(data), mLength(length) {}
StringPiece(const std::string& str) : mData(str.data()), mLength(str.size()) {}
+ StringPiece(std::string_view str) : mData(str.data()), mLength(str.size()) {}
inline const char* data() const { return mData; }
inline size_t length() const { return mLength; }
diff --git a/tests/perftests/Android.bp b/tests/perftests/Android.bp
index 362ba10..c4260b3 100644
--- a/tests/perftests/Android.bp
+++ b/tests/perftests/Android.bp
@@ -46,5 +46,7 @@
"libicu",
"liblog",
"libcutils",
+ "aconfig_text_flags_c_lib",
+ "server_configurable_flags",
],
}
diff --git a/tests/stresstest/Android.bp b/tests/stresstest/Android.bp
index 7e0379c..420a4b1 100644
--- a/tests/stresstest/Android.bp
+++ b/tests/stresstest/Android.bp
@@ -42,6 +42,8 @@
"libutils",
"libz",
"libcutils",
+ "aconfig_text_flags_c_lib",
+ "server_configurable_flags",
],
srcs: [
diff --git a/tests/unittest/FontFeatureTest.cpp b/tests/unittest/FontFeatureTest.cpp
index 7f9cdf4..6f01842 100644
--- a/tests/unittest/FontFeatureTest.cpp
+++ b/tests/unittest/FontFeatureTest.cpp
@@ -14,10 +14,12 @@
* limitations under the License.
*/
+#include <com_android_text_flags.h>
+#include <flag_macros.h>
#include <gtest/gtest.h>
-#include "FontFeatureUtils.h"
#include "FontTestUtils.h"
+#include "minikin/FontFeature.h"
#include "minikin/MinikinPaint.h"
namespace minikin {
@@ -53,7 +55,7 @@
TEST_F(DefaultFontFeatureTest, disable) {
auto paint = MinikinPaint(font);
- paint.fontFeatureSettings = "\"chws\" off";
+ paint.fontFeatureSettings = FontFeature::parse("\"chws\" off");
auto f = cleanAndAddDefaultFontFeatures(paint);
std::sort(f.begin(), f.end(), compareFeatureTag);
@@ -65,7 +67,7 @@
TEST_F(DefaultFontFeatureTest, preserve) {
auto paint = MinikinPaint(font);
- paint.fontFeatureSettings = "\"ruby\" on";
+ paint.fontFeatureSettings = FontFeature::parse("\"ruby\" on");
auto f = cleanAndAddDefaultFontFeatures(paint);
std::sort(f.begin(), f.end(), compareFeatureTag);
@@ -95,7 +97,7 @@
TEST_F(DefaultFontFeatureTest, halt_disable_chws) {
auto paint = MinikinPaint(font);
- paint.fontFeatureSettings = "\"halt\" on";
+ paint.fontFeatureSettings = FontFeature::parse("\"halt\" on");
auto f = cleanAndAddDefaultFontFeatures(paint);
EXPECT_EQ(1u, f.size());
@@ -105,7 +107,7 @@
TEST_F(DefaultFontFeatureTest, palt_disable_chws) {
auto paint = MinikinPaint(font);
- paint.fontFeatureSettings = "\"palt\" on";
+ paint.fontFeatureSettings = FontFeature::parse("\"palt\" on");
auto f = cleanAndAddDefaultFontFeatures(paint);
EXPECT_EQ(1u, f.size());
@@ -116,7 +118,7 @@
TEST_F(DefaultFontFeatureTest, halt_disable_chws_large_letter_spacing) {
auto paint = MinikinPaint(font);
paint.letterSpacing = 1.0; // em
- paint.fontFeatureSettings = "\"halt\" on";
+ paint.fontFeatureSettings = FontFeature::parse("\"halt\" on");
auto f = cleanAndAddDefaultFontFeatures(paint);
std::sort(f.begin(), f.end(), compareFeatureTag);
@@ -133,7 +135,7 @@
TEST_F(DefaultFontFeatureTest, palt_disable_chws_large_letter_spacing) {
auto paint = MinikinPaint(font);
paint.letterSpacing = 1.0; // em
- paint.fontFeatureSettings = "\"palt\" on";
+ paint.fontFeatureSettings = FontFeature::parse("\"palt\" on");
auto f = cleanAndAddDefaultFontFeatures(paint);
std::sort(f.begin(), f.end(), compareFeatureTag);
@@ -147,4 +149,27 @@
EXPECT_TRUE(f[2].value);
}
+class FontFeatureTest : public testing::Test {
+protected:
+ std::shared_ptr<FontCollection> font;
+
+ virtual void SetUp() override { font = buildFontCollection("Ascii.ttf"); }
+};
+
+TEST_F_WITH_FLAGS(FontFeatureTest, do_not_skip_cache_if_flagEnabled,
+ REQUIRES_FLAGS_ENABLED(ACONFIG_FLAG(com::android::text::flags,
+ inter_character_justification))) {
+ auto paint = MinikinPaint(font);
+ paint.fontFeatureSettings = FontFeature::parse("\"palt\" on");
+ EXPECT_FALSE(paint.skipCache());
+}
+
+TEST_F_WITH_FLAGS(FontFeatureTest, do_not_skip_cache_if_flagDisabled,
+ REQUIRES_FLAGS_DISABLED(ACONFIG_FLAG(com::android::text::flags,
+ inter_character_justification))) {
+ auto paint = MinikinPaint(font);
+ paint.fontFeatureSettings = FontFeature::parse("\"palt\" on");
+ EXPECT_TRUE(paint.skipCache());
+}
+
} // namespace minikin
diff --git a/tests/unittest/LayoutCacheTest.cpp b/tests/unittest/LayoutCacheTest.cpp
index b1d60ba..5d20456 100644
--- a/tests/unittest/LayoutCacheTest.cpp
+++ b/tests/unittest/LayoutCacheTest.cpp
@@ -246,11 +246,11 @@
SCOPED_TRACE("Different font feature settings");
auto collection = buildFontCollection("Ascii.ttf");
MinikinPaint paint1(collection);
- paint1.fontFeatureSettings = "";
+ paint1.fontFeatureSettings = FontFeature::parse("");
layoutCache.getOrCreate(text1, Range(0, text1.size()), paint1, false /* LTR */,
StartHyphenEdit::NO_EDIT, EndHyphenEdit::NO_EDIT, false, layout1);
MinikinPaint paint2(collection);
- paint2.fontFeatureSettings = "'liga' on";
+ paint2.fontFeatureSettings = FontFeature::parse("'liga' on");
layoutCache.getOrCreate(text1, Range(0, text1.size()), paint2, false /* LTR */,
StartHyphenEdit::NO_EDIT, EndHyphenEdit::NO_EDIT, false, layout2);
EXPECT_NE(layout1.get(), layout2.get());
diff --git a/tests/unittest/LayoutCoreTest.cpp b/tests/unittest/LayoutCoreTest.cpp
index 139a7ea..264454a 100644
--- a/tests/unittest/LayoutCoreTest.cpp
+++ b/tests/unittest/LayoutCoreTest.cpp
@@ -54,7 +54,7 @@
const std::string fontFeaturesSettings) {
MinikinPaint paint(fc);
paint.size = 10.0f; // make 1em = 10px
- paint.fontFeatureSettings = fontFeaturesSettings;
+ paint.fontFeatureSettings = FontFeature::parse(fontFeaturesSettings);
return buildLayout(text, paint);
}
