Some testing images in Google3 look slightly different with this change. Need to change them, too.

Revert "Roll back #10487 fix (so now spaces always resolved in the main font)"

This reverts commit ff5d8700b3.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> Roll back #10487 fix (so now spaces always resolved in the main font)
>
> Bug: skia:10715
> Change-Id: I06bb881998d00228c79a218967eda6468cbc5bed
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334053
> Reviewed-by: Ben Wagner <bungeman@google.com>
> Commit-Queue: Julia Lavrova <jlavrova@google.com>

TBR=bungeman@google.com,jlavrova@google.com

Change-Id: Icc1f262f60fa2ec9b4425ce3088b5eaac9313374
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10715
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334158
Reviewed-by: Julia Lavrova <jlavrova@google.com>
Commit-Queue: Julia Lavrova <jlavrova@google.com>
This commit is contained in:
Julia Lavrova 2020-11-11 20:46:52 +00:00 committed by Skia Commit-Bot
parent 3a004dfd91
commit b776a105e5
3 changed files with 18 additions and 84 deletions

View File

@ -3151,66 +3151,6 @@ private:
using INHERITED = Sample;
};
class ParagraphView52 : public ParagraphView_Base {
protected:
SkString name() override { return SkString("Paragraph52"); }
void onDrawContent(SkCanvas* canvas) override {
canvas->drawColor(SK_ColorWHITE);
//const char* text = "😀😃😄 ABC 😀😃😄 DEF GHI";
auto fontCollection = sk_make_sp<FontCollection>();
fontCollection->setDefaultFontManager(SkFontMgr::RefDefault());
fontCollection->enableFontFallback();
{
const char* text = " 😀 😃";
ParagraphStyle paragraph_style;
paragraph_style.turnHintingOff();
ParagraphBuilderImpl builder(paragraph_style, fontCollection);
TextStyle text_style;
//text_style.setFontFamilies({SkString("sans-serif")});
text_style.setFontFamilies({SkString("Roboto"), SkString("Noto Color Emoji")});
text_style.setFontSize(40);
text_style.setColor(SK_ColorBLACK);
builder.pushStyle(text_style);
builder.addText(text, strlen(text));
builder.pop();
auto paragraph = builder.Build();
paragraph->layout(width());
paragraph->paint(canvas, 0, 0);
}
{
const char* text = " 😀 A";
ParagraphStyle paragraph_style;
paragraph_style.turnHintingOff();
ParagraphBuilderImpl builder(paragraph_style, fontCollection);
TextStyle text_style;
//text_style.setFontFamilies({SkString("sans-serif")});
text_style.setFontFamilies({SkString("Roboto"), SkString("Noto Color Emoji")});
text_style.setFontSize(40);
text_style.setColor(SK_ColorBLACK);
builder.pushStyle(text_style);
builder.addText(text, strlen(text));
builder.pop();
auto paragraph = builder.Build();
paragraph->layout(width());
paragraph->paint(canvas, 0, 400);
}
}
private:
using INHERITED = Sample;
};
} // namespace
@ -3264,4 +3204,3 @@ DEF_SAMPLE(return new ParagraphView48();)
DEF_SAMPLE(return new ParagraphView49();)
DEF_SAMPLE(return new ParagraphView50();)
DEF_SAMPLE(return new ParagraphView51();)
DEF_SAMPLE(return new ParagraphView52();)

View File

@ -303,9 +303,7 @@ void OneLineShaper::sortOutGlyphs(std::function<void(GlyphRange)>&& sortOutUnres
const char* cluster = text.begin() + clusterIndex(i);
SkUnichar codepoint = nextUtf8Unit(&cluster, text.end());
bool isControl8 = fParagraph->getUnicode()->isControl(codepoint);
// TODO: This is a temp change to match space handiling in LibTxt
// (all spaces are resolved with the main font)
bool isWhitespace8 = false; // fParagraph->getUnicode()->isWhitespace(codepoint);
bool isWhitespace8 = fParagraph->getUnicode()->isWhitespace(codepoint);
// Inspect the glyph
auto glyph = fCurrentRun->fGlyphs[i];

View File

@ -1239,7 +1239,7 @@ DEF_TEST(SkParagraph_HeightOverrideParagraph, reporter) {
paragraph->layout(550);
auto impl = static_cast<ParagraphImpl*>(paragraph.get());
REPORTER_ASSERT(reporter, impl->runs().size() == 5);
REPORTER_ASSERT(reporter, impl->runs().size() == 4);
REPORTER_ASSERT(reporter, impl->styles().size() == 1); // paragraph style does not count
REPORTER_ASSERT(reporter, impl->styles()[0].fStyle.equals(text_style));
@ -3992,29 +3992,28 @@ DEF_TEST(SkParagraph_FontFallbackParagraph, reporter) {
paragraph->layout(TestCanvasWidth);
paragraph->paint(canvas.get(), 10.0, 15.0);
REPORTER_ASSERT(reporter, paragraph->unresolvedGlyphs() == 2); // From the text1 ("字典 " - excluding the last space)
REPORTER_ASSERT(reporter, paragraph->unresolvedGlyphs() == 3); // From the text1 ("字典 " - including the last space)
auto impl = static_cast<ParagraphImpl*>(paragraph.get());
REPORTER_ASSERT(reporter, impl->runs().size() == 7);
REPORTER_ASSERT(reporter, impl->runs().size() == 6);
// Font resolution in Skia produces 6 runs because 2 parts of "Roboto 字典 " have different
// script (Minikin merges the first 2 into one because of unresolved)
// [Apple + Unresolved + ' '] 0, 1, 2
// [Apple + Noto] 3, 4
// [Apple + Han] 5, 6
// [Apple + Unresolved ] 0, 1
// [Apple + Noto] 2, 3
// [Apple + Han] 4, 5
auto robotoAdvance = impl->runs()[0].advance().fX +
impl->runs()[1].advance().fX +
impl->runs()[2].advance().fX;
impl->runs()[1].advance().fX;
REPORTER_ASSERT(reporter, SkScalarNearlyEqual(robotoAdvance, 64.199f, EPSILON50));
REPORTER_ASSERT(reporter, SkScalarNearlyEqual(impl->runs()[3].advance().fX, 139.125f, EPSILON100));
REPORTER_ASSERT(reporter, SkScalarNearlyEqual(impl->runs()[4].advance().fX, 27.999f, EPSILON100));
REPORTER_ASSERT(reporter, SkScalarNearlyEqual(impl->runs()[5].advance().fX, 62.248f, EPSILON100));
REPORTER_ASSERT(reporter, SkScalarNearlyEqual(impl->runs()[6].advance().fX, 27.999f, EPSILON100));
REPORTER_ASSERT(reporter, SkScalarNearlyEqual(impl->runs()[2].advance().fX, 139.125f, EPSILON100));
REPORTER_ASSERT(reporter, SkScalarNearlyEqual(impl->runs()[3].advance().fX, 27.999f, EPSILON100));
REPORTER_ASSERT(reporter, SkScalarNearlyEqual(impl->runs()[4].advance().fX, 62.248f, EPSILON100));
REPORTER_ASSERT(reporter, SkScalarNearlyEqual(impl->runs()[5].advance().fX, 27.999f, EPSILON100));
// When a different font is resolved, then the metrics are different.
REPORTER_ASSERT(reporter, impl->runs()[4].correctAscent() != impl->runs()[6].correctAscent());
REPORTER_ASSERT(reporter, impl->runs()[4].correctDescent() != impl->runs()[6].correctDescent());
REPORTER_ASSERT(reporter, impl->runs()[3].correctAscent() != impl->runs()[5].correctAscent());
REPORTER_ASSERT(reporter, impl->runs()[3].correctDescent() != impl->runs()[5].correctDescent());
}
// Checked: NO DIFF
@ -5496,7 +5495,7 @@ DEF_TEST(SkParagraph_FontResolutionInRTL, reporter) {
paragraph->paint(canvas.get(), 0, 0);
auto impl = static_cast<ParagraphImpl*>(paragraph.get());
REPORTER_ASSERT(reporter, impl->runs().size() == (10 + 11));
REPORTER_ASSERT(reporter, impl->runs().size() == 1);
}
DEF_TEST(SkParagraph_FontResolutionInLTR, reporter) {
@ -5523,10 +5522,8 @@ DEF_TEST(SkParagraph_FontResolutionInLTR, reporter) {
paragraph->paint(canvas.get(), 0, 0);
auto impl = static_cast<ParagraphImpl*>(paragraph.get());
REPORTER_ASSERT(reporter, impl->runs().size() == 5);
REPORTER_ASSERT(reporter, impl->runs().size() == 3);
REPORTER_ASSERT(reporter, impl->runs()[0].textRange().width() == 4); // "abc "
REPORTER_ASSERT(reporter, impl->runs()[1].textRange().width() == 2); // "{unresolved}"
REPORTER_ASSERT(reporter, impl->runs()[2].textRange().width() == 1); // " "
REPORTER_ASSERT(reporter, impl->runs()[3].textRange().width() == 2); // "{unresolved}"
REPORTER_ASSERT(reporter, impl->runs()[4].textRange().width() == 4); // " def"
REPORTER_ASSERT(reporter, impl->runs()[1].textRange().width() == 5); // "{unresolved} {unresolved}"
REPORTER_ASSERT(reporter, impl->runs()[2].textRange().width() == 4); // " def"
}