Bugs
Change-Id: I3d45c6c8cef0109377812de0a3aab5d457a29b86
Bugs: skia:9849, skia:9850
9849 is related to font resolution (we may have to try different fallback
fonts to resolve all codepoints)
9850 is related to finding a position inside a glyph ("ffi" is an example)
Change-Id: I3d45c6c8cef0109377812de0a3aab5d457a29b86
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/271745
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Julia Lavrova <jlavrova@google.com>
diff --git a/modules/skparagraph/src/OneLineShaper.cpp b/modules/skparagraph/src/OneLineShaper.cpp
index e52e6d1..5ba6e8a 100644
--- a/modules/skparagraph/src/OneLineShaper.cpp
+++ b/modules/skparagraph/src/OneLineShaper.cpp
@@ -44,7 +44,7 @@
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.width() == back.fText.width()) {
+ front.fText == back.fText) {
// The entire block remains unresolved!
if (front.fRun != nullptr) {
back.fRun = front.fRun;
@@ -394,25 +394,38 @@
std::vector<sk_sp<SkTypeface>> typefaces = fParagraph->fFontCollection->findTypefaces(textStyle.getFontFamilies(), textStyle.getFontStyle());
for (const auto& typeface : typefaces) {
- if (!visitor(typeface))
+ if (!visitor(typeface)) {
+ // Resolved everything
return;
+ }
}
if (fParagraph->fFontCollection->fontFallbackEnabled()) {
// Give fallback a clue
- SkASSERT(!fUnresolvedBlocks.empty());
- auto unresolvedRange = fUnresolvedBlocks.front().fText;
- auto unresolvedText = fParagraph->text(unresolvedRange);
- const char* ch = unresolvedText.begin();
- SkUnichar unicode = utf8_next(&ch, unresolvedText.end());
+ // Some unresolved subblocks might be resolved with different fallback fonts
+ while (!fUnresolvedBlocks.empty()) {
+ auto unresolvedRange = fUnresolvedBlocks.front().fText;
+ auto unresolvedText = fParagraph->text(unresolvedRange);
+ const char* ch = unresolvedText.begin();
+ SkUnichar unicode = utf8_next(&ch, unresolvedText.end());
- auto typeface = fParagraph->fFontCollection->defaultFallback(
- unicode, textStyle.getFontStyle(), textStyle.getLocale());
+ auto typeface = fParagraph->fFontCollection->defaultFallback(
+ unicode, textStyle.getFontStyle(), textStyle.getLocale());
- if (typeface != nullptr) {
- if (!visitor(typeface)) {
+ if (typeface == nullptr) {
return;
}
+
+ if (!visitor(typeface)) {
+ // Resolved everything
+ return;
+ } else {
+ // Check if anything was resolved and stop it it was not
+ auto last = fUnresolvedBlocks.back();
+ if (unresolvedRange == last.fText) {
+ return;
+ }
+ }
}
}
}
diff --git a/modules/skparagraph/src/ParagraphImpl.cpp b/modules/skparagraph/src/ParagraphImpl.cpp
index 45c7bd4..13e5e9b 100644
--- a/modules/skparagraph/src/ParagraphImpl.cpp
+++ b/modules/skparagraph/src/ParagraphImpl.cpp
@@ -933,6 +933,8 @@
auto glyphStart = context.run->positionX(found) + context.fTextShift + offsetX;
auto glyphWidth = context.run->positionX(found + 1) - context.run->positionX(found);
auto clusterIndex8 = context.run->fClusterIndexes[found];
+ auto clusterEnd8 = context.run->fClusterIndexes[found + 1];
+ TextRange clusterText (clusterIndex8, clusterEnd8);
// Find the grapheme positions in codepoints that contains the point
auto codepoint = std::lower_bound(
@@ -941,21 +943,29 @@
[](const Codepoint& lhs,size_t rhs) -> bool { return lhs.fTextIndex < rhs; });
auto codepointIndex = codepoint - fCodePoints.begin();
- auto codepoints = fGraphemes16[codepoint->fGrapheme].fCodepointRange;
+ CodepointRange codepoints(codepointIndex, codepointIndex);
+ for (codepoints.end = codepointIndex + 1; codepoints.end < fCodePoints.size(); ++codepoints.end) {
+ auto& cp = fCodePoints[codepoints.end];
+ if (cp.fTextIndex >= clusterText.end) {
+ break;
+ }
+ }
auto graphemeSize = codepoints.width();
// We only need to inspect one glyph (maybe not even the entire glyph)
SkScalar center;
+ bool insideGlyph = false;
if (graphemeSize > 1) {
- auto averageCodepoint = glyphWidth / graphemeSize;
- auto codepointStart = glyphStart + averageCodepoint * (codepointIndex - codepoints.start);
- auto codepointEnd = codepointStart + averageCodepoint;
- center = (codepointStart + codepointEnd) / 2;
+ auto averageCodepointWidth = glyphWidth / graphemeSize;
+ auto delta = dx - glyphStart;
+ auto insideIndex = SkScalarFloorToInt(delta / averageCodepointWidth);
+ insideGlyph = delta > averageCodepointWidth;
+ center = glyphStart + averageCodepointWidth * insideIndex + averageCodepointWidth / 2;
+ codepointIndex += insideIndex;
} else {
- SkASSERT(graphemeSize == 1);
center = glyphStart + glyphWidth / 2;
}
- if ((dx < center) == context.run->leftToRight()) {
+ if ((dx < center) == context.run->leftToRight() || insideGlyph) {
result = { SkToS32(codepointIndex), kDownstream };
} else {
result = { SkToS32(codepointIndex + 1), kUpstream };
diff --git a/modules/skparagraph/src/Run.cpp b/modules/skparagraph/src/Run.cpp
index 9bcdf90..a9404ee 100644
--- a/modules/skparagraph/src/Run.cpp
+++ b/modules/skparagraph/src/Run.cpp
@@ -12,6 +12,7 @@
SkUnichar val = SkUTF::NextUTF8(ptr, end);
return val < 0 ? 0xFFFD : val;
}
+
}
namespace skia {
@@ -88,7 +89,7 @@
}
}
-std::tuple<bool, ClusterIndex, ClusterIndex> Run::findLimitingClusters(TextRange text, bool onlyInnerClusters) const {
+std::tuple<bool, ClusterIndex, ClusterIndex> Run::findLimitingClusters(TextRange text, bool extendToClusters) const {
if (text.width() == 0) {
for (auto i = fClusterRange.start; i != fClusterRange.end; ++i) {
@@ -101,19 +102,27 @@
}
Cluster* start = nullptr;
Cluster* end = nullptr;
- if (onlyInnerClusters) {
+ if (extendToClusters) {
for (auto i = fClusterRange.start; i != fClusterRange.end; ++i) {
auto& cluster = fMaster->cluster(i);
- if (cluster.textRange().start >= text.start && start == nullptr) {
- start = &cluster;
- }
- if (cluster.textRange().end <= text.end) {
- end = &cluster;
- } else {
+ auto clusterRange = cluster.textRange();
+ if (clusterRange.end <= text.start) {
+ continue;
+ } else if (clusterRange.start >= text.end) {
break;
}
+
+ TextRange s = TextRange(std::max(clusterRange.start, text.start),
+ std::min(clusterRange.end, text.end));
+ if (s.width() > 0) {
+ if (start == nullptr) {
+ start = &cluster;
+ }
+ end = &cluster;
+ }
}
} else {
+ // TODO: Do we need to use this branch?..
for (auto i = fClusterRange.start; i != fClusterRange.end; ++i) {
auto& cluster = fMaster->cluster(i);
if (cluster.textRange().end > text.start && start == nullptr) {
diff --git a/samplecode/SampleParagraph.cpp b/samplecode/SampleParagraph.cpp
index 6391670..d7d30ab 100644
--- a/samplecode/SampleParagraph.cpp
+++ b/samplecode/SampleParagraph.cpp
@@ -2139,91 +2139,50 @@
void onDrawContent(SkCanvas* canvas) override {
- const char* text = "PESTO";
+ const char* text = "ffi";
canvas->drawColor(SK_ColorWHITE);
- SkPaint paint;
- paint.setColor(SK_ColorRED);
- paint.setStyle(SkPaint::kStroke_Style);
- paint.setAntiAlias(true);
- paint.setStrokeWidth(1);
+ auto collection = getFontCollection();
ParagraphStyle paragraph_style;
- paragraph_style.setTextAlign(TextAlign::kCenter);
- auto collection = getFontCollection();
ParagraphBuilderImpl builder(paragraph_style, collection);
TextStyle text_style;
text_style.setColor(SK_ColorBLACK);
text_style.setFontFamilies({SkString("Roboto")});
- text_style.setFontSize(48);
- text_style.setFontStyle(SkFontStyle::Bold());
- text_style.setLetterSpacing(3);
+ text_style.setFontSize(60);
builder.pushStyle(text_style);
builder.addText(text);
auto paragraph = builder.Build();
- auto w = width() / 2;
- paragraph->layout(w);
+ paragraph->layout(width());
paragraph->paint(canvas, 0, 0);
- canvas->drawRect(SkRect::MakeXYWH(0, 0, width() / 2, paragraph->getHeight()), paint);
+ auto width = paragraph->getLongestLine();
+ auto height = paragraph->getHeight();
+
+ auto f1 = paragraph->getGlyphPositionAtCoordinate(width/6, height/2);
+ auto f2 = paragraph->getGlyphPositionAtCoordinate(width/2, height/2);
+ auto i = paragraph->getGlyphPositionAtCoordinate(width*5/6, height/2);
+
+ SkDebugf("%d(%s) %d(%s) %d(%s)\n",
+ f1.position, f1.affinity == Affinity::kUpstream ? "up" : "down",
+ 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];
+
+ 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");
}
private:
typedef Sample INHERITED;
};
-class ParagraphView30 : public ParagraphView_Base {
-protected:
- SkString name() override { return SkString("Paragraph30"); }
-
- void onDrawContent(SkCanvas* canvas) override {
-
- /*
- * text: TextSpan(
- text: 'aaaa bbbb ',
- style: TextStyle(fontSize: 48.0),
- children: <TextSpan>[
- TextSpan(text: 'cc dd', style:TextStyle(fontFamily: 'serif', fontSize: 64.0)),
- ],
- ),
- textDirection: TextDirection.ltr,
- textAlign: TextAlign.justify,
-
- */
-
- const char* text1 = "aaaa bbbb ";
- const char* text2 = "cc dd";
-
- canvas->drawColor(SK_ColorWHITE);
-
- ParagraphStyle paragraph_style;
- paragraph_style.setTextAlign(TextAlign::kJustify);
- auto collection = getFontCollection();
- SkPaint red;
- red.setColor(SK_ColorRED);
- TextStyle text_style;
- text_style.setColor(SK_ColorBLACK);
-
- ParagraphBuilderImpl builder(paragraph_style, collection);
-
- text_style.setFontFamilies({SkString("Roboto")});
- text_style.setFontSize(48);
- builder.pushStyle(text_style);
- builder.addText(text1);
-
- text_style.setFontFamilies({SkString("Google Sans")});
- text_style.setFontSize(64);
- builder.pushStyle(text_style);
- builder.addText(text2);
-
- auto paragraph = builder.Build();
-
- paragraph->layout(310);
- paragraph->paint(canvas, 0, 0);
- }
-
-private:
- typedef Sample INHERITED;
-};
//////////////////////////////////////////////////////////////////////////////
DEF_SAMPLE(return new ParagraphView1();)
@@ -2254,4 +2213,3 @@
DEF_SAMPLE(return new ParagraphView27();)
DEF_SAMPLE(return new ParagraphView28();)
DEF_SAMPLE(return new ParagraphView29();)
-DEF_SAMPLE(return new ParagraphView30();)
