[skparagraph] Soft hyphen (U+00AD) rendering — CL 1193736 ready for review

[David H Bebawy]

Hi,

I've implemented support for rendering visible hyphens at soft hyphen (U+00AD) line break positions in the skparagraph module.

CL: https://skia-review.googlesource.com/c/skia/+/1193736

Flutter issue: https://github.com/flutter/flutter/issues/18443 (302+ thumbs-up, open since 2018)

Problem: SkParagraph correctly breaks lines at U+00AD positions but never renders a visible hyphen at the break point. Per the Unicode standard, U+00AD should render as a visible hyphen when at a line break and remain invisible otherwise.

Root cause: Text is shaped as one long line first (via HarfBuzz), where U+00AD is shaped as zero-width/invisible. After line breaking, the invisible shaping is preserved.

The fix (6 files, +204 lines): When a line breaks at a soft hyphen, a visible hyphen glyph ("-") is shaped and appended to the line end, following the existing ellipsis pattern:

• Cluster::isSoftHyphen() detects U+00AD clusters
• TextLine::fHyphen stores the shaped hyphen Run (parallel to fEllipsis)
• createSoftHyphen() reuses shapeEllipsis() infrastructure
• Hyphen width is included in line width; painting works for both LTR and RTL
• 4 new tests added; all 144 existing SkParagraph tests pass

Two requests:

1. Bug tracker access: I'm unable to file bugs on issues.skia.org ("You do not have permission to create issues in this component"). Could someone file a bug for this, or grant me access? Draft below:

   Title: [skparagraph] Render visible hyphen at soft hyphen (U+00AD) line breaks

   Component: Skia > Paragraph

   When text containing soft hyphens (U+00AD) is laid out and a line break occurs at a soft hyphen position, no visible hyphen is rendered at the end of the line. Per the Unicode standard, U+00AD should render as a visible hyphen when at a line break, and remain invisible otherwise. SkParagraph correctly identifies U+00AD as a line break opportunity (via ICU), but since text is shaped as one long line first (where U+00AD is zero-width), the invisible shaping is preserved after line breaking.

   Related: https://github.com/flutter/flutter/issues/18443

   Fix: https://skia-review.googlesource.com/c/skia/+/1193736

2. Code review: Who would be the right reviewer for skparagraph changes?

Thanks,
--

David H Bebawy
http://davidbebawy.com

[Julia Lavrova]

I reviewed the suggested changes.

The approach looks solid, and this new behavior is a useful addition to SkParagraph.
However, because several internal customers currently rely on the existing behavior, we can't safely roll this out as the new default without breaking their layouts.
Could we wrap this new functionality behind a run-time flag (perhaps added to ParagraphStyle)?
We will need the flag to default to the legacy behavior so that our existing users aren't impacted unless they explicitly opt-in to the new logic.
Let me know if you have any questions about where that flag should live. 

Thanks for the contribution.

--
You received this message because you are subscribed to the Google Groups "skia-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to skia-discuss...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/skia-discuss/CANbOCGXQdBMxf7YNxD397kNkUFY7Y0OAMK2qwAG9DXqrCMDenw%40mail.gmail.com.

[David H Bebawy]

Soft hyphen tests: dynamic width measurement to avoid font-metric sensitivity?

Hi Julia, Kaylee,

The soft hyphen rendering CL has been reverted twice now because the tests used a hardcoded layout width of 60px that was too close to the actual advance width of "inter" at 20px Roboto on Ubuntu-NativeFonts bots. The Reland^2 (https://skia-review.googlesource.com/c/skia/+/1208837) bumps this to 80px, which gives more margin.

However, any hardcoded width is fundamentally dependent on font metrics that can vary across platforms and font versions. I'd like your feedback on a more robust pattern for a follow-up CL: measure the text first, then compute the layout width dynamically:

```
auto paragraph = builder.Build();

// Layout wide to measure actual text advance
paragraph->layout(TestCanvasWidth);
auto impl = static_cast<ParagraphImpl*>(paragraph.get());
REPORTER_ASSERT(reporter, impl->lines().size() == 1);
SkScalar halfWidth = impl->lines()[0].width() / 2;

// Re-layout at half the measured width. "inter" is ~40% of
// "international" by character count, so it fits on line 1
// while the full word does not -- guaranteed regardless of
// font metrics.
paragraph->layout(halfWidth);
REPORTER_ASSERT(reporter, impl->lines().size() == 2);
```

This is completely font-metric-independent: since halfWidth comes from the same font that does the layout, it's guaranteed that the first segment fits and the full word doesn't. The only break opportunity in the word is the soft hyphen (U+00AD), so the break must occur there.

The tradeoff: this pattern isn't used elsewhere in SkParagraphTest.cpp -- existing tests all use hardcoded widths. But for tests that verify line-breaking at specific positions (like soft hyphen), the dynamic approach eliminates an entire class of bot failures.

Is this something you'd be comfortable seeing in the test file, or would you prefer sticking with a wider static margin (80px)? Happy to implement either way.

Thanks,
David

[David H Bebawy]

Soft hyphen tests: dynamic width measurement to avoid font-metric sensitivity?

Hi Julia, Kaylee,

The soft hyphen rendering CL has been reverted twice now because the tests used a hardcoded layout width of 60px that was too close to the actual advance width of "inter" at 20px Roboto on Ubuntu-NativeFonts bots. The Reland^2 (https://skia-review.googlesource.com/c/skia/+/1208837) bumps this to 80px for LTR and 40px for RTL, which gives more margin.

However, any hardcoded width is fundamentally dependent on font metrics that can vary across platforms and font versions. I'd like your feedback on a more robust pattern for a potential follow-up CL: measure the text first, then compute the layout width dynamically:

```
auto paragraph = builder.Build();

// Layout wide to measure actual text advance
paragraph->layout(TestCanvasWidth);
auto impl = static_cast<ParagraphImpl*>(paragraph.get());
REPORTER_ASSERT(reporter, impl->lines().size() == 1);

// Re-layout at 2/3 the measured width. Both "inter" (~40% of the
// full width) and "national" (~60%) fit within 67%, but the full
// word does not -- so the break must occur at the soft hyphen.
SkScalar narrowWidth = impl->lines()[0].width() * 2 / 3;
paragraph->layout(narrowWidth);

REPORTER_ASSERT(reporter, impl->lines().size() == 2);
```

Note: the ratio must be 2/3, not 1/2 -- at 1/2, the second segment ("national", ~60% of the full width) doesn't fit either and you get 3 lines instead of 2. We validated this on both Linux (Docker, Ubuntu 24.04 with CIPD skparagraph fonts) and macOS.

This is completely font-metric-independent: since narrowWidth comes from the same font that does the layout, both segments are guaranteed to fit on their respective lines regardless of platform. The only break opportunity in the word is the soft hyphen (U+00AD), so the break must occur there.

The tradeoff: this pattern isn't used elsewhere in SkParagraphTest.cpp -- existing tests all use hardcoded widths. But for tests that verify line-breaking at specific positions (like soft hyphen), the dynamic approach eliminates an entire class of bot failures.

Is this something you'd be comfortable seeing in the test file as a follow-up, or is the wider static margin (80px) sufficient? Happy to implement either way.

Thanks,
David

[jlav...@google.com]

There are few ways to make sure if you had a hyphen. I would take the last run on the first line (it's possible via ParagraphImpl) and check if it has a hyphen flag. 

That way you do not depend on measurements at all. In general, checking the structures rather than numbers is safer and more clear.