Reverts "Make TextSpan hit testing precise." (#140468)

Reverts flutter/flutter#139717
Initiated by: LongCatIsLooong
This change reverts the following previous change:
Original Description:
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: auto-submit[bot]

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

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

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

@@ -6,7 +6,6 @@ import 'dart:math' show max, min;
 import 'dart:ui' as ui show
   BoxHeightStyle,
   BoxWidthStyle,
-  GlyphInfo,
   LineMetrics,
   Paragraph,
   ParagraphBuilder,
@@ -25,7 +24,6 @@ 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.
@@ -1495,24 +1493,7 @@ class TextPainter {
       : boxes.map((TextBox box) => _shiftTextBox(box, offset)).toList(growable: false);
   }
 
-  /// 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.
+  /// Returns the 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,20 +343,18 @@ 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) {
-    final String? text = this.text;
-    if (text == null || text.isEmpty) {
+    if (text == null) {
       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,16 +1941,8 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
   @protected
   bool hitTestChildren(BoxHitTestResult result, { required Offset position }) {
     final Offset effectivePosition = position - _paintOffset;
-    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) {
+    final InlineSpan? textSpan = _textPainter.text;
+    switch (textSpan?.getSpanForPosition(_textPainter.getPositionForOffset(effectivePosition))) {
       case final HitTestTarget span:
         result.add(HitTestEntry(span));
         return true;

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

@@ -303,7 +303,6 @@ class RenderParagraph extends RenderBox with ContainerRenderObjectMixin<RenderBo
   }
 
   static final String _placeholderCharacter = String.fromCharCode(PlaceholderSpan.placeholderCodeUnit);
-
   final TextPainter _textPainter;
 
   List<AttributedString>? _cachedAttributedLabels;
@@ -731,18 +730,9 @@ class RenderParagraph extends RenderBox with ContainerRenderObjectMixin<RenderBo
   bool hitTestSelf(Offset position) => true;
 
   @override
-  @protected
   bool hitTestChildren(BoxHitTestResult result, { required Offset position }) {
-    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) {
+    final TextPosition textPosition = _textPainter.getPositionForOffset(position);
+    switch (_textPainter.text!.getSpanForPosition(textPosition)) {
       case final HitTestTarget span:
         result.add(HitTestEntry(span));
         return true;

packages/flutter/test/painting/text_span_test.dart

@@ -250,24 +250,6 @@ 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,10 +12,6 @@ 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;
@@ -1188,107 +1184,8 @@ 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),
@@ -1795,8 +1692,7 @@ void main() {
       // Prepare for painting after layout.
       pumpFrame(phase: EnginePhase.compositingBits);
       BoxHitTestResult result = BoxHitTestResult();
-      // 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));
+      editable.hitTest(result, position: Offset.zero);
       // We expect two hit test entries in the path because the RenderEditable
       // will add itself as well.
       expect(result.path, hasLength(2));
@@ -1806,7 +1702,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, 5.0));
+      editable.hitTest(result, position: const Offset(15.0, 0.0));
       expect(result.path, hasLength(2));
       target = result.path.first.target;
       expect(target, isA<TextSpan>());
@@ -1879,8 +1775,7 @@ void main() {
       // Prepare for painting after layout.
       pumpFrame(phase: EnginePhase.compositingBits);
       BoxHitTestResult result = BoxHitTestResult();
-      // 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));
+      editable.hitTest(result, position: Offset.zero);
       // We expect two hit test entries in the path because the RenderEditable
       // will add itself as well.
       expect(result.path, hasLength(2));
@@ -1890,14 +1785,13 @@ 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, 4.0));
+      editable.hitTest(result, position: const Offset(15.0, 0.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;
@@ -1920,7 +1814,7 @@ void main() {
 
       result = BoxHitTestResult();
       editable.hitTest(result, position: const Offset(5.0, 15.0));
-      expect(result.path, hasLength(1)); // Only the RenderEditable.
+      expect(result.path, hasLength(2));
     }, skip: isBrowser); // https://github.com/flutter/flutter/issues/61020
   });