Skip to content

Commit 98daf58

Browse files
dbebawyskcq-be@skia-corp.google.com.iam.gserviceaccount.com
dbebawy
authored and
[email protected]
committed
Reland^2 "[skparagraph] Render visible hyphen at soft hyphen (U+00AD) line breaks"
This is a reland of commit 9a0def8 The reland was reverted (339e3c3) because soft hyphen tests still failed on Ubuntu-NativeFonts bots: the hardcoded layout width of 60px was too close to the actual advance width of "inter" at 20px Roboto on those bots, causing the text to not break at the expected position. Fix: increase the test layout width from 60 to 80px. At 20px Roboto, "inter" measures ~40-55px across font versions (fits in 80), while "international" measures ~100-130px (does not fit in 80), so the break at the soft hyphen is reliably triggered. Original change's description: > When text is laid out and a line break occurs at a soft hyphen > (U+00AD) position, a visible hyphen glyph ("-") is now shaped > and appended to the end of that line. Soft hyphens that are not > at line break positions remain invisible (zero-width), as before. > > This behavior is gated behind ParagraphStyle::setRenderSoftHyphens(), > which defaults to false to preserve existing behavior for current > consumers. Callers must opt in to enable visible hyphen rendering. [email protected], [email protected] Change-Id: I23b21e5d2b5fbd6bcb990e0a631c87456e46ad10 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1208837 Commit-Queue: Julia Lavrova <[email protected]> Reviewed-by: Kaylee Lubick <[email protected]> Reviewed-by: Julia Lavrova <[email protected]>
1 parent b3829c1 commit 98daf58

8 files changed

Lines changed: 300 additions & 9 deletions

File tree

‎AUTHORS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ Christian Swinehart <[email protected]>
3838
Christian Plesner Hansen <[email protected]>
3939
ColdPaleLight <[email protected]>
4040
Collabora Ltd <*@collabora.com>
41+
David Bebawy <[email protected]>
4142
Dawson Coleman <[email protected]>
4243
Deepak Mohan <[email protected]>
4344
Ehsan Akhgari <[email protected]>

‎modules/skparagraph/include/ParagraphStyle.h‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,14 +79,13 @@ struct ParagraphStyle {
7979
ParagraphStyle();
8080

8181
bool operator==(const ParagraphStyle& rhs) const {
82-
return this->fHeight == rhs.fHeight &&
83-
this->fEllipsis == rhs.fEllipsis &&
82+
return this->fHeight == rhs.fHeight && this->fEllipsis == rhs.fEllipsis &&
8483
this->fEllipsisUtf16 == rhs.fEllipsisUtf16 &&
8584
this->fTextDirection == rhs.fTextDirection && this->fTextAlign == rhs.fTextAlign &&
8685
this->fDefaultTextStyle == rhs.fDefaultTextStyle &&
8786
this->fReplaceTabCharacters == rhs.fReplaceTabCharacters &&
88-
this->fFakeMissingFontStyles == rhs.fFakeMissingFontStyles;
89-
87+
this->fFakeMissingFontStyles == rhs.fFakeMissingFontStyles &&
88+
this->fRenderSoftHyphens == rhs.fRenderSoftHyphens;
9089
}
9190

9291
const StrutStyle& getStrutStyle() const { return fStrutStyle; }
@@ -135,6 +134,9 @@ struct ParagraphStyle {
135134
bool getLetterSpacingByCSSSpec() const { return fLetterSpacingByCSSSpec; }
136135
void setLetterSpacingByCSSSpec(bool value) { fLetterSpacingByCSSSpec = value; }
137136

137+
bool getRenderSoftHyphens() const { return fRenderSoftHyphens; }
138+
void setRenderSoftHyphens(bool value) { fRenderSoftHyphens = value; }
139+
138140
private:
139141
StrutStyle fStrutStyle;
140142
TextStyle fDefaultTextStyle;
@@ -150,6 +152,7 @@ struct ParagraphStyle {
150152
bool fFakeMissingFontStyles;
151153
bool fApplyRoundingHack = true;
152154
bool fLetterSpacingByCSSSpec = false;
155+
bool fRenderSoftHyphens = false;
153156
};
154157
} // namespace textlayout
155158
} // namespace skia

‎modules/skparagraph/src/ParagraphImpl.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -673,6 +673,9 @@ void ParagraphImpl::breakShapedTextIntoLines(SkScalar maxWidth) {
673673
if (addEllipsis) {
674674
line.createEllipsis(maxWidth, this->getEllipsis(), true);
675675
}
676+
if (this->paragraphStyle().getRenderSoftHyphens()) {
677+
line.createSoftHyphen();
678+
}
676679
fLongestLine = std::max(fLongestLine, nearlyZero(line.width()) ? widthWithSpaces : line.width());
677680
});
678681

‎modules/skparagraph/src/Run.cpp‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,17 @@ bool Cluster::isSoftBreak() const {
365365
SkUnicode::CodeUnitFlags::kSoftLineBreakBefore);
366366
}
367367

368+
bool Cluster::isSoftHyphen() const {
369+
auto text = fOwner->text();
370+
const char* p = text.begin() + fTextRange.start;
371+
const char* end = text.begin() + fTextRange.end;
372+
if (p >= end) {
373+
return false;
374+
}
375+
SkUnichar cp = SkUTF::NextUTF8(&p, end);
376+
return cp == 0x00AD && p == end;
377+
}
378+
368379
bool Cluster::isGraphemeBreak() const {
369380
return fOwner->codeUnitHasProperty(fTextRange.end, SkUnicode::CodeUnitFlags::kGraphemeStart);
370381
}

‎modules/skparagraph/src/Run.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,7 @@ class Cluster {
315315
bool isIdeographic() const { return fIsIdeographic; }
316316

317317
bool isSoftBreak() const;
318+
bool isSoftHyphen() const;
318319
bool isGraphemeBreak() const;
319320
bool canBreakLineAfter() const { return isHardBreak() || isSoftBreak(); }
320321
size_t startPos() const { return fStart; }

‎modules/skparagraph/src/TextLine.cpp‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -237,10 +237,8 @@ void TextLine::ensureTextBlobCachePopulated() {
237237
if (fTextBlobCachePopulated) {
238238
return;
239239
}
240-
if (fBlockRange.width() == 1 &&
241-
fRunsInVisualOrder.size() == 1 &&
242-
fEllipsis == nullptr &&
243-
fOwner->run(fRunsInVisualOrder[0]).placeholderStyle() == nullptr) {
240+
if (fBlockRange.width() == 1 && fRunsInVisualOrder.size() == 1 && fEllipsis == nullptr &&
241+
fHyphen == nullptr && fOwner->run(fRunsInVisualOrder[0]).placeholderStyle() == nullptr) {
244242
if (fClusterRange.width() == 0) {
245243
return;
246244
}
@@ -631,6 +629,27 @@ void TextLine::createEllipsis(SkScalar maxWidth, const SkString& ellipsis, bool)
631629
}
632630
}
633631

632+
void TextLine::createSoftHyphen() {
633+
if (fEllipsis) {
634+
return;
635+
}
636+
if (fClusterRange.width() == 0) {
637+
return;
638+
}
639+
640+
auto& lastCluster = fOwner->cluster(fClusterRange.end - 1);
641+
if (!lastCluster.isSoftHyphen()) {
642+
return;
643+
}
644+
645+
SkString hyphenStr("-");
646+
fHyphen = this->shapeEllipsis(hyphenStr, &lastCluster);
647+
if (fHyphen) {
648+
fHyphen->setOwner(fOwner);
649+
fHyphen->fClusterStart = lastCluster.textRange().start;
650+
}
651+
}
652+
634653
std::unique_ptr<Run> TextLine::shapeEllipsis(const SkString& ellipsis, const Cluster* cluster) {
635654

636655
class ShapeHandler final : public SkShaper::RunHandler {
@@ -1066,6 +1085,13 @@ void TextLine::iterateThroughVisualRuns(bool includingGhostSpaces, const RunVisi
10661085
}
10671086
}
10681087

1088+
if (this->hyphen() != nullptr &&
1089+
fOwner->paragraphStyle().getTextDirection() == TextDirection::kRtl) {
1090+
runOffset = this->hyphen()->offset().fX;
1091+
if (visitor(hyphen(), runOffset, hyphen()->textRange(), &width)) {
1092+
}
1093+
}
1094+
10691095
for (auto& runIndex : fRunsInVisualOrder) {
10701096

10711097
const auto run = &this->fOwner->run(runIndex);
@@ -1101,6 +1127,13 @@ void TextLine::iterateThroughVisualRuns(bool includingGhostSpaces, const RunVisi
11011127
}
11021128
}
11031129

1130+
if (this->hyphen() != nullptr &&
1131+
fOwner->paragraphStyle().getTextDirection() == TextDirection::kLtr) {
1132+
if (visitor(hyphen(), runOffset, hyphen()->textRange(), &width)) {
1133+
totalWidth += width;
1134+
}
1135+
}
1136+
11041137
if (!includingGhostSpaces && compareRound(totalWidth, this->width(), fOwner->getApplyRoundingHack()) != 0) {
11051138
// This is a very important assert!
11061139
// It asserts that 2 different ways of calculation come with the same results

‎modules/skparagraph/src/TextLine.h‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,15 @@ class TextLine {
7070
ClusterRange clusters() const { return fClusterRange; }
7171
ClusterRange clustersWithSpaces() const { return fGhostClusterRange; }
7272
Run* ellipsis() const { return fEllipsis.get(); }
73+
Run* hyphen() const { return fHyphen.get(); }
7374
InternalLineMetrics sizes() const { return fSizes; }
7475
bool empty() const { return fTextExcludingSpaces.empty(); }
7576

7677
SkScalar spacesWidth() const { return fWidthWithSpaces - width(); }
7778
SkScalar height() const { return fAdvance.fY; }
7879
SkScalar width() const {
79-
return fAdvance.fX + (fEllipsis != nullptr ? fEllipsis->fAdvance.fX : 0);
80+
return fAdvance.fX + (fEllipsis != nullptr ? fEllipsis->fAdvance.fX : 0) +
81+
(fHyphen != nullptr ? fHyphen->fAdvance.fX : 0);
8082
}
8183
SkScalar widthWithoutEllipsis() const { return fAdvance.fX; }
8284
SkVector offset() const;
@@ -108,6 +110,7 @@ class TextLine {
108110
void ensureTextBlobCachePopulated();
109111

110112
void createEllipsis(SkScalar maxWidth, const SkString& ellipsis, bool ltr);
113+
void createSoftHyphen();
111114

112115
// For testing internal structures
113116
void scanStyles(StyleType style, const RunStyleVisitor& visitor);
@@ -182,6 +185,7 @@ class TextLine {
182185
SkScalar fShift; // Let right
183186
SkScalar fWidthWithSpaces;
184187
std::unique_ptr<Run> fEllipsis; // In case the line ends with the ellipsis
188+
std::unique_ptr<Run> fHyphen; // Visible hyphen for soft hyphen line breaks
185189
InternalLineMetrics fSizes; // Line metrics as a max of all run metrics and struts
186190
InternalLineMetrics fMaxRunMetrics; // No struts - need it for GetRectForRange(max height)
187191
bool fHasBackground;

0 commit comments

Comments
 (0)