From 1148a2a8bac5e338836568ac090e4bb99b4aba5c Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Mon, 30 Jan 2023 17:39:09 -0800 Subject: [PATCH] Migrate EditableTextState from addPostFrameCallbacks to compositionCallbacks (#119359) * PostFrameCallbacks -> compositionCallbacks * review * review --- packages/flutter/lib/src/rendering/layer.dart | 4 +- .../lib/src/widgets/editable_text.dart | 318 ++++++++++-------- .../test/rendering/recording_canvas.dart | 3 + .../test/widgets/editable_text_test.dart | 60 ++++ 4 files changed, 251 insertions(+), 134 deletions(-) diff --git a/packages/flutter/lib/src/rendering/layer.dart b/packages/flutter/lib/src/rendering/layer.dart index 9b98c407d4..bad15e3eb8 100644 --- a/packages/flutter/lib/src/rendering/layer.dart +++ b/packages/flutter/lib/src/rendering/layer.dart @@ -168,9 +168,7 @@ abstract class Layer extends AbstractNode with DiagnosticableTreeMixin { assert(delta != 0); _compositionCallbackCount += delta; assert(_compositionCallbackCount >= 0); - if (parent != null) { - parent!._updateSubtreeCompositionObserverCount(delta); - } + parent?._updateSubtreeCompositionObserverCount(delta); } void _fireCompositionCallbacks({required bool includeChildren}) { diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart index 3b7197609e..56633e4d1d 100644 --- a/packages/flutter/lib/src/widgets/editable_text.dart +++ b/packages/flutter/lib/src/widgets/editable_text.dart @@ -84,6 +84,51 @@ const Duration _kCursorBlinkHalfPeriod = Duration(milliseconds: 500); // is shown in an obscured text field. const int _kObscureShowLatestCharCursorTicks = 3; +class _CompositionCallback extends SingleChildRenderObjectWidget { + const _CompositionCallback({ required this.compositeCallback, required this.enabled, super.child }); + final CompositionCallback compositeCallback; + final bool enabled; + + @override + RenderObject createRenderObject(BuildContext context) { + return _RenderCompositionCallback(compositeCallback, enabled); + } + @override + void updateRenderObject(BuildContext context, _RenderCompositionCallback renderObject) { + super.updateRenderObject(context, renderObject); + // _EditableTextState always uses the same callback. + assert(renderObject.compositeCallback == compositeCallback); + renderObject.enabled = enabled; + } +} + +class _RenderCompositionCallback extends RenderProxyBox { + _RenderCompositionCallback(this.compositeCallback, this._enabled); + + final CompositionCallback compositeCallback; + VoidCallback? _cancelCallback; + + bool get enabled => _enabled; + bool _enabled = false; + set enabled(bool newValue) { + _enabled = newValue; + if (!newValue) { + _cancelCallback?.call(); + _cancelCallback = null; + } else if (_cancelCallback == null) { + markNeedsPaint(); + } + } + + @override + void paint(PaintingContext context, ui.Offset offset) { + if (enabled) { + _cancelCallback ??= context.addCompositionCallback(compositeCallback); + } + super.paint(context, offset); + } +} + /// A controller for an editable text field. /// /// Whenever the user modifies a text field with an associated @@ -2970,8 +3015,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien ? currentAutofillScope!.attach(this, _effectiveAutofillClient.textInputConfiguration) : TextInput.attach(this, _effectiveAutofillClient.textInputConfiguration); _updateSizeAndTransform(); - _updateComposingRectIfNeeded(); - _updateCaretRectIfNeeded(); + _schedulePeriodicPostFrameCallbacks(); final TextStyle style = widget.style; _textInputConnection! ..setStyle( @@ -2999,6 +3043,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien _textInputConnection!.close(); _textInputConnection = null; _lastKnownRemoteTextEditingValue = null; + removeTextPlaceholder(); } } @@ -3523,6 +3568,33 @@ class EditableTextState extends State with AutomaticKeepAliveClien updateKeepAlive(); } + void _compositeCallback(Layer layer) { + // The callback can be invoked when the layer is detached. + // The input connection can be closed by the platform in which case this + // widget doesn't rebuild. + if (!renderEditable.attached || !_hasInputConnection) { + return; + } + assert(mounted); + assert((context as Element).debugIsActive); + _updateSizeAndTransform(); + } + + void _updateSizeAndTransform() { + final Size size = renderEditable.size; + final Matrix4 transform = renderEditable.getTransformTo(null); + _textInputConnection!.setEditableSizeAndTransform(size, transform); + } + + void _schedulePeriodicPostFrameCallbacks([Duration? duration]) { + if (!_hasInputConnection) { + return; + } + _updateSelectionRects(); + _updateComposingRectIfNeeded(); + _updateCaretRectIfNeeded(); + SchedulerBinding.instance.addPostFrameCallback(_schedulePeriodicPostFrameCallbacks); + } _ScribbleCacheKey? _scribbleCacheKey; void _updateSelectionRects({bool force = false}) { @@ -3585,18 +3657,6 @@ class EditableTextState extends State with AutomaticKeepAliveClien _textInputConnection!.setSelectionRects(rects); } - void _updateSizeAndTransform() { - if (_hasInputConnection) { - final Size size = renderEditable.size; - final Matrix4 transform = renderEditable.getTransformTo(null); - _textInputConnection!.setEditableSizeAndTransform(size, transform); - _updateSelectionRects(); - SchedulerBinding.instance.addPostFrameCallback((Duration _) => _updateSizeAndTransform()); - } else if (_placeholderLocation != -1) { - removeTextPlaceholder(); - } - } - // Sends the current composing rect to the iOS text input plugin via the text // input channel. We need to keep sending the information even if no text is // currently marked, as the information usually lags behind. The text input @@ -3604,42 +3664,34 @@ class EditableTextState extends State with AutomaticKeepAliveClien // when the composing rect info didn't arrive in time. void _updateComposingRectIfNeeded() { final TextRange composingRange = _value.composing; - if (_hasInputConnection) { - assert(mounted); - Rect? composingRect = renderEditable.getRectForComposingRange(composingRange); - // Send the caret location instead if there's no marked text yet. - if (composingRect == null) { - assert(!composingRange.isValid || composingRange.isCollapsed); - final int offset = composingRange.isValid ? composingRange.start : 0; - composingRect = renderEditable.getLocalRectForCaret(TextPosition(offset: offset)); - } - _textInputConnection!.setComposingRect(composingRect); - SchedulerBinding.instance.addPostFrameCallback((Duration _) => _updateComposingRectIfNeeded()); + assert(mounted); + Rect? composingRect = renderEditable.getRectForComposingRange(composingRange); + // Send the caret location instead if there's no marked text yet. + if (composingRect == null) { + assert(!composingRange.isValid || composingRange.isCollapsed); + final int offset = composingRange.isValid ? composingRange.start : 0; + composingRect = renderEditable.getLocalRectForCaret(TextPosition(offset: offset)); } + _textInputConnection!.setComposingRect(composingRect); } void _updateCaretRectIfNeeded() { - if (_hasInputConnection) { - if (renderEditable.selection != null && renderEditable.selection!.isValid && - renderEditable.selection!.isCollapsed) { - final TextPosition currentTextPosition = TextPosition(offset: renderEditable.selection!.baseOffset); - final Rect caretRect = renderEditable.getLocalRectForCaret(currentTextPosition); - _textInputConnection!.setCaretRect(caretRect); - } - SchedulerBinding.instance.addPostFrameCallback((Duration _) => _updateCaretRectIfNeeded()); + final TextSelection? selection = renderEditable.selection; + if (selection == null || !selection.isValid || !selection.isCollapsed) { + return; } + final TextPosition currentTextPosition = TextPosition(offset: selection.baseOffset); + final Rect caretRect = renderEditable.getLocalRectForCaret(currentTextPosition); + _textInputConnection!.setCaretRect(caretRect); } - TextDirection get _textDirection { - final TextDirection result = widget.textDirection ?? Directionality.of(context); - return result; - } + TextDirection get _textDirection => widget.textDirection ?? Directionality.of(context); /// The renderer for this widget's descendant. /// /// This property is typically used to notify the renderer of input gestures /// when [RenderEditable.ignorePointer] is true. - RenderEditable get renderEditable => _editableKey.currentContext!.findRenderObject()! as RenderEditable; + late final RenderEditable renderEditable = _editableKey.currentContext!.findRenderObject()! as RenderEditable; @override TextEditingValue get textEditingValue => _value; @@ -3812,7 +3864,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien @override void removeTextPlaceholder() { - if (!widget.scribbleEnabled) { + if (!widget.scribbleEnabled || _placeholderLocation == -1) { return; } @@ -4243,100 +4295,104 @@ class EditableTextState extends State with AutomaticKeepAliveClien super.build(context); // See AutomaticKeepAliveClientMixin. final TextSelectionControls? controls = widget.selectionControls; - return TextFieldTapRegion( - onTapOutside: widget.onTapOutside ?? _defaultOnTapOutside, - debugLabel: kReleaseMode ? null : 'EditableText', - child: MouseRegion( - cursor: widget.mouseCursor ?? SystemMouseCursors.text, - child: Actions( - actions: _actions, - child: _TextEditingHistory( - controller: widget.controller, - onTriggered: (TextEditingValue value) { - userUpdateTextEditingValue(value, SelectionChangedCause.keyboard); - }, - child: Focus( - focusNode: widget.focusNode, - includeSemantics: false, - debugLabel: kReleaseMode ? null : 'EditableText', - child: Scrollable( - key: _scrollableKey, - excludeFromSemantics: true, - axisDirection: _isMultiline ? AxisDirection.down : AxisDirection.right, - controller: _scrollController, - physics: widget.scrollPhysics, - dragStartBehavior: widget.dragStartBehavior, - restorationId: widget.restorationId, - // If a ScrollBehavior is not provided, only apply scrollbars when - // multiline. The overscroll indicator should not be applied in - // either case, glowing or stretching. - scrollBehavior: widget.scrollBehavior ?? ScrollConfiguration.of(context).copyWith( - scrollbars: _isMultiline, - overscroll: false, - ), - viewportBuilder: (BuildContext context, ViewportOffset offset) { - return CompositedTransformTarget( - link: _toolbarLayerLink, - child: Semantics( - onCopy: _semanticsOnCopy(controls), - onCut: _semanticsOnCut(controls), - onPaste: _semanticsOnPaste(controls), - child: _ScribbleFocusable( - focusNode: widget.focusNode, - editableKey: _editableKey, - enabled: widget.scribbleEnabled, - updateSelectionRects: () { - _openInputConnection(); - _updateSelectionRects(force: true); - }, - child: _Editable( - key: _editableKey, - startHandleLayerLink: _startHandleLayerLink, - endHandleLayerLink: _endHandleLayerLink, - inlineSpan: buildTextSpan(), - value: _value, - cursorColor: _cursorColor, - backgroundCursorColor: widget.backgroundCursorColor, - showCursor: EditableText.debugDeterministicCursor - ? ValueNotifier(widget.showCursor) - : _cursorVisibilityNotifier, - forceLine: widget.forceLine, - readOnly: widget.readOnly, - hasFocus: _hasFocus, - maxLines: widget.maxLines, - minLines: widget.minLines, - expands: widget.expands, - strutStyle: widget.strutStyle, - selectionColor: widget.selectionColor, - textScaleFactor: widget.textScaleFactor ?? MediaQuery.textScaleFactorOf(context), - textAlign: widget.textAlign, - textDirection: _textDirection, - locale: widget.locale, - textHeightBehavior: widget.textHeightBehavior ?? DefaultTextHeightBehavior.maybeOf(context), - textWidthBasis: widget.textWidthBasis, - obscuringCharacter: widget.obscuringCharacter, - obscureText: widget.obscureText, - offset: offset, - onCaretChanged: _handleCaretChanged, - rendererIgnoresPointer: widget.rendererIgnoresPointer, - cursorWidth: widget.cursorWidth, - cursorHeight: widget.cursorHeight, - cursorRadius: widget.cursorRadius, - cursorOffset: widget.cursorOffset ?? Offset.zero, - selectionHeightStyle: widget.selectionHeightStyle, - selectionWidthStyle: widget.selectionWidthStyle, - paintCursorAboveText: widget.paintCursorAboveText, - enableInteractiveSelection: widget._userSelectionEnabled, - textSelectionDelegate: this, - devicePixelRatio: _devicePixelRatio, - promptRectRange: _currentPromptRectRange, - promptRectColor: widget.autocorrectionTextRectColor, - clipBehavior: widget.clipBehavior, + return _CompositionCallback( + compositeCallback: _compositeCallback, + enabled: _hasInputConnection, + child: TextFieldTapRegion( + onTapOutside: widget.onTapOutside ?? _defaultOnTapOutside, + debugLabel: kReleaseMode ? null : 'EditableText', + child: MouseRegion( + cursor: widget.mouseCursor ?? SystemMouseCursors.text, + child: Actions( + actions: _actions, + child: _TextEditingHistory( + controller: widget.controller, + onTriggered: (TextEditingValue value) { + userUpdateTextEditingValue(value, SelectionChangedCause.keyboard); + }, + child: Focus( + focusNode: widget.focusNode, + includeSemantics: false, + debugLabel: kReleaseMode ? null : 'EditableText', + child: Scrollable( + key: _scrollableKey, + excludeFromSemantics: true, + axisDirection: _isMultiline ? AxisDirection.down : AxisDirection.right, + controller: _scrollController, + physics: widget.scrollPhysics, + dragStartBehavior: widget.dragStartBehavior, + restorationId: widget.restorationId, + // If a ScrollBehavior is not provided, only apply scrollbars when + // multiline. The overscroll indicator should not be applied in + // either case, glowing or stretching. + scrollBehavior: widget.scrollBehavior ?? ScrollConfiguration.of(context).copyWith( + scrollbars: _isMultiline, + overscroll: false, + ), + viewportBuilder: (BuildContext context, ViewportOffset offset) { + return CompositedTransformTarget( + link: _toolbarLayerLink, + child: Semantics( + onCopy: _semanticsOnCopy(controls), + onCut: _semanticsOnCut(controls), + onPaste: _semanticsOnPaste(controls), + child: _ScribbleFocusable( + focusNode: widget.focusNode, + editableKey: _editableKey, + enabled: widget.scribbleEnabled, + updateSelectionRects: () { + _openInputConnection(); + _updateSelectionRects(force: true); + }, + child: _Editable( + key: _editableKey, + startHandleLayerLink: _startHandleLayerLink, + endHandleLayerLink: _endHandleLayerLink, + inlineSpan: buildTextSpan(), + value: _value, + cursorColor: _cursorColor, + backgroundCursorColor: widget.backgroundCursorColor, + showCursor: EditableText.debugDeterministicCursor + ? ValueNotifier(widget.showCursor) + : _cursorVisibilityNotifier, + forceLine: widget.forceLine, + readOnly: widget.readOnly, + hasFocus: _hasFocus, + maxLines: widget.maxLines, + minLines: widget.minLines, + expands: widget.expands, + strutStyle: widget.strutStyle, + selectionColor: widget.selectionColor, + textScaleFactor: widget.textScaleFactor ?? MediaQuery.textScaleFactorOf(context), + textAlign: widget.textAlign, + textDirection: _textDirection, + locale: widget.locale, + textHeightBehavior: widget.textHeightBehavior ?? DefaultTextHeightBehavior.maybeOf(context), + textWidthBasis: widget.textWidthBasis, + obscuringCharacter: widget.obscuringCharacter, + obscureText: widget.obscureText, + offset: offset, + onCaretChanged: _handleCaretChanged, + rendererIgnoresPointer: widget.rendererIgnoresPointer, + cursorWidth: widget.cursorWidth, + cursorHeight: widget.cursorHeight, + cursorRadius: widget.cursorRadius, + cursorOffset: widget.cursorOffset ?? Offset.zero, + selectionHeightStyle: widget.selectionHeightStyle, + selectionWidthStyle: widget.selectionWidthStyle, + paintCursorAboveText: widget.paintCursorAboveText, + enableInteractiveSelection: widget._userSelectionEnabled, + textSelectionDelegate: this, + devicePixelRatio: _devicePixelRatio, + promptRectRange: _currentPromptRectRange, + promptRectColor: widget.autocorrectionTextRectColor, + clipBehavior: widget.clipBehavior, + ), ), ), - ), - ); - }, + ); + }, + ), ), ), ), diff --git a/packages/flutter/test/rendering/recording_canvas.dart b/packages/flutter/test/rendering/recording_canvas.dart index b46cf35c6b..62031fd07f 100644 --- a/packages/flutter/test/rendering/recording_canvas.dart +++ b/packages/flutter/test/rendering/recording_canvas.dart @@ -179,6 +179,9 @@ class TestRecordingPaintingContext extends ClipContext implements PaintingContex painter(this, offset); } + @override + VoidCallback addCompositionCallback(CompositionCallback callback) => () {}; + @override void noSuchMethod(Invocation invocation) { } } diff --git a/packages/flutter/test/widgets/editable_text_test.dart b/packages/flutter/test/widgets/editable_text_test.dart index 16cfe506ed..992df3cc68 100644 --- a/packages/flutter/test/widgets/editable_text_test.dart +++ b/packages/flutter/test/widgets/editable_text_test.dart @@ -14611,6 +14611,66 @@ testWidgets('Floating cursor ending with selection', (WidgetTester tester) async // Shouldn't crash. state.didChangeMetrics(); }); + + testWidgets('_CompositionCallback widget does not skip frames', (WidgetTester tester) async { + EditableText.debugDeterministicCursor = true; + final FocusNode focusNode = FocusNode(); + final TextEditingController controller = TextEditingController.fromValue( + const TextEditingValue(selection: TextSelection.collapsed(offset: 0)), + ); + + Offset offset = Offset.zero; + late StateSetter setState; + + await tester.pumpWidget( + MaterialApp( + home: StatefulBuilder( + builder: (BuildContext context, StateSetter stateSetter) { + setState = stateSetter; + return Transform.translate( + offset: offset, + // The EditableText is configured in a way that the it doesn't + // explicitly request repaint on focus change. + child: TickerMode( + enabled: false, + child: RepaintBoundary( + child: EditableText( + controller: controller, + focusNode: focusNode, + style: const TextStyle(), + showCursor: false, + cursorColor: Colors.blue, + backgroundCursorColor: Colors.grey, + ), + ), + ), + ); + } + ), + ), + ); + + focusNode.requestFocus(); + await tester.pump(); + tester.testTextInput.log.clear(); + + // The composition callback should be registered. To verify, change the + // parent layer's transform. + setState(() { offset = const Offset(42, 0); }); + await tester.pump(); + + expect( + tester.testTextInput.log, + contains( + matchesMethodCall( + 'TextInput.setEditableSizeAndTransform', + args: containsPair('transform', Matrix4.translationValues(offset.dx, offset.dy, 0).storage), + ), + ), + ); + + EditableText.debugDeterministicCursor = false; + }); } class UnsettableController extends TextEditingController {