DEV/flutter_flutter
1
0
Fork 0
mirror of https://github.com/flutter/flutter.git synced 2026-02-20 02:29:02 +08:00
Code Issues Packages Projects Releases Wiki Activity

Do not keep FontCollection reference in Layout.

Browse Source 
LayoutCache only keeps result of layout and can live after
FontCollection is destructed by GC.

This kind of failure will be captured by minikin_stress_tests in the
subsequent CL (I1bf4ba43e6e97cd04e7d6dd42d388dd17ce64c7b)

Test: ran minikin_tests
Bug: 36223724
Change-Id: I639b73c0f1041549158c43212a901c82df4b02db
This commit is contained in:
Seigo Nonaka 2017-03-14 15:34:06 -07:00
parent fd4124c533
commit 99ef32cef0
3 changed files with 60 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
include/minikin/Layout.h
View File

@ -75,8 +75,7 @@ enum {
class Layout {
public:
Layout(const std::shared_ptr<FontCollection>& collection)
: mGlyphs(), mAdvances(), mCollection(collection), mFaces(), mAdvance(0), mBounds() {
Layout() : mGlyphs(), mAdvances(), mFaces(), mAdvance(0), mBounds() {
mBounds.setEmpty();
}
@ -89,7 +88,8 @@ public:
void dump() const;
void doLayout(const uint16_t* buf, size_t start, size_t count, size_t bufSize,
int bidiFlags, const FontStyle &style, const MinikinPaint &paint);
int bidiFlags, const FontStyle &style, const MinikinPaint &paint,
const std::shared_ptr<FontCollection>& collection);
static float measureText(const uint16_t* buf, size_t start, size_t count, size_t bufSize,
int bidiFlags, const FontStyle &style, const MinikinPaint &paint,
@ -143,7 +143,7 @@ private:
// Lay out a single bidi run
void doLayoutRun(const uint16_t* buf, size_t start, size_t count, size_t bufSize,
bool isRtl, LayoutContext* ctx);
bool isRtl, LayoutContext* ctx, const std::shared_ptr<FontCollection>& collection);
// Append another layout (for example, cached value) into this one
void appendLayout(Layout* src, size_t start, float extraAdvance);
@ -151,7 +151,6 @@ private:
std::vector<LayoutGlyph> mGlyphs;
std::vector<float> mAdvances;
std::shared_ptr<FontCollection> mCollection;
std::vector<FakedFont> mFaces;
float mAdvance;
MinikinRect mBounds;

22
libs/minikin/Layout.cpp
View File

@ -131,10 +131,11 @@ public:
mChars = NULL;
}
void doLayout(Layout* layout, LayoutContext* ctx) const {
void doLayout(Layout* layout, LayoutContext* ctx,
const std::shared_ptr<FontCollection>& collection) const {
layout->mAdvances.resize(mCount, 0);
ctx->clearHbFonts();
layout->doLayoutRun(mChars, mStart, mCount, mNchars, mIsRtl, ctx);
layout->doLayoutRun(mChars, mStart, mCount, mNchars, mIsRtl, ctx, collection);
}
private:
@ -173,8 +174,8 @@ public:
Layout* layout = mCache.get(key);
if (layout == NULL) {
key.copyText();
layout = new Layout(collection);
key.doLayout(layout, ctx);
layout = new Layout();
key.doLayout(layout, ctx, collection);
mCache.put(key, layout);
}
return layout;
@ -584,7 +585,8 @@ BidiText::BidiText(const uint16_t* buf, size_t start, size_t count, size_t bufSi
}
void Layout::doLayout(const uint16_t* buf, size_t start, size_t count, size_t bufSize,
int bidiFlags, const FontStyle &style, const MinikinPaint &paint) {
int bidiFlags, const FontStyle &style, const MinikinPaint &paint,
const std::shared_ptr<FontCollection>& collection) {
android::AutoMutex _l(gMinikinLock);
LayoutContext ctx;
@ -596,7 +598,7 @@ void Layout::doLayout(const uint16_t* buf, size_t start, size_t count, size_t bu
for (const BidiText::Iter::RunInfo& runInfo : BidiText(buf, start, count, bufSize, bidiFlags)) {
doLayoutRunCached(buf, runInfo.mRunStart, runInfo.mRunLength, bufSize, runInfo.mIsRtl, &ctx,
start, mCollection, this, NULL);
start, collection, this, NULL);
}
ctx.clearHbFonts();
}
@ -684,8 +686,8 @@ float Layout::doLayoutWord(const uint16_t* buf, size_t start, size_t count, size
float advance;
if (ctx->paint.skipCache()) {
Layout layoutForWord(collection);
key.doLayout(&layoutForWord, ctx);
Layout layoutForWord;
key.doLayout(&layoutForWord, ctx, collection);
if (layout) {
layout->appendLayout(&layoutForWord, bufStart, wordSpacing);
}
@ -863,10 +865,10 @@ static inline uint32_t addToHbBuffer(hb_buffer_t* buffer,
void Layout::doLayoutRun(const uint16_t* buf, size_t start, size_t count, size_t bufSize,
bool isRtl, LayoutContext* ctx) {
bool isRtl, LayoutContext* ctx, const std::shared_ptr<FontCollection>& collection) {
hb_buffer_t* buffer = LayoutEngine::getInstance().hbBuffer;
vector<FontCollection::Run> items;
mCollection->itemize(buf + start, count, ctx->style, &items);
collection->itemize(buf + start, count, ctx->style, &items);
vector<hb_feature_t> features;
// Disable default-on non-required ligature features if letter-spacing

69
tests/unittest/LayoutTest.cpp
View File

@ -70,14 +70,15 @@ TEST_F(LayoutTest, doLayoutTest) {
float advances[kMaxAdvanceLength];
std::vector<float> expectedValues;
Layout layout(mCollection);
Layout layout;
std::vector<uint16_t> 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);
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint,
mCollection);
EXPECT_EQ(70.0f, layout.getAdvance());
layout.getBounds(&rect);
EXPECT_EQ(0.0f, rect.mLeft);
@ -95,7 +96,8 @@ TEST_F(LayoutTest, doLayoutTest) {
{
SCOPED_TRACE("two words");
text = utf8ToUtf16("two words");
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint);
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint,
mCollection);
EXPECT_EQ(90.0f, layout.getAdvance());
layout.getBounds(&rect);
EXPECT_EQ(0.0f, rect.mLeft);
@ -113,7 +115,8 @@ TEST_F(LayoutTest, doLayoutTest) {
{
SCOPED_TRACE("three words");
text = utf8ToUtf16("three words test");
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint);
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint,
mCollection);
EXPECT_EQ(160.0f, layout.getAdvance());
layout.getBounds(&rect);
EXPECT_EQ(0.0f, rect.mLeft);
@ -131,7 +134,8 @@ TEST_F(LayoutTest, doLayoutTest) {
{
SCOPED_TRACE("two spaces");
text = utf8ToUtf16("two spaces");
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint);
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint,
mCollection);
EXPECT_EQ(110.0f, layout.getAdvance());
layout.getBounds(&rect);
EXPECT_EQ(0.0f, rect.mLeft);
@ -156,7 +160,7 @@ TEST_F(LayoutTest, doLayoutTest_wordSpacing) {
std::vector<float> expectedValues;
std::vector<uint16_t> text;
Layout layout(mCollection);
Layout layout;
paint.wordSpacing = 5.0f;
@ -164,7 +168,8 @@ TEST_F(LayoutTest, doLayoutTest_wordSpacing) {
{
SCOPED_TRACE("one word");
text = utf8ToUtf16("oneword");
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint);
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint,
mCollection);
EXPECT_EQ(70.0f, layout.getAdvance());
layout.getBounds(&rect);
EXPECT_EQ(0.0f, rect.mLeft);
@ -182,7 +187,8 @@ TEST_F(LayoutTest, doLayoutTest_wordSpacing) {
{
SCOPED_TRACE("two words");
text = utf8ToUtf16("two words");
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint);
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint,
mCollection);
EXPECT_EQ(95.0f, layout.getAdvance());
layout.getBounds(&rect);
EXPECT_EQ(0.0f, rect.mLeft);
@ -204,7 +210,8 @@ TEST_F(LayoutTest, doLayoutTest_wordSpacing) {
{
SCOPED_TRACE("three words test");
text = utf8ToUtf16("three words test");
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint);
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint,
mCollection);
EXPECT_EQ(170.0f, layout.getAdvance());
layout.getBounds(&rect);
EXPECT_EQ(0.0f, rect.mLeft);
@ -224,7 +231,8 @@ TEST_F(LayoutTest, doLayoutTest_wordSpacing) {
{
SCOPED_TRACE("two spaces");
text = utf8ToUtf16("two spaces");
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint);
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint,
mCollection);
EXPECT_EQ(120.0f, layout.getAdvance());
layout.getBounds(&rect);
EXPECT_EQ(0.0f, rect.mLeft);
@ -250,7 +258,7 @@ TEST_F(LayoutTest, doLayoutTest_negativeWordSpacing) {
float advances[kMaxAdvanceLength];
std::vector<float> expectedValues;
Layout layout(mCollection);
Layout layout;
std::vector<uint16_t> text;
// Negative word spacing also should work.
@ -259,7 +267,8 @@ TEST_F(LayoutTest, doLayoutTest_negativeWordSpacing) {
{
SCOPED_TRACE("one word");
text = utf8ToUtf16("oneword");
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint);
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint,
mCollection);
EXPECT_EQ(70.0f, layout.getAdvance());
layout.getBounds(&rect);
EXPECT_EQ(0.0f, rect.mLeft);
@ -277,7 +286,8 @@ TEST_F(LayoutTest, doLayoutTest_negativeWordSpacing) {
{
SCOPED_TRACE("two words");
text = utf8ToUtf16("two words");
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint);
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint,
mCollection);
EXPECT_EQ(85.0f, layout.getAdvance());
layout.getBounds(&rect);
EXPECT_EQ(0.0f, rect.mLeft);
@ -296,7 +306,8 @@ TEST_F(LayoutTest, doLayoutTest_negativeWordSpacing) {
{
SCOPED_TRACE("three words");
text = utf8ToUtf16("three word test");
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint);
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint,
mCollection);
EXPECT_EQ(140.0f, layout.getAdvance());
layout.getBounds(&rect);
EXPECT_EQ(0.0f, rect.mLeft);
@ -316,7 +327,8 @@ TEST_F(LayoutTest, doLayoutTest_negativeWordSpacing) {
{
SCOPED_TRACE("two spaces");
text = utf8ToUtf16("two spaces");
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint);
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint,
mCollection);
EXPECT_EQ(100.0f, layout.getAdvance());
layout.getBounds(&rect);
EXPECT_EQ(0.0f, rect.mLeft);
@ -340,11 +352,13 @@ TEST_F(LayoutTest, doLayoutTest_rtlTest) {
std::vector<uint16_t> text = parseUnicodeString("'a' 'b' U+3042 U+3043 'c' 'd'");
Layout ltrLayout(mCollection);
ltrLayout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint);
Layout ltrLayout;
ltrLayout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint,
mCollection);
Layout rtlLayout(mCollection);
rtlLayout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_RTL, FontStyle(), paint);
Layout rtlLayout;
rtlLayout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_RTL, FontStyle(), paint,
mCollection);
ASSERT_EQ(ltrLayout.nGlyphs(), rtlLayout.nGlyphs());
ASSERT_EQ(6u, ltrLayout.nGlyphs());
@ -357,7 +371,7 @@ TEST_F(LayoutTest, doLayoutTest_rtlTest) {
}
TEST_F(LayoutTest, hyphenationTest) {
Layout layout(mCollection);
Layout layout;
std::vector<uint16_t> text;
// The mock implementation returns 10.0f advance for all glyphs.
@ -366,7 +380,8 @@ TEST_F(LayoutTest, hyphenationTest) {
text = utf8ToUtf16("oneword");
MinikinPaint paint;
paint.hyphenEdit = HyphenEdit::NO_EDIT;
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint);
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint,
mCollection);
EXPECT_EQ(70.0f, layout.getAdvance());
}
{
@ -374,7 +389,8 @@ TEST_F(LayoutTest, hyphenationTest) {
text = utf8ToUtf16("oneword");
MinikinPaint paint;
paint.hyphenEdit = HyphenEdit::INSERT_HYPHEN_AT_END;
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint);
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint,
mCollection);
EXPECT_EQ(80.0f, layout.getAdvance());
}
{
@ -382,7 +398,8 @@ TEST_F(LayoutTest, hyphenationTest) {
text = utf8ToUtf16("oneword");
MinikinPaint paint;
paint.hyphenEdit = HyphenEdit::REPLACE_WITH_HYPHEN_AT_END;
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint);
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint,
mCollection);
EXPECT_EQ(70.0f, layout.getAdvance());
}
{
@ -390,7 +407,8 @@ TEST_F(LayoutTest, hyphenationTest) {
text = utf8ToUtf16("oneword");
MinikinPaint paint;
paint.hyphenEdit = HyphenEdit::INSERT_HYPHEN_AT_START;
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint);
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint,
mCollection);
EXPECT_EQ(80.0f, layout.getAdvance());
}
{
@ -398,7 +416,8 @@ TEST_F(LayoutTest, hyphenationTest) {
text = utf8ToUtf16("oneword");
MinikinPaint paint;
paint.hyphenEdit = HyphenEdit::INSERT_HYPHEN_AT_START | HyphenEdit::INSERT_HYPHEN_AT_END;
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint);
layout.doLayout(text.data(), 0, text.size(), text.size(), kBidi_LTR, FontStyle(), paint,
mCollection);
EXPECT_EQ(90.0f, layout.getAdvance());
}
}