Reland "Make `TextSpan` hit testing precise." (#140468) (#140621)

Fixes https://github.com/flutter/flutter/issues/131435, https://github.com/flutter/flutter/issues/104594, https://github.com/flutter/flutter/issues/43400

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 its character's layout bounds.

Potential issues:

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.

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/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/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
   });
 

packages/flutter/test/rendering/paragraph_test.dart

@@ -796,6 +796,84 @@ void main() {
     expect(node.childrenCount, 2);
   }, skip: isBrowser); // https://github.com/flutter/flutter/issues/61020
 
+  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(
+      style: const TextStyle(fontSize: 10.0),
+      children: <InlineSpan>[textSpanA, textSpanBC],
+    );
+
+    final RenderParagraph paragraph = RenderParagraph(text, textDirection: TextDirection.ltr);
+    layout(paragraph, constraints: const BoxConstraints.tightFor(width: 100.0));
+
+    BoxHitTestResult result;
+
+    // Hit-testing the first line
+    // First A
+    expect(paragraph.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(paragraph.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(paragraph.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(paragraph.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(paragraph.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(paragraph.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(paragraph.hitTest(result = BoxHitTestResult(), position: const Offset(100.0, 15.0)), isFalse);
+    expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[]);
+
+    // Not even remotely close.
+    expect(paragraph.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 RenderParagraph paragraph = RenderParagraph(text, textDirection: TextDirection.ltr, textAlign: TextAlign.justify);
+    layout(paragraph, constraints: const BoxConstraints.tightFor(width: 100.0));
+    BoxHitTestResult result;
+
+    // Tapping on A.
+    expect(paragraph.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(paragraph.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(paragraph.hitTest(result = BoxHitTestResult(), position: const Offset(95.0, 5.0)), isTrue);
+    expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[textSpanB]);
+  });
+
   group('Selection', () {
     void selectionParagraph(RenderParagraph paragraph, TextPosition start, TextPosition end) {
       for (final Selectable selectable in (paragraph.registrar! as TestSelectionRegistrar).selectables) {

packages/flutter/test/widgets/slivers_padding_test.dart

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'package:flutter/gestures.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter/rendering.dart';
 import 'package:flutter_test/flutter_test.dart';
@@ -15,7 +16,6 @@ class _MockRenderSliver extends RenderSliver {
       maxPaintExtent: 10,
     );
   }
-
 }
 
 Future<void> test(WidgetTester tester, double offset, EdgeInsetsGeometry padding, AxisDirection axisDirection, TextDirection textDirection) {
@@ -180,15 +180,15 @@ void main() {
     ]);
     HitTestResult result;
     result = tester.hitTestOnBinding(const Offset(10.0, 10.0));
-    expectIsTextSpan(result.path.first.target, 'before');
+    hitsText(result, 'before');
     result = tester.hitTestOnBinding(const Offset(10.0, 60.0));
     expect(result.path.first.target, isA<RenderView>());
     result = tester.hitTestOnBinding(const Offset(100.0, 100.0));
-    expectIsTextSpan(result.path.first.target, 'padded');
+    hitsText(result, 'padded');
     result = tester.hitTestOnBinding(const Offset(100.0, 490.0));
     expect(result.path.first.target, isA<RenderView>());
     result = tester.hitTestOnBinding(const Offset(10.0, 520.0));
-    expectIsTextSpan(result.path.first.target, 'after');
+    hitsText(result, 'after');
   });
 
   testWidgets('Viewport+SliverPadding hit testing up', (WidgetTester tester) async {
@@ -202,15 +202,15 @@ void main() {
     ]);
     HitTestResult result;
     result = tester.hitTestOnBinding(const Offset(10.0, 600.0-10.0));
-    expectIsTextSpan(result.path.first.target, 'before');
+    hitsText(result, 'before');
     result = tester.hitTestOnBinding(const Offset(10.0, 600.0-60.0));
     expect(result.path.first.target, isA<RenderView>());
     result = tester.hitTestOnBinding(const Offset(100.0, 600.0-100.0));
-    expectIsTextSpan(result.path.first.target, 'padded');
+    hitsText(result, 'padded');
     result = tester.hitTestOnBinding(const Offset(100.0, 600.0-490.0));
     expect(result.path.first.target, isA<RenderView>());
     result = tester.hitTestOnBinding(const Offset(10.0, 600.0-520.0));
-    expectIsTextSpan(result.path.first.target, 'after');
+    hitsText(result, 'after');
   });
 
   testWidgets('Viewport+SliverPadding hit testing left', (WidgetTester tester) async {
@@ -224,15 +224,15 @@ void main() {
     ]);
     HitTestResult result;
     result = tester.hitTestOnBinding(const Offset(800.0-10.0, 10.0));
-    expectIsTextSpan(result.path.first.target, 'before');
+    hitsText(result, 'before');
     result = tester.hitTestOnBinding(const Offset(800.0-60.0, 10.0));
     expect(result.path.first.target, isA<RenderView>());
     result = tester.hitTestOnBinding(const Offset(800.0-100.0, 100.0));
-    expectIsTextSpan(result.path.first.target, 'padded');
+    hitsText(result, 'padded');
     result = tester.hitTestOnBinding(const Offset(800.0-490.0, 100.0));
     expect(result.path.first.target, isA<RenderView>());
     result = tester.hitTestOnBinding(const Offset(800.0-520.0, 10.0));
-    expectIsTextSpan(result.path.first.target, 'after');
+    hitsText(result, 'after');
   });
 
   testWidgets('Viewport+SliverPadding hit testing right', (WidgetTester tester) async {
@@ -246,15 +246,15 @@ void main() {
     ]);
     HitTestResult result;
     result = tester.hitTestOnBinding(const Offset(10.0, 10.0));
-    expectIsTextSpan(result.path.first.target, 'before');
+    hitsText(result, 'before');
     result = tester.hitTestOnBinding(const Offset(60.0, 10.0));
     expect(result.path.first.target, isA<RenderView>());
     result = tester.hitTestOnBinding(const Offset(100.0, 100.0));
-    expectIsTextSpan(result.path.first.target, 'padded');
+    hitsText(result, 'padded');
     result = tester.hitTestOnBinding(const Offset(490.0, 100.0));
     expect(result.path.first.target, isA<RenderView>());
     result = tester.hitTestOnBinding(const Offset(520.0, 10.0));
-    expectIsTextSpan(result.path.first.target, 'after');
+    hitsText(result, 'after');
   });
 
   testWidgets('Viewport+SliverPadding no child', (WidgetTester tester) async {
@@ -617,7 +617,15 @@ void main() {
   });
 }
 
-void expectIsTextSpan(Object target, String text) {
-  expect(target, isA<TextSpan>());
-  expect((target as TextSpan).text, text);
+void hitsText(HitTestResult hitTestResult, String text) {
+  switch (hitTestResult.path.first.target) {
+    case final TextSpan span:
+      expect(span.text, text);
+    case final RenderParagraph paragraph:
+      final InlineSpan span = paragraph.text;
+      expect(span, isA<TextSpan>());
+      expect((span as TextSpan).text, text);
+    case final HitTestTarget target:
+      fail('$target is not a TextSpan or a RenderParagraph.');
+  }
 }