Fixing crash when some fonts are unresolved
Bug: Skia:10255
Change-Id: I5b6569603a49977c7fa96731ce46c41bf681a6b1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/293569
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Julia Lavrova <jlavrova@google.com>
diff --git a/modules/skparagraph/samples/SampleParagraph.cpp b/modules/skparagraph/samples/SampleParagraph.cpp
index 97e7f93..ccbb49c 100644
--- a/modules/skparagraph/samples/SampleParagraph.cpp
+++ b/modules/skparagraph/samples/SampleParagraph.cpp
@@ -1356,7 +1356,9 @@
paint.setStyle(SkPaint::kStroke_Style);
paint.setAntiAlias(true);
paint.setStrokeWidth(1);
- canvas->drawRect(result.front().rect, paint);
+ if (!result.empty()) {
+ canvas->drawRect(result.front().rect, paint);
+ }
}
private:
@@ -1494,8 +1496,8 @@
auto ltgray = SkColorSetRGB(211, 211, 211);
auto multiplier = 5.67;
- const char* text = ">Sͬ͑̀͐̈͒̈́̋̎ͮͩ̽̓ͬ̂̆̔͗́̓ͣͧ͊ͫ͛̉͌̐̑ͪ͗̚͝҉̴͉͢k̡̊̓ͫͭͩ͂͊ͨͪͬ̑ͫ̍̌̄͛̌̂̑̂̋̊̔ͫ͛̽̑ͨ̍ͭ̓̀ͪͪ̉͐͗̌̓̃̚͟͝҉̢͏̫̞̙͇͖̮͕̗̟͕͇͚̻͈̣̻̪͉̰̲̣̫ͅͅP̴̅̍͒̿͗͗̇ͩ̃͆͌̀̽͏̧̡͕͖̝̖̼̺̰̣̬͔͖͔̼͙̞̦̫͓̘͜a̸̴̸̴̢̢̨̨̫͍͓̥̼̭̼̻̤̯̙̤̻̠͚̍̌͋̂ͦͨ̽̇͌͌͆̀̽̎͒̄ͪ̐ͦ̈ͫ͐͗̓̚̚͜ͅr͐͐ͤͫ̐ͥ͂̈́̿́ͮ̃͗̓̏ͫ̀̿͏̸̵̧́͘̕͟͝͠͞͠҉̷̧͚͢͟a̓̽̎̄͗̔͛̄̐͊͛ͫ͂͌̂̂̈̈̓̔̅̅̄͊̉́ͪ̑̄͆ͬ̍͆ͭ͋̐ͬ͏̷̵̨̢̩̹̖͓̥̳̰͔̱̬͖̙͓̙͇̀̀̕͜͟͟͢͟͜͠͡g̨̅̇ͦ͋̂ͦͨͭ̓͐͆̏̂͛̉ͧ̑ͫ̐̒͛ͫ̍̒͛́̚҉̷̨̛̛̀͜͢͞҉̩̘̲͍͎̯̹̝̭̗̱͇͉̲̱͔̯̠̹̥̻͉̲̜̤̰̪̗̺̖̺r̷͌̓̇̅ͭ̀̐̃̃ͭ͑͗̉̈̇̈́ͥ̓ͣ́ͤ͂ͤ͂̏͌̆̚҉̴̸̧̢̢̛̫͉̦̥̤̙͈͉͈͉͓̙̗̟̳̜͈̗̺̟̠̠͖͓̖̪͕̠̕̕͝ͅả̸̴̡̡̧͠͞͡͞҉̛̕͟͏̷̘̪̱͈̲͉̞̠̞̪̫͎̲̬̖̀̀͟͝͞͞͠p̛͂̈͐̚͠҉̵̸̡̢̢̩̹͙̯͖̙̙̮̥̙͚̠͔̥̭̮̞̣̪̬̥̠̖̝̥̪͎́̀̕͜͡͡ͅͅh̵̷̵̡̛ͤ̂͌̐̓̐̋̋͊̒̆̽́̀̀̀͢͠͞͞҉̷̸̢̕҉͚̯͖̫̜̞̟̠̱͉̝̲̹̼͉̟͉̩̮͔̤͖̞̭̙̹̬ͅ<";
-
+ //const char* text = ">Sͬ͑̀͐̈͒̈́̋̎ͮͩ̽̓ͬ̂̆̔͗́̓ͣͧ͊ͫ͛̉͌̐̑ͪ͗̚͝҉̴͉͢k̡̊̓ͫͭͩ͂͊ͨͪͬ̑ͫ̍̌̄͛̌̂̑̂̋̊̔ͫ͛̽̑ͨ̍ͭ̓̀ͪͪ̉͐͗̌̓̃̚͟͝҉̢͏̫̞̙͇͖̮͕̗̟͕͇͚̻͈̣̻̪͉̰̲̣̫ͅͅP̴̅̍͒̿͗͗̇ͩ̃͆͌̀̽͏̧̡͕͖̝̖̼̺̰̣̬͔͖͔̼͙̞̦̫͓̘͜a̸̴̸̴̢̢̨̨̫͍͓̥̼̭̼̻̤̯̙̤̻̠͚̍̌͋̂ͦͨ̽̇͌͌͆̀̽̎͒̄ͪ̐ͦ̈ͫ͐͗̓̚̚͜ͅr͐͐ͤͫ̐ͥ͂̈́̿́ͮ̃͗̓̏ͫ̀̿͏̸̵̧́͘̕͟͝͠͞͠҉̷̧͚͢͟a̓̽̎̄͗̔͛̄̐͊͛ͫ͂͌̂̂̈̈̓̔̅̅̄͊̉́ͪ̑̄͆ͬ̍͆ͭ͋̐ͬ͏̷̵̨̢̩̹̖͓̥̳̰͔̱̬͖̙͓̙͇̀̀̕͜͟͟͢͟͜͠͡g̨̅̇ͦ͋̂ͦͨͭ̓͐͆̏̂͛̉ͧ̑ͫ̐̒͛ͫ̍̒͛́̚҉̷̨̛̛̀͜͢͞҉̩̘̲͍͎̯̹̝̭̗̱͇͉̲̱͔̯̠̹̥̻͉̲̜̤̰̪̗̺̖̺r̷͌̓̇̅ͭ̀̐̃̃ͭ͑͗̉̈̇̈́ͥ̓ͣ́ͤ͂ͤ͂̏͌̆̚҉̴̸̧̢̢̛̫͉̦̥̤̙͈͉͈͉͓̙̗̟̳̜͈̗̺̟̠̠͖͓̖̪͕̠̕̕͝ͅả̸̴̡̡̧͠͞͡͞҉̛̕͟͏̷̘̪̱͈̲͉̞̠̞̪̫͎̲̬̖̀̀͟͝͞͞͠p̛͂̈͐̚͠҉̵̸̡̢̢̩̹͙̯͖̙̙̮̥̙͚̠͔̥̭̮̞̣̪̬̥̠̖̝̥̪͎́̀̕͜͡͡ͅͅh̵̷̵̡̛ͤ̂͌̐̓̐̋̋͊̒̆̽́̀̀̀͢͠͞͞҉̷̸̢̕҉͚̯͖̫̜̞̟̠̱͉̝̲̹̼͉̟͉̩̮͔̤͖̞̭̙̹̬ͅ<";
+ const char* text = ">S͛ͭ̋͆̈̔̇͗̍͑̎ͪͮͧͣ̽ͫͣ́ͬ̀͌͑͂͗͒̍̔̄ͧ̏̉̌̊̊̿̀̌̃̄͐̓̓̚̚҉̵̡͜͟͝͠͏̸̵̡̧͜҉̷̡͇̜̘̻̺̘̟̝͙̬̘̩͇̭̼̥̖̤̦͎k͉̩̘͚̜̹̗̗͍̤̥̱͉̳͕͖̤̲̣͚̮̞̬̲͍͔̯̻̮̞̭͈̗̫͓̂ͨ̉ͪ̒͋͛̀̍͊ͧ̿̅͆̓̔̔ͬ̇̑̿ͩ͗ͮ̎͌̿̄ͅP̴̵̡̡̛̪͙̼̣̟̩̭̫̱͙̬͔͉͍̘̠͉̦̝̘̥̟̗͖̫̤͕̙̬̦͍̱̖̮̱͑͐̎̃̒͐͋̚͘͞a̶̶̵̵̵̶̶̡̧̢̢̺͔̣͖̭̺͍̤͚̱̜̰̥͕̬̥̲̞̥̘͇͚̺̰͚̪̺͔̤͍̓̿͆̎͋̓ͦ̈́ͦ̌́̄͗̌̓͌̕͜͜͟͢͝͡ŕ͎̝͕͉̻͎̤̭͚̗̳̖̙̘͚̫͖͓͚͉͔͈̟̰̟̬̗͓̟͚̱̕͡ͅͅͅa̸̶̢̛̛̽ͮͩ̅͒ͫ͗͂̎ͦ̈́̓̚͘͜͢͡҉̷̵̶̢̡̜̮̦̜̥̜̯̙͓͔̼̗̻͜͜ͅḡ̢̛͕̗͖̖̤̦̘͔ͨͨ̊͒ͩͭͤ̍̅̃ͪ̋̏̓̍̋͗̋ͨ̏̽̈́̔̀̋̉ͫ̅̂ͭͫ̏͒͋ͥ̚͜r̶̢̧̧̥̤̼̀̂̒ͪ͌̿͌̅͛ͨͪ͒̍ͥ̉ͤ̌̿̆́ͭ͆̃̒ͤ͛̊ͧ̽͘͝͠a̧̢̧̢͑͑̓͑ͮ̃͂̄͛́̈́͋̂͌̽̄͒̔́̇ͨͧͭ͐ͦ̋ͨ̍ͦ̍̋͆̔ͧ͑͋͌̈̓͛͛̚͢͜͜͏̴̢̧̛̳͍̹͚̰̹̻͔p̨̡͆ͦͣ͊̽̔͂̉ͣ̔ͣ̌̌̉̃̋̂͒ͫ̄̎̐͗̉̌̃̽̽́̀̚͘͜͟҉̱͉h̭̮̘̗͔̜̯͔͈̯̺͔̗̣̭͚̱̰̙̼̹͚̣̻̥̲̮͍̤͜͝<";
ParagraphStyle paragraph_style;
ParagraphBuilderImpl builder(paragraph_style, fontCollection);
SkPaint paint;
@@ -1628,13 +1630,11 @@
for (size_t i = 0; i < run.size(); ++i) {
auto glyph = run.glyphs().begin() + i;
if (*glyph == 0) {
- SkDebugf("Run[%d] @pos=%d\n", run.index(), i);
- SkASSERT(false);
+ //SkDebugf("Run[%d] @pos=%d\n", run.index(), i);
}
}
} else {
- SkDebugf("Run[%d]: %s\n", run.index(), fontFamily.c_str());
- SkASSERT(false);
+ //SkDebugf("Run[%d]: %s\n", run.index(), fontFamily.c_str());
}
}
fRedraw = false;
@@ -2190,16 +2190,32 @@
f2.position, f2.affinity == Affinity::kUpstream ? "up" : "down",
i.position, i.affinity == Affinity::kUpstream ? "up" : "down");
- auto rf1 = paragraph->getRectsForRange(0, 1, RectHeightStyle::kTight, RectWidthStyle::kTight)[0];
- auto rf2 = paragraph->getRectsForRange(1, 2, RectHeightStyle::kTight, RectWidthStyle::kTight)[0];
- auto rfi = paragraph->getRectsForRange(2, 3, RectHeightStyle::kTight, RectWidthStyle::kTight)[0];
+ auto f1 = paragraph->getRectsForRange(0, 1, RectHeightStyle::kTight, RectWidthStyle::kTight);
+ if (f1.empty()) {
+ SkDebugf("F1 is empty\n");
+ } else {
+ auto rf1 = f1[0];
+ SkDebugf("f1: [%f:%f] %s\n",
+ rf1.rect.fLeft, rf1.rect.fRight, rf1.direction == TextDirection::kRtl ? "rtl" : "ltr");
+ }
- SkDebugf("f1: [%f:%f] %s\n",
- rf1.rect.fLeft, rf1.rect.fRight, rf1.direction == TextDirection::kRtl ? "rtl" : "ltr");
- SkDebugf("f2: [%f:%f] %s\n",
- rf2.rect.fLeft, rf2.rect.fRight, rf2.direction == TextDirection::kRtl ? "rtl" : "ltr");
- SkDebugf("i: [%f:%f] %s\n",
- rfi.rect.fLeft, rfi.rect.fRight, rfi.direction == TextDirection::kRtl ? "rtl" : "ltr");
+ auto f2 = paragraph->getRectsForRange(1, 2, RectHeightStyle::kTight, RectWidthStyle::kTight);
+ if (f2.empty()) {
+ SkDebugf("F2 is empty\n");
+ } else {
+ auto rf2 = f2[0];
+ SkDebugf("f2: [%f:%f] %s\n",
+ rf2.rect.fLeft, rf2.rect.fRight, rf2.direction == TextDirection::kRtl ? "rtl" : "ltr");
+ }
+
+ auto fi = paragraph->getRectsForRange(2, 3, RectHeightStyle::kTight, RectWidthStyle::kTight);
+ if (fi.empty()) {
+ SkDebugf("FI is empty\n");
+ } else {
+ auto rfi = fi[0];
+ SkDebugf("i: [%f:%f] %s\n",
+ rfi.rect.fLeft, rfi.rect.fRight, rfi.direction == TextDirection::kRtl ? "rtl" : "ltr");
+ }
}
}
@@ -2846,7 +2862,7 @@
DEF_SAMPLE(return new ParagraphView16();)
DEF_SAMPLE(return new ParagraphView17();)
DEF_SAMPLE(return new ParagraphView18();)
-//DEF_SAMPLE(return new ParagraphView19();)
+DEF_SAMPLE(return new ParagraphView19();)
DEF_SAMPLE(return new ParagraphView20();)
DEF_SAMPLE(return new ParagraphView21();)
DEF_SAMPLE(return new ParagraphView22();)
diff --git a/modules/skparagraph/src/OneLineShaper.cpp b/modules/skparagraph/src/OneLineShaper.cpp
index 5f93b4b..c09b298 100644
--- a/modules/skparagraph/src/OneLineShaper.cpp
+++ b/modules/skparagraph/src/OneLineShaper.cpp
@@ -26,33 +26,32 @@
auto oldUnresolvedCount = fUnresolvedBlocks.size();
// Find all unresolved blocks
- bool nothingWasUnresolved = true;
sortOutGlyphs([&](GlyphRange block){
if (block.width() == 0) {
return;
}
- nothingWasUnresolved = false;
addUnresolvedWithRun(block);
});
// Fill all the gaps between unresolved blocks with resolved ones
- if (nothingWasUnresolved) {
+ if (oldUnresolvedCount == fUnresolvedBlocks.size()) {
// No unresolved blocks added - we resolved the block with one run entirely
addFullyResolved();
return;
+ } else if (oldUnresolvedCount == fUnresolvedBlocks.size() - 1) {
+ auto& unresolved = fUnresolvedBlocks.back();
+ if (fCurrentRun->textRange() == unresolved.fText) {
+ // Nothing was resolved; preserve the initial run if it makes sense
+ auto& front = fUnresolvedBlocks.front();
+ if (front.fRun != nullptr) {
+ unresolved.fRun = front.fRun;
+ unresolved.fGlyphs = front.fGlyphs;
+ }
+ return;
+ }
}
- auto& front = fUnresolvedBlocks.front(); // The one we need to resolve
- auto& back = fUnresolvedBlocks.back(); // The one we have from shaper
- if (fUnresolvedBlocks.size() == oldUnresolvedCount + 1 &&
- front.fText == back.fText) {
- // The entire block remains unresolved!
- if (front.fRun != nullptr) {
- back.fRun = front.fRun;
- }
- } else {
- fillGaps(oldUnresolvedCount);
- }
+ fillGaps(oldUnresolvedCount);
}
#ifdef SK_DEBUG
@@ -94,9 +93,18 @@
auto begin = fUnresolvedBlocks.begin();
auto end = fUnresolvedBlocks.end();
begin += startingCount; // Skip the old ones, do the new ones
+ TextRange prevText = EMPTY_TEXT;
for (; begin != end; ++begin) {
auto& unresolved = *begin;
+ if (unresolved.fText == prevText) {
+ // Clean up repetitive blocks that appear inside the same grapheme block
+ unresolved.fText = EMPTY_TEXT;
+ continue;
+ } else {
+ prevText = unresolved.fText;
+ }
+
TextRange resolvedText(resolvedTextStart, fCurrentRun->leftToRight() ? unresolved.fText.start : unresolved.fText.end);
if (resolvedText.width() > 0) {
if (!fCurrentRun->leftToRight()) {
diff --git a/modules/skparagraph/src/OneLineShaper.h b/modules/skparagraph/src/OneLineShaper.h
index 1b916ef..569fadb 100644
--- a/modules/skparagraph/src/OneLineShaper.h
+++ b/modules/skparagraph/src/OneLineShaper.h
@@ -42,8 +42,8 @@
// Entire run comes as one block fully resolved
explicit RunBlock(std::shared_ptr<Run> run)
: fRun(std::move(run))
- , fText(run->fTextRange)
- , fGlyphs(GlyphRange(0, run->size())) { }
+ , fText(fRun->fTextRange)
+ , fGlyphs(GlyphRange(0, fRun->size())) { }
std::shared_ptr<Run> fRun;
TextRange fText;
diff --git a/modules/skparagraph/src/TextLine.cpp b/modules/skparagraph/src/TextLine.cpp
index 583e868..a7b6059 100644
--- a/modules/skparagraph/src/TextLine.cpp
+++ b/modules/skparagraph/src/TextLine.cpp
@@ -370,13 +370,13 @@
// TODO: This is the change for flutter, must be removed later
SkScalar correctedBaseline = SkScalarFloorToScalar(this->baseline() + 0.5);
SkTextBlobBuilder builder;
- context.run->copyTo(builder, SkToU32(context.pos), context.size, SkVector::Make(0, correctedBaseline));
+ context.run->copyTo(builder, SkToU32(context.pos), context.size, SkVector::Make(context.fTextShift, correctedBaseline));
if (context.clippingNeeded) {
canvas->save();
canvas->clipRect(extendHeight(context).makeOffset(this->offset()));
}
- canvas->drawTextBlob(builder.make(), this->offset().fX + context.fTextShift, this->offset().fY, paint);
+ canvas->drawTextBlob(builder.make(), this->offset().fX, this->offset().fY, paint);
if (context.clippingNeeded) {
canvas->restore();
@@ -403,7 +403,7 @@
}
SkTextBlobBuilder builder;
- context.run->copyTo(builder, context.pos, context.size, SkVector::Make(0, shiftDown));
+ context.run->copyTo(builder, context.pos, context.size, SkVector::Make(context.fTextShift, shiftDown));
if (context.clippingNeeded) {
canvas->save();
@@ -411,7 +411,7 @@
clip.offset(this->offset());
canvas->clipRect(clip);
}
- canvas->drawTextBlob(builder.make(), this->offset().fX + context.fTextShift + shadow.fOffset.x(), this->offset().fY + shadow.fOffset.y(), paint);
+ canvas->drawTextBlob(builder.make(), this->offset().fX + shadow.fOffset.x(), this->offset().fY + shadow.fOffset.y(), paint);
if (context.clippingNeeded) {
canvas->restore();
@@ -678,6 +678,12 @@
result.clip.fRight = fAdvance.fX;
}
+ if (result.clip.width() < 0) {
+ // Weird situation when glyph offsets move the glyph to the left
+ // (happens with zalgo texts, for instance)
+ result.clip.fRight = result.clip.fLeft;
+ }
+
// The text must be aligned with the lineOffset
result.fTextShift = textStartInLine - textStartInRun;
diff --git a/modules/skparagraph/src/TypefaceFontProvider.cpp b/modules/skparagraph/src/TypefaceFontProvider.cpp
index cb5c262..d1b9f26 100644
--- a/modules/skparagraph/src/TypefaceFontProvider.cpp
+++ b/modules/skparagraph/src/TypefaceFontProvider.cpp
@@ -76,7 +76,9 @@
}
void TypefaceFontStyleSet::appendTypeface(sk_sp<SkTypeface> typeface) {
- fStyles.emplace_back(std::move(typeface));
+ if (typeface.get() != nullptr) {
+ fStyles.emplace_back(std::move(typeface));
+ }
}
} // namespace textlayout
