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

This reverts commit b776a105e5.

Reason for revert: This will be the first step of a staged change.

Original change's description:
> 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>

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

Change-Id: I0f2f46fc21eec795026ea98696f68112915f7979
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10715
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334159
Commit-Queue: Julia Lavrova <jlavrova@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
This commit is contained in:
Julia Lavrova 2020-11-11 20:49:40 +00:00 committed by Skia Commit-Bot
parent a1e8fe3a2b
commit c90084a9a3
3 changed files with 105 additions and 11 deletions

View File

@ -3151,6 +3151,66 @@ 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
@ -3204,3 +3264,4 @@ 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,8 +303,13 @@ 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)
#ifdef SK_PARAGRAPH_LIBTXT_SPACES_RESOLUTION
bool isWhitespace8 = false; // fParagraph->getUnicode()->isWhitespace(codepoint);
#else
bool isWhitespace8 = fParagraph->getUnicode()->isWhitespace(codepoint);
#endif
// Inspect the glyph
auto glyph = fCurrentRun->fGlyphs[i];
if (glyph == 0 && !isControl8) { // Unresolved glyph and not control codepoint

View File

@ -1239,7 +1239,11 @@ DEF_TEST(SkParagraph_HeightOverrideParagraph, reporter) {
paragraph->layout(550);
auto impl = static_cast<ParagraphImpl*>(paragraph.get());
#ifdef SK_PARAGRAPH_LIBTXT_SPACES_RESOLUTION
REPORTER_ASSERT(reporter, impl->runs().size() == 5);
#else
REPORTER_ASSERT(reporter, impl->runs().size() == 4);
#endif
REPORTER_ASSERT(reporter, impl->styles().size() == 1); // paragraph style does not count
REPORTER_ASSERT(reporter, impl->styles()[0].fStyle.equals(text_style));
@ -3992,28 +3996,38 @@ DEF_TEST(SkParagraph_FontFallbackParagraph, reporter) {
paragraph->layout(TestCanvasWidth);
paragraph->paint(canvas.get(), 10.0, 15.0);
#ifdef SK_PARAGRAPH_LIBTXT_SPACES_RESOLUTION
size_t spaceRun = 1;
REPORTER_ASSERT(reporter, paragraph->unresolvedGlyphs() == 2); // From the text1 ("字典" - excluding the last space)
#else
size_t spaceRun = 0;
REPORTER_ASSERT(reporter, paragraph->unresolvedGlyphs() == 3); // From the text1 ("字典 " - including the last space)
#endif
auto impl = static_cast<ParagraphImpl*>(paragraph.get());
REPORTER_ASSERT(reporter, impl->runs().size() == 6);
REPORTER_ASSERT(reporter, impl->runs().size() == 6 + spaceRun);
// 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
// [Apple + Noto] 2, 3
// [Apple + Han] 4, 5
// [Apple + Unresolved + ' '] 0, 1, 2
// [Apple + Noto] 3, 4
// [Apple + Han] 5, 6
auto robotoAdvance = impl->runs()[0].advance().fX +
impl->runs()[1].advance().fX;
#ifdef SK_PARAGRAPH_LIBTXT_SPACES_RESOLUTION
robotoAdvance += impl->runs()[2].advance().fX;
#endif
REPORTER_ASSERT(reporter, SkScalarNearlyEqual(robotoAdvance, 64.199f, EPSILON50));
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));
REPORTER_ASSERT(reporter, SkScalarNearlyEqual(impl->runs()[2 + spaceRun].advance().fX, 139.125f, EPSILON100));
REPORTER_ASSERT(reporter, SkScalarNearlyEqual(impl->runs()[3 + spaceRun].advance().fX, 27.999f, EPSILON100));
REPORTER_ASSERT(reporter, SkScalarNearlyEqual(impl->runs()[4 + spaceRun].advance().fX, 62.248f, EPSILON100));
REPORTER_ASSERT(reporter, SkScalarNearlyEqual(impl->runs()[5 + spaceRun].advance().fX, 27.999f, EPSILON100));
// When a different font is resolved, then the metrics are different.
REPORTER_ASSERT(reporter, impl->runs()[3].correctAscent() != impl->runs()[5].correctAscent());
REPORTER_ASSERT(reporter, impl->runs()[3].correctDescent() != impl->runs()[5].correctDescent());
REPORTER_ASSERT(reporter, impl->runs()[3 + spaceRun].correctAscent() != impl->runs()[5 + spaceRun].correctAscent());
REPORTER_ASSERT(reporter, impl->runs()[3 + spaceRun].correctDescent() != impl->runs()[5 + spaceRun].correctDescent());
}
// Checked: NO DIFF
@ -5495,7 +5509,11 @@ DEF_TEST(SkParagraph_FontResolutionInRTL, reporter) {
paragraph->paint(canvas.get(), 0, 0);
auto impl = static_cast<ParagraphImpl*>(paragraph.get());
#ifdef SK_PARAGRAPH_LIBTXT_SPACES_RESOLUTION
REPORTER_ASSERT(reporter, impl->runs().size() == (10 + 11));
#else
REPORTER_ASSERT(reporter, impl->runs().size() == 1);
#endif
}
DEF_TEST(SkParagraph_FontResolutionInLTR, reporter) {
@ -5522,8 +5540,18 @@ DEF_TEST(SkParagraph_FontResolutionInLTR, reporter) {
paragraph->paint(canvas.get(), 0, 0);
auto impl = static_cast<ParagraphImpl*>(paragraph.get());
#ifdef SK_PARAGRAPH_LIBTXT_SPACES_RESOLUTION
REPORTER_ASSERT(reporter, impl->runs().size() == 5);
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"
#else
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() == 5); // "{unresolved} {unresolved}"
REPORTER_ASSERT(reporter, impl->runs()[2].textRange().width() == 4); // " def"
#endif
}