Skip to content

Commit

Permalink
fix(textblock): adjust selection to account for surrogate pairs being…
Browse files Browse the repository at this point in the history
… 2 chars
  • Loading branch information
ramezgerges committed Mar 12, 2024
1 parent 582ceaf commit 8acee8b
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 7 deletions.
71 changes: 69 additions & 2 deletions src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,67 @@ private Range Selection
}
}

/// <summary>
/// When we read the length of a span or segment from Skia/HarfBuzz, what we actually get is the
/// number of glyphs (i.e. Unicode runes), not the number of c# chars, so surrogate pairs will
/// only be counted as a single "unit". This method stretches the range to account for surrogate
/// pairs being 2 characters, not one.
/// </summary>
/// <remarks>
/// Make sure not to call this on an already-adjusted range with surrogate pairs in it, as it will
/// double count the surrogate pairs.
/// </remarks>
private Range AdjustSelectionForSurrogatePairs(Range range)
{
var start = Math.Min(range.start, range.end);
var end = Math.Max(range.start, range.end);

var count = 0;
for (int i = 0, j = 0; i < Text.Length && j < start; i++, j++, count += 1)
{
if (i < Text.Length - 1 && char.IsSurrogatePair(Text[i], Text[i + 1]))
{
// Notice how j didn't move here
count += 1;
i++;
}
}

var adjustedStart = count;

for (int i = start, j = 0; i < Text.Length && j < end - start; i++, j++, count += 1)
{
if (i < Text.Length - 1 && char.IsSurrogatePair(Text[i], Text[i + 1]))
{
// Notice how j didn't move here
count += 1;
i++;
}
}

var adjustedEnd = count;

// keep direction
return range.start < range.end ? new Range(adjustedStart, adjustedEnd) : new Range(adjustedEnd, adjustedStart);
}

// equivalent to AdjustSelectionForSurrogatePairs for a single index
private int AdjustIndexForSurrogatePairs(int index)
{
var count = 0;
for (int i = 0, j = 0; i < Text.Length && j < index; i++, j++, count += 1)
{
if (i < Text.Length - 1 && char.IsSurrogatePair(Text[i], Text[i + 1]))
{
// Notice how j didn't move here
count += 1;
i++;
}
}

return count;
}

partial void OnSelectionChanged();

#if !UNO_REFERENCE_API
Expand Down Expand Up @@ -941,13 +1002,15 @@ private void UpdateHyperlinks() { } // Events are subscribed in Hyperlink's ctor
#endif
if (index >= 0) // should always be true if above TODO is addressed
{
#if __SKIA__
that.Selection = that.AdjustSelectionForSurrogatePairs(new Range(index, index));
#else
that.Selection = new Range(index, index);
#endif
}

e.Handled = true;
that.Focus(FocusState.Pointer);

that.CapturePointer(e.Pointer);
}
};

Expand Down Expand Up @@ -1027,7 +1090,11 @@ private void UpdateHyperlinks() { } // Events are subscribed in Hyperlink's ctor
#endif
if (index >= 0) // should always be true if above TODO is addressed
{
#if __SKIA__
that.Selection = that.Selection with { end = that.AdjustIndexForSurrogatePairs(index) };
#else
that.Selection = that.Selection with { end = index };
#endif
}
}
};
Expand Down
23 changes: 18 additions & 5 deletions src/Uno.UI/UI/Xaml/Controls/TextBlock/TextBlock.skia.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using Windows.Foundation;
using SkiaSharp;
using Microsoft.UI.Composition;
Expand Down Expand Up @@ -192,7 +193,17 @@ partial void OnSelectionHighlightColorChangedPartial(SolidColorBrush brush)
void IBlock.Invalidate(bool updateText) => InvalidateInlines(updateText);

partial void OnSelectionChanged()
=> Inlines.Selection = (Math.Min(Selection.start, Selection.end), Math.Max(Selection.start, Selection.end));
{
// we "unadjust" the adjustment that comes from AdjustSelectionForSurrogatePairs or wherever.
var start = Math.Min(Selection.start, Selection.end);
var end = Math.Max(Selection.start, Selection.end);
// If this affects performance heavily, we could go through the runes once instead of twice
var unadjustedStart = Text[..start].EnumerateRunes().Count();
var unadjustedEnd = Text[..end].EnumerateRunes().Count();

// keep direction
Inlines.Selection = Selection.start < Selection.end ? (unadjustedStart, unadjustedEnd) : (unadjustedEnd, unadjustedStart);
}

partial void SetupInlines()
{
Expand Down Expand Up @@ -237,12 +248,13 @@ private void OnDoubleTapped(DoubleTappedRoutedEventArgs e)
if (nullableSpan is { span: var span })
{
// Index
var index = Inlines.GetIndexAt(position, false, true);
var adjustedIndex = AdjustIndexForSurrogatePairs(Inlines.GetIndexAt(position, false, true));
var spanRange = Inlines.GetStartAndEndIndicesForSpan(span, false);
var chunk = GetChunkAt(Text[spanRange.start..spanRange.end], index - spanRange.start);
var adjustedRange = AdjustSelectionForSurrogatePairs(new Range(spanRange.start, spanRange.end));
var chunk = GetChunkAt(Text[adjustedRange.start..adjustedRange.end], adjustedIndex - adjustedRange.start);

// the chunk range will be relative to the span, so we have to add the offset of the span relative to the entire Text
Selection = new Range(spanRange.start + chunk.start, spanRange.start + chunk.start + chunk.length);
Selection = new Range(adjustedRange.start + chunk.start, adjustedRange.start + chunk.start + chunk.length);
}
}
}
Expand Down Expand Up @@ -291,7 +303,8 @@ private void OnDoubleTapped(DoubleTappedRoutedEventArgs e)
}
}

if (start <= index && index < i)
// the second condition handles the case of index == length, which happens when you e.g. click at the very end of a chunk
if (start <= index && index < i || i == length)
{
return (start, i - start);
}
Expand Down

0 comments on commit 8acee8b

Please sign in to comment.