diff --git a/bin/internal/goldens.version b/bin/internal/goldens.version index 4a51cfcb2df..acb3aae6dc3 100644 --- a/bin/internal/goldens.version +++ b/bin/internal/goldens.version @@ -1 +1 @@ -cb8264b1953000a603ecc2659ece3483859438a1 +3048ed023e5cc1d8e201f7abbba346e73cf5fa2c diff --git a/packages/flutter/lib/src/painting/text_painter.dart b/packages/flutter/lib/src/painting/text_painter.dart index 915b26f02f9..c558b588df1 100644 --- a/packages/flutter/lib/src/painting/text_painter.dart +++ b/packages/flutter/lib/src/painting/text_painter.dart @@ -15,6 +15,16 @@ import 'text_span.dart'; export 'package:flutter/services.dart' show TextRange, TextSelection; +class _CaretMetrics { + const _CaretMetrics({this.offset, this.fullHeight}); + /// The offset of the top left corner of the caret from the top left + /// corner of the paragraph. + final Offset offset; + + /// The full height of the glyph at the caret position. + final double fullHeight; +} + /// An object that paints a [TextSpan] tree into a [Canvas]. /// /// To use a [TextPainter], follow these steps: @@ -444,11 +454,11 @@ class TextPainter { // Unicode value for a zero width joiner character. static const int _zwjUtf16 = 0x200d; - // Get the Offset of the cursor (in logical pixels) based off the near edge + // Get the Rect of the cursor (in logical pixels) based off the near edge // of the character upstream from the given string offset. // TODO(garyq): Use actual extended grapheme cluster length instead of // an increasing cluster length amount to achieve deterministic performance. - Offset _getOffsetFromUpstream(int offset, Rect caretPrototype) { + Rect _getRectFromUpstream(int offset, Rect caretPrototype) { final String flattenedText = _text.toPlainText(); final int prevCodeUnit = _text.codeUnitAt(max(0, offset - 1)); if (prevCodeUnit == null) @@ -481,21 +491,21 @@ class TextPainter { // If the upstream character is a newline, cursor is at start of next line const int NEWLINE_CODE_UNIT = 10; if (prevCodeUnit == NEWLINE_CODE_UNIT) { - return Offset(_emptyOffset.dx, box.bottom); + return Rect.fromLTRB(_emptyOffset.dx, box.bottom, _emptyOffset.dx, box.bottom + box.bottom - box.top); } final double caretEnd = box.end; final double dx = box.direction == TextDirection.rtl ? caretEnd - caretPrototype.width : caretEnd; - return Offset(min(dx, width), box.top); + return Rect.fromLTRB(min(dx, width), box.top, min(dx, width), box.bottom); } return null; } - // Get the Offset of the cursor (in logical pixels) based off the near edge + // Get the Rect of the cursor (in logical pixels) based off the near edge // of the character downstream from the given string offset. // TODO(garyq): Use actual extended grapheme cluster length instead of // an increasing cluster length amount to achieve deterministic performance. - Offset _getOffsetFromDownstream(int offset, Rect caretPrototype) { + Rect _getRectFromDownstream(int offset, Rect caretPrototype) { final String flattenedText = _text.toPlainText(); // We cap the offset at the final index of the _text. final int nextCodeUnit = _text.codeUnitAt(min(offset, flattenedText == null ? 0 : flattenedText.length - 1)); @@ -526,7 +536,7 @@ class TextPainter { final TextBox box = boxes.last; final double caretStart = box.start; final double dx = box.direction == TextDirection.rtl ? caretStart - caretPrototype.width : caretStart; - return Offset(min(dx, width), box.top); + return Rect.fromLTRB(min(dx, width), box.top, min(dx, width), box.bottom); } return null; } @@ -568,20 +578,52 @@ class TextPainter { /// /// Valid only after [layout] has been called. Offset getOffsetForCaret(TextPosition position, Rect caretPrototype) { + _computeCaretMetrics(position, caretPrototype); + return _caretMetrics.offset; + } + + /// Returns the tight bounded height of the glyph at the given [position]. + /// + /// Valid only after [layout] has been called. + double getFullHeightForCaret(TextPosition position, Rect caretPrototype) { + _computeCaretMetrics(position, caretPrototype); + return _caretMetrics.fullHeight; + } + + // Cached caret metrics. This allows multiple invokes of [getOffsetForCaret] and + // [getFullHeightForCaret] in a row without performing redundant and expensive + // get rect calls to the paragraph. + _CaretMetrics _caretMetrics; + + // Holds the TextPosition and caretPrototype the last caret metrics were + // computed with. When new values are passed in, we recompute the caret metrics. + // only as nessecary. + TextPosition _previousCaretPosition; + Rect _previousCaretPrototype; + + // Checks if the [position] and [caretPrototype] have changed from the cached + // version and recomputes the metrics required to position the caret. + void _computeCaretMetrics(TextPosition position, Rect caretPrototype) { assert(!_needsLayout); + if (position == _previousCaretPosition && caretPrototype == _previousCaretPrototype) + return; final int offset = position.offset; assert(position.affinity != null); + Rect rect; switch (position.affinity) { - case TextAffinity.upstream: - return _getOffsetFromUpstream(offset, caretPrototype) - ?? _getOffsetFromDownstream(offset, caretPrototype) - ?? _emptyOffset; - case TextAffinity.downstream: - return _getOffsetFromDownstream(offset, caretPrototype) - ?? _getOffsetFromUpstream(offset, caretPrototype) - ?? _emptyOffset; + case TextAffinity.upstream: { + rect = _getRectFromUpstream(offset, caretPrototype) ?? _getRectFromDownstream(offset, caretPrototype); + break; + } + case TextAffinity.downstream: { + rect = _getRectFromDownstream(offset, caretPrototype) ?? _getRectFromUpstream(offset, caretPrototype); + break; + } } - return null; + _caretMetrics = _CaretMetrics( + offset: rect != null ? Offset(rect.left, rect.top) : _emptyOffset, + fullHeight: rect != null ? rect.bottom - rect.top : null, + ); } /// Returns a list of rects that bound the given selection. diff --git a/packages/flutter/lib/src/rendering/editable.dart b/packages/flutter/lib/src/rendering/editable.dart index 78a5ecc9a3c..92381fc775c 100644 --- a/packages/flutter/lib/src/rendering/editable.dart +++ b/packages/flutter/lib/src/rendering/editable.dart @@ -1476,6 +1476,13 @@ class RenderEditable extends RenderBox { _textLayoutLastWidth = constraintWidth; } + // TODO(garyq): This is no longer producing the highest-fidelity caret + // heights for Android, especially when non-alphabetic languages + // are involved. The current implementation overrides the height set + // here with the full measured height of the text on Android which looks + // superior (subjectively and in terms of fidelity). We should rework + // this properly to once again match the platform. + // /// On iOS, the cursor is taller than the the cursor on Android. The height /// of the cursor for iOS is approximate and obtained through an eyeball /// comparison. @@ -1520,17 +1527,28 @@ class RenderEditable extends RenderBox { void _paintCaret(Canvas canvas, Offset effectiveOffset, TextPosition textPosition) { assert(_textLayoutLastWidth == constraints.maxWidth); - final Offset caretOffset = _textPainter.getOffsetForCaret(textPosition, _caretPrototype); // If the floating cursor is enabled, the text cursor's color is [backgroundCursorColor] while // the floating cursor's color is _cursorColor; final Paint paint = Paint() ..color = _floatingCursorOn ? backgroundCursorColor : _cursorColor; - Rect caretRect = _caretPrototype.shift(caretOffset + effectiveOffset); + Rect caretRect = _caretPrototype.shift(_textPainter.getOffsetForCaret(textPosition, _caretPrototype) + effectiveOffset); if (_cursorOffset != null) caretRect = caretRect.shift(_cursorOffset); + // Override the height to take the full height of the glyph at the TextPosition + // when not on iOS. iOS has special handling that creates a taller caret. + // TODO(garyq): See the TODO for _getCaretPrototype. + if (defaultTargetPlatform != TargetPlatform.iOS && _textPainter.getFullHeightForCaret(textPosition, _caretPrototype) != null) { + caretRect = Rect.fromLTRB( + caretRect.left, + (caretRect.bottom + caretRect.top) / 2 - (_textPainter.getFullHeightForCaret(textPosition, _caretPrototype) / 2), + caretRect.right, + (caretRect.bottom + caretRect.top) / 2 + (_textPainter.getFullHeightForCaret(textPosition, _caretPrototype) / 2), + ); + } + caretRect = caretRect.shift(_getPixelPerfectCursorOffset(caretRect)); if (cursorRadius == null) { diff --git a/packages/flutter/test/rendering/editable_test.dart b/packages/flutter/test/rendering/editable_test.dart index d5e28bc89e9..28f0be549ad 100644 --- a/packages/flutter/test/rendering/editable_test.dart +++ b/packages/flutter/test/rendering/editable_test.dart @@ -134,7 +134,7 @@ void main() { expect(editable, paints..rect( color: const Color.fromARGB(0xFF, 0xFF, 0x00, 0x00), - rect: Rect.fromLTWH(40, 2, 1, 6), + rect: Rect.fromLTWH(40, 0, 1, 10), )); // Now change to a rounded caret. @@ -146,7 +146,7 @@ void main() { expect(editable, paints..rrect( color: const Color.fromARGB(0xFF, 0x00, 0x00, 0xFF), rrect: RRect.fromRectAndRadius( - Rect.fromLTWH(40, 2, 4, 6), + Rect.fromLTWH(40, 0, 4, 10), const Radius.circular(3), ), )); @@ -158,7 +158,80 @@ void main() { expect(editable, paints..rrect( color: const Color.fromARGB(0xFF, 0x00, 0x00, 0xFF), rrect: RRect.fromRectAndRadius( - Rect.fromLTWH(80, 2, 4, 16), + Rect.fromLTWH(80, 0, 4, 20), + const Radius.circular(3), + ), + )); + + // Can turn off caret. + showCursor.value = false; + pumpFrame(); + + expect(editable, paintsExactlyCountTimes(#drawRRect, 0)); + }); + + test('Cursor with ideographic script', () { + final TextSelectionDelegate delegate = FakeEditableTextState(); + final ValueNotifier showCursor = ValueNotifier(true); + EditableText.debugDeterministicCursor = true; + + final RenderEditable editable = RenderEditable( + backgroundCursorColor: Colors.grey, + textDirection: TextDirection.ltr, + cursorColor: const Color.fromARGB(0xFF, 0xFF, 0x00, 0x00), + offset: ViewportOffset.zero(), + textSelectionDelegate: delegate, + text: const TextSpan( + text: '中文测试文本是否正确', + style: TextStyle( + height: 1.0, fontSize: 10.0, fontFamily: 'Ahem', + ), + ), + selection: const TextSelection.collapsed( + offset: 4, + affinity: TextAffinity.upstream, + ), + ); + + layout(editable); + + editable.layout(BoxConstraints.loose(const Size(100, 100))); + expect( + editable, + // Draw no cursor by default. + paintsExactlyCountTimes(#drawRect, 0), + ); + + editable.showCursor = showCursor; + pumpFrame(); + + expect(editable, paints..rect( + color: const Color.fromARGB(0xFF, 0xFF, 0x00, 0x00), + rect: Rect.fromLTWH(40, 0, 1, 10), + )); + + // Now change to a rounded caret. + editable.cursorColor = const Color.fromARGB(0xFF, 0x00, 0x00, 0xFF); + editable.cursorWidth = 4; + editable.cursorRadius = const Radius.circular(3); + pumpFrame(); + + expect(editable, paints..rrect( + color: const Color.fromARGB(0xFF, 0x00, 0x00, 0xFF), + rrect: RRect.fromRectAndRadius( + Rect.fromLTWH(40, 0, 4, 10), + const Radius.circular(3), + ), + )); + + editable.textScaleFactor = 2; + pumpFrame(); + + // Now the caret height is much bigger due to the bigger font scale. + expect(editable, paints..rrect( + color: const Color.fromARGB(0xFF, 0x00, 0x00, 0xFF), + rrect: RRect.fromRectAndRadius( + Rect.fromLTWH(80, 0, 4, 20), const Radius.circular(3), ), )); diff --git a/packages/flutter/test/widgets/editable_text_cursor_test.dart b/packages/flutter/test/widgets/editable_text_cursor_test.dart index ba232ddd441..88a186dc6db 100644 --- a/packages/flutter/test/widgets/editable_text_cursor_test.dart +++ b/packages/flutter/test/widgets/editable_text_cursor_test.dart @@ -91,7 +91,7 @@ void main() { await expectLater( find.byKey(const ValueKey(1)), - matchesGoldenFile('editable_text_test.0.0.png'), + matchesGoldenFile('editable_text_test.0.1.png'), ); }, skip: !Platform.isLinux); @@ -142,7 +142,7 @@ void main() { await expectLater( find.byKey(const ValueKey(1)), - matchesGoldenFile('editable_text_test.1.0.png'), + matchesGoldenFile('editable_text_test.1.1.png'), ); }, skip: !Platform.isLinux); @@ -617,7 +617,7 @@ void main() { expect(editable, paints ..rrect( rrect: RRect.fromRectAndRadius( - Rect.fromLTRB(463.3333435058594, 2.0833332538604736, 465.3333435058594, 14.083333015441895), + Rect.fromLTRB(463.3333435058594, 0.0833332538604736, 465.3333435058594, 16.083333015441895), const Radius.circular(2.0), ), color: const Color(0xff8e8e93), @@ -642,7 +642,7 @@ void main() { expect(find.byType(EditableText), paints ..rrect( rrect: RRect.fromRectAndRadius( - Rect.fromLTRB(191.3333282470703, 2.0833332538604736, 193.3333282470703, 14.083333015441895), + Rect.fromLTRB(191.3333282470703, 0.0833332538604736, 193.3333282470703, 16.083333015441895), const Radius.circular(2.0), ), color: const Color(0xff8e8e93),