diff --git a/packages/flutter/lib/src/painting/text_painter.dart b/packages/flutter/lib/src/painting/text_painter.dart index 34de9f5ac5..dc5c53d6bb 100644 --- a/packages/flutter/lib/src/painting/text_painter.dart +++ b/packages/flutter/lib/src/painting/text_painter.dart @@ -1044,6 +1044,7 @@ class TextPainter { // Get the caret metrics (in logical pixels) based off the near edge of the // character upstream from the given string offset. _CaretMetrics? _getMetricsFromUpstream(int offset) { + assert(offset >= 0); final int plainTextLength = plainText.length; if (plainTextLength == 0 || offset > plainTextLength) { return null; @@ -1061,7 +1062,7 @@ class TextPainter { final int prevRuneOffset = offset - graphemeClusterLength; // Use BoxHeightStyle.strut to ensure that the caret's height fits within // the line's height and is consistent throughout the line. - boxes = _paragraph!.getBoxesForRange(prevRuneOffset, offset, boxHeightStyle: ui.BoxHeightStyle.strut); + boxes = _paragraph!.getBoxesForRange(max(0, prevRuneOffset), offset, boxHeightStyle: ui.BoxHeightStyle.strut); // When the range does not include a full cluster, no boxes will be returned. if (boxes.isEmpty) { // When we are at the beginning of the line, a non-surrogate position will @@ -1079,7 +1080,12 @@ class TextPainter { graphemeClusterLength *= 2; continue; } - final TextBox box = boxes.first; + + // Try to identify the box nearest the offset. This logic works when + // there's just one box, and when all boxes have the same direction. + // It may not work in bidi text: https://github.com/flutter/flutter/issues/123424 + final TextBox box = boxes.last.direction == TextDirection.ltr + ? boxes.last : boxes.first; return prevCodeUnit == NEWLINE_CODE_UNIT ? _EmptyLineCaretMetrics(lineVerticalOffset: box.bottom) @@ -1087,11 +1093,13 @@ class TextPainter { } return null; } + // Get the caret metrics (in logical pixels) based off the near edge of the // character downstream from the given string offset. _CaretMetrics? _getMetricsFromDownstream(int offset) { + assert(offset >= 0); final int plainTextLength = plainText.length; - if (plainTextLength == 0 || offset < 0) { + if (plainTextLength == 0) { return null; } // We cap the offset at the final index of plain text. @@ -1123,7 +1131,13 @@ class TextPainter { graphemeClusterLength *= 2; continue; } - final TextBox box = boxes.last; + + // Try to identify the box nearest the offset. This logic works when + // there's just one box, and when all boxes have the same direction. + // It may not work in bidi text: https://github.com/flutter/flutter/issues/123424 + final TextBox box = boxes.first.direction == TextDirection.ltr + ? boxes.first : boxes.last; + return _LineCaretMetrics(offset: Offset(box.start, box.top), writingDirection: box.direction, fullHeight: box.bottom - box.top); } return null; @@ -1159,7 +1173,13 @@ class TextPainter { /// /// Valid only after [layout] has been called. Offset getOffsetForCaret(TextPosition position, Rect caretPrototype) { - final _CaretMetrics caretMetrics = _computeCaretMetrics(position); + final _CaretMetrics caretMetrics; + if (position.offset < 0) { + // TODO(LongCatIsLooong): make this case impossible; see https://github.com/flutter/flutter/issues/79495 + caretMetrics = const _EmptyLineCaretMetrics(lineVerticalOffset: 0); + } else { + caretMetrics = _computeCaretMetrics(position); + } if (caretMetrics is _EmptyLineCaretMetrics) { final double paintOffsetAlignment = _computePaintOffsetFraction(textAlign, textDirection!); @@ -1192,6 +1212,10 @@ class TextPainter { /// /// Valid only after [layout] has been called. double? getFullHeightForCaret(TextPosition position, Rect caretPrototype) { + if (position.offset < 0) { + // TODO(LongCatIsLooong): make this case impossible; see https://github.com/flutter/flutter/issues/79495 + return null; + } final _CaretMetrics caretMetrics = _computeCaretMetrics(position); return caretMetrics is _LineCaretMetrics ? caretMetrics.fullHeight : null; } @@ -1232,10 +1256,11 @@ class TextPainter { /// Returns a list of rects that bound the given selection. /// + /// The [selection] must be a valid range (with [TextSelection.isValid] true). + /// /// The [boxHeightStyle] and [boxWidthStyle] arguments may be used to select /// the shape of the [TextBox]s. These properties default to - /// [ui.BoxHeightStyle.tight] and [ui.BoxWidthStyle.tight] respectively and - /// must not be null. + /// [ui.BoxHeightStyle.tight] and [ui.BoxWidthStyle.tight] respectively. /// /// A given selection might have more than one rect if this text painter /// contains bidirectional text because logically contiguous text might not be @@ -1253,6 +1278,7 @@ class TextPainter { ui.BoxWidthStyle boxWidthStyle = ui.BoxWidthStyle.tight, }) { assert(_debugAssertTextLayoutIsValid); + assert(selection.isValid); return _paragraph!.getBoxesForRange( selection.start, selection.end, diff --git a/packages/flutter/lib/src/rendering/editable.dart b/packages/flutter/lib/src/rendering/editable.dart index 274af193ca..ffdad4cff6 100644 --- a/packages/flutter/lib/src/rendering/editable.dart +++ b/packages/flutter/lib/src/rendering/editable.dart @@ -720,6 +720,11 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin, void _updateSelectionExtentsVisibility(Offset effectiveOffset) { assert(selection != null); + if (!selection!.isValid) { + _selectionStartInViewport.value = false; + _selectionEndInViewport.value = false; + return; + } final Rect visibleRegion = Offset.zero & size; final Offset startOffset = _textPainter.getOffsetForCaret( @@ -3010,8 +3015,7 @@ class _FloatingCursorPainter extends RenderEditablePainter { final TextSelection? selection = renderEditable.selection; - // TODO(LongCatIsLooong): skip painting the caret when the selection is - // (-1, -1). + // TODO(LongCatIsLooong): skip painting caret when selection is (-1, -1): https://github.com/flutter/flutter/issues/79495 if (selection == null || !selection.isCollapsed) { return; } diff --git a/packages/flutter/test/painting/text_painter_test.dart b/packages/flutter/test/painting/text_painter_test.dart index 75aae4e275..9c7eb0bdee 100644 --- a/packages/flutter/test/painting/text_painter_test.dart +++ b/packages/flutter/test/painting/text_painter_test.dart @@ -8,12 +8,121 @@ import 'package:flutter/foundation.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; +void _checkCaretOffsetsLtrAt(String text, List boundaries) { + expect(boundaries.first, 0); + expect(boundaries.last, text.length); + + final TextPainter painter = TextPainter() + ..textDirection = TextDirection.ltr; + + // Lay out the string up to each boundary, and record the width. + final List prefixWidths = []; + for (final int boundary in boundaries) { + painter.text = TextSpan(text: text.substring(0, boundary)); + painter.layout(); + prefixWidths.add(painter.width); + } + + // The painter has the full text laid out. Check the caret offsets. + double caretOffset(int offset) { + final TextPosition position = ui.TextPosition(offset: offset); + return painter.getOffsetForCaret(position, ui.Rect.zero).dx; + } + expect(boundaries.map(caretOffset).toList(), prefixWidths); + double lastOffset = caretOffset(0); + for (int i = 1; i <= text.length; i++) { + final double offset = caretOffset(i); + expect(offset, greaterThanOrEqualTo(lastOffset)); + lastOffset = offset; + } + painter.dispose(); +} + +/// Check the caret offsets are accurate for the given single line of LTR text. +/// +/// This lays out the given text as a single line with [TextDirection.ltr] +/// and checks the following invariants, which should always hold if the text +/// is made up of LTR characters: +/// * The caret offsets go monotonically from 0.0 to the width of the text. +/// * At each character (that is, grapheme cluster) boundary, the caret +/// offset equals the width that the text up to that point would have +/// if laid out on its own. +/// +/// If you have a [TextSpan] instead of a plain [String], +/// see [caretOffsetsForTextSpan]. +void checkCaretOffsetsLtr(String text) { + final List characterBoundaries = []; + final CharacterRange range = CharacterRange.at(text, 0); + while (true) { + characterBoundaries.add(range.current.length); + if (range.stringAfterLength <= 0) { + break; + } + range.expandNext(); + } + _checkCaretOffsetsLtrAt(text, characterBoundaries); +} + +/// Check the caret offsets are accurate for the given single line of LTR text, +/// ignoring character boundaries within each given cluster. +/// +/// This concatenates [clusters] into a string and then performs the same +/// checks as [checkCaretOffsetsLtr], except that instead of checking the +/// offset-equals-prefix-width invariant at every character boundary, +/// it does so only at the boundaries between the elements of [clusters]. +/// +/// The elements of [clusters] should be composed of whole characters: each +/// element should be a valid character range in the concatenated string. +/// +/// Consider using [checkCaretOffsetsLtr] instead of this function. If that +/// doesn't pass, you may have an instance of . +void checkCaretOffsetsLtrFromPieces(List clusters) { + final StringBuffer buffer = StringBuffer(); + final List boundaries = []; + boundaries.add(buffer.length); + for (final String cluster in clusters) { + buffer.write(cluster); + boundaries.add(buffer.length); + } + _checkCaretOffsetsLtrAt(buffer.toString(), boundaries); +} + +/// Compute the caret offsets for the given single line of text, a [TextSpan]. +/// +/// This lays out the given text as a single line with the given [textDirection] +/// and returns a full list of caret offsets, one at each code unit boundary. +/// +/// This also checks that the offset at the very start or very end, if the text +/// direction is RTL or LTR respectively, equals the line's width. +/// +/// If you have a [String] instead of a nontrivial [TextSpan], +/// consider using [checkCaretOffsetsLtr] instead. +List caretOffsetsForTextSpan(TextDirection textDirection, TextSpan text) { + final TextPainter painter = TextPainter() + ..textDirection = textDirection + ..text = text + ..layout(); + final int length = text.toPlainText().length; + final List result = List.generate(length + 1, (int offset) { + final TextPosition position = ui.TextPosition(offset: offset); + return painter.getOffsetForCaret(position, ui.Rect.zero).dx; + }); + switch (textDirection) { + case TextDirection.ltr: expect(result[length], painter.width); + case TextDirection.rtl: expect(result[0], painter.width); + } + painter.dispose(); + return result; +} + void main() { test('TextPainter caret test', () { final TextPainter painter = TextPainter() ..textDirection = TextDirection.ltr; String text = 'A'; + checkCaretOffsetsLtr(text); + painter.text = TextSpan(text: text); painter.layout(); @@ -28,6 +137,7 @@ void main() { // Check that getOffsetForCaret handles a character that is encoded as a // surrogate pair. text = 'A\u{1F600}'; + checkCaretOffsetsLtr(text); painter.text = TextSpan(text: text); painter.layout(); caretOffset = painter.getOffsetForCaret(ui.TextPosition(offset: text.length), ui.Rect.zero); @@ -87,6 +197,8 @@ void main() { // Format: 'πŸ‘©β€πŸ‘©β€πŸ‘¦πŸ‘©β€πŸ‘©β€πŸ‘§β€πŸ‘§πŸ‘' // One three-person family, one four-person family, one clapping hands (medium skin tone). const String text = 'πŸ‘©β€πŸ‘©β€πŸ‘¦πŸ‘©β€πŸ‘©β€πŸ‘§β€πŸ‘§πŸ‘πŸ½'; + checkCaretOffsetsLtr(text); + painter.text = const TextSpan(text: text); painter.layout(maxWidth: 10000); @@ -147,6 +259,90 @@ void main() { painter.dispose(); }, skip: isBrowser && !isCanvasKit); // https://github.com/flutter/flutter/issues/56308 + test('TextPainter caret emoji tests: single, long emoji', () { + // Regression test for https://github.com/flutter/flutter/issues/50563 + checkCaretOffsetsLtr('πŸ‘©β€πŸš€'); + checkCaretOffsetsLtr('πŸ‘©β€β€οΈβ€πŸ’‹β€πŸ‘©'); + checkCaretOffsetsLtr('πŸ‘¨β€πŸ‘©β€πŸ‘¦β€πŸ‘¦'); + checkCaretOffsetsLtr('πŸ‘¨πŸΎβ€πŸ€β€πŸ‘¨πŸ»'); + checkCaretOffsetsLtr('πŸ‘¨β€πŸ‘¦'); + checkCaretOffsetsLtr('πŸ‘©β€πŸ‘¦'); + checkCaretOffsetsLtr('πŸŒπŸΏβ€β™€οΈ'); + checkCaretOffsetsLtr('πŸŠβ€β™€οΈ'); + checkCaretOffsetsLtr('πŸ„πŸ»β€β™‚οΈ'); + + // These actually worked even before #50563 was fixed (because + // their lengths in code units are powers of 2, namely 4 and 8). + checkCaretOffsetsLtr('πŸ‡ΊπŸ‡³'); + checkCaretOffsetsLtr('πŸ‘©β€β€οΈβ€πŸ‘¨'); + }, skip: isBrowser && !isCanvasKit); // https://github.com/flutter/flutter/issues/56308 + + test('TextPainter caret emoji test: letters, then 1 emoji of 5 code units', () { + // Regression test for https://github.com/flutter/flutter/issues/50563 + checkCaretOffsetsLtr('aπŸ‘©β€πŸš€'); + checkCaretOffsetsLtr('abπŸ‘©β€πŸš€'); + checkCaretOffsetsLtr('abcπŸ‘©β€πŸš€'); + checkCaretOffsetsLtr('abcdπŸ‘©β€πŸš€'); + }, skip: isBrowser && !isCanvasKit); // https://github.com/flutter/flutter/issues/56308 + + test('TextPainter caret zalgo test', () { + // Regression test for https://github.com/flutter/flutter/issues/98516 + checkCaretOffsetsLtr('ZΝ₯Ν¬ΜΎΝ‰Μ³ΜΊaΜ’Μ’ΝŒΜ‹ΝͺΜ΄Ν•Μ²lΝ€Μ€ΜšΜˆΝœΜ¨ΝŽΜ°Μ˜Ν‰ΜŸgΜ’ΝΝ…Ν•Ν”Μ€Ν–ΜŸoΜΝ―ΜšΜ…ΝͺΜ†Ν£Μ‘Μ΅Μ‘ΜΌΝš'); + }, skip: isBrowser && !isCanvasKit); // https://github.com/flutter/flutter/issues/56308 + + test('TextPainter caret Devanagari test', () { + // Regression test for https://github.com/flutter/flutter/issues/118403 + checkCaretOffsetsLtrFromPieces( + ['ΰ€ͺΰ₯ΰ€°ΰ€Ύ', 'ΰ€ͺΰ₯ΰ€€', ' ', 'ΰ€΅', 'ΰ€°ΰ₯ΰ€£', 'ΰ€¨', ' ', 'ΰ€ͺΰ₯ΰ€°', 'ΰ€΅ΰ₯ΰ€°ΰ₯', 'ΰ€€ΰ€Ώ']); + }, skip: isBrowser && !isCanvasKit); // https://github.com/flutter/flutter/issues/56308 + + test('TextPainter caret Devanagari test, full strength', () { + // Regression test for https://github.com/flutter/flutter/issues/118403 + checkCaretOffsetsLtr('ΰ€ͺΰ₯ΰ€°ΰ€Ύΰ€ͺΰ₯ΰ€€ ΰ€΅ΰ€°ΰ₯ΰ€£ΰ€¨ ΰ€ͺΰ₯ΰ€°ΰ€΅ΰ₯ΰ€°ΰ₯ΰ€€ΰ€Ώ'); + }, skip: true); // https://github.com/flutter/flutter/issues/122478 + + test('TextPainter caret emoji test LTR: letters next to emoji, as separate TextBoxes', () { + // Regression test for https://github.com/flutter/flutter/issues/122477 + // The trigger for this bug was to have SkParagraph report separate + // TextBoxes for the emoji and for the characters next to it. + // In normal usage on a real device, this can happen by simply typing + // letters and then an emoji, presumably because they get different fonts. + // In these tests, our single test font covers both letters and emoji, + // so we provoke the same effect by adding styles. + expect(caretOffsetsForTextSpan( + TextDirection.ltr, + const TextSpan(children: [ + TextSpan(text: 'πŸ‘©β€πŸš€', style: TextStyle()), + TextSpan(text: ' words', style: TextStyle(fontWeight: FontWeight.bold)), + ])), + [0, 28, 28, 28, 28, 28, 42, 56, 70, 84, 98, 112]); + expect(caretOffsetsForTextSpan( + TextDirection.ltr, + const TextSpan(children: [ + TextSpan(text: 'words ', style: TextStyle(fontWeight: FontWeight.bold)), + TextSpan(text: 'πŸ‘©β€πŸš€', style: TextStyle()), + ])), + [0, 14, 28, 42, 56, 70, 84, 84, 84, 84, 84, 112]); + }, skip: isBrowser && !isCanvasKit); // https://github.com/flutter/flutter/issues/56308 + + test('TextPainter caret emoji test RTL: letters next to emoji, as separate TextBoxes', () { + // Regression test for https://github.com/flutter/flutter/issues/122477 + expect(caretOffsetsForTextSpan( + TextDirection.rtl, + const TextSpan(children: [ + TextSpan(text: 'πŸ‘©β€πŸš€', style: TextStyle()), + TextSpan(text: ' ΧžΧ™ΧœΧ™Χ', style: TextStyle(fontWeight: FontWeight.bold)), + ])), + [112, 84, 84, 84, 84, 84, 70, 56, 42, 28, 14, 0]); + expect(caretOffsetsForTextSpan( + TextDirection.rtl, + const TextSpan(children: [ + TextSpan(text: 'ΧžΧ™ΧœΧ™Χ ', style: TextStyle(fontWeight: FontWeight.bold)), + TextSpan(text: 'πŸ‘©β€πŸš€', style: TextStyle()), + ])), + [112, 98, 84, 70, 56, 42, 28, 28, 28, 28, 28, 0]); + }, skip: isBrowser && !isCanvasKit); // https://github.com/flutter/flutter/issues/56308 + test('TextPainter caret center space test', () { final TextPainter painter = TextPainter() ..textDirection = TextDirection.ltr;