Cache the layout result if the font feature is specified

Before this patch, the layout is not cahced if the font feature settings
are specified. This CL caches the layout even if the font feature is
specifid.

This is necessary because the Java layer will use font feature settings
for disabling ligature if the inter word justification is specified.

Bug: 283193133
Test: minikin_tests
Change-Id: If54c448d3f496b3323e2d3177803b815f433b65c

17 files changed, 208 insertions, 79 deletions

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 Point;
 struct MinikinRect;
 struct MinikinExtent;
 struct MinikinPaint;
+struct FontFeature;
 class Range;
 class U16StringPiece;
 class LayoutPiece;
@@ -40,6 +41,8 @@ std::string toString(const Range& range);
 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/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 @@ public:
         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 @@ public:
               mStartHyphen(startHyphen),
               mEndHyphen(endHyphen),
               mIsRtl(dir),
+              mFontFeatureSettings(paint.fontFeatureSettings),
               mHash(computeHash()) {}
 
     bool operator==(const LayoutCacheKey& o) const {
@@ -64,6 +65,7 @@ public:
                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 @@ public:
     void freeText() {
         delete[] mChars;
         mChars = NULL;
+        mFontFeatureSettings.clear();
     }
 
     uint32_t getMemoryUsage() const { return sizeof(LayoutCacheKey) + sizeof(uint16_t) * mNchars; }
@@ -99,6 +102,7 @@ private:
     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 @@ private:
                 .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 @@ enum MinikinFontFlags {
 };
 
 // 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 @@ struct MinikinPaint {
               fontFeatureSettings(),
               font(font) {}
 
-    bool skipCache() const { return !fontFeatureSettings.empty(); }
+    bool skipCache() const;
 
     float size;
     float scaleX;
@@ -70,7 +71,7 @@ struct MinikinPaint {
     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 @@ struct MinikinPaint {
                 .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 @@ std::string toString(const MinikinExtent& extent) {
     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..f1da23d 100644
--- a/libs/minikin/FeatureFlags.h
+++ b/libs/minikin/FeatureFlags.h
@@ -39,6 +39,14 @@ inline bool word_style_auto() {
 #endif  // __ANDROID__
 }
 
+inline bool inter_character_justification() {
+#ifdef __ANDROID__
+    return com_android_text_flags_inter_character_justification();
+#else
+    return true;
+#endif  // __ANDROID__
+}
+
 }  // namespace features
 
 #endif  // FEATURE_FLAGS
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 @@ std::vector<hb_feature_t> cleanAndAddDefaultFontFeatures(const MinikinPaint& pai
     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 b89958e..10c932a 100644
--- a/libs/minikin/LayoutCore.cpp
+++ b/libs/minikin/LayoutCore.cpp
@@ -34,11 +34,11 @@
 #include <vector>
 
 #include "BidiUtils.h"
-#include "FontFeatureUtils.h"
 #include "LayoutUtils.h"
 #include "LocaleListCache.h"
 #include "MinikinInternal.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 @@ bool isVariationSelector(uint32_t codePoint) {
     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 @@ public:
     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 @@ cc_benchmark {
         "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 @@ cc_test {
         "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, default) {
 
 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, disable) {
 
 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, large_letter_spacing) {
 
 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, halt_disable_chws) {
 
 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, palt_disable_chws) {
 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, halt_disable_chws_large_letter_spacing) {
 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 @@ TEST_F(DefaultFontFeatureTest, palt_disable_chws_large_letter_spacing) {
     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 @@ TEST(LayoutCacheTest, cacheMissTest) {
         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 @@ static LayoutPiece buildLayout(const std::string& text, std::shared_ptr<FontColl
                                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);
 }