Round unresolved font blocks to grapheme edges
I had to stage the change with SK_PARAGRAPH_GRAPHEME_EDGES because of
some google3 tests.
Bug: skia:11196
Change-Id: Ic45e43e6c69f649c6bc0fedcc6f303891139b85e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359566
Commit-Queue: Julia Lavrova <jlavrova@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
diff --git a/modules/skparagraph/samples/SampleParagraph.cpp b/modules/skparagraph/samples/SampleParagraph.cpp
index 9e7faeb..1a761a9 100644
--- a/modules/skparagraph/samples/SampleParagraph.cpp
+++ b/modules/skparagraph/samples/SampleParagraph.cpp
@@ -3268,6 +3268,116 @@
using INHERITED = Sample;
};
+class ParagraphView54 : public ParagraphView_Base {
+protected:
+ SkString name() override { return SkString("Paragraph54"); }
+
+ void onDrawContent(SkCanvas* canvas) override {
+ canvas->drawColor(SK_ColorWHITE);
+ //std::string text("يَهْدِيْكُمُ اللَّهُ وَيُصْلِحُ بَالَكُمُ");
+ //auto text = "ד👨👩👧👦😀";
+ auto text = "👨👩👧👦😀";
+
+ //auto fontCollection = sk_make_sp<FontCollection>();
+ auto fontCollection = getFontCollection();
+ fontCollection->setDefaultFontManager(SkFontMgr::RefDefault());
+ fontCollection->enableFontFallback();
+ //fontCollection->disableFontFallback();
+
+ ParagraphStyle paragraph_style;
+ //paragraph_style.setTextDirection(TextDirection::kRtl);
+
+ ParagraphBuilderImpl builder(paragraph_style, fontCollection);
+ TextStyle text_style;
+ text_style.setFontFamilies({SkString("Noto Naskh Arabic")});
+ text_style.setFontSize(36);
+ text_style.setColor(SK_ColorBLACK);
+ builder.pushStyle(text_style);
+ builder.addText(text);
+
+ auto paragraph = builder.Build();
+ paragraph->layout(/*360*/width());
+ paragraph->paint(canvas, 0, 0);
+ }
+
+private:
+ using INHERITED = Sample;
+};
+
+class ParagraphView55 : public ParagraphView_Base {
+protected:
+ SkString name() override { return SkString("Paragraph55"); }
+
+ void onDrawContent(SkCanvas* canvas) override {
+ canvas->drawColor(SK_ColorWHITE);
+ std::string text("يَهْدِيْكُمُ اللَّهُ وَيُصْلِحُ بَالَكُمُ");
+
+ //auto fontCollection = sk_make_sp<FontCollection>();
+ //fontCollection->setDefaultFontManager(SkFontMgr::RefDefault());
+ //fontCollection->enableFontFallback();
+ auto fontCollection = getFontCollection();
+ fontCollection->disableFontFallback();
+
+ ParagraphStyle paragraph_style;
+ paragraph_style.setTextDirection(TextDirection::kRtl);
+
+ ParagraphBuilderImpl builder(paragraph_style, fontCollection);
+ TextStyle text_style;
+ text_style.setFontFamilies({SkString("Noto Naskh Arabic")});
+ text_style.setFontSize(64);
+ text_style.setColor(SK_ColorBLACK);
+ builder.pushStyle(text_style);
+ builder.addText(text.substr(0, 10).data());
+ text_style.setColor(SK_ColorRED);
+ builder.pushStyle(text_style);
+ builder.addText(text.substr(10, 20).data());
+ text_style.setColor(SK_ColorBLACK);
+ builder.pushStyle(text_style);
+ builder.addText(text.substr(30, 50).data());
+
+ auto paragraph = builder.Build();
+ paragraph->layout(/*360*/width());
+ paragraph->paint(canvas, 0, 0);
+ }
+
+private:
+ using INHERITED = Sample;
+};
+
+class ParagraphView56 : public ParagraphView_Base {
+protected:
+ SkString name() override { return SkString("Paragraph56"); }
+
+ void onDrawContent(SkCanvas* canvas) override {
+ canvas->drawColor(SK_ColorWHITE);
+ auto text = "BAM BAM BAM by Jade Baraldo\n"
+ "Now on Top 100 Music Videos United States";
+
+ auto fontCollection = sk_make_sp<TestFontCollection>(GetResourcePath("fonts").c_str(), false);
+ fontCollection->addFontFromFile("music/Roboto-Regular.ttf", "roboto");
+ fontCollection->addFontFromFile("music/NotoSansCJK-Regular.ttc", "noto");
+ fontCollection->addFontFromFile("music/NotoColorEmoji.ttf", "emoji");
+
+ ParagraphStyle paragraph_style;
+ ParagraphBuilderImpl builder(paragraph_style, fontCollection);
+ TextStyle text_style;
+ //text_style.setFontFamilies({SkString("Noto Naskh Arabic")});
+ text_style.setFontFamilies({SkString("roboto"),
+ SkString("noto"),
+ SkString("emoji")});
+ text_style.setFontSize(20);
+ text_style.setColor(SK_ColorBLACK);
+ builder.pushStyle(text_style);
+ builder.addText(text);
+ auto paragraph = builder.Build();
+ paragraph->layout(width());
+ paragraph->paint(canvas, 0, 0);
+ }
+
+private:
+ using INHERITED = Sample;
+};
+
} // namespace
//////////////////////////////////////////////////////////////////////////////
@@ -3322,3 +3432,6 @@
DEF_SAMPLE(return new ParagraphView51();)
DEF_SAMPLE(return new ParagraphView52();)
DEF_SAMPLE(return new ParagraphView53();)
+DEF_SAMPLE(return new ParagraphView54();)
+DEF_SAMPLE(return new ParagraphView55();)
+DEF_SAMPLE(return new ParagraphView56();)
diff --git a/modules/skparagraph/src/OneLineShaper.cpp b/modules/skparagraph/src/OneLineShaper.cpp
index 3beb5dc..d1707b8 100644
--- a/modules/skparagraph/src/OneLineShaper.cpp
+++ b/modules/skparagraph/src/OneLineShaper.cpp
@@ -19,7 +19,12 @@
fCurrentRun->commit();
auto oldUnresolvedCount = fUnresolvedBlocks.size();
-
+/*
+ SkDebugf("Run [%d:%d)\n", fCurrentRun->fTextRange.start, fCurrentRun->fTextRange.end);
+ for (size_t i = 0; i < fCurrentRun->size(); ++i) {
+ SkDebugf("[%d] %d %d %f\n", i, fCurrentRun->fGlyphs[i], fCurrentRun->fClusterIndexes[i], fCurrentRun->fPositions[i].fX);
+ }
+*/
// Find all unresolved blocks
sortOutGlyphs([&](GlyphRange block){
if (block.width() == 0) {
@@ -291,6 +296,60 @@
// Glue whitespaces to the next/prev unresolved blocks
// (so we don't have chinese text with english whitespaces broken into millions of tiny runs)
+#ifndef SK_PARAGRAPH_GRAPHEME_EDGES
+void OneLineShaper::sortOutGlyphs(std::function<void(GlyphRange)>&& sortOutUnresolvedBLock) {
+
+ size_t unresolvedGlyphs = 0;
+
+ GlyphRange block = EMPTY_RANGE;
+ bool graphemeResolved = false;
+ TextIndex graphemeStart = EMPTY_INDEX;
+ for (size_t i = 0; i < fCurrentRun->size(); ++i) {
+
+ ClusterIndex ci = clusterIndex(i);
+ // Removing all pretty optimizations for whitespaces
+ // because they get in a way of grapheme rounding
+ // Inspect the glyph
+ auto glyph = fCurrentRun->fGlyphs[i];
+
+ GraphemeIndex gi = fParagraph->findPreviousGraphemeBoundary(ci);
+ if ((fCurrentRun->leftToRight() ? gi > graphemeStart : gi < graphemeStart) || graphemeStart == EMPTY_INDEX) {
+ // We only count glyph resolved if all the glyphs in its grapheme are resolved
+ graphemeResolved = glyph != 0;
+ graphemeStart = gi;
+ } else if (glyph == 0) {
+ // Found unresolved glyph - the entire grapheme is unresolved now
+ graphemeResolved = false;
+ }
+
+ if (!graphemeResolved) { // Unresolved glyph and not control codepoint
+ ++unresolvedGlyphs;
+ if (block.start == EMPTY_INDEX) {
+ // Start new unresolved block
+ block.start = i;
+ block.end = EMPTY_INDEX;
+ } else {
+ // Keep skipping unresolved block
+ }
+ } else { // Resolved glyph or control codepoint
+ if (block.start == EMPTY_INDEX) {
+ // Keep skipping resolved code points
+ } else {
+ // This is the end of unresolved block
+ block.end = i;
+ sortOutUnresolvedBLock(block);
+ block = EMPTY_RANGE;
+ }
+ }
+ }
+
+ // One last block could have been left
+ if (block.start != EMPTY_INDEX) {
+ block.end = fCurrentRun->size();
+ sortOutUnresolvedBLock(block);
+ }
+}
+#else
void OneLineShaper::sortOutGlyphs(std::function<void(GlyphRange)>&& sortOutUnresolvedBLock) {
auto text = fCurrentRun->fOwner->text();
@@ -353,6 +412,7 @@
sortOutUnresolvedBLock(block);
}
}
+#endif
void OneLineShaper::iterateThroughFontStyles(TextRange textRange,
SkSpan<Block> styleSpan,
diff --git a/modules/skparagraph/tests/SkParagraphTest.cpp b/modules/skparagraph/tests/SkParagraphTest.cpp
index b6b5a9c..154a22e 100644
--- a/modules/skparagraph/tests/SkParagraphTest.cpp
+++ b/modules/skparagraph/tests/SkParagraphTest.cpp
@@ -45,7 +45,6 @@
struct GrContextOptions;
-
#define VeryLongCanvasWidth 1000000
#define TestCanvasWidth 1000
#define TestCanvasHeight 600
@@ -1239,11 +1238,7 @@
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));
@@ -3990,13 +3985,8 @@
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());
@@ -4009,9 +3999,7 @@
// [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 + spaceRun].advance().fX, 139.125f, EPSILON100));
@@ -5503,11 +5491,7 @@
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) {
@@ -5534,20 +5518,12 @@
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
}
DEF_TEST(SkParagraph_Intrinsic, reporter) {
@@ -5818,3 +5794,5 @@
REPORTER_ASSERT(reporter, SkScalarNearlyEqual(len1, 300.0f));
REPORTER_ASSERT(reporter, SkScalarNearlyEqual(len2, 250.0f));
}
+
+// "\u05D3\u{1F468}\u200D\u{1F469}\u200D\u{1F467}\u200D\u{1F466}\uD83D\uDE00"
diff --git a/public.bzl b/public.bzl
index d8421a9..d33a9bc 100644
--- a/public.bzl
+++ b/public.bzl
@@ -723,6 +723,7 @@
# Google3 probably doesn't want this feature yet
"SK_DISABLE_REDUCE_OPLIST_SPLITTING",
# Staging flags for API changes
+ "SK_PARAGRAPH_GRAPHEME_EDGES",
# Should remove after we update golden images
"SK_WEBP_ENCODER_USE_DEFAULT_METHOD",
# Experiment to diagnose image diffs in Google3
