Skip to content

Commit fee8ce7

Browse files
ramezgergesclaude
andcommitted
refactor(text): drop IFont.Shape direction-guessing overload
Shaping should shape a run of known direction; guessing direction from script is a layout concern, not a font capability, and it forced every IFont backend to replicate HarfBuzz's GuessSegmentProperties direction inference. The only caller was the legacy non-bidi Run.Segments/ParsedText path (superseded by UnicodeText); it now passes the run's own FlowDirection to the explicit Shape(text, direction) overload, dropping the per-segment shaper guess and the spaces-only RTL hack that compensated for it. SkiaFont/ManagedFont collapse ShapeCore into the single Shape (GuessSegment- Properties stays — it also sets script/language — with direction set explicitly). Validated: text + acrylic runtime tests unchanged (the one Given_TextBlock gripper failure is pre-existing — fails identically on baseline without this change). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 6d50985 commit fee8ce7

4 files changed

Lines changed: 11 additions & 47 deletions

File tree

src/Uno.UI.Composition.Drawing/IFont.cs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,6 @@ public interface IFont
2525
/// </summary>
2626
GlyphRun Shape(ReadOnlySpan<char> text, TextDirection direction, bool enableLigatures = true);
2727

28-
/// <summary>
29-
/// Shapes a run, letting the shaper guess the run's direction from its script (used by the segment itemizer that
30-
/// hasn't resolved bidi itself) and reporting it back via <paramref name="resolvedDirection"/>. Otherwise
31-
/// identical to <see cref="Shape(ReadOnlySpan{char}, TextDirection, bool)"/>.
32-
/// </summary>
33-
GlyphRun Shape(ReadOnlySpan<char> text, out TextDirection resolvedDirection, bool enableLigatures = true);
34-
3528
/// <summary>
3629
/// Builds the combined filled outline of the run's outline glyphs (color glyphs are excluded — draw those
3730
/// via <see cref="AppendColorGlyphImages"/>). Each glyph is placed at its position, shifted by <paramref name="baselineY"/>.

src/Uno.UI.Composition.Skia/Composition/Uno/Drawing/SkiaFont.cs

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,21 +65,11 @@ public float GetGlyphAdvance(ushort glyph)
6565
public string FamilyName => _font.Typeface?.FamilyName ?? string.Empty;
6666

6767
public GlyphRun Shape(ReadOnlySpan<char> text, TextDirection direction, bool enableLigatures = true)
68-
=> ShapeCore(text, direction, enableLigatures, out _);
69-
70-
public GlyphRun Shape(ReadOnlySpan<char> text, out TextDirection resolvedDirection, bool enableLigatures = true)
71-
=> ShapeCore(text, null, enableLigatures, out resolvedDirection);
72-
73-
private GlyphRun ShapeCore(ReadOnlySpan<char> text, TextDirection? direction, bool enableLigatures, out TextDirection resolvedDirection)
7468
{
7569
using var buffer = new HbBuffer();
7670
buffer.AddUtf16(text);
77-
buffer.GuessSegmentProperties();
78-
if (direction is { } requested)
79-
{
80-
buffer.Direction = requested == TextDirection.RightToLeft ? HarfBuzzSharp.Direction.RightToLeft : HarfBuzzSharp.Direction.LeftToRight;
81-
}
82-
resolvedDirection = buffer.Direction == HarfBuzzSharp.Direction.RightToLeft ? TextDirection.RightToLeft : TextDirection.LeftToRight;
71+
buffer.GuessSegmentProperties(); // sets the run's script/language for the shaper; direction is set explicitly below
72+
buffer.Direction = direction == TextDirection.RightToLeft ? HarfBuzzSharp.Direction.RightToLeft : HarfBuzzSharp.Direction.LeftToRight;
8373

8474
if (enableLigatures)
8575
{

src/Uno.UI.Composition/Composition/Uno/Drawing/ManagedFont.cs

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -149,21 +149,11 @@ public int GetAdvanceWidth(ushort glyph)
149149
public string FamilyName => _familyName ??= ParseFamilyName(_data, _name);
150150

151151
public GlyphRun Shape(ReadOnlySpan<char> text, TextDirection direction, bool enableLigatures = true)
152-
=> ShapeCore(text, direction, enableLigatures, out _);
153-
154-
public GlyphRun Shape(ReadOnlySpan<char> text, out TextDirection resolvedDirection, bool enableLigatures = true)
155-
=> ShapeCore(text, null, enableLigatures, out resolvedDirection);
156-
157-
private GlyphRun ShapeCore(ReadOnlySpan<char> text, TextDirection? direction, bool enableLigatures, out TextDirection resolvedDirection)
158152
{
159153
using var buffer = new HbBuffer();
160154
buffer.AddUtf16(text);
161-
buffer.GuessSegmentProperties();
162-
if (direction is { } requested)
163-
{
164-
buffer.Direction = requested == TextDirection.RightToLeft ? HarfBuzzSharp.Direction.RightToLeft : HarfBuzzSharp.Direction.LeftToRight;
165-
}
166-
resolvedDirection = buffer.Direction == HarfBuzzSharp.Direction.RightToLeft ? TextDirection.RightToLeft : TextDirection.LeftToRight;
155+
buffer.GuessSegmentProperties(); // sets the run's script/language for the shaper; direction is set explicitly below
156+
buffer.Direction = direction == TextDirection.RightToLeft ? HarfBuzzSharp.Direction.RightToLeft : HarfBuzzSharp.Direction.LeftToRight;
167157

168158
if (enableLigatures)
169159
{

src/Uno.UI/UI/Xaml/Documents/Run.skia.cs

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -215,22 +215,13 @@ private List<Segment> GetSegments()
215215
// Skip the second line break char so it stays part of the same cluster as the first.
216216
var shapedLength = lineBreakLength == 2 ? length - 1 : length;
217217

218-
// The direction is guessed by the shaper from the run's script (bidi itemization isn't done here
219-
// yet — see the TODO above). Ligatures are disabled because a TextBox needs each source char to
220-
// stay separately addressable (uno#15528, uno#16788).
221-
var glyphRun = segmentFont.Shape(text.Slice(i, shapedLength), out var resolvedDirection, enableLigatures: false);
222-
var direction = resolvedDirection is TextDirection.LeftToRight ? FlowDirection.LeftToRight : FlowDirection.RightToLeft;
223-
if (direction == FlowDirection.LeftToRight &&
224-
segments.Count > 0 && segments[segments.Count - 1].Direction == FlowDirection.RightToLeft &&
225-
trailingSpaces + leadingSpaces == length)
226-
{
227-
// If the current segment consists of spaces only, it will be considered LeftToRight.
228-
// But if the previous segment was RightToLeft, we want the current segment to also be RTL.
229-
// This is quite hacky, it feels like GetRenderOrderedSegmentSpans is buggy and a real fix needs to go there.
230-
direction = FlowDirection.RightToLeft;
231-
}
232-
233-
var glyphs = GetGlyphs(glyphRun, i, resolvedDirection is TextDirection.RightToLeft);
218+
// Legacy non-bidi path (superseded by UnicodeText): shape each segment in the run's own
219+
// FlowDirection rather than resolving bidi. Ligatures are disabled because a TextBox needs each
220+
// source char to stay separately addressable (uno#15528, uno#16788).
221+
var direction = this.FlowDirection;
222+
var textDirection = direction == FlowDirection.RightToLeft ? TextDirection.RightToLeft : TextDirection.LeftToRight;
223+
var glyphRun = segmentFont.Shape(text.Slice(i, shapedLength), textDirection, enableLigatures: false);
224+
var glyphs = GetGlyphs(glyphRun, i, textDirection is TextDirection.RightToLeft);
234225

235226
Debug.Assert(!(Text.AsSpan(i, length).Contains('\t')) || length == 1);
236227
if (length == 1 && text[i] == '\t')

0 commit comments

Comments
 (0)