From abeb164fe28945197d7dc0f76bb9b1b17483563b Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Mon, 15 May 2017 14:06:58 -0700 Subject: [PATCH] Add intrinsic dimensions to RenderEditable (#10093) Also: * Make TextPainter.preferredLineHeight honour root fontSize * Remove bogus docs. * More aggressively track dirty state for RenderEditable. * Some tests. --- .../lib/src/painting/text_painter.dart | 33 ++++++----- .../flutter/lib/src/rendering/editable.dart | 58 +++++++++++++++---- .../test/painting/text_painter_test.dart | 14 +++++ .../flutter/test/rendering/editable_test.dart | 22 +++++++ 4 files changed, 101 insertions(+), 26 deletions(-) create mode 100644 packages/flutter/test/rendering/editable_test.dart diff --git a/packages/flutter/lib/src/painting/text_painter.dart b/packages/flutter/lib/src/painting/text_painter.dart index 9b7860f0e7..a3e77426e4 100644 --- a/packages/flutter/lib/src/painting/text_painter.dart +++ b/packages/flutter/lib/src/painting/text_painter.dart @@ -136,15 +136,30 @@ class TextPainter { ui.Paragraph _layoutTemplate; + ui.ParagraphStyle _createParagraphStyle() { + return _text.style?.getParagraphStyle( + textAlign: textAlign, + textScaleFactor: textScaleFactor, + maxLines: _maxLines, + ellipsis: _ellipsis, + ) ?? new ui.ParagraphStyle( + textAlign: textAlign, + maxLines: maxLines, + ellipsis: ellipsis, + ); + } + /// The height of a zero-width space in [text] in logical pixels. /// /// Not every line of text in [text] will have this height, but this height /// is "typical" for text in [text] and useful for sizing other objects /// relative a typical line of text. + /// + /// Obtaining this value does not require calling [layout]. double get preferredLineHeight { assert(text != null); if (_layoutTemplate == null) { - final ui.ParagraphBuilder builder = new ui.ParagraphBuilder(new ui.ParagraphStyle()); + final ui.ParagraphBuilder builder = new ui.ParagraphBuilder(_createParagraphStyle()); if (text.style != null) builder.pushStyle(text.style.getTextStyle(textScaleFactor: textScaleFactor)); builder.addText(_kZeroWidthSpace); @@ -165,7 +180,8 @@ class TextPainter { return layoutValue.ceilToDouble(); } - /// The width at which decreasing the width of the text would prevent it from painting itself completely within its bounds. + /// The width at which decreasing the width of the text would prevent it from + /// painting itself completely within its bounds. /// /// Valid only after [layout] has been called. double get minIntrinsicWidth { @@ -251,18 +267,7 @@ class TextPainter { return; _needsLayout = false; if (_paragraph == null) { - ui.ParagraphStyle paragraphStyle = _text.style?.getParagraphStyle( - textAlign: textAlign, - textScaleFactor: textScaleFactor, - maxLines: _maxLines, - ellipsis: _ellipsis, - ); - paragraphStyle ??= new ui.ParagraphStyle( - textAlign: textAlign, - maxLines: maxLines, - ellipsis: ellipsis, - ); - final ui.ParagraphBuilder builder = new ui.ParagraphBuilder(paragraphStyle); + final ui.ParagraphBuilder builder = new ui.ParagraphBuilder(_createParagraphStyle()); _text.build(builder, textScaleFactor: textScaleFactor); _paragraph = builder.build(); } diff --git a/packages/flutter/lib/src/rendering/editable.dart b/packages/flutter/lib/src/rendering/editable.dart index 6147b9fe3e..2cde1ae4a3 100644 --- a/packages/flutter/lib/src/rendering/editable.dart +++ b/packages/flutter/lib/src/rendering/editable.dart @@ -53,9 +53,7 @@ class TextSelectionPoint { } } -/// A single line of editable text. class RenderEditable extends RenderBox { - /// Creates a render object for a single line of editable text. RenderEditable({ TextSpan text, TextAlign textAlign, @@ -89,6 +87,18 @@ class RenderEditable extends RenderBox { /// Called when the selection changes. SelectionChangedHandler onSelectionChanged; + double _textLayoutLastWidth; + + /// Marks the render object as needing to be laid out again and have its text + /// metrics recomputed. + /// + /// Implies [markNeedsLayout]. + @protected + void markNeedsTextLayout() { + _textLayoutLastWidth = null; + markNeedsLayout(); + } + /// The text to display TextSpan get text => _textPainter.text; final TextPainter _textPainter; @@ -96,7 +106,7 @@ class RenderEditable extends RenderBox { if (_textPainter.text == value) return; _textPainter.text = value; - markNeedsLayout(); + markNeedsTextLayout(); } /// How the text should be aligned horizontally. @@ -143,7 +153,7 @@ class RenderEditable extends RenderBox { if (_maxLines == value) return; _maxLines = value; - markNeedsLayout(); + markNeedsTextLayout(); } /// The color to use when painting the selection. @@ -166,7 +176,7 @@ class RenderEditable extends RenderBox { if (_textPainter.textScaleFactor == value) return; _textPainter.textScaleFactor = value; - markNeedsLayout(); + markNeedsTextLayout(); } List _selectionRects; @@ -261,10 +271,8 @@ class RenderEditable extends RenderBox { /// points might actually be co-located (e.g., because of a bidirectional /// selection that contains some text but whose ends meet in the middle). List getEndpointsForSelection(TextSelection selection) { - // TODO(mpcomplete): We should be more disciplined about when we dirty the - // layout state of the text painter so that we can know that the layout is - // clean at this point. - _layoutText(); + assert(constraints != null); + _layoutText(constraints.maxWidth); final Offset paintOffset = _paintOffset; @@ -286,6 +294,7 @@ class RenderEditable extends RenderBox { /// Returns the position in the text for the given global coordinate. TextPosition getPositionForPoint(Offset globalPosition) { + _layoutText(constraints.maxWidth); globalPosition += -_paintOffset; return _textPainter.getPositionForOffset(globalToLocal(globalPosition)); } @@ -293,11 +302,25 @@ class RenderEditable extends RenderBox { /// Returns the Rect in local coordinates for the caret at the given text /// position. Rect getLocalRectForCaret(TextPosition caretPosition) { + _layoutText(constraints.maxWidth); final Offset caretOffset = _textPainter.getOffsetForCaret(caretPosition, _caretPrototype); // This rect is the same as _caretPrototype but without the vertical padding. return new Rect.fromLTWH(0.0, 0.0, _kCaretWidth, _preferredLineHeight).shift(caretOffset + _paintOffset); } + @override + double computeMinIntrinsicWidth(double height) { + _layoutText(double.INFINITY); + return _textPainter.minIntrinsicWidth; + } + + @override + double computeMaxIntrinsicWidth(double height) { + _layoutText(double.INFINITY); + return _textPainter.maxIntrinsicWidth; + } + + // This does not required the layout to be updated. double get _preferredLineHeight => _textPainter.preferredLineHeight; @override @@ -332,6 +355,7 @@ class RenderEditable extends RenderBox { } void _handleTap() { + _layoutText(constraints.maxWidth); assert(_lastTapDownPosition != null); final Offset globalPosition = _lastTapDownPosition; _lastTapDownPosition = null; @@ -348,6 +372,7 @@ class RenderEditable extends RenderBox { } void _handleLongPress() { + _layoutText(constraints.maxWidth); final Offset globalPosition = _longPressPosition; _longPressPosition = null; if (onSelectionChanged != null) { @@ -357,6 +382,7 @@ class RenderEditable extends RenderBox { } TextSelection _selectWordAtOffset(TextPosition position) { + assert(_textLayoutLastWidth == constraints.maxWidth); final TextRange word = _textPainter.getWordBoundary(position); // When long-pressing past the end of the text, we want a collapsed cursor. if (position.offset >= word.end) @@ -366,18 +392,22 @@ class RenderEditable extends RenderBox { Rect _caretPrototype; - void _layoutText() { + void _layoutText(double constraintWidth) { + assert(constraintWidth != null); + if (_textLayoutLastWidth == constraintWidth) + return; final double caretMargin = _kCaretGap + _kCaretWidth; - final double availableWidth = math.max(0.0, constraints.maxWidth - caretMargin); + final double availableWidth = math.max(0.0, constraintWidth - caretMargin); final double maxWidth = _maxLines > 1 ? availableWidth : double.INFINITY; _textPainter.layout(minWidth: availableWidth, maxWidth: maxWidth); + _textLayoutLastWidth = constraintWidth; } @override void performLayout() { + _layoutText(constraints.maxWidth); _caretPrototype = new Rect.fromLTWH(0.0, _kCaretHeightOffset, _kCaretWidth, _preferredLineHeight - 2.0 * _kCaretHeightOffset); _selectionRects = null; - _layoutText(); size = new Size(constraints.maxWidth, constraints.constrainHeight( _textPainter.height.clamp(_preferredLineHeight, _preferredLineHeight * _maxLines) )); @@ -389,12 +419,14 @@ class RenderEditable extends RenderBox { } void _paintCaret(Canvas canvas, Offset effectiveOffset) { + assert(_textLayoutLastWidth == constraints.maxWidth); final Offset caretOffset = _textPainter.getOffsetForCaret(_selection.extent, _caretPrototype); final Paint paint = new Paint()..color = _cursorColor; canvas.drawRect(_caretPrototype.shift(caretOffset + effectiveOffset), paint); } void _paintSelection(Canvas canvas, Offset effectiveOffset) { + assert(_textLayoutLastWidth == constraints.maxWidth); assert(_selectionRects != null); final Paint paint = new Paint()..color = _selectionColor; for (ui.TextBox box in _selectionRects) @@ -402,6 +434,7 @@ class RenderEditable extends RenderBox { } void _paintContents(PaintingContext context, Offset offset) { + assert(_textLayoutLastWidth == constraints.maxWidth); final Offset effectiveOffset = offset + _paintOffset; if (_selection != null) { @@ -418,6 +451,7 @@ class RenderEditable extends RenderBox { @override void paint(PaintingContext context, Offset offset) { + _layoutText(constraints.maxWidth); if (_hasVisualOverflow) context.pushClipRect(needsCompositing, offset, Offset.zero & size, _paintContents); else diff --git a/packages/flutter/test/painting/text_painter_test.dart b/packages/flutter/test/painting/text_painter_test.dart index ef018547a1..3db5ea4fd9 100644 --- a/packages/flutter/test/painting/text_painter_test.dart +++ b/packages/flutter/test/painting/text_painter_test.dart @@ -48,4 +48,18 @@ void main() { painter.layout(); expect(painter.size, const Size(123.0, 123.0)); }); + + test('TextPainter default text height is 14 pixels', () { + final TextPainter painter = new TextPainter(text: const TextSpan(text: 'x')); + painter.layout(); + expect(painter.preferredLineHeight, 14.0); + expect(painter.size, const Size(14.0, 14.0)); + }); + + test('TextPainter sets paragraph size from root', () { + final TextPainter painter = new TextPainter(text: const TextSpan(text: 'x', style: const TextStyle(fontSize: 100.0))); + painter.layout(); + expect(painter.preferredLineHeight, 100.0); + expect(painter.size, const Size(100.0, 100.0)); + }); } diff --git a/packages/flutter/test/rendering/editable_test.dart b/packages/flutter/test/rendering/editable_test.dart new file mode 100644 index 0000000000..d9096c28d2 --- /dev/null +++ b/packages/flutter/test/rendering/editable_test.dart @@ -0,0 +1,22 @@ +// Copyright 2017 The Chromium 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/rendering.dart'; +import 'package:test/test.dart'; + +void main() { + test('editable intrinsics', () { + final RenderEditable editable = new RenderEditable( + text: const TextSpan( + style: const TextStyle(height: 1.0, fontSize: 10.0, fontFamily: 'Ahem'), + text: '12345', + ), + offset: new ViewportOffset.zero(), + ); + expect(editable.getMinIntrinsicWidth(double.INFINITY), 50.0); + expect(editable.getMaxIntrinsicWidth(double.INFINITY), 50.0); + expect(editable.getMinIntrinsicHeight(double.INFINITY), 10.0); + expect(editable.getMaxIntrinsicHeight(double.INFINITY), 10.0); + }); +} \ No newline at end of file