Skip to content

Commit

Permalink
Avoid using TextAffinity in TextBoundary (flutter#117446)
Browse files Browse the repository at this point in the history 
* Avoid affinity like the plague

* ignore lint

* clean up

* fix test

* review

* Move wordboundary to text painter

* docs

* fix tests
  • Loading branch information
@LongCatIsLooong @loic-sharma
LongCatIsLooong authored and loic-sharma committed Jan 6, 2023
1 parent 7b58bf3 commit a60f474
Show file tree
Hide file tree
Showing 13 changed files with 551 additions and 726 deletions.
163 changes: 154 additions & 9 deletions packages/flutter/lib/src/painting/text_painter.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,133 @@ enum TextWidthBasis {
longestLine,
}

/// A [TextBoundary] subclass for locating word breaks.
///
/// The underlying implementation uses [UAX #29](https://unicode.org/reports/tr29/)
/// defined default word boundaries.
///
/// The default word break rules can be tailored to meet the requirements of
/// different use cases. For instance, the default rule set keeps horizontal
/// whitespaces together as a single word, which may not make sense in a
/// word-counting context -- "hello world" counts as 3 words instead of 2.
/// An example is the [moveByWordBoundary] variant, which is a tailored
/// word-break locator that more closely matches the default behavior of most
/// platforms and editors when it comes to handling text editing keyboard
/// shortcuts that move or delete word by word.
class WordBoundary extends TextBoundary {
/// Creates a [WordBoundary] with the text and layout information.
WordBoundary._(this._text, this._paragraph);

final InlineSpan _text;
final ui.Paragraph _paragraph;

@override
TextRange getTextBoundaryAt(int position) => _paragraph.getWordBoundary(TextPosition(offset: max(position, 0)));

// Combines two UTF-16 code units (high surrogate + low surrogate) into a
// single code point that represents a supplementary character.
static int _codePointFromSurrogates(int highSurrogate, int lowSurrogate) {
assert(
TextPainter._isHighSurrogate(highSurrogate),
'U+${highSurrogate.toRadixString(16).toUpperCase().padLeft(4, "0")}) is not a high surrogate.',
);
assert(
TextPainter._isLowSurrogate(lowSurrogate),
'U+${lowSurrogate.toRadixString(16).toUpperCase().padLeft(4, "0")}) is not a low surrogate.',
);
const int base = 0x010000 - (0xD800 << 10) - 0xDC00;
return (highSurrogate << 10) + lowSurrogate + base;
}

// The Runes class does not provide random access with a code unit offset.
int? _codePointAt(int index) {
final int? codeUnitAtIndex = _text.codeUnitAt(index);
if (codeUnitAtIndex == null) {
return null;
}
switch (codeUnitAtIndex & 0xFC00) {
case 0xD800:
return _codePointFromSurrogates(codeUnitAtIndex, _text.codeUnitAt(index + 1)!);
case 0xDC00:
return _codePointFromSurrogates(_text.codeUnitAt(index - 1)!, codeUnitAtIndex);
default:
return codeUnitAtIndex;
}
}

static bool _isNewline(int codePoint) {
switch (codePoint) {
case 0x000A:
case 0x0085:
case 0x000B:
case 0x000C:
case 0x2028:
case 0x2029:
return true;
default:
return false;
}
}

bool _skipSpacesAndPunctuations(int offset, bool forward) {
// Use code point since some punctuations are supplementary characters.
// "inner" here refers to the code unit that's before the break in the
// search direction (`forward`).
final int? innerCodePoint = _codePointAt(forward ? offset - 1 : offset);
final int? outerCodeUnit = _text.codeUnitAt(forward ? offset : offset - 1);

// Make sure the hard break rules in UAX#29 take precedence over the ones we
// add below. Luckily there're only 4 hard break rules for word breaks, and
// dictionary based breaking does not introduce new hard breaks:
// https://unicode-org.github.io/icu/userguide/boundaryanalysis/break-rules.html#word-dictionaries
//
// WB1 & WB2: always break at the start or the end of the text.
final bool hardBreakRulesApply = innerCodePoint == null || outerCodeUnit == null
// WB3a & WB3b: always break before and after newlines.
|| _isNewline(innerCodePoint) || _isNewline(outerCodeUnit);
return hardBreakRulesApply || !RegExp(r'[\p{Space_Separator}\p{Punctuation}]', unicode: true).hasMatch(String.fromCharCode(innerCodePoint));
}

/// Returns a [TextBoundary] suitable for handling keyboard navigation
/// commands that change the current selection word by word.
///
/// This [TextBoundary] is used by text widgets in the flutter framework to
/// provide default implementation for text editing shortcuts, for example,
/// "delete to the previous word".
///
/// The implementation applies the same set of rules [WordBoundary] uses,
/// except that word breaks end on a space separator or a punctuation will be
/// skipped, to match the behavior of most platforms. Additional rules may be
/// added in the future to better match platform behaviors.
late final TextBoundary moveByWordBoundary = _UntilTextBoundary(this, _skipSpacesAndPunctuations);
}

class _UntilTextBoundary extends TextBoundary {
const _UntilTextBoundary(this._textBoundary, this._predicate);

final UntilPredicate _predicate;
final TextBoundary _textBoundary;

@override
int? getLeadingTextBoundaryAt(int position) {
if (position < 0) {
return null;
}
final int? offset = _textBoundary.getLeadingTextBoundaryAt(position);
return offset == null || _predicate(offset, false)
? offset
: getLeadingTextBoundaryAt(offset - 1);
}

@override
int? getTrailingTextBoundaryAt(int position) {
final int? offset = _textBoundary.getTrailingTextBoundaryAt(max(position, 0));
return offset == null || _predicate(offset, true)
? offset
: getTrailingTextBoundaryAt(offset);
}
}

/// This is used to cache and pass the computed metrics regarding the
/// caret's size and position. This is preferred due to the expensive
/// nature of the calculation.
Expand Down Expand Up @@ -750,7 +877,7 @@ class TextPainter {

// Creates a ui.Paragraph using the current configurations in this class and
// assign it to _paragraph.
void _createParagraph() {
ui.Paragraph _createParagraph() {
assert(_paragraph == null || _rebuildParagraphForPaint);
final InlineSpan? text = this.text;
if (text == null) {
Expand All @@ -763,8 +890,9 @@ class TextPainter {
_debugMarkNeedsLayoutCallStack = null;
return true;
}());
_paragraph = builder.build();
final ui.Paragraph paragraph = _paragraph = builder.build();
_rebuildParagraphForPaint = false;
return paragraph;
}

void _layoutParagraph(double minWidth, double maxWidth) {
Expand Down Expand Up @@ -861,13 +989,18 @@ class TextPainter {
canvas.drawParagraph(_paragraph!, offset);
}

// Returns true iff the given value is a valid UTF-16 surrogate. The value
// Returns true iff the given value is a valid UTF-16 high surrogate. The value
// must be a UTF-16 code unit, meaning it must be in the range 0x0000-0xFFFF.
//
// See also:
// * https://en.wikipedia.org/wiki/UTF-16#Code_points_from_U+010000_to_U+10FFFF
static bool _isUtf16Surrogate(int value) {
return value & 0xF800 == 0xD800;
static bool _isHighSurrogate(int value) {
return value & 0xFC00 == 0xD800;
}

// Whether the given UTF-16 code unit is a low (second) surrogate.
static bool _isLowSurrogate(int value) {
return value & 0xFC00 == 0xDC00;
}

// Checks if the glyph is either [Unicode.RLM] or [Unicode.LRM]. These values take
Expand All @@ -886,7 +1019,7 @@ class TextPainter {
return null;
}
// TODO(goderbauer): doesn't handle extended grapheme clusters with more than one Unicode scalar value (https://github.com/flutter/flutter/issues/13404).
return _isUtf16Surrogate(nextCodeUnit) ? offset + 2 : offset + 1;
return _isHighSurrogate(nextCodeUnit) ? offset + 2 : offset + 1;
}

/// Returns the closest offset before `offset` at which the input cursor can
Expand All @@ -897,7 +1030,7 @@ class TextPainter {
return null;
}
// TODO(goderbauer): doesn't handle extended grapheme clusters with more than one Unicode scalar value (https://github.com/flutter/flutter/issues/13404).
return _isUtf16Surrogate(prevCodeUnit) ? offset - 2 : offset - 1;
return _isLowSurrogate(prevCodeUnit) ? offset - 2 : offset - 1;
}

// Unicode value for a zero width joiner character.
Expand All @@ -916,7 +1049,7 @@ class TextPainter {
const int NEWLINE_CODE_UNIT = 10;

// Check for multi-code-unit glyphs such as emojis or zero width joiner.
final bool needsSearch = _isUtf16Surrogate(prevCodeUnit) || _text!.codeUnitAt(offset) == _zwjUtf16 || _isUnicodeDirectionality(prevCodeUnit);
final bool needsSearch = _isHighSurrogate(prevCodeUnit) || _isLowSurrogate(prevCodeUnit) || _text!.codeUnitAt(offset) == _zwjUtf16 || _isUnicodeDirectionality(prevCodeUnit);
int graphemeClusterLength = needsSearch ? 2 : 1;
List<TextBox> boxes = <TextBox>[];
while (boxes.isEmpty) {
Expand Down Expand Up @@ -966,7 +1099,7 @@ class TextPainter {
final int nextCodeUnit = plainText.codeUnitAt(min(offset, plainTextLength - 1));

// Check for multi-code-unit glyphs such as emojis or zero width joiner
final bool needsSearch = _isUtf16Surrogate(nextCodeUnit) || nextCodeUnit == _zwjUtf16 || _isUnicodeDirectionality(nextCodeUnit);
final bool needsSearch = _isHighSurrogate(nextCodeUnit) || _isLowSurrogate(nextCodeUnit) || nextCodeUnit == _zwjUtf16 || _isUnicodeDirectionality(nextCodeUnit);
int graphemeClusterLength = needsSearch ? 2 : 1;
List<TextBox> boxes = <TextBox>[];
while (boxes.isEmpty) {
Expand Down Expand Up @@ -1141,6 +1274,18 @@ class TextPainter {
return _paragraph!.getWordBoundary(position);
}

/// {@template flutter.painting.TextPainter.wordBoundaries}
/// Returns a [TextBoundary] that can be used to perform word boundary analysis
/// on the current [text].
///
/// This [TextBoundary] uses word boundary rules defined in [Unicode Standard
/// Annex #29](http://www.unicode.org/reports/tr29/#Word_Boundaries).
/// {@endtemplate}
///
/// Currently word boundary analysis can only be performed after [layout]
/// has been called.
WordBoundary get wordBoundaries => WordBoundary._(text!, _paragraph!);

/// Returns the text range of the line at the given offset.
///
/// The newline (if any) is not returned as part of the range.
Expand Down
3 changes: 3 additions & 0 deletions packages/flutter/lib/src/rendering/editable.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2098,6 +2098,9 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
_setSelection(newSelection, cause);
}

/// {@macro flutter.painting.TextPainter.wordBoundaries}
WordBoundary get wordBoundaries => _textPainter.wordBoundaries;

/// Select a word around the location of the last tap down.
///
/// {@macro flutter.rendering.RenderEditable.selectPosition}
Expand Down
54 changes: 41 additions & 13 deletions packages/flutter/lib/src/rendering/paragraph.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1559,21 +1559,21 @@ class _SelectableFragment with Selectable, ChangeNotifier implements TextLayoutM
switch (granularity) {
case TextGranularity.character:
final String text = range.textInside(fullText);
newPosition = _getNextPosition(CharacterBoundary(text), targetedEdge, forward);
newPosition = _moveBeyondTextBoundaryAtDirection(targetedEdge, forward, CharacterBoundary(text));
result = SelectionResult.end;
break;
case TextGranularity.word:
final String text = range.textInside(fullText);
newPosition = _getNextPosition(WhitespaceBoundary(text) + WordBoundary(this), targetedEdge, forward);
final TextBoundary textBoundary = paragraph._textPainter.wordBoundaries.moveByWordBoundary;
newPosition = _moveBeyondTextBoundaryAtDirection(targetedEdge, forward, textBoundary);
result = SelectionResult.end;
break;
case TextGranularity.line:
newPosition = _getNextPosition(LineBreak(this), targetedEdge, forward);
newPosition = _moveToTextBoundaryAtDirection(targetedEdge, forward, LineBoundary(this));
result = SelectionResult.end;
break;
case TextGranularity.document:
final String text = range.textInside(fullText);
newPosition = _getNextPosition(DocumentBoundary(text), targetedEdge, forward);
newPosition = _moveBeyondTextBoundaryAtDirection(targetedEdge, forward, DocumentBoundary(text));
if (forward && newPosition.offset == range.end) {
result = SelectionResult.next;
} else if (!forward && newPosition.offset == range.start) {
Expand All @@ -1592,15 +1592,43 @@ class _SelectableFragment with Selectable, ChangeNotifier implements TextLayoutM
return result;
}

TextPosition _getNextPosition(TextBoundary boundary, TextPosition position, bool forward) {
if (forward) {
return _clampTextPosition(
(PushTextPosition.forward + boundary).getTrailingTextBoundaryAt(position)
);
// Move **beyond** the local boundary of the given type (unless range.start or
// range.end is reached). Used for most TextGranularity types except for
// TextGranularity.line, to ensure the selection movement doesn't get stuck at
// a local fixed point.
TextPosition _moveBeyondTextBoundaryAtDirection(TextPosition end, bool forward, TextBoundary textBoundary) {
final int newOffset = forward
? textBoundary.getTrailingTextBoundaryAt(end.offset) ?? range.end
: textBoundary.getLeadingTextBoundaryAt(end.offset - 1) ?? range.start;
return TextPosition(offset: newOffset);
}

// Move **to** the local boundary of the given type. Typically used for line
// boundaries, such that performing "move to line start" more than once never
// moves the selection to the previous line.
TextPosition _moveToTextBoundaryAtDirection(TextPosition end, bool forward, TextBoundary textBoundary) {
assert(end.offset >= 0);
final int caretOffset;
switch (end.affinity) {
case TextAffinity.upstream:
if (end.offset < 1 && !forward) {
assert (end.offset == 0);
return const TextPosition(offset: 0);
}
final CharacterBoundary characterBoundary = CharacterBoundary(fullText);
caretOffset = math.max(
0,
characterBoundary.getLeadingTextBoundaryAt(range.start + end.offset) ?? range.start,
) - 1;
break;
case TextAffinity.downstream:
caretOffset = end.offset;
break;
}
return _clampTextPosition(
(PushTextPosition.backward + boundary).getLeadingTextBoundaryAt(position),
);
final int offset = forward
? textBoundary.getTrailingTextBoundaryAt(caretOffset) ?? range.end
: textBoundary.getLeadingTextBoundaryAt(caretOffset) ?? range.start;
return TextPosition(offset: offset);
}

MapEntry<TextPosition, SelectionResult> _handleVerticalMovement(TextPosition position, {required double horizontalBaselineInParagraphCoordinates, required bool below}) {
Expand Down
Loading

0 comments on commit a60f474

Please sign in to comment.