From 210774f392d8a842f2349e0a5ac68f8e9b5db0c7 Mon Sep 17 00:00:00 2001 From: Adam Barth Date: Sat, 1 Apr 2017 23:47:17 -0700 Subject: [PATCH] Make cursor blinking more efficient (#9142) Rather than rebuilding to blink the cursor, we now pass a ValueNotifier to the RenderEditable so that it can simply repaint. This patch also contains some refactoring towards being able to do the same thing with the text being edited, but I didn't quite get it working. --- .../flutter/lib/src/rendering/editable.dart | 34 ++++- .../lib/src/widgets/editable_text.dart | 142 +++++++++--------- 2 files changed, 99 insertions(+), 77 deletions(-) diff --git a/packages/flutter/lib/src/rendering/editable.dart b/packages/flutter/lib/src/rendering/editable.dart index 96ab78f7cc..9d586006ad 100644 --- a/packages/flutter/lib/src/rendering/editable.dart +++ b/packages/flutter/lib/src/rendering/editable.dart @@ -49,7 +49,7 @@ class RenderEditable extends RenderBox { TextSpan text, TextAlign textAlign, Color cursorColor, - bool showCursor: false, + ValueNotifier showCursor, int maxLines: 1, Color selectionColor, double textScaleFactor: 1.0, @@ -58,15 +58,15 @@ class RenderEditable extends RenderBox { this.onSelectionChanged, }) : _textPainter = new TextPainter(text: text, textAlign: textAlign, textScaleFactor: textScaleFactor), _cursorColor = cursorColor, - _showCursor = showCursor, + _showCursor = showCursor ?? new ValueNotifier(false), _maxLines = maxLines, _selection = selection, _offset = offset { - assert(showCursor != null); + assert(_showCursor != null); assert(maxLines != null); assert(textScaleFactor != null); assert(offset != null); - assert(!showCursor || cursorColor != null); + assert(!_showCursor.value || cursorColor != null); _tap = new TapGestureRecognizer() ..onTapDown = _handleTapDown ..onTap = _handleTap @@ -108,13 +108,17 @@ class RenderEditable extends RenderBox { } /// Whether to paint the cursor. - bool get showCursor => _showCursor; - bool _showCursor; - set showCursor(bool value) { + ValueNotifier get showCursor => _showCursor; + ValueNotifier _showCursor; + set showCursor(ValueNotifier value) { assert(value != null); if (_showCursor == value) return; + if (attached) + _showCursor.removeListener(markNeedsPaint); _showCursor = value; + if (attached) + _showCursor.addListener(markNeedsPaint); markNeedsPaint(); } @@ -186,6 +190,20 @@ class RenderEditable extends RenderBox { markNeedsLayout(); } + @override + void attach(PipelineOwner owner) { + super.attach(owner); + _offset.addListener(markNeedsPaint); + _showCursor.addListener(markNeedsPaint); + } + + @override + void detach() { + _offset.removeListener(markNeedsPaint); + _showCursor.removeListener(markNeedsPaint); + super.detach(); + } + bool get _isMultiline => maxLines > 1; Axis get _viewportAxis => _isMultiline ? Axis.vertical : Axis.horizontal; @@ -377,7 +395,7 @@ class RenderEditable extends RenderBox { final Offset effectiveOffset = offset + _paintOffset; if (_selection != null) { - if (_selection.isCollapsed && _showCursor && cursorColor != null) { + if (_selection.isCollapsed && _showCursor.value && cursorColor != null) { _paintCaret(context.canvas, effectiveOffset); } else if (!_selection.isCollapsed && _selectionColor != null) { _selectionRects ??= _textPainter.getBoxesForSelection(_selection); diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart index c6890c4b2d..3ecdf086f2 100644 --- a/packages/flutter/lib/src/widgets/editable_text.dart +++ b/packages/flutter/lib/src/widgets/editable_text.dart @@ -23,29 +23,19 @@ export 'package:flutter/services.dart' show TextEditingValue, TextSelection, Tex const Duration _kCursorBlinkHalfPeriod = const Duration(milliseconds: 500); -class TextEditingController extends ChangeNotifier { +class TextEditingController extends ValueNotifier { TextEditingController({ String text }) - : _value = text == null ? TextEditingValue.empty : new TextEditingValue(text: text); + : super(text == null ? TextEditingValue.empty : new TextEditingValue(text: text)); TextEditingController.fromValue(TextEditingValue value) - : _value = value ?? TextEditingValue.empty; + : super(value ?? TextEditingValue.empty); - TextEditingValue get value => _value; - TextEditingValue _value; - set value(TextEditingValue newValue) { - assert(newValue != null); - if (_value == newValue) - return; - _value = newValue; - notifyListeners(); - } - - String get text => _value.text; + String get text => value.text; set text(String newText) { value = value.copyWith(text: newText, composing: TextRange.empty); } - TextSelection get selection => _value.selection; + TextSelection get selection => value.selection; set selection(TextSelection newSelection) { value = value.copyWith(selection: newSelection, composing: TextRange.empty); } @@ -174,7 +164,7 @@ class EditableText extends StatefulWidget { /// State for a [EditableText]. class EditableTextState extends State implements TextInputClient { Timer _cursorTimer; - bool _showCursor = false; + final ValueNotifier _showCursor = new ValueNotifier(false); TextInputConnection _textInputConnection; TextSelectionOverlay _selectionOverlay; @@ -206,7 +196,7 @@ class EditableTextState extends State implements TextInputClient { if (config.controller != oldConfig.controller) { oldConfig.controller.removeListener(_didChangeTextEditingValue); config.controller.addListener(_didChangeTextEditingValue); - if (_isAttachedToKeyboard && config.controller.value != oldConfig.controller.value) + if (_hasInputConnection && config.controller.value != oldConfig.controller.value) _textInputConnection.setEditingState(config.controller.value); } if (config.focusNode != oldConfig.focusNode) { @@ -218,13 +208,9 @@ class EditableTextState extends State implements TextInputClient { @override void dispose() { config.controller.removeListener(_didChangeTextEditingValue); - if (_isAttachedToKeyboard) { - _textInputConnection.close(); - _textInputConnection = null; - } - assert(!_isAttachedToKeyboard); - if (_cursorTimer != null) - _stopCursorTimer(); + _closeInputConnectionIfNeeded(); + assert(!_hasInputConnection); + _stopCursorTimer(); assert(_cursorTimer == null); _selectionOverlay?.dispose(); _selectionOverlay = null; @@ -256,12 +242,7 @@ class EditableTextState extends State implements TextInputClient { config.controller.value = value; } - void _didChangeTextEditingValue() { - setState(() { /* We use config.controller.value in build(). */ }); - } - - bool get _isAttachedToKeyboard => _textInputConnection != null && _textInputConnection.attached; - + bool get _hasFocus => config.focusNode.hasFocus; bool get _isMultiline => config.maxLines > 1; // Calculate the new scroll offset so the cursor remains visible. @@ -278,17 +259,28 @@ class EditableTextState extends State implements TextInputClient { } bool _didRequestKeyboard = false; + bool get _hasInputConnection => _textInputConnection != null && _textInputConnection.attached; - void _attachOrDetachKeyboard(bool focused) { - if (focused && !_isAttachedToKeyboard && _didRequestKeyboard) { + void _openInputConnectionIfNeeded() { + if (!_hasInputConnection) { _textInputConnection = TextInput.attach(this, new TextInputConfiguration(inputType: config.keyboardType)) ..setEditingState(_value) ..show(); - } else if (!focused) { - if (_isAttachedToKeyboard) { - _textInputConnection.close(); - _textInputConnection = null; - } + } + } + + void _closeInputConnectionIfNeeded() { + if (_hasInputConnection) { + _textInputConnection.close(); + _textInputConnection = null; + } + } + + void _openOrCloseInputConnectionIfNeeded() { + if (_hasFocus && _didRequestKeyboard) { + _openInputConnectionIfNeeded(); + } else if (!_hasFocus) { + _closeInputConnectionIfNeeded(); config.controller.clearComposing(); } _didRequestKeyboard = false; @@ -302,14 +294,15 @@ class EditableTextState extends State implements TextInputClient { /// focus, the control will then attach to the keyboard and request that the /// keyboard become visible. void requestKeyboard() { - if (_isAttachedToKeyboard) { + if (_hasInputConnection) { _textInputConnection.show(); } else { - _didRequestKeyboard = true; - if (config.focusNode.hasFocus) - _attachOrDetachKeyboard(true); - else + if (_hasFocus) { + _openInputConnectionIfNeeded(); + } else { + _didRequestKeyboard = true; FocusScope.of(context).requestFocus(config.focusNode); + } } } @@ -318,6 +311,17 @@ class EditableTextState extends State implements TextInputClient { _selectionOverlay = null; } + void _updateOrDisposeSelectionOverlayIfNeeded() { + if (_selectionOverlay != null) { + if (_hasFocus) { + _selectionOverlay.update(_value); + } else { + _selectionOverlay.dispose(); + _selectionOverlay = null; + } + } + } + void _handleSelectionChanged(TextSelection selection, RenderEditable renderObject, bool longPress) { // Note that this will show the keyboard for all selection changes on the // EditableWidget, not just changes triggered by user gestures. @@ -351,7 +355,7 @@ class EditableTextState extends State implements TextInputClient { /// Whether the blinking cursor is actually visible at this precise moment /// (it's hidden half the time, since it blinks). @visibleForTesting - bool get cursorCurrentlyVisible => _showCursor; + bool get cursorCurrentlyVisible => _showCursor.value; /// The cursor blink interval (the amount of time the cursor is in the "on" /// state or the "off" state). A complete cursor blink period is twice this @@ -360,39 +364,39 @@ class EditableTextState extends State implements TextInputClient { Duration get cursorBlinkInterval => _kCursorBlinkHalfPeriod; void _cursorTick(Timer timer) { - setState(() { - _showCursor = !_showCursor; - }); + _showCursor.value = !_showCursor.value; } void _startCursorTimer() { - _showCursor = true; + _showCursor.value = true; _cursorTimer = new Timer.periodic(_kCursorBlinkHalfPeriod, _cursorTick); } - void _handleFocusChanged() { - final bool focused = config.focusNode.hasFocus; - _attachOrDetachKeyboard(focused); - - if (_cursorTimer == null && focused && _value.selection.isCollapsed) - _startCursorTimer(); - else if (_cursorTimer != null && (!focused || !_value.selection.isCollapsed)) - _stopCursorTimer(); - - if (_selectionOverlay != null) { - if (focused) { - _selectionOverlay.update(_value); - } else { - _selectionOverlay.dispose(); - _selectionOverlay = null; - } - } + void _stopCursorTimer() { + _cursorTimer?.cancel(); + _cursorTimer = null; + _showCursor.value = false; } - void _stopCursorTimer() { - _cursorTimer.cancel(); - _cursorTimer = null; - _showCursor = false; + void _startOrStopCursorTimerIfNeeded() { + if (_cursorTimer == null && _hasFocus && _value.selection.isCollapsed) + _startCursorTimer(); + else if (_cursorTimer != null && (!_hasFocus || !_value.selection.isCollapsed)) + _stopCursorTimer(); + } + + void _didChangeTextEditingValue() { + _startOrStopCursorTimerIfNeeded(); + _updateOrDisposeSelectionOverlayIfNeeded(); + // TODO(abarth): Teach RenderEditable about ValueNotifier + // to avoid this setState(). + setState(() { /* We use config.controller.value in build(). */ }); + } + + void _handleFocusChanged() { + _openOrCloseInputConnectionIfNeeded(); + _startOrStopCursorTimerIfNeeded(); + _updateOrDisposeSelectionOverlayIfNeeded(); } @override @@ -440,7 +444,7 @@ class _Editable extends LeafRenderObjectWidget { final TextEditingValue value; final TextStyle style; final Color cursorColor; - final bool showCursor; + final ValueNotifier showCursor; final int maxLines; final Color selectionColor; final double textScaleFactor;