Skip to content

Commit ea5b972

Browse files
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.
1 parent e86b825 commit ea5b972

File tree

13 files changed

+747
-40
lines changed

13 files changed

+747
-40
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ export 'dart:ui' show
1616
FontStyle,
1717
FontVariation,
1818
FontWeight,
19+
GlyphInfo,
1920
ImageShader,
2021
Locale,
2122
MaskFilter,

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

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'dart:math' show max, min;
66
import 'dart:ui' as ui show
77
BoxHeightStyle,
88
BoxWidthStyle,
9+
GlyphInfo,
910
LineMetrics,
1011
Paragraph,
1112
ParagraphBuilder,
@@ -24,6 +25,7 @@ import 'strut_style.dart';
2425
import 'text_scaler.dart';
2526
import 'text_span.dart';
2627

28+
export 'dart:ui' show LineMetrics;
2729
export 'package:flutter/services.dart' show TextRange, TextSelection;
2830

2931
/// The default font size if none is specified.
@@ -1493,7 +1495,24 @@ class TextPainter {
14931495
: boxes.map((TextBox box) => _shiftTextBox(box, offset)).toList(growable: false);
14941496
}
14951497

1496-
/// Returns the position within the text for the given pixel offset.
1498+
/// Returns the [GlyphInfo] of the glyph closest to the given `offset` in the
1499+
/// paragraph coordinate system, or null if the text is empty, or is entirely
1500+
/// clipped or ellipsized away.
1501+
///
1502+
/// This method first finds the line closest to `offset.dy`, and then returns
1503+
/// the [GlyphInfo] of the closest glyph(s) within that line.
1504+
ui.GlyphInfo? getClosestGlyphForOffset(Offset offset) {
1505+
assert(_debugAssertTextLayoutIsValid);
1506+
assert(!_debugNeedsRelayout);
1507+
final _TextPainterLayoutCacheWithOffset cachedLayout = _layoutCache!;
1508+
final ui.GlyphInfo? rawGlyphInfo = cachedLayout.paragraph.getClosestGlyphInfoForOffset(offset - cachedLayout.paintOffset);
1509+
if (rawGlyphInfo == null || cachedLayout.paintOffset == Offset.zero) {
1510+
return rawGlyphInfo;
1511+
}
1512+
return ui.GlyphInfo(rawGlyphInfo.graphemeClusterLayoutBounds.shift(cachedLayout.paintOffset), rawGlyphInfo.graphemeClusterCodeUnitRange, rawGlyphInfo.writingDirection);
1513+
}
1514+
1515+
/// Returns the closest position within the text for the given pixel offset.
14971516
TextPosition getPositionForOffset(Offset offset) {
14981517
assert(_debugAssertTextLayoutIsValid);
14991518
assert(!_debugNeedsRelayout);

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -343,18 +343,20 @@ class TextSpan extends InlineSpan implements HitTestTarget, MouseTrackerAnnotati
343343
/// Returns the text span that contains the given position in the text.
344344
@override
345345
InlineSpan? getSpanForPositionVisitor(TextPosition position, Accumulator offset) {
346-
if (text == null) {
346+
final String? text = this.text;
347+
if (text == null || text.isEmpty) {
347348
return null;
348349
}
349350
final TextAffinity affinity = position.affinity;
350351
final int targetOffset = position.offset;
351-
final int endOffset = offset.value + text!.length;
352+
final int endOffset = offset.value + text.length;
353+
352354
if (offset.value == targetOffset && affinity == TextAffinity.downstream ||
353355
offset.value < targetOffset && targetOffset < endOffset ||
354356
endOffset == targetOffset && affinity == TextAffinity.upstream) {
355357
return this;
356358
}
357-
offset.increment(text!.length);
359+
offset.increment(text.length);
358360
return null;
359361
}
360362

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1941,8 +1941,16 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
19411941
@protected
19421942
bool hitTestChildren(BoxHitTestResult result, { required Offset position }) {
19431943
final Offset effectivePosition = position - _paintOffset;
1944-
final InlineSpan? textSpan = _textPainter.text;
1945-
switch (textSpan?.getSpanForPosition(_textPainter.getPositionForOffset(effectivePosition))) {
1944+
final GlyphInfo? glyph = _textPainter.getClosestGlyphForOffset(effectivePosition);
1945+
// The hit-test can't fall through the horizontal gaps between visually
1946+
// adjacent characters on the same line, even with a large letter-spacing or
1947+
// text justification, as graphemeClusterLayoutBounds.width is the advance
1948+
// width to the next character, so there's no gap between their
1949+
// graphemeClusterLayoutBounds rects.
1950+
final InlineSpan? spanHit = glyph != null && glyph.graphemeClusterLayoutBounds.contains(effectivePosition)
1951+
? _textPainter.text!.getSpanForPosition(TextPosition(offset: glyph.graphemeClusterCodeUnitRange.start))
1952+
: null;
1953+
switch (spanHit) {
19461954
case final HitTestTarget span:
19471955
result.add(HitTestEntry(span));
19481956
return true;

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,7 @@ class RenderParagraph extends RenderBox with ContainerRenderObjectMixin<RenderBo
303303
}
304304

305305
static final String _placeholderCharacter = String.fromCharCode(PlaceholderSpan.placeholderCodeUnit);
306+
306307
final TextPainter _textPainter;
307308

308309
List<AttributedString>? _cachedAttributedLabels;
@@ -730,9 +731,18 @@ class RenderParagraph extends RenderBox with ContainerRenderObjectMixin<RenderBo
730731
bool hitTestSelf(Offset position) => true;
731732

732733
@override
734+
@protected
733735
bool hitTestChildren(BoxHitTestResult result, { required Offset position }) {
734-
final TextPosition textPosition = _textPainter.getPositionForOffset(position);
735-
switch (_textPainter.text!.getSpanForPosition(textPosition)) {
736+
final GlyphInfo? glyph = _textPainter.getClosestGlyphForOffset(position);
737+
// The hit-test can't fall through the horizontal gaps between visually
738+
// adjacent characters on the same line, even with a large letter-spacing or
739+
// text justification, as graphemeClusterLayoutBounds.width is the advance
740+
// width to the next character, so there's no gap between their
741+
// graphemeClusterLayoutBounds rects.
742+
final InlineSpan? spanHit = glyph != null && glyph.graphemeClusterLayoutBounds.contains(position)
743+
? _textPainter.text!.getSpanForPosition(TextPosition(offset: glyph.graphemeClusterCodeUnitRange.start))
744+
: null;
745+
switch (spanHit) {
736746
case final HitTestTarget span:
737747
result.add(HitTestEntry(span));
738748
return true;

packages/flutter/test/painting/text_span_test.dart

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,24 @@ void main() {
250250
expect(textSpan2.compareTo(textSpan2), RenderComparison.identical);
251251
});
252252

253+
test('GetSpanForPosition', () {
254+
const TextSpan textSpan = TextSpan(
255+
text: '',
256+
children: <InlineSpan>[
257+
TextSpan(text: '', children: <InlineSpan>[
258+
TextSpan(text: 'a'),
259+
]),
260+
TextSpan(text: 'b'),
261+
TextSpan(text: 'c'),
262+
],
263+
);
264+
265+
expect((textSpan.getSpanForPosition(const TextPosition(offset: 0)) as TextSpan?)?.text, 'a');
266+
expect((textSpan.getSpanForPosition(const TextPosition(offset: 1)) as TextSpan?)?.text, 'b');
267+
expect((textSpan.getSpanForPosition(const TextPosition(offset: 2)) as TextSpan?)?.text, 'c');
268+
expect((textSpan.getSpanForPosition(const TextPosition(offset: 3)) as TextSpan?)?.text, isNull);
269+
});
270+
253271
test('GetSpanForPosition with WidgetSpan', () {
254272
const TextSpan textSpan = TextSpan(
255273
text: 'a',

packages/flutter/test/rendering/editable_test.dart

Lines changed: 112 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ import 'package:flutter_test/flutter_test.dart';
1212

1313
import 'rendering_tester.dart';
1414

15+
double _caretMarginOf(RenderEditable renderEditable) {
16+
return renderEditable.cursorWidth + 1.0;
17+
}
18+
1519
void _applyParentData(List<RenderBox> inlineRenderBoxes, InlineSpan span) {
1620
int index = 0;
1721
RenderBox? previousBox;
@@ -1184,8 +1188,107 @@ void main() {
11841188
});
11851189

11861190
group('hit testing', () {
1191+
final TextSelectionDelegate delegate = _FakeEditableTextState();
1192+
1193+
test('Basic TextSpan Hit testing', () {
1194+
final TextSpan textSpanA = TextSpan(text: 'A' * 10);
1195+
const TextSpan textSpanBC = TextSpan(text: 'BC', style: TextStyle(letterSpacing: 26.0));
1196+
1197+
final TextSpan text = TextSpan(
1198+
text: '',
1199+
style: const TextStyle(fontSize: 10.0),
1200+
children: <InlineSpan>[textSpanA, textSpanBC],
1201+
);
1202+
1203+
final RenderEditable renderEditable = RenderEditable(
1204+
text: text,
1205+
maxLines: null,
1206+
startHandleLayerLink: LayerLink(),
1207+
endHandleLayerLink: LayerLink(),
1208+
textDirection: TextDirection.ltr,
1209+
offset: ViewportOffset.fixed(0.0),
1210+
textSelectionDelegate: delegate,
1211+
selection: const TextSelection.collapsed(offset: 0),
1212+
);
1213+
layout(renderEditable, constraints: BoxConstraints.tightFor(width: 100.0 + _caretMarginOf(renderEditable)));
1214+
1215+
BoxHitTestResult result;
1216+
1217+
// Hit-testing the first line
1218+
// First A
1219+
expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(5.0, 5.0)), isTrue);
1220+
expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[textSpanA]);
1221+
// The last A.
1222+
expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(95.0, 5.0)), isTrue);
1223+
expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[textSpanA]);
1224+
// Far away from the line.
1225+
expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(200.0, 5.0)), isFalse);
1226+
expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[]);
1227+
1228+
// Hit-testing the second line
1229+
// Tapping on B (startX = letter-spacing / 2 = 13.0).
1230+
expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(18.0, 15.0)), isTrue);
1231+
expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[textSpanBC]);
1232+
1233+
// Between B and C, with large letter-spacing.
1234+
expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(31.0, 15.0)), isTrue);
1235+
expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[textSpanBC]);
1236+
1237+
// On C.
1238+
expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(54.0, 15.0)), isTrue);
1239+
expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[textSpanBC]);
1240+
1241+
// After C.
1242+
expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(100.0, 15.0)), isTrue);
1243+
expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[]);
1244+
1245+
// Not even remotely close.
1246+
expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(9999.0, 9999.0)), isFalse);
1247+
expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[]);
1248+
});
1249+
1250+
test('TextSpan Hit testing with text justification', () {
1251+
const TextSpan textSpanA = TextSpan(text: 'A '); // The space is a word break.
1252+
const TextSpan textSpanB = TextSpan(text: 'B\u200B'); // The zero-width space is used as a line break.
1253+
final TextSpan textSpanC = TextSpan(text: 'C' * 10); // The third span starts a new line since it's too long for the first line.
1254+
1255+
// The text should look like:
1256+
// A B
1257+
// CCCCCCCCCC
1258+
final TextSpan text = TextSpan(
1259+
text: '',
1260+
style: const TextStyle(fontSize: 10.0),
1261+
children: <InlineSpan>[textSpanA, textSpanB, textSpanC],
1262+
);
1263+
final RenderEditable renderEditable = RenderEditable(
1264+
text: text,
1265+
maxLines: null,
1266+
startHandleLayerLink: LayerLink(),
1267+
endHandleLayerLink: LayerLink(),
1268+
textDirection: TextDirection.ltr,
1269+
textAlign: TextAlign.justify,
1270+
offset: ViewportOffset.fixed(0.0),
1271+
textSelectionDelegate: delegate,
1272+
selection: const TextSelection.collapsed(offset: 0),
1273+
);
1274+
1275+
layout(renderEditable, constraints: BoxConstraints.tightFor(width: 100.0 + _caretMarginOf(renderEditable)));
1276+
BoxHitTestResult result;
1277+
1278+
// Tapping on A.
1279+
expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(5.0, 5.0)), isTrue);
1280+
expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[textSpanA]);
1281+
1282+
// Between A and B.
1283+
expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(50.0, 5.0)), isTrue);
1284+
expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[textSpanA]);
1285+
1286+
// On B.
1287+
expect(renderEditable.hitTest(result = BoxHitTestResult(), position: const Offset(95.0, 5.0)), isTrue);
1288+
expect(result.path.map((HitTestEntry<HitTestTarget> entry) => entry.target).whereType<TextSpan>(), <TextSpan>[textSpanB]);
1289+
});
1290+
11871291
test('hits correct TextSpan when not scrolled', () {
1188-
final TextSelectionDelegate delegate = _FakeEditableTextState();
11891292
final RenderEditable editable = RenderEditable(
11901293
text: const TextSpan(
11911294
style: TextStyle(height: 1.0, fontSize: 10.0),
@@ -1692,7 +1795,8 @@ void main() {
16921795
// Prepare for painting after layout.
16931796
pumpFrame(phase: EnginePhase.compositingBits);
16941797
BoxHitTestResult result = BoxHitTestResult();
1695-
editable.hitTest(result, position: Offset.zero);
1798+
// The WidgetSpans have a height of 14.0, so "test" has a y offset of 4.0.
1799+
editable.hitTest(result, position: const Offset(1.0, 5.0));
16961800
// We expect two hit test entries in the path because the RenderEditable
16971801
// will add itself as well.
16981802
expect(result.path, hasLength(2));
@@ -1702,7 +1806,7 @@ void main() {
17021806
// Only testing the RenderEditable entry here once, not anymore below.
17031807
expect(result.path.last.target, isA<RenderEditable>());
17041808
result = BoxHitTestResult();
1705-
editable.hitTest(result, position: const Offset(15.0, 0.0));
1809+
editable.hitTest(result, position: const Offset(15.0, 5.0));
17061810
expect(result.path, hasLength(2));
17071811
target = result.path.first.target;
17081812
expect(target, isA<TextSpan>());
@@ -1775,7 +1879,8 @@ void main() {
17751879
// Prepare for painting after layout.
17761880
pumpFrame(phase: EnginePhase.compositingBits);
17771881
BoxHitTestResult result = BoxHitTestResult();
1778-
editable.hitTest(result, position: Offset.zero);
1882+
// The WidgetSpans have a height of 14.0, so "test" has a y offset of 4.0.
1883+
editable.hitTest(result, position: const Offset(0.0, 4.0));
17791884
// We expect two hit test entries in the path because the RenderEditable
17801885
// will add itself as well.
17811886
expect(result.path, hasLength(2));
@@ -1785,13 +1890,14 @@ void main() {
17851890
// Only testing the RenderEditable entry here once, not anymore below.
17861891
expect(result.path.last.target, isA<RenderEditable>());
17871892
result = BoxHitTestResult();
1788-
editable.hitTest(result, position: const Offset(15.0, 0.0));
1893+
editable.hitTest(result, position: const Offset(15.0, 4.0));
17891894
expect(result.path, hasLength(2));
17901895
target = result.path.first.target;
17911896
expect(target, isA<TextSpan>());
17921897
expect((target as TextSpan).text, text);
17931898

17941899
result = BoxHitTestResult();
1900+
// "test" is 40 pixel wide.
17951901
editable.hitTest(result, position: const Offset(41.0, 0.0));
17961902
expect(result.path, hasLength(3));
17971903
target = result.path.first.target;
@@ -1814,7 +1920,7 @@ void main() {
18141920

18151921
result = BoxHitTestResult();
18161922
editable.hitTest(result, position: const Offset(5.0, 15.0));
1817-
expect(result.path, hasLength(2));
1923+
expect(result.path, hasLength(1)); // Only the RenderEditable.
18181924
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/61020
18191925
});
18201926

0 commit comments

Comments
 (0)