Make TextSpan hit testing precise. (#139717)

Fixes flutter/flutter#131435, #104594, #43400
Needs flutter/engine#48774 (to fix the web test failure).

Currently the method we use for text span hit testing `TextPainter.getPositionForOffset` always returns the closest `TextPosition`, even when the given offset is far away from the text. 

The new TextPaintes method tells you the layout bounds (`width =  letterspacing / 2 + x_advance + letterspacing / 2`, `height = font ascent + font descent`) of a character, the PR changes the hit testing implementation such that a TextSpan is only considered hit if the point-down event landed in one of it's character's layout bounds.

Potential issues:

1. In theory since the text is baseline aligned, we should use the max ascent and max descent of each character to calculate the height of the text span's hit-test region, in case some characters in the span have to fall back to a different font, but that will be slower and it typically doesn't make a huge difference. 

This is a breaking change. It also introduces a new finder and a new method `WidgetTester.tapOnText`: `await tester.tapOnText('string to match')` for ease of migration.

Author: LongCatIsLooong

packages/flutter/lib/src/painting/basic_types.dart

@@ -16,6 +16,7 @@ export 'dart:ui' show
   FontStyle,
   FontVariation,
   FontWeight,
+  GlyphInfo,
   ImageShader,
   Locale,
   MaskFilter,

packages/flutter/lib/src/painting/text_painter.dart

@@ -6,6 +6,7 @@ import 'dart:math' show max, min;
 import 'dart:ui' as ui show
   BoxHeightStyle,
   BoxWidthStyle,
+  GlyphInfo,
   LineMetrics,
   Paragraph,
   ParagraphBuilder,
@@ -24,6 +25,7 @@ import 'strut_style.dart';
 import 'text_scaler.dart';
 import 'text_span.dart';
 
+export 'dart:ui' show LineMetrics;
 export 'package:flutter/services.dart' show TextRange, TextSelection;
 
 /// The default font size if none is specified.
@@ -1493,7 +1495,24 @@ class TextPainter {
       : boxes.map((TextBox box) => _shiftTextBox(box, offset)).toList(growable: false);
   }
 
-  /// Returns the position within the text for the given pixel offset.
+  /// Returns the [GlyphInfo] of the glyph closest to the given `offset` in the
+  /// paragraph coordinate system, or null if the text is empty, or is entirely
+  /// clipped or ellipsized away.
+  ///
+  /// This method first finds the line closest to `offset.dy`, and then returns
+  /// the [GlyphInfo] of the closest glyph(s) within that line.
+   ui.GlyphInfo? getClosestGlyphForOffset(Offset offset) {
+    assert(_debugAssertTextLayoutIsValid);
+    assert(!_debugNeedsRelayout);
+    final _TextPainterLayoutCacheWithOffset cachedLayout = _layoutCache!;
+    final ui.GlyphInfo? rawGlyphInfo = cachedLayout.paragraph.getClosestGlyphInfoForOffset(offset - cachedLayout.paintOffset);
+    if (rawGlyphInfo == null || cachedLayout.paintOffset == Offset.zero) {
+      return rawGlyphInfo;
+    }
+    return ui.GlyphInfo(rawGlyphInfo.graphemeClusterLayoutBounds.shift(cachedLayout.paintOffset), rawGlyphInfo.graphemeClusterCodeUnitRange, rawGlyphInfo.writingDirection);
+  }
+
+  /// Returns the closest position within the text for the given pixel offset.
   TextPosition getPositionForOffset(Offset offset) {
     assert(_debugAssertTextLayoutIsValid);
     assert(!_debugNeedsRelayout);

packages/flutter/lib/src/painting/text_span.dart

@@ -343,18 +343,20 @@ class TextSpan extends InlineSpan implements HitTestTarget, MouseTrackerAnnotati
   /// Returns the text span that contains the given position in the text.
   @override
   InlineSpan? getSpanForPositionVisitor(TextPosition position, Accumulator offset) {
-    if (text == null) {
+    final String? text = this.text;
+    if (text == null || text.isEmpty) {
       return null;
     }
     final TextAffinity affinity = position.affinity;
     final int targetOffset = position.offset;
-    final int endOffset = offset.value + text!.length;
+    final int endOffset = offset.value + text.length;
+
     if (offset.value == targetOffset && affinity == TextAffinity.downstream ||
         offset.value < targetOffset && targetOffset < endOffset ||
         endOffset == targetOffset && affinity == TextAffinity.upstream) {
       return this;
     }
-    offset.increment(text!.length);
+    offset.increment(text.length);
     return null;
   }
 

packages/flutter/lib/src/rendering/editable.dart

@@ -1941,8 +1941,16 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
   @protected
   bool hitTestChildren(BoxHitTestResult result, { required Offset position }) {
     final Offset effectivePosition = position - _paintOffset;
-    final InlineSpan? textSpan = _textPainter.text;
-    switch (textSpan?.getSpanForPosition(_textPainter.getPositionForOffset(effectivePosition))) {
+    final GlyphInfo? glyph = _textPainter.getClosestGlyphForOffset(effectivePosition);
+    // The hit-test can't fall through the horizontal gaps between visually
+    // adjacent characters on the same line, even with a large letter-spacing or
+    // text justification, as graphemeClusterLayoutBounds.width is the advance
+    // width to the next character, so there's no gap between their
+    // graphemeClusterLayoutBounds rects.
+    final InlineSpan? spanHit = glyph != null && glyph.graphemeClusterLayoutBounds.contains(effectivePosition)
+      ? _textPainter.text!.getSpanForPosition(TextPosition(offset: glyph.graphemeClusterCodeUnitRange.start))
+      : null;
+    switch (spanHit) {
       case final HitTestTarget span:
         result.add(HitTestEntry(span));
         return true;

packages/flutter/lib/src/rendering/paragraph.dart

@@ -303,6 +303,7 @@ class RenderParagraph extends RenderBox with ContainerRenderObjectMixin<RenderBo
   }
 
   static final String _placeholderCharacter = String.fromCharCode(PlaceholderSpan.placeholderCodeUnit);
+
   final TextPainter _textPainter;
 
   List<AttributedString>? _cachedAttributedLabels;
@@ -730,9 +731,18 @@ class RenderParagraph extends RenderBox with ContainerRenderObjectMixin<RenderBo
   bool hitTestSelf(Offset position) => true;
 
   @override
+  @protected
   bool hitTestChildren(BoxHitTestResult result, { required Offset position }) {
-    final TextPosition textPosition = _textPainter.getPositionForOffset(position);
-    switch (_textPainter.text!.getSpanForPosition(textPosition)) {
+    final GlyphInfo? glyph = _textPainter.getClosestGlyphForOffset(position);
+    // The hit-test can't fall through the horizontal gaps between visually
+    // adjacent characters on the same line, even with a large letter-spacing or
+    // text justification, as graphemeClusterLayoutBounds.width is the advance
+    // width to the next character, so there's no gap between their
+    // graphemeClusterLayoutBounds rects.
+    final InlineSpan? spanHit = glyph != null && glyph.graphemeClusterLayoutBounds.contains(position)
+      ? _textPainter.text!.getSpanForPosition(TextPosition(offset: glyph.graphemeClusterCodeUnitRange.start))
+      : null;
+    switch (spanHit) {
       case final HitTestTarget span:
         result.add(HitTestEntry(span));
         return true;

packages/flutter/test/painting/text_span_test.dart

@@ -250,6 +250,24 @@ void main() {
     expect(textSpan2.compareTo(textSpan2), RenderComparison.identical);
   });
 
+  test('GetSpanForPosition', () {
+    const TextSpan textSpan = TextSpan(
+      text: '',
+      children: <InlineSpan>[
+        TextSpan(text: '', children: <InlineSpan>[
+          TextSpan(text: 'a'),
+        ]),
+        TextSpan(text: 'b'),
+        TextSpan(text: 'c'),
+      ],
+    );
+
+    expect((textSpan.getSpanForPosition(const TextPosition(offset: 0)) as TextSpan?)?.text, 'a');
+    expect((textSpan.getSpanForPosition(const TextPosition(offset: 1)) as TextSpan?)?.text, 'b');
+    expect((textSpan.getSpanForPosition(const TextPosition(offset: 2)) as TextSpan?)?.text, 'c');
+    expect((textSpan.getSpanForPosition(const TextPosition(offset: 3)) as TextSpan?)?.text, isNull);
+  });
+
   test('GetSpanForPosition with WidgetSpan', () {
     const TextSpan textSpan = TextSpan(
       text: 'a',

packages/flutter/test/rendering/editable_test.dart

@@ -12,6 +12,10 @@ import 'package:flutter_test/flutter_test.dart';
 
 import 'rendering_tester.dart';
 
+double _caretMarginOf(RenderEditable renderEditable) {
+  return renderEditable.cursorWidth + 1.0;
+}
+
 void _applyParentData(List<RenderBox> inlineRenderBoxes, InlineSpan span) {
   int index = 0;
   RenderBox? previousBox;
@@ -1184,8 +1188,107 @@ void main() {
     });
 
     group('hit testing', () {
+      final TextSelectionDelegate delegate = _FakeEditableTextState();
+
+      test('Basic TextSpan Hit testing', () {
+        final TextSpan textSpanA = TextSpan(text: 'A' * 10);
+        const TextSpan textSpanBC = TextSpan(text: 'BC', style: TextStyle(letterSpacing: 26.0));
+
+        final TextSpan text = TextSpan(
+          text: '',
+          style: const TextStyle(fontSize: 10.0),
+          children: <InlineSpan>[textSpanA, textSpanBC],
+        );
+
+        final RenderEditable renderEditable = RenderEditable(
+          text: text,
+          maxLines: null,
+          startHandleLayerLink: LayerLink(),
+          endHandleLayerLink: LayerLink(),
+          textDirection: TextDirection.ltr,
+          offset: ViewportOffset.fixed(0.0),
+          textSelectionDelegate: delegate,
+          selection: const TextSelection.collapsed(offset: 0),
+        );
+        layout(renderEditable, constraints:  BoxConstraints.tightFor(width: 100.0 + _caretMarginOf(renderEditable)));
+
+        BoxHitTestResult result;
+
+        // Hit-testing the first line
+        // First A
+        expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(5.0, 5.0)), isTrue);
+        expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[textSpanA]);
+        // The last A.
+        expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(95.0, 5.0)), isTrue);
+        expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[textSpanA]);
+        // Far away from the line.
+        expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(200.0, 5.0)), isFalse);
+        expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[]);
+
+        // Hit-testing the second line
+        // Tapping on B (startX = letter-spacing / 2 = 13.0).
+        expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(18.0, 15.0)), isTrue);
+        expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[textSpanBC]);
+
+        // Between B and C, with large letter-spacing.
+        expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(31.0, 15.0)), isTrue);
+        expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[textSpanBC]);
+
+        // On C.
+        expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(54.0, 15.0)), isTrue);
+        expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[textSpanBC]);
+
+        // After C.
+        expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(100.0, 15.0)), isTrue);
+        expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[]);
+
+        // Not even remotely close.
+        expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(9999.0, 9999.0)), isFalse);
+        expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[]);
+      });
+
+      test('TextSpan Hit testing with text justification', () {
+        const TextSpan textSpanA = TextSpan(text: 'A ');      // The space is a word break.
+        const TextSpan textSpanB = TextSpan(text: 'B\u200B'); // The zero-width space is used as a line break.
+        final TextSpan textSpanC = TextSpan(text: 'C' * 10);  // The third span starts a new line since it's too long for the first line.
+
+        // The text should look like:
+        // A        B
+        // CCCCCCCCCC
+        final TextSpan text = TextSpan(
+          text: '',
+          style: const TextStyle(fontSize: 10.0),
+          children: <InlineSpan>[textSpanA, textSpanB, textSpanC],
+        );
+        final RenderEditable renderEditable = RenderEditable(
+          text: text,
+          maxLines: null,
+          startHandleLayerLink: LayerLink(),
+          endHandleLayerLink: LayerLink(),
+          textDirection: TextDirection.ltr,
+          textAlign: TextAlign.justify,
+          offset: ViewportOffset.fixed(0.0),
+          textSelectionDelegate: delegate,
+          selection: const TextSelection.collapsed(offset: 0),
+        );
+
+        layout(renderEditable, constraints: BoxConstraints.tightFor(width: 100.0 + _caretMarginOf(renderEditable)));
+        BoxHitTestResult result;
+
+        // Tapping on A.
+        expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(5.0, 5.0)), isTrue);
+        expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[textSpanA]);
+
+        // Between A and B.
+        expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(50.0, 5.0)), isTrue);
+        expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[textSpanA]);
+
+        // On B.
+        expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(95.0, 5.0)), isTrue);
+        expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[textSpanB]);
+      });
+
       test('hits correct TextSpan when not scrolled', () {
-        final TextSelectionDelegate delegate = _FakeEditableTextState();
         final RenderEditable editable = RenderEditable(
           text: const TextSpan(
             style: TextStyle(height: 1.0, fontSize: 10.0),
@@ -1692,7 +1795,8 @@ void main() {
       // Prepare for painting after layout.
       pumpFrame(phase: EnginePhase.compositingBits);
       BoxHitTestResult result = BoxHitTestResult();
-      editable.hitTest(result, position: Offset.zero);
+      // The WidgetSpans have a height of 14.0, so "test" has a y offset of 4.0.
+      editable.hitTest(result, position: const Offset(1.0, 5.0));
       // We expect two hit test entries in the path because the RenderEditable
       // will add itself as well.
       expect(result.path, hasLength(2));
@@ -1702,7 +1806,7 @@ void main() {
       // Only testing the RenderEditable entry here once, not anymore below.
       expect(result.path.last.target, isA<RenderEditable>());
       result = BoxHitTestResult();
-      editable.hitTest(result, position: const Offset(15.0, 0.0));
+      editable.hitTest(result, position: const Offset(15.0, 5.0));
       expect(result.path, hasLength(2));
       target = result.path.first.target;
       expect(target, isA<TextSpan>());
@@ -1775,7 +1879,8 @@ void main() {
       // Prepare for painting after layout.
       pumpFrame(phase: EnginePhase.compositingBits);
       BoxHitTestResult result = BoxHitTestResult();
-      editable.hitTest(result, position: Offset.zero);
+      // The WidgetSpans have a height of 14.0, so "test" has a y offset of 4.0.
+      editable.hitTest(result, position: const Offset(0.0, 4.0));
       // We expect two hit test entries in the path because the RenderEditable
       // will add itself as well.
       expect(result.path, hasLength(2));
@@ -1785,13 +1890,14 @@ void main() {
       // Only testing the RenderEditable entry here once, not anymore below.
       expect(result.path.last.target, isA<RenderEditable>());
       result = BoxHitTestResult();
-      editable.hitTest(result, position: const Offset(15.0, 0.0));
+      editable.hitTest(result, position: const Offset(15.0, 4.0));
       expect(result.path, hasLength(2));
       target = result.path.first.target;
       expect(target, isA<TextSpan>());
       expect((target as TextSpan).text, text);
 
       result = BoxHitTestResult();
+      // "test" is 40 pixel wide.
       editable.hitTest(result, position: const Offset(41.0, 0.0));
       expect(result.path, hasLength(3));
       target = result.path.first.target;
@@ -1814,7 +1920,7 @@ void main() {
 
       result = BoxHitTestResult();
       editable.hitTest(result, position: const Offset(5.0, 15.0));
-      expect(result.path, hasLength(2));
+      expect(result.path, hasLength(1)); // Only the RenderEditable.
     }, skip: isBrowser); // https://github.com/flutter/flutter/issues/61020
   });