Unverified Commit 1306d7f1 authored by LongCatIsLooong's avatar LongCatIsLooong Committed by GitHub

Fix Caret Height On Empty Lines (#120834)

* improve caret caching, fix caret for empty text/line, `getLocalRectForCaret` now reports the real rect that will be painted.
move caret x-coordinate clamping to RenderEditable since TextPainter doesn't know about clipping.

* comments

* review
parent 7dd53fef
......@@ -273,14 +273,32 @@ class _UntilTextBoundary extends TextBoundary {
/// This is used to cache and pass the computed metrics regarding the
/// caret's size and position. This is preferred due to the expensive
/// nature of the calculation.
class _CaretMetrics {
const _CaretMetrics({required this.offset, this.fullHeight});
///
// This should be a sealed class: A _CaretMetrics is either a _LineCaretMetrics
// or an _EmptyLineCaretMetrics.
@immutable
abstract class _CaretMetrics { }
/// The _CaretMetrics for carets located in a non-empty line. Carets located in a
/// non-empty line are associated with a glyph within the same line.
class _LineCaretMetrics implements _CaretMetrics {
const _LineCaretMetrics({required this.offset, required this.writingDirection, required this.fullHeight});
/// The offset of the top left corner of the caret from the top left
/// corner of the paragraph.
final Offset offset;
/// The writing direction of the glyph the _CaretMetrics is associated with.
final TextDirection writingDirection;
/// The full height of the glyph at the caret position.
final double? fullHeight;
final double fullHeight;
}
/// The _CaretMetrics for carets located in an empty line (when the text is
/// empty, or the caret is between two a newline characters).
class _EmptyLineCaretMetrics implements _CaretMetrics {
const _EmptyLineCaretMetrics({ required this.lineVerticalOffset });
/// The y offset of the unoccupied line.
final double lineVerticalOffset;
}
/// An object that paints a [TextSpan] tree into a [Canvas].
......@@ -466,7 +484,6 @@ class TextPainter {
_paragraph = null;
_lineMetricsCache = null;
_previousCaretPosition = null;
_previousCaretPrototype = null;
}
/// The (potentially styled) text to paint.
......@@ -935,7 +952,6 @@ class TextPainter {
// A change in layout invalidates the cached caret and line metrics as well.
_lineMetricsCache = null;
_previousCaretPosition = null;
_previousCaretPrototype = null;
_layoutParagraph(minWidth, maxWidth);
_inlinePlaceholderBoxes = _paragraph!.getBoxesForPlaceholders();
}
......@@ -1027,9 +1043,9 @@ class TextPainter {
// Unicode value for a zero width joiner character.
static const int _zwjUtf16 = 0x200d;
// Get the Rect of the cursor (in logical pixels) based off the near edge
// of the character upstream from the given string offset.
Rect? _getRectFromUpstream(int offset, Rect caretPrototype) {
// Get the caret metrics (in logical pixels) based off the near edge of the
// character upstream from the given string offset.
_CaretMetrics? _getMetricsFromUpstream(int offset) {
final int plainTextLength = plainText.length;
if (plainTextLength == 0 || offset > plainTextLength) {
return null;
......@@ -1067,21 +1083,15 @@ class TextPainter {
}
final TextBox box = boxes.first;
if (prevCodeUnit == NEWLINE_CODE_UNIT) {
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 Rect.fromLTRB(clampDouble(dx, 0, _paragraph!.width), box.top,
clampDouble(dx, 0, _paragraph!.width), box.bottom);
return prevCodeUnit == NEWLINE_CODE_UNIT
? _EmptyLineCaretMetrics(lineVerticalOffset: box.bottom)
: _LineCaretMetrics(offset: Offset(box.end, box.top), writingDirection: box.direction, fullHeight: box.bottom - box.top);
}
return null;
}
// Get the Rect of the cursor (in logical pixels) based off the near edge
// of the character downstream from the given string offset.
Rect? _getRectFromDownstream(int offset, Rect caretPrototype) {
// Get the caret metrics (in logical pixels) based off the near edge of the
// character downstream from the given string offset.
_CaretMetrics? _getMetricsFromDownstream(int offset) {
final int plainTextLength = plainText.length;
if (plainTextLength == 0 || offset < 0) {
return null;
......@@ -1116,38 +1126,33 @@ class TextPainter {
continue;
}
final TextBox box = boxes.last;
final double caretStart = box.start;
final double dx = box.direction == TextDirection.rtl ? caretStart - caretPrototype.width : caretStart;
return Rect.fromLTRB(clampDouble(dx, 0, _paragraph!.width), box.top, clampDouble(dx, 0, _paragraph!.width), box.bottom);
return _LineCaretMetrics(offset: Offset(box.start, box.top), writingDirection: box.direction, fullHeight: box.bottom - box.top);
}
return null;
}
Offset get _emptyOffset {
assert(_debugAssertTextLayoutIsValid); // implies textDirection is non-null
static double _computePaintOffsetFraction(TextAlign textAlign, TextDirection textDirection) {
switch (textAlign) {
case TextAlign.left:
return Offset.zero;
return 0.0;
case TextAlign.right:
return Offset(width, 0.0);
return 1.0;
case TextAlign.center:
return Offset(width / 2.0, 0.0);
case TextAlign.justify:
return 0.5;
case TextAlign.start:
assert(textDirection != null);
switch (textDirection!) {
case TextAlign.justify:
switch (textDirection) {
case TextDirection.rtl:
return Offset(width, 0.0);
return 1.0;
case TextDirection.ltr:
return Offset.zero;
return 0.0;
}
case TextAlign.end:
assert(textDirection != null);
switch (textDirection!) {
switch (textDirection) {
case TextDirection.rtl:
return Offset.zero;
return 0.0;
case TextDirection.ltr:
return Offset(width, 0.0);
return 1.0;
}
}
}
......@@ -1156,8 +1161,33 @@ class TextPainter {
///
/// Valid only after [layout] has been called.
Offset getOffsetForCaret(TextPosition position, Rect caretPrototype) {
_computeCaretMetrics(position, caretPrototype);
return _caretMetrics.offset;
final _CaretMetrics caretMetrics = _computeCaretMetrics(position);
if (caretMetrics is _EmptyLineCaretMetrics) {
final double paintOffsetAlignment = _computePaintOffsetFraction(textAlign, textDirection!);
// The full width is not (width - caretPrototype.width)
// because RenderEditable reserves cursor width on the right. Ideally this
// should be handled by RenderEditable instead.
final double dx = paintOffsetAlignment == 0 ? 0 : paintOffsetAlignment * width;
return Offset(dx, caretMetrics.lineVerticalOffset);
}
final Offset offset;
switch ((caretMetrics as _LineCaretMetrics).writingDirection) {
case TextDirection.rtl:
offset = Offset(caretMetrics.offset.dx - caretPrototype.width, caretMetrics.offset.dy);
break;
case TextDirection.ltr:
offset = caretMetrics.offset;
break;
}
// If offset.dx is outside of the advertised content area, then the associated
// glyph cluster belongs to a trailing newline character. Ideally the behavior
// should be handled by higher-level implementations (for instance,
// RenderEditable reserves width for showing the caret, it's best to handle
// the clamping there).
final double adjustedDx = clampDouble(offset.dx, 0, width);
return Offset(adjustedDx, offset.dy);
}
/// {@template flutter.painting.textPainter.getFullHeightForCaret}
......@@ -1166,8 +1196,8 @@ class TextPainter {
///
/// Valid only after [layout] has been called.
double? getFullHeightForCaret(TextPosition position, Rect caretPrototype) {
_computeCaretMetrics(position, caretPrototype);
return _caretMetrics.fullHeight;
final _CaretMetrics caretMetrics = _computeCaretMetrics(position);
return caretMetrics is _LineCaretMetrics ? caretMetrics.fullHeight : null;
}
// Cached caret metrics. This allows multiple invokes of [getOffsetForCaret] and
......@@ -1179,35 +1209,29 @@ class TextPainter {
// computed with. When new values are passed in, we recompute the caret metrics.
// only as necessary.
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) {
_CaretMetrics _computeCaretMetrics(TextPosition position) {
assert(_debugAssertTextLayoutIsValid);
if (position == _previousCaretPosition && caretPrototype == _previousCaretPrototype) {
return;
if (position == _previousCaretPosition) {
return _caretMetrics;
}
final int offset = position.offset;
Rect? rect;
final _CaretMetrics? metrics;
switch (position.affinity) {
case TextAffinity.upstream: {
rect = _getRectFromUpstream(offset, caretPrototype) ?? _getRectFromDownstream(offset, caretPrototype);
metrics = _getMetricsFromUpstream(offset) ?? _getMetricsFromDownstream(offset);
break;
}
case TextAffinity.downstream: {
rect = _getRectFromDownstream(offset, caretPrototype) ?? _getRectFromUpstream(offset, caretPrototype);
metrics = _getMetricsFromDownstream(offset) ?? _getMetricsFromUpstream(offset);
break;
}
}
_caretMetrics = _CaretMetrics(
offset: rect != null ? Offset(rect.left, rect.top) : _emptyOffset,
fullHeight: rect != null ? rect.bottom - rect.top : null,
);
// Cache the input parameters to prevent repeat work later.
_previousCaretPosition = position;
_previousCaretPrototype = caretPrototype;
return _caretMetrics = metrics ?? const _EmptyLineCaretMetrics(lineVerticalOffset: 0);
}
/// Returns a list of rects that bound the given selection.
......
......@@ -406,14 +406,14 @@ class TextSpan extends InlineSpan implements HitTestTarget, MouseTrackerAnnotati
@override
int? codeUnitAtVisitor(int index, Accumulator offset) {
final String? text = this.text;
if (text == null) {
return null;
}
if (index - offset.value < text!.length) {
return text!.codeUnitAt(index - offset.value);
}
offset.increment(text!.length);
return null;
final int localOffset = index - offset.value;
assert(localOffset >= 0);
offset.increment(text.length);
return localOffset < text.length ? text.codeUnitAt(localOffset) : null;
}
/// Populates the `semanticsOffsets` and `semanticsElements` with the appropriate data
......
......@@ -1790,11 +1790,46 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
/// [TextPainter] object.
Rect getLocalRectForCaret(TextPosition caretPosition) {
_computeTextMetricsIfNeeded();
final Offset caretOffset = _textPainter.getOffsetForCaret(caretPosition, _caretPrototype);
// This rect is the same as _caretPrototype but without the vertical padding.
final Rect rect = Rect.fromLTWH(0.0, 0.0, cursorWidth, cursorHeight).shift(caretOffset + _paintOffset + cursorOffset);
// Add additional cursor offset (generally only if on iOS).
return rect.shift(_snapToPhysicalPixel(rect.topLeft));
final Rect caretPrototype = _caretPrototype;
final Offset caretOffset = _textPainter.getOffsetForCaret(caretPosition, caretPrototype);
Rect caretRect = caretPrototype.shift(caretOffset + cursorOffset);
final double scrollableWidth = math.max(_textPainter.width + _caretMargin, size.width);
final double caretX = clampDouble(caretRect.left, 0, math.max(scrollableWidth - _caretMargin, 0));
caretRect = Offset(caretX, caretRect.top) & caretRect.size;
final double caretHeight = cursorHeight;
switch (defaultTargetPlatform) {
case TargetPlatform.iOS:
case TargetPlatform.macOS:
final double fullHeight = _textPainter.getFullHeightForCaret(caretPosition, caretPrototype) ?? _textPainter.preferredLineHeight;
final double heightDiff = fullHeight - caretRect.height;
// Center the caret vertically along the text.
caretRect = Rect.fromLTWH(
caretRect.left,
caretRect.top + heightDiff / 2,
caretRect.width,
caretRect.height,
);
break;
case TargetPlatform.android:
case TargetPlatform.fuchsia:
case TargetPlatform.linux:
case TargetPlatform.windows:
// 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 _computeCaretPrototype().
caretRect = Rect.fromLTWH(
caretRect.left,
caretRect.top - _kCaretHeightOffset,
caretRect.width,
caretHeight,
);
break;
}
caretRect = caretRect.shift(_paintOffset);
return caretRect.shift(_snapToPhysicalPixel(caretRect.topLeft));
}
@override
......@@ -2311,13 +2346,7 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
late Rect _caretPrototype;
// 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) in _paintCaret. We
// should rework this properly to once again match the platform. The constant
// _kCaretHeightOffset scales poorly for small font sizes.
// TODO(LongCatIsLooong): https://github.com/flutter/flutter/issues/120836
//
/// On iOS, the cursor is taller than the cursor on Android. The height
/// of the cursor for iOS is approximate and obtained through an eyeball
......@@ -2970,44 +2999,7 @@ class _FloatingCursorPainter extends RenderEditablePainter {
}
void paintRegularCursor(Canvas canvas, RenderEditable renderEditable, Color caretColor, TextPosition textPosition) {
final Rect caretPrototype = renderEditable._caretPrototype;
final Offset caretOffset = renderEditable._textPainter.getOffsetForCaret(textPosition, caretPrototype);
Rect caretRect = caretPrototype.shift(caretOffset + cursorOffset);
final double? caretHeight = renderEditable._textPainter.getFullHeightForCaret(textPosition, caretPrototype);
if (caretHeight != null) {
switch (defaultTargetPlatform) {
case TargetPlatform.iOS:
case TargetPlatform.macOS:
final double heightDiff = caretHeight - caretRect.height;
// Center the caret vertically along the text.
caretRect = Rect.fromLTWH(
caretRect.left,
caretRect.top + heightDiff / 2,
caretRect.width,
caretRect.height,
);
break;
case TargetPlatform.android:
case TargetPlatform.fuchsia:
case TargetPlatform.linux:
case TargetPlatform.windows:
// 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 _computeCaretPrototype().
caretRect = Rect.fromLTWH(
caretRect.left,
caretRect.top - _kCaretHeightOffset,
caretRect.width,
caretHeight,
);
break;
}
}
caretRect = caretRect.shift(renderEditable._paintOffset);
final Rect integralRect = caretRect.shift(renderEditable._snapToPhysicalPixel(caretRect.topLeft));
final Rect integralRect = renderEditable.getLocalRectForCaret(textPosition);
if (shouldPaint) {
final Radius? radius = cursorRadius;
caretPaint.color = caretColor;
......
......@@ -136,8 +136,10 @@ class WidgetSpan extends PlaceholderSpan {
@override
int? codeUnitAtVisitor(int index, Accumulator offset) {
final int localOffset = index - offset.value;
assert(localOffset >= 0);
offset.increment(1);
return PlaceholderSpan.placeholderCodeUnit;
return localOffset == 0 ? PlaceholderSpan.placeholderCodeUnit : null;
}
@override
......
......@@ -3108,7 +3108,7 @@ void main() {
);
expect(firstCharEndpoint.length, 1);
// The first character is now offscreen to the left.
expect(firstCharEndpoint[0].point.dx, moreOrLessEquals(-309.30, epsilon: 1));
expect(firstCharEndpoint[0].point.dx, moreOrLessEquals(-310.30, epsilon: 1));
}, variant: TargetPlatformVariant.all(excluding: <TargetPlatform>{ TargetPlatform.iOS, TargetPlatform.macOS }));
testWidgets('long press drag can edge scroll on Apple platforms', (WidgetTester tester) async {
......@@ -4813,7 +4813,7 @@ void main() {
// The ListView has scrolled to keep the TextField and cursor handle
// visible.
expect(scrollController.offset, 25.0);
expect(scrollController.offset, 27.0);
});
testWidgets('disabled state golden', (WidgetTester tester) async {
......
......@@ -4960,6 +4960,7 @@ void main() {
);
final RenderEditable editable = findRenderEditable(tester);
assert(editable.size.width == 300);
Offset topLeft = editable.localToGlobal(
editable.getLocalRectForCaret(const TextPosition(offset: 0)).topLeft,
);
......@@ -11765,7 +11766,7 @@ void main() {
// The ListView has scrolled to keep the TextField and cursor handle
// visible.
expect(scrollController.offset, 48.0);
expect(scrollController.offset, 50.0);
});
// Regression test for https://github.com/flutter/flutter/issues/74566
......@@ -11798,7 +11799,7 @@ void main() {
await tester.pumpAndSettle();
// The ListView has scrolled to keep the TextField visible.
expect(scrollController.offset, 48.0);
expect(scrollController.offset, 50.0);
expect(textFieldScrollController.offset, 0.0);
// After entering some long text, the last input character remains on the screen.
......
// Copyright 2014 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.dart';
void main() {
test('WidgetSpan codeUnitAt', () {
const InlineSpan span = WidgetSpan(child: SizedBox());
expect(span.codeUnitAt(-1), isNull);
expect(span.codeUnitAt(0), PlaceholderSpan.placeholderCodeUnit);
expect(span.codeUnitAt(1), isNull);
expect(span.codeUnitAt(2), isNull);
const InlineSpan nestedSpan = TextSpan(
text: 'AAA',
children: <InlineSpan>[span, span],
);
expect(nestedSpan.codeUnitAt(-1), isNull);
expect(nestedSpan.codeUnitAt(0), 65);
expect(nestedSpan.codeUnitAt(1), 65);
expect(nestedSpan.codeUnitAt(2), 65);
expect(nestedSpan.codeUnitAt(3), PlaceholderSpan.placeholderCodeUnit);
expect(nestedSpan.codeUnitAt(4), PlaceholderSpan.placeholderCodeUnit);
expect(nestedSpan.codeUnitAt(5), isNull);
});
}
......@@ -8,6 +8,8 @@
@TestOn('!chrome')
library;
import 'dart:math' as math;
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
......@@ -1105,4 +1107,137 @@ void main() {
await tester.pump(const Duration(milliseconds: 500));
expect((findRenderEditable(tester).text! as TextSpan).text, '•••');
});
testWidgets('getLocalRectForCaret with empty text', (WidgetTester tester) async {
EditableText.debugDeterministicCursor = true;
addTearDown(() { EditableText.debugDeterministicCursor = false; });
const String text = '12';
final TextEditingController controller = TextEditingController.fromValue(
const TextEditingValue(
text: text,
selection: TextSelection.collapsed(offset: text.length),
),
);
final Widget widget = EditableText(
autofocus: true,
backgroundCursorColor: Colors.grey,
controller: controller,
focusNode: FocusNode(),
style: const TextStyle(fontSize: 20),
textAlign: TextAlign.center,
keyboardType: TextInputType.text,
cursorColor: cursorColor,
maxLines: null,
);
await tester.pumpWidget(MaterialApp(home: widget));
final EditableTextState editableTextState = tester.firstState(find.byWidget(widget));
final RenderEditable renderEditable = editableTextState.renderEditable;
final Rect initialLocalCaretRect = renderEditable.getLocalRectForCaret(const TextPosition(offset: text.length));
for (int i = 0; i < 3; i++) {
Actions.invoke(primaryFocus!.context!, const DeleteCharacterIntent(forward: false));
await tester.pump();
expect(controller.text.length, math.max(0, text.length - 1 - i));
final Rect localRect = renderEditable.getLocalRectForCaret(
TextPosition(offset: controller.text.length),
);
expect(localRect.size, initialLocalCaretRect.size);
expect(localRect.top, initialLocalCaretRect.top);
expect(localRect.left, lessThan(initialLocalCaretRect.left));
}
expect(controller.text, isEmpty);
});
testWidgets('Caret center space test', (WidgetTester tester) async {
EditableText.debugDeterministicCursor = true;
addTearDown(() { EditableText.debugDeterministicCursor = false; });
final String text = 'test${' ' * 1000}';
final Widget widget = EditableText(
autofocus: true,
backgroundCursorColor: Colors.grey,
controller: TextEditingController.fromValue(
TextEditingValue(
text: text,
selection: TextSelection.collapsed(offset: text.length, affinity: TextAffinity.upstream),
),
),
focusNode: FocusNode(),
style: const TextStyle(),
textAlign: TextAlign.center,
keyboardType: TextInputType.text,
cursorColor: cursorColor,
cursorWidth: 13.0,
cursorHeight: 17.0,
maxLines: null,
);
await tester.pumpWidget(MaterialApp(home: widget));
final EditableTextState editableTextState = tester.firstState(find.byWidget(widget));
final Rect editableTextRect = tester.getRect(find.byWidget(widget));
final RenderEditable renderEditable = editableTextState.renderEditable;
// The trailing whitespaces are not line break opportunities.
expect(renderEditable.getLineAtOffset(TextPosition(offset: text.length)).start, 0);
// The caretRect shouldn't be outside of the RenderEditable.
final Rect caretRect = Rect.fromLTWH(
editableTextRect.right - 13.0 - 1.0,
editableTextRect.top,
13.0,
17.0,
);
expect(
renderEditable,
paints..rect(color: cursorColor, rect: caretRect),
);
}, skip: isBrowser && !isCanvasKit); // https://github.com/flutter/flutter/issues/56308
testWidgets('getLocalRectForCaret reports the real caret Rect', (WidgetTester tester) async {
EditableText.debugDeterministicCursor = true;
addTearDown(() { EditableText.debugDeterministicCursor = false; });
final String text = 'test${' ' * 50}\n'
'2nd line\n'
'\n';
final TextEditingController controller = TextEditingController.fromValue(TextEditingValue(
text: text,
selection: const TextSelection.collapsed(offset: 0),
));
final Widget widget = EditableText(
autofocus: true,
backgroundCursorColor: Colors.grey,
controller: controller,
focusNode: FocusNode(),
style: const TextStyle(fontSize: 20),
textAlign: TextAlign.center,
keyboardType: TextInputType.text,
cursorColor: cursorColor,
maxLines: null,
);
await tester.pumpWidget(MaterialApp(home: widget));
final EditableTextState editableTextState = tester.firstState(find.byWidget(widget));
final Rect editableTextRect = tester.getRect(find.byWidget(widget));
final RenderEditable renderEditable = editableTextState.renderEditable;
final Iterable<TextPosition> positions = List<int>
.generate(text.length + 1, (int index) => index)
.expand((int i) => <TextPosition>[TextPosition(offset: i, affinity: TextAffinity.upstream), TextPosition(offset: i)]);
for (final TextPosition position in positions) {
controller.selection = TextSelection.fromPosition(position);
await tester.pump();
final Rect localRect = renderEditable.getLocalRectForCaret(position);
expect(
renderEditable,
paints..rect(color: cursorColor, rect: localRect.shift(editableTextRect.topLeft)),
);
}
}, variant: TargetPlatformVariant.all());
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment