Skip to content

F
Front-End
Unverified Commit 0f5de872 authored by Gary Qian's avatar Gary Qian Committed by GitHub
Browse files
Options

Revert "Use full height of the glyph for caret height on Android" (#31159) 

* Revert "Use full height of the glyph for caret height on Android (#30991)"

This reverts commit 96e1fc9c.
parent c4e16629
Hide whitespace changes
Inline Side-by-side
Showing with 25 additions and 158 deletions
packages/flutter/lib/src/painting/text_painter.dart
View file @ 0f5de872
......@@ -15,16 +15,6 @@ import 'text_span.dart';
export 'package:flutter/services.dart' show TextRange, TextSelection;
class _CaretMetrics {
const _CaretMetrics({this.offset, this.fullHeight});
/// The offset of the top left corner of the caret from the top left
/// corner of the paragraph.
final Offset offset;
/// The full height of the glyph at the caret position.
final double fullHeight;
}
/// An object that paints a [TextSpan] tree into a [Canvas].
///
/// To use a [TextPainter], follow these steps:
......@@ -454,11 +444,11 @@ 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
// Get the Offset of the cursor (in logical pixels) based off the near edge
// of the character upstream from the given string offset.
// TODO(garyq): Use actual extended grapheme cluster length instead of
// an increasing cluster length amount to achieve deterministic performance.
Rect _getRectFromUpstream(int offset, Rect caretPrototype) {
Offset _getOffsetFromUpstream(int offset, Rect caretPrototype) {
final String flattenedText = _text.toPlainText();
final int prevCodeUnit = _text.codeUnitAt(max(0, offset - 1));
if (prevCodeUnit == null)
......@@ -491,21 +481,21 @@ class TextPainter {
// If the upstream character is a newline, cursor is at start of next line
const int NEWLINE_CODE_UNIT = 10;
if (prevCodeUnit == NEWLINE_CODE_UNIT) {
return Rect.fromLTRB(_emptyOffset.dx, box.bottom, _emptyOffset.dx, box.bottom + box.bottom - box.top);
return Offset(_emptyOffset.dx, box.bottom);
}
final double caretEnd = box.end;
final double dx = box.direction == TextDirection.rtl ? caretEnd - caretPrototype.width : caretEnd;
return Rect.fromLTRB(min(dx, width), box.top, min(dx, width), box.bottom);
return Offset(min(dx, width), box.top);
}
return null;
}
// Get the Rect of the cursor (in logical pixels) based off the near edge
// Get the Offset of the cursor (in logical pixels) based off the near edge
// of the character downstream from the given string offset.
// TODO(garyq): Use actual extended grapheme cluster length instead of
// an increasing cluster length amount to achieve deterministic performance.
Rect _getRectFromDownstream(int offset, Rect caretPrototype) {
Offset _getOffsetFromDownstream(int offset, Rect caretPrototype) {
final String flattenedText = _text.toPlainText();
// We cap the offset at the final index of the _text.
final int nextCodeUnit = _text.codeUnitAt(min(offset, flattenedText == null ? 0 : flattenedText.length - 1));
......@@ -536,7 +526,7 @@ class TextPainter {
final TextBox box = boxes.last;
final double caretStart = box.start;
final double dx = box.direction == TextDirection.rtl ? caretStart - caretPrototype.width : caretStart;
return Rect.fromLTRB(min(dx, width), box.top, min(dx, width), box.bottom);
return Offset(min(dx, width), box.top);
}
return null;
}
......@@ -578,52 +568,20 @@ class TextPainter {
///
/// Valid only after [layout] has been called.
Offset getOffsetForCaret(TextPosition position, Rect caretPrototype) {
_computeCaretMetrics(position, caretPrototype);
return _caretMetrics.offset;
}
/// Returns the tight bounded height of the glyph at the given [position].
///
/// Valid only after [layout] has been called.
double getFullHeightForCaret(TextPosition position, Rect caretPrototype) {
_computeCaretMetrics(position, caretPrototype);
return _caretMetrics.fullHeight;
}
// Cached caret metrics. This allows multiple invokes of [getOffsetForCaret] and
// [getFullHeightForCaret] in a row without performing redundant and expensive
// get rect calls to the paragraph.
_CaretMetrics _caretMetrics;
// Holds the TextPosition and caretPrototype the last caret metrics were
// computed with. When new values are passed in, we recompute the caret metrics.
// only as nessecary.
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) {
assert(!_needsLayout);
if (position == _previousCaretPosition && caretPrototype == _previousCaretPrototype)
return;
final int offset = position.offset;
assert(position.affinity != null);
Rect rect;
switch (position.affinity) {
case TextAffinity.upstream: {
rect = _getRectFromUpstream(offset, caretPrototype) ?? _getRectFromDownstream(offset, caretPrototype);
break;
}
case TextAffinity.downstream: {
rect = _getRectFromDownstream(offset, caretPrototype) ?? _getRectFromUpstream(offset, caretPrototype);
break;
}
case TextAffinity.upstream:
return _getOffsetFromUpstream(offset, caretPrototype)
?? _getOffsetFromDownstream(offset, caretPrototype)
?? _emptyOffset;
case TextAffinity.downstream:
return _getOffsetFromDownstream(offset, caretPrototype)
?? _getOffsetFromUpstream(offset, caretPrototype)
?? _emptyOffset;
}
_caretMetrics = _CaretMetrics(
offset: rect != null ? Offset(rect.left, rect.top) : _emptyOffset,
fullHeight: rect != null ? rect.bottom - rect.top : null,
);
return null;
}
/// Returns a list of rects that bound the given selection.
......
packages/flutter/lib/src/rendering/editable.dart
View file @ 0f5de872
......@@ -1476,13 +1476,6 @@ class RenderEditable extends RenderBox {
_textLayoutLastWidth = constraintWidth;
}
// 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). We should rework
// this properly to once again match the platform.
//
/// On iOS, the cursor is taller than the the cursor on Android. The height
/// of the cursor for iOS is approximate and obtained through an eyeball
/// comparison.
......@@ -1527,28 +1520,17 @@ class RenderEditable extends RenderBox {
void _paintCaret(Canvas canvas, Offset effectiveOffset, TextPosition textPosition) {
assert(_textLayoutLastWidth == constraints.maxWidth);
final Offset caretOffset = _textPainter.getOffsetForCaret(textPosition, _caretPrototype);
// If the floating cursor is enabled, the text cursor's color is [backgroundCursorColor] while
// the floating cursor's color is _cursorColor;
final Paint paint = Paint()
..color = _floatingCursorOn ? backgroundCursorColor : _cursorColor;
Rect caretRect = _caretPrototype.shift(_textPainter.getOffsetForCaret(textPosition, _caretPrototype) + effectiveOffset);
Rect caretRect = _caretPrototype.shift(caretOffset + effectiveOffset);
if (_cursorOffset != null)
caretRect = caretRect.shift(_cursorOffset);
// 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 _getCaretPrototype.
if (defaultTargetPlatform != TargetPlatform.iOS && _textPainter.getFullHeightForCaret(textPosition, _caretPrototype) != null) {
caretRect = Rect.fromLTRB(
caretRect.left,
(caretRect.bottom + caretRect.top) / 2 - (_textPainter.getFullHeightForCaret(textPosition, _caretPrototype) / 2),
caretRect.right,
(caretRect.bottom + caretRect.top) / 2 + (_textPainter.getFullHeightForCaret(textPosition, _caretPrototype) / 2),
);
}
caretRect = caretRect.shift(_getPixelPerfectCursorOffset(caretRect));
if (cursorRadius == null) {
......
packages/flutter/test/rendering/editable_test.dart
View file @ 0f5de872
......@@ -134,7 +134,7 @@ void main() {
expect(editable, paints..rect(
color: const Color.fromARGB(0xFF, 0xFF, 0x00, 0x00),
rect: Rect.fromLTWH(40, 0, 1, 10),
rect: Rect.fromLTWH(40, 2, 1, 6),
));
// Now change to a rounded caret.
......@@ -146,7 +146,7 @@ void main() {
expect(editable, paints..rrect(
color: const Color.fromARGB(0xFF, 0x00, 0x00, 0xFF),
rrect: RRect.fromRectAndRadius(
Rect.fromLTWH(40, 0, 4, 10),
Rect.fromLTWH(40, 2, 4, 6),
const Radius.circular(3),
),
));
......@@ -158,80 +158,7 @@ void main() {
expect(editable, paints..rrect(
color: const Color.fromARGB(0xFF, 0x00, 0x00, 0xFF),
rrect: RRect.fromRectAndRadius(
Rect.fromLTWH(80, 0, 4, 20),
const Radius.circular(3),
),
));
// Can turn off caret.
showCursor.value = false;
pumpFrame();
expect(editable, paintsExactlyCountTimes(#drawRRect, 0));
});
test('Cursor with ideographic script', () {
final TextSelectionDelegate delegate = FakeEditableTextState();
final ValueNotifier<bool> showCursor = ValueNotifier<bool>(true);
EditableText.debugDeterministicCursor = true;
final RenderEditable editable = RenderEditable(
backgroundCursorColor: Colors.grey,
textDirection: TextDirection.ltr,
cursorColor: const Color.fromARGB(0xFF, 0xFF, 0x00, 0x00),
offset: ViewportOffset.zero(),
textSelectionDelegate: delegate,
text: const TextSpan(
text: '中文测试文本是否正确',
style: TextStyle(
height: 1.0, fontSize: 10.0, fontFamily: 'Ahem',
),
),
selection: const TextSelection.collapsed(
offset: 4,
affinity: TextAffinity.upstream,
),
);
layout(editable);
editable.layout(BoxConstraints.loose(const Size(100, 100)));
expect(
editable,
// Draw no cursor by default.
paintsExactlyCountTimes(#drawRect, 0),
);
editable.showCursor = showCursor;
pumpFrame();
expect(editable, paints..rect(
color: const Color.fromARGB(0xFF, 0xFF, 0x00, 0x00),
rect: Rect.fromLTWH(40, 0, 1, 10),
));
// Now change to a rounded caret.
editable.cursorColor = const Color.fromARGB(0xFF, 0x00, 0x00, 0xFF);
editable.cursorWidth = 4;
editable.cursorRadius = const Radius.circular(3);
pumpFrame();
expect(editable, paints..rrect(
color: const Color.fromARGB(0xFF, 0x00, 0x00, 0xFF),
rrect: RRect.fromRectAndRadius(
Rect.fromLTWH(40, 0, 4, 10),
const Radius.circular(3),
),
));
editable.textScaleFactor = 2;
pumpFrame();
// Now the caret height is much bigger due to the bigger font scale.
expect(editable, paints..rrect(
color: const Color.fromARGB(0xFF, 0x00, 0x00, 0xFF),
rrect: RRect.fromRectAndRadius(
Rect.fromLTWH(80, 0, 4, 20),
Rect.fromLTWH(80, 2, 4, 16),
const Radius.circular(3),
),
));
......
packages/flutter/test/widgets/editable_text_cursor_test.dart
View file @ 0f5de872
......@@ -91,7 +91,7 @@ void main() {
await expectLater(
find.byKey(const ValueKey<int>(1)),
matchesGoldenFile('editable_text_test.0.1.png'),
matchesGoldenFile('editable_text_test.0.0.png'),
);
}, skip: !Platform.isLinux);
......@@ -142,7 +142,7 @@ void main() {
await expectLater(
find.byKey(const ValueKey<int>(1)),
matchesGoldenFile('editable_text_test.1.1.png'),
matchesGoldenFile('editable_text_test.1.0.png'),
);
}, skip: !Platform.isLinux);
......@@ -617,7 +617,7 @@ void main() {
expect(editable, paints
..rrect(
rrect: RRect.fromRectAndRadius(
Rect.fromLTRB(463.3333435058594, 0.0833332538604736, 465.3333435058594, 16.083333015441895),
Rect.fromLTRB(463.3333435058594, 2.0833332538604736, 465.3333435058594, 14.083333015441895),
const Radius.circular(2.0),
),
color: const Color(0xff8e8e93),
......@@ -642,7 +642,7 @@ void main() {
expect(find.byType(EditableText), paints
..rrect(
rrect: RRect.fromRectAndRadius(
Rect.fromLTRB(191.3333282470703, 0.0833332538604736, 193.3333282470703, 16.083333015441895),
Rect.fromLTRB(191.3333282470703, 2.0833332538604736, 193.3333282470703, 14.083333015441895),
const Radius.circular(2.0),
),
color: const Color(0xff8e8e93),
......
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

HIAST - GitLab