Selection in justification: Bugs: skia: 9969
Make sure getGlyphPositionAtCoordinate works in RTL
Change-Id: I394d868bbbd4a3042e1a2f50901d137c65f1f2b9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/274544
Commit-Queue: Julia Lavrova <jlavrova@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
diff --git a/modules/skparagraph/src/OneLineShaper.cpp b/modules/skparagraph/src/OneLineShaper.cpp
index 422f37d..2483abc 100644
--- a/modules/skparagraph/src/OneLineShaper.cpp
+++ b/modules/skparagraph/src/OneLineShaper.cpp
@@ -242,11 +242,15 @@
// Make it [left:right) regardless of a text direction
TextRange OneLineShaper::normalizeTextRange(GlyphRange glyphRange) {
- TextRange textRange(clusterIndex(glyphRange.start), clusterIndex(glyphRange.end));
- if (!fCurrentRun->leftToRight()) {
- std::swap(textRange.start, textRange.end);
+
+ if (fCurrentRun->leftToRight()) {
+ return TextRange(clusterIndex(glyphRange.start), clusterIndex(glyphRange.end));
+ } else {
+ return TextRange(clusterIndex(glyphRange.end - 1),
+ glyphRange.start > 0
+ ? clusterIndex(glyphRange.start - 1)
+ : fCurrentRun->fClusterStart + fCurrentRun->fTextRange.width());
}
- return textRange;
}
void OneLineShaper::addFullyResolved() {
@@ -396,7 +400,7 @@
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) {
// Resolved everything
return;
}
@@ -420,20 +424,19 @@
return;
}
- if (!visitor(typeface)) {
- // Resolved everything
+ auto resolved = visitor(typeface);
+ if (resolved == Resolved::Everything) {
+ // Resolved everything, no need to try another font
return;
}
- // Check if anything was resolved and stop it it was not
- auto last = fUnresolvedBlocks.back();
- if (!(unresolvedRange == last.fText)) {
- // Resolved something, no need to repeat
+ if (resolved == Resolved::Something) {
+ // Resolved something, no need to try another codepoint
break;
}
if (ch == unresolvedText.end()) {
- // Not a single codepoint could be resolved but we can switch to another block
+ // Not a single codepoint could be resolved but we finished the block
break;
}
@@ -552,6 +555,7 @@
fUnresolvedBlocks.emplace(RunBlock(block.fRange));
matchResolvedFonts(block.fStyle, [&](sk_sp<SkTypeface> typeface) {
+
// Create one more font to try
SkFont font(std::move(typeface), block.fStyle.getFontSize());
font.setEdging(SkFont::Edging::kAntiAlias);
@@ -572,8 +576,9 @@
// Walk through all the currently unresolved blocks
// (ignoring those that appear later)
- auto count = fUnresolvedBlocks.size();
- while (count-- > 0) {
+ auto resolvedCount = fResolvedBlocks.size();
+ auto unresolvedCount = fUnresolvedBlocks.size();
+ while (unresolvedCount-- > 0) {
auto unresolvedRange = fUnresolvedBlocks.front().fText;
auto unresolvedText = fParagraph->text(unresolvedRange);
@@ -594,8 +599,13 @@
this->dropUnresolved();
}
- // Continue until we resolved all the code points
- return !fUnresolvedBlocks.empty();
+ if (fUnresolvedBlocks.empty()) {
+ return Resolved::Everything;
+ } else if (resolvedCount < fResolvedBlocks.size()) {
+ return Resolved::Something;
+ } else {
+ return Resolved::Nothing;
+ }
});
this->finish(block.fRange, fHeight, advanceX);
@@ -615,9 +625,6 @@
// [left: right)
auto findBaseChar = [&](TextIndex index, Dir dir) -> TextIndex {
- if (!fCurrentRun->leftToRight()) {
- ++index;
- }
if (dir == Dir::right) {
while (index < fCurrentRun->fTextRange.end) {
if (this->fParagraph->fGraphemes.contains(index)) {
diff --git a/modules/skparagraph/src/OneLineShaper.h b/modules/skparagraph/src/OneLineShaper.h
index eaae28e..c1bcc64 100644
--- a/modules/skparagraph/src/OneLineShaper.h
+++ b/modules/skparagraph/src/OneLineShaper.h
@@ -60,7 +60,13 @@
using ShapeSingleFontVisitor = std::function<void(Block, SkTArray<SkShaper::Feature>)>;
void iterateThroughFontStyles(TextRange textRange, SkSpan<Block> styleSpan, const ShapeSingleFontVisitor& visitor);
- using TypefaceVisitor = std::function<bool(sk_sp<SkTypeface> typeface)>;
+ enum Resolved {
+ Nothing,
+ Something,
+ Everything
+ };
+
+ using TypefaceVisitor = std::function<Resolved(sk_sp<SkTypeface> typeface)>;
void matchResolvedFonts(const TextStyle& textStyle, const TypefaceVisitor& visitor);
#ifdef SK_DEBUG
void printState();
diff --git a/modules/skparagraph/src/ParagraphImpl.cpp b/modules/skparagraph/src/ParagraphImpl.cpp
index 865c2c6..840ab4c 100644
--- a/modules/skparagraph/src/ParagraphImpl.cpp
+++ b/modules/skparagraph/src/ParagraphImpl.cpp
@@ -725,7 +725,8 @@
&line != &fLines.back())
{
// TODO: this is just a patch. Make it right later (when it's clear what and how)
- clip.fLeft = 0;
+ trailingSpaces = clip;
+ trailingSpaces.fLeft = line.width();
clip.fRight = line.width();
} else if (this->fParagraphStyle.getTextDirection() == TextDirection::kRtl &&
!run->leftToRight())
@@ -852,7 +853,6 @@
return boxes;
}
-// TODO: Deal with RTL here
// TODO: Optimize (save cluster <-> codepoint connection)
PositionWithAffinity ParagraphImpl::getGlyphPositionAtCoordinate(SkScalar dx, SkScalar dy) {
@@ -875,9 +875,10 @@
line.iterateThroughVisualRuns(true,
[this, &line, dx, &result]
(const Run* run, SkScalar runOffsetInLine, TextRange textRange, SkScalar* runWidthInLine) {
+ bool lookingForHit = true;
*runWidthInLine = line.iterateThroughSingleRunByStyles(
run, runOffsetInLine, textRange, StyleType::kNone,
- [this, line, dx, &result]
+ [this, line, dx, &result, &lookingForHit]
(TextRange textRange, const TextStyle& style, const TextLine::ClipContext& context) {
auto findCodepointByTextIndex = [this](ClusterIndex clusterIndex8) {
@@ -894,12 +895,12 @@
// All the other runs are placed right of this one
auto codepointIndex = findCodepointByTextIndex(context.run->globalClusterIndex(context.pos));
result = { SkToS32(codepointIndex), kDownstream };
+ lookingForHit = false;
return false;
}
if (dx >= context.clip.fRight + offsetX) {
- // We have to keep looking but just in case keep the last one as the closes
- // so far
+ // We have to keep looking ; just in case keep the last one as the closest
auto codepointIndex = findCodepointByTextIndex(context.run->globalClusterIndex(context.pos + context.size));
result = { SkToS32(codepointIndex), kUpstream };
return true;
@@ -909,35 +910,42 @@
// Find the glyph position in the run that is the closest left of our point
// TODO: binary search
size_t found = context.pos;
- for (size_t i = context.pos; i < context.pos + context.size; ++i) {
+ for (size_t index = context.pos; index < context.pos + context.size; ++index) {
// TODO: this rounding is done to match Flutter tests. Must be removed..
- auto index = context.run->leftToRight() ? i : context.size - i;
auto end = littleRound(context.run->positionX(index) + context.fTextShift + offsetX);
- if ((context.run->leftToRight() ? end > dx : dx > end)) {
+ if (end > dx) {
break;
}
found = index;
}
- if (!context.run->leftToRight()) {
- --found;
- }
-
auto glyphStart = context.run->positionX(found) + context.fTextShift + offsetX;
auto glyphWidth = context.run->positionX(found + 1) - context.run->positionX(found);
auto clusterIndex8 = context.run->globalClusterIndex(found);
auto clusterEnd8 = context.run->globalClusterIndex(found + 1);
- TextRange clusterText (clusterIndex8, clusterEnd8);
// Find the grapheme positions in codepoints that contains the point
auto codepointIndex = findCodepointByTextIndex(clusterIndex8);
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;
+ if (context.run->leftToRight()) {
+ for (codepoints.end = codepointIndex;
+ codepoints.end < fCodePoints.size(); ++codepoints.end) {
+ auto& cp = fCodePoints[codepoints.end];
+ if (cp.fTextIndex >= clusterEnd8) {
+ break;
+ }
}
+ } else {
+ for (codepoints.end = codepointIndex;
+ codepoints.end > 0; --codepoints.end) {
+ auto& cp = fCodePoints[codepoints.end];
+ if (cp.fTextIndex <= clusterEnd8) {
+ break;
+ }
+ }
+ std::swap(codepoints.start, codepoints.end);
}
+
auto graphemeSize = codepoints.width();
// We only need to inspect one glyph (maybe not even the entire glyph)
@@ -959,10 +967,11 @@
result = { SkToS32(codepointIndex + 1), kUpstream };
}
// No need to continue
+ lookingForHit = false;
return false;
});
- return true;
+ return lookingForHit;
});
break;
}
diff --git a/samplecode/SampleParagraph.cpp b/samplecode/SampleParagraph.cpp
index f97c1ce..4c577d2 100644
--- a/samplecode/SampleParagraph.cpp
+++ b/samplecode/SampleParagraph.cpp
@@ -2341,28 +2341,110 @@
void onDrawContent(SkCanvas* canvas) override {
canvas->drawColor(SK_ColorWHITE);
-
- auto fontCollection = getFontCollection();
+ auto text = "ضخمة ص ،😁😂🤣ضضض ؤ،،😗😗😍😋شسي،😗😁😁ؤرى،😗😃😄😍ببب،🥰😅🥰🥰🥰ثيلااتن";
+ //auto text = "ى،😗😃😄😍بب";
+ //auto text1 = "World domination is such an ugly phrase - I prefer to call it world optimisation";
+ auto fontCollection = sk_make_sp<FontCollection>();
+ fontCollection->setDefaultFontManager(SkFontMgr::RefDefault());
+ fontCollection->enableFontFallback();
ParagraphStyle paragraph_style;
- paragraph_style.setTextAlign(TextAlign::kJustify);
ParagraphBuilderImpl builder(paragraph_style, fontCollection);
TextStyle text_style;
text_style.setColor(SK_ColorBLACK);
- text_style.setFontFamilies({SkString("Roboto")});
- text_style.setFontSize(36);
+ text_style.setFontFamilies({SkString("Noto Color Emoji")});
+ text_style.setFontSize(50);
builder.pushStyle(text_style);
- builder.addText("Something");
+ builder.addText(text);
auto paragraph = builder.Build();
- paragraph->layout(width());
- paragraph->layout(0);
+ paragraph->layout(1041); // 1041
+
+ SkColor colors[] = {SK_ColorBLUE, SK_ColorCYAN, SK_ColorLTGRAY, SK_ColorGREEN,
+ SK_ColorRED, SK_ColorWHITE, SK_ColorYELLOW, SK_ColorMAGENTA };
+ SkPaint paint;
+ size_t wordPos = 0;
+ size_t index = 0;
+ while (wordPos < 72) {
+ auto res2 = paragraph->getWordBoundary(wordPos);
+ if (res2.width() == 0) {
+ break;
+ }
+ wordPos = res2.end;
+ auto res3 = paragraph->getRectsForRange(
+ res2.start, res2.end,
+ RectHeightStyle::kTight, RectWidthStyle::kTight);
+ paint.setColor(colors[index % 8]);
+ ++index;
+ if (!res3.empty()) {
+ canvas->drawRect(res3[0].rect, paint);
+ }
+ }
paragraph->paint(canvas, 0, 0);
}
private:
typedef Sample INHERITED;
};
-//
+
+class ParagraphView35 : public ParagraphView_Base {
+protected:
+ SkString name() override { return SkString("Paragraph35"); }
+
+ Click* onFindClickHandler(SkScalar x, SkScalar y, skui::ModifierKey modi) override {
+ return new Click;
+ }
+
+ bool onClick(Click* click) override {
+ fPoint = click->fCurr;
+ return true;
+ }
+
+ void onDrawContent(SkCanvas* canvas) override {
+
+ canvas->drawColor(SK_ColorWHITE);
+
+ auto text = u"hzbzzj sjsjjs sjkkahgafa\u09A4\u09A1\u09A4\u09A0\u09A4\u09A0 jsjzjgvsh sjsjsksbsbsjs sjjajajahhav jssjbxx jsisudg \u09AF\u09A0\u09AF\u09A0\u09A4\u09A0\u09A4\u09A0\u09A5 \u062A\u0624\u062A\u064A\u0646\u0646\u064A\u0621\u0646\u0627\u0644\u0631\u0631\u064A\u0644\u0627 \u062A\u062A\u0644\u0649 \u062A\u0627\u0631\u064A\u062E \u062A\u0633\u0628\u0628 \u0624\u062A\u064A\u062A\u0624\u062A\u0624\u062A\u0624\u062A\u0624 dhishsbs \u7238\u7238\u4E0D\u5BF9\u52B2\u5927\u5BB6\u90FD\u597D\u8BB0\u5F97\u8BB0\u5F97hshs\u099B\u09A1\u099B\u09A1\u099A jdjdj jdjdjd dbbdbdbdbddbnd\u09A2\u099B\u09A1\u09A2\u09A3\u099B\u09B0\u099A\u0998\u09A0\u09A0\u09B8\u09AB\u0997\u09A3\u09A4\u099C\u09B0\u09A5\u099B\u099B\u09A5\u09A6\u099D\u09A6\u09B2\u09A5\u09A4\u09A3\u09A2\u0997\u0996\u09A0\u0998\u0999\u09A3\u099A\u09A5\u09A4\u09A3\u062A\u0628\u0646\u064A\u0646 \u09A5\u09A3\u09A3 \u09A4\u0998\u0998\u0998\u099B\u09A4 \u09A4\u09A3 \u09A3\u0998\u09A2\u09A3\u0999\u0648\u064A\u0648\u0621\u062A\u064A\u0632\u0633\u0646\u0632\u0624\u0624\u0645\u0645\u0624\u0648\u0624\u0648\u0648\u064A\u0646\u0624\u0646\u0624\u0646\u0624\u0624 \u09A4\u09A4\u09A2\u09A2\u09A4\u09A4 \u0999\u0998\u0997\u09C1\u099B\u09A5 \u09A4\u0997\u0998\u09A3\u099A\u099C\u09A6\u09A5\u0632\u0624\u0648\u0624\u0648\u0624 \u09A4\u09A4\u09A3\u0998\u09A2\u09A4\u099B\u09A6\u09A5\u09A4\u0999\u0998\u09A3 \u0648\u0624\u0648\u0624\u0648\u0624\u0632\u0624\u0646\u0633\u0643\u0633\u0643\u0628\u0646\u09A4\u09AD\u0996\u0996\u099F\u09C0\u09C1\u099B\u09A6\u09C0\u09C1\u09C2\u09C7\u0648\u0624\u0646\u0621\u0646\u0624\u0646 \u09C7\u09C2\u09C0\u09C2\u099A\u09A3\u09A2\u09A4\u09A5\u09A5\u0632\u064A\u09C7\u09C2\u09C0\u09C2\u099A\u09A3\u09A2\u09AE\u09A4\u09A5\u09A5 \U0001f34d\U0001f955\U0001f4a7\U0001f4a7\U0001f4a6\U0001f32a";
+ auto fontCollection = sk_make_sp<FontCollection>();
+ fontCollection->setDefaultFontManager(SkFontMgr::RefDefault());
+ fontCollection->enableFontFallback();
+
+ ParagraphStyle paragraph_style;
+ paragraph_style.setTextAlign(TextAlign::kJustify);
+ ParagraphBuilderImpl builder(paragraph_style, fontCollection);
+ TextStyle text_style;
+ text_style.setColor(SK_ColorBLACK);
+ text_style.setFontFamilies({SkString("Roboto"), SkString("Noto Color Emoji")});
+ text_style.setFontSize(40);
+ builder.pushStyle(text_style);
+ builder.addText(text);
+ auto paragraph = builder.Build();
+ paragraph->layout(width());
+ paragraph->paint(canvas, 0, 0);
+ for (size_t i = 0; i < 402; ++i) {
+ //auto res1 = paragraph->getGlyphPositionAtCoordinate(fPoint.fX, fPoint.fY);
+ //auto res2 = paragraph->getWordBoundary(res1.position);
+ auto res3 = paragraph->getRectsForRange(i, i + 1, RectHeightStyle::kTight, RectWidthStyle::kTight);
+ if (res3.empty()) {
+ SkDebugf("empty: %f %d %d\n", width(), i, i + 1);
+ }
+ }
+/*
+ SkPaint paint;
+ paint.setColor(SK_ColorLTGRAY);
+ for (auto& r : res3) {
+ if (SkScalarNearlyZero(r.rect.fLeft) && SkScalarNearlyZero(r.rect.fTop)) {
+ SkDebugf("0, 0: %f %d %d\n", width(), res2.start, res2.end);
+ }
+ canvas->drawRect(r.rect, paint);
+ }
+*/
+ }
+
+private:
+ typedef Sample INHERITED;
+ SkPoint fPoint;
+};
+
//////////////////////////////////////////////////////////////////////////////
DEF_SAMPLE(return new ParagraphView1();)
@@ -2398,3 +2480,4 @@
DEF_SAMPLE(return new ParagraphView32();)
DEF_SAMPLE(return new ParagraphView33();)
DEF_SAMPLE(return new ParagraphView34();)
+DEF_SAMPLE(return new ParagraphView35();)