From 57e577a5d691ee0ac0f64a71040d516e5d656889 Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Mon, 17 Oct 2022 12:47:37 -0700 Subject: [PATCH] Clean up `_updateSelectionRects` (#113425) --- .../lib/src/widgets/editable_text.dart | 171 ++++++++++-------- .../test/widgets/editable_text_test.dart | 145 ++++++++++++--- 2 files changed, 220 insertions(+), 96 deletions(-) diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart index 745bf8e8a6..c287ac5430 100644 --- a/packages/flutter/lib/src/widgets/editable_text.dart +++ b/packages/flutter/lib/src/widgets/editable_text.dart @@ -1779,6 +1779,8 @@ class EditableTextState extends State with AutomaticKeepAliveClien final ClipboardStatusNotifier? _clipboardStatus = kIsWeb ? null : ClipboardStatusNotifier(); TextInputConnection? _textInputConnection; + bool get _hasInputConnection => _textInputConnection?.attached ?? false; + TextSelectionOverlay? _selectionOverlay; ScrollController? _internalScrollController; @@ -2037,7 +2039,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien _clipboardStatus?.addListener(_onChangedClipboardStatus); widget.controller.addListener(_didChangeTextEditingValue); widget.focusNode.addListener(_handleFocusChanged); - _scrollController.addListener(_updateSelectionOverlayForScroll); + _scrollController.addListener(_onEditableScroll); _cursorVisibilityNotifier.value = widget.showCursor; _spellCheckConfiguration = _inferSpellCheckConfiguration(widget.spellCheckConfiguration); } @@ -2125,8 +2127,8 @@ class EditableTextState extends State with AutomaticKeepAliveClien } if (widget.scrollController != oldWidget.scrollController) { - (oldWidget.scrollController ?? _internalScrollController)?.removeListener(_updateSelectionOverlayForScroll); - _scrollController.addListener(_updateSelectionOverlayForScroll); + (oldWidget.scrollController ?? _internalScrollController)?.removeListener(_onEditableScroll); + _scrollController.addListener(_onEditableScroll); } if (!_shouldCreateInputConnection) { @@ -2567,7 +2569,6 @@ class EditableTextState extends State with AutomaticKeepAliveClien return RevealedOffset(rect: rect.shift(unitOffset * offsetDelta), offset: targetOffset); } - bool get _hasInputConnection => _textInputConnection?.attached ?? false; /// Whether to send the autofill information to the autofill service. True by /// default. bool get _needsAutofill => _effectiveAutofillClient.textInputConfiguration.autofillConfiguration.enabled; @@ -2718,8 +2719,9 @@ class EditableTextState extends State with AutomaticKeepAliveClien } } - void _updateSelectionOverlayForScroll() { + void _onEditableScroll() { _selectionOverlay?.updateForScroll(); + _scribbleCacheKey = null; } void _createSelectionOverlay() { @@ -3115,11 +3117,6 @@ class EditableTextState extends State with AutomaticKeepAliveClien // Place cursor at the end if the selection is invalid when we receive focus. _handleSelectionChanged(TextSelection.collapsed(offset: _value.text.length), null); } - - _cachedText = ''; - _cachedFirstRect = null; - _cachedSize = Size.zero; - _cachedPlaceholder = -1; } else { WidgetsBinding.instance.removeObserver(this); setState(() { _currentPromptRectRange = null; }); @@ -3127,74 +3124,66 @@ class EditableTextState extends State with AutomaticKeepAliveClien updateKeepAlive(); } - String _cachedText = ''; - Rect? _cachedFirstRect; - Size _cachedSize = Size.zero; - int _cachedPlaceholder = -1; - TextStyle? _cachedTextStyle; + _ScribbleCacheKey? _scribbleCacheKey; void _updateSelectionRects({bool force = false}) { - if (!widget.scribbleEnabled) { - return; - } - if (defaultTargetPlatform != TargetPlatform.iOS) { + if (!widget.scribbleEnabled || defaultTargetPlatform != TargetPlatform.iOS) { return; } - final String text = renderEditable.text?.toPlainText(includeSemanticsLabels: false) ?? ''; - final List firstSelectionBoxes = renderEditable.getBoxesForSelection(const TextSelection(baseOffset: 0, extentOffset: 1)); - final Rect? firstRect = firstSelectionBoxes.isNotEmpty ? firstSelectionBoxes.first : null; final ScrollDirection scrollDirection = _scrollController.position.userScrollDirection; - final Size size = renderEditable.size; - final bool textChanged = text != _cachedText; - final bool textStyleChanged = _cachedTextStyle != widget.style; - final bool firstRectChanged = _cachedFirstRect != firstRect; - final bool sizeChanged = _cachedSize != size; - final bool placeholderChanged = _cachedPlaceholder != _placeholderLocation; - if (scrollDirection == ScrollDirection.idle && (force || textChanged || textStyleChanged || firstRectChanged || sizeChanged || placeholderChanged)) { - _cachedText = text; - _cachedFirstRect = firstRect; - _cachedTextStyle = widget.style; - _cachedSize = size; - _cachedPlaceholder = _placeholderLocation; - bool belowRenderEditableBottom = false; - final List rects = List.generate( - _cachedText.characters.length, - (int i) { - if (belowRenderEditableBottom) { - return null; - } - - final int offset = _cachedText.characters.getRange(0, i).string.length; - final List boxes = renderEditable.getBoxesForSelection(TextSelection(baseOffset: offset, extentOffset: offset + _cachedText.characters.characterAt(i).string.length)); - if (boxes.isEmpty) { - return null; - } - - final SelectionRect selectionRect = SelectionRect( - bounds: boxes.first, - position: offset, - ); - if (renderEditable.paintBounds.bottom < selectionRect.bounds.top) { - belowRenderEditableBottom = true; - return null; - } - return selectionRect; - }, - ).where((SelectionRect? selectionRect) { - if (selectionRect == null) { - return false; - } - if (renderEditable.paintBounds.right < selectionRect.bounds.left || selectionRect.bounds.right < renderEditable.paintBounds.left) { - return false; - } - if (renderEditable.paintBounds.bottom < selectionRect.bounds.top || selectionRect.bounds.bottom < renderEditable.paintBounds.top) { - return false; - } - return true; - }).map((SelectionRect? selectionRect) => selectionRect!).toList(); - _textInputConnection!.setSelectionRects(rects); + if (scrollDirection != ScrollDirection.idle) { + return; } + + final InlineSpan inlineSpan = renderEditable.text!; + final _ScribbleCacheKey newCacheKey = _ScribbleCacheKey( + inlineSpan: inlineSpan, + textAlign: widget.textAlign, + textDirection: _textDirection, + textScaleFactor: widget.textScaleFactor ?? MediaQuery.textScaleFactorOf(context), + textHeightBehavior: widget.textHeightBehavior ?? DefaultTextHeightBehavior.of(context), + locale: widget.locale, + structStyle: widget.strutStyle, + placeholder: _placeholderLocation, + size: renderEditable.size, + ); + + final RenderComparison comparison = force + ? RenderComparison.layout + : _scribbleCacheKey?.compare(newCacheKey) ?? RenderComparison.layout; + if (comparison.index < RenderComparison.layout.index) { + return; + } + _scribbleCacheKey = newCacheKey; + + final List rects = []; + int graphemeStart = 0; + // Can't use _value.text here: the controller value could change between + // frames. + final String plainText = inlineSpan.toPlainText(includeSemanticsLabels: false); + final CharacterRange characterRange = CharacterRange(plainText); + while (characterRange.moveNext()) { + final int graphemeEnd = graphemeStart + characterRange.current.length; + final List boxes = renderEditable.getBoxesForSelection( + TextSelection(baseOffset: graphemeStart, extentOffset: graphemeEnd), + ); + + final Rect? box = boxes.isEmpty ? null : boxes.first; + if (box != null) { + final Rect paintBounds = renderEditable.paintBounds; + // Stop early when characters are already below the bottom edge of the + // RenderEditable, regardless of its clipBehavior. + if (paintBounds.bottom <= box.top) { + break; + } + if (paintBounds.contains(box.topLeft) || paintBounds.contains(box.bottomRight)) { + rects.add(SelectionRect(position: graphemeStart, bounds: box)); + } + } + graphemeStart = graphemeEnd; + } + _textInputConnection!.setSelectionRects(rects); } void _updateSizeAndTransform() { @@ -4103,6 +4092,46 @@ class _Editable extends MultiChildRenderObjectWidget { } } +@immutable +class _ScribbleCacheKey { + const _ScribbleCacheKey({ + required this.inlineSpan, + required this.textAlign, + required this.textDirection, + required this.textScaleFactor, + required this.textHeightBehavior, + required this.locale, + required this.structStyle, + required this.placeholder, + required this.size, + }); + + final TextAlign textAlign; + final TextDirection textDirection; + final double textScaleFactor; + final TextHeightBehavior? textHeightBehavior; + final Locale? locale; + final StrutStyle structStyle; + final int placeholder; + final Size size; + final InlineSpan inlineSpan; + + RenderComparison compare(_ScribbleCacheKey other) { + if (identical(other, this)) { + return RenderComparison.identical; + } + final bool needsLayout = textAlign != other.textAlign + || textDirection != other.textDirection + || textScaleFactor != other.textScaleFactor + || (textHeightBehavior ?? const TextHeightBehavior()) != (other.textHeightBehavior ?? const TextHeightBehavior()) + || locale != other.locale + || structStyle != other.structStyle + || placeholder != other.placeholder + || size != other.size; + return needsLayout ? RenderComparison.layout : inlineSpan.compareTo(other.inlineSpan); + } +} + class _ScribbleFocusable extends StatefulWidget { const _ScribbleFocusable({ required this.child, diff --git a/packages/flutter/test/widgets/editable_text_test.dart b/packages/flutter/test/widgets/editable_text_test.dart index f96d9a7ff3..5b3f6b97ba 100644 --- a/packages/flutter/test/widgets/editable_text_test.dart +++ b/packages/flutter/test/widgets/editable_text_test.dart @@ -4628,41 +4628,136 @@ void main() { // Ensure selection rects are sent on iPhone (using SE 3rd gen size) tester.binding.window.physicalSizeTestValue = const Size(750.0, 1334.0); - final List log = []; + final List> log = >[]; SystemChannels.textInput.setMockMethodCallHandler((MethodCall methodCall) async { - log.add(methodCall); + if (methodCall.method == 'TextInput.setSelectionRects') { + final List args = methodCall.arguments as List; + final List selectionRects = []; + for (final dynamic rect in args) { + selectionRects.add(SelectionRect( + position: (rect as List)[4] as int, + bounds: Rect.fromLTWH(rect[0] as double, rect[1] as double, rect[2] as double, rect[3] as double), + )); + } + log.add(selectionRects); + } }); final TextEditingController controller = TextEditingController(); + final ScrollController scrollController = ScrollController(); controller.text = 'Text1'; - await tester.pumpWidget( - MediaQuery( - data: const MediaQueryData(), - child: Directionality( - textDirection: TextDirection.ltr, - child: Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - EditableText( - key: ValueKey(controller.text), - controller: controller, - focusNode: FocusNode(), - style: Typography.material2018().black.titleMedium!, - cursorColor: Colors.blue, - backgroundCursorColor: Colors.grey, + Future pumpEditableText({ double? width, double? height, TextAlign textAlign = TextAlign.start }) async { + await tester.pumpWidget( + MediaQuery( + data: const MediaQueryData(), + child: Directionality( + textDirection: TextDirection.ltr, + child: Center( + child: SizedBox( + width: width, + height: height, + child: EditableText( + controller: controller, + textAlign: textAlign, + scrollController: scrollController, + maxLines: null, + focusNode: focusNode, + cursorWidth: 0, + style: Typography.material2018().black.titleMedium!, + cursorColor: Colors.blue, + backgroundCursorColor: Colors.grey, + ), ), - ], + ), ), ), - ), - ); - await tester.showKeyboard(find.byKey(ValueKey(controller.text))); + ); + } - // There should be a new platform message updating the selection rects. - final MethodCall methodCall = log.firstWhere((MethodCall m) => m.method == 'TextInput.setSelectionRects'); - expect(methodCall.method, 'TextInput.setSelectionRects'); - expect((methodCall.arguments as List).length, 5); + await pumpEditableText(); + expect(log, isEmpty); + + await tester.showKeyboard(find.byType(EditableText)); + // First update. + expect(log.single, const [ + SelectionRect(position: 0, bounds: Rect.fromLTRB(0.0, 0.0, 14.0, 14.0)), + SelectionRect(position: 1, bounds: Rect.fromLTRB(14.0, 0.0, 28.0, 14.0)), + SelectionRect(position: 2, bounds: Rect.fromLTRB(28.0, 0.0, 42.0, 14.0)), + SelectionRect(position: 3, bounds: Rect.fromLTRB(42.0, 0.0, 56.0, 14.0)), + SelectionRect(position: 4, bounds: Rect.fromLTRB(56.0, 0.0, 70.0, 14.0)) + ]); + log.clear(); + + await tester.pumpAndSettle(); + expect(log, isEmpty); + + await pumpEditableText(); + expect(log, isEmpty); + + // Change the width such that each character occupies a line. + await pumpEditableText(width: 20); + expect(log.single, const [ + SelectionRect(position: 0, bounds: Rect.fromLTRB(0.0, 0.0, 14.0, 14.0)), + SelectionRect(position: 1, bounds: Rect.fromLTRB(0.0, 14.0, 14.0, 28.0)), + SelectionRect(position: 2, bounds: Rect.fromLTRB(0.0, 28.0, 14.0, 42.0)), + SelectionRect(position: 3, bounds: Rect.fromLTRB(0.0, 42.0, 14.0, 56.0)), + SelectionRect(position: 4, bounds: Rect.fromLTRB(0.0, 56.0, 14.0, 70.0)) + ]); + log.clear(); + + await tester.enterText(find.byType(EditableText), 'Text1👨‍👩‍👦'); + await tester.pump(); + expect(log.single, const [ + SelectionRect(position: 0, bounds: Rect.fromLTRB(0.0, 0.0, 14.0, 14.0)), + SelectionRect(position: 1, bounds: Rect.fromLTRB(0.0, 14.0, 14.0, 28.0)), + SelectionRect(position: 2, bounds: Rect.fromLTRB(0.0, 28.0, 14.0, 42.0)), + SelectionRect(position: 3, bounds: Rect.fromLTRB(0.0, 42.0, 14.0, 56.0)), + SelectionRect(position: 4, bounds: Rect.fromLTRB(0.0, 56.0, 14.0, 70.0)), + SelectionRect(position: 5, bounds: Rect.fromLTRB(0.0, 70.0, 42.0, 84.0)), + ]); + log.clear(); + + // The 4th line will be partially visible. + await pumpEditableText(width: 20, height: 45); + expect(log.single, const [ + SelectionRect(position: 0, bounds: Rect.fromLTRB(0.0, 0.0, 14.0, 14.0)), + SelectionRect(position: 1, bounds: Rect.fromLTRB(0.0, 14.0, 14.0, 28.0)), + SelectionRect(position: 2, bounds: Rect.fromLTRB(0.0, 28.0, 14.0, 42.0)), + SelectionRect(position: 3, bounds: Rect.fromLTRB(0.0, 42.0, 14.0, 56.0)), + ]); + log.clear(); + + await pumpEditableText(width: 20, height: 45, textAlign: TextAlign.right); + // This is 1px off from being completely right-aligned. The 1px width is + // reserved for caret. + expect(log.single, const [ + SelectionRect(position: 0, bounds: Rect.fromLTRB(5.0, 0.0, 19.0, 14.0)), + SelectionRect(position: 1, bounds: Rect.fromLTRB(5.0, 14.0, 19.0, 28.0)), + SelectionRect(position: 2, bounds: Rect.fromLTRB(5.0, 28.0, 19.0, 42.0)), + SelectionRect(position: 3, bounds: Rect.fromLTRB(5.0, 42.0, 19.0, 56.0)), + // These 2 lines will be out of bounds. + // SelectionRect(position: 4, bounds: Rect.fromLTRB(5.0, 56.0, 19.0, 70.0)), + // SelectionRect(position: 5, bounds: Rect.fromLTRB(-23.0, 70.0, 19.0, 84.0)), + ]); + log.clear(); + + expect(scrollController.offset, 0); + + // Scrolling also triggers update. + scrollController.jumpTo(14); + await tester.pumpAndSettle(); + expect(log.single, const [ + SelectionRect(position: 0, bounds: Rect.fromLTRB(5.0, -14.0, 19.0, 0.0)), + SelectionRect(position: 1, bounds: Rect.fromLTRB(5.0, 0.0, 19.0, 14.0)), + SelectionRect(position: 2, bounds: Rect.fromLTRB(5.0, 14.0, 19.0, 28.0)), + SelectionRect(position: 3, bounds: Rect.fromLTRB(5.0, 28.0, 19.0, 42.0)), + SelectionRect(position: 4, bounds: Rect.fromLTRB(5.0, 42.0, 19.0, 56.0)), + // This line is skipped because it's below the bottom edge of the render + // object. + // SelectionRect(position: 5, bounds: Rect.fromLTRB(5.0, 56.0, 47.0, 70.0)), + ]); + log.clear(); // On web, we should rely on the browser's implementation of Scribble, so we will not send selection rects. }, skip: kIsWeb, variant: const TargetPlatformVariant({ TargetPlatform.iOS })); // [intended]