Use full height of the glyph for caret height on Android (#30991)

bin/internal/goldens.version

@@ -1 +1 @@
-cb8264b1953000a603ecc2659ece3483859438a1
+3048ed023e5cc1d8e201f7abbba346e73cf5fa2c

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;
       }
-    return null;
+      case TextAffinity.downstream: {
+        rect = _getRectFromDownstream(offset, caretPrototype) ??  _getRectFromUpstream(offset, caretPrototype);
+        break;
+      }
+    }
+    _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.

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) {

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<bool> showCursor = ValueNotifier<bool>(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),
       ),
     ));

packages/flutter/test/widgets/editable_text_cursor_test.dart

@@ -91,7 +91,7 @@ void main() {
 
     await expectLater(
       find.byKey(const ValueKey<int>(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<int>(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),