Use a separate `TextPainter` for intrinsics calculation in `RenderEditable`...

Use a separate `TextPainter` for intrinsics calculation in `RenderEditable` and `RenderParagraph` (#144577)

Use a dedicated `TextPainter` for intrinsic size calculation in `RenderEditable` and `RenderParagraph`.

This is an implementation detail so the change should be covered by existing tests.  Performance wise this shouldn't be significantly slower since SkParagraph [caches the result of slower operations across different paragraphs](https://github.com/google/skia/blob/9c62e7b382cf387195ef82895530c97ccceda690/modules/skparagraph/src/ParagraphCache.cpp#L254-L272). Existing benchmarks should be able to catch potential regressions (??).

The reason for making this change is to make sure that intrinsic size computations don't destroy text layout artifacts, so I can expose the text layout as a stream of immutable `TextLayout` objects, to signify other render objects that text-layout-dependent-cache (such as caches for `getBoxesForRange` which can be relatively slow to compute) should be invalidated and  `markNeedsPaint` needs to be called if the painting logic depended on text layout.
Without this change, the intrinsics/dry layout calculations will add additional events to the text layout stream, which violates the "dry"/non-destructive contract.

packages/flutter/lib/src/rendering/paragraph.dart

@@ -306,6 +306,27 @@ class RenderParagraph extends RenderBox with ContainerRenderObjectMixin<RenderBo
 
   final TextPainter _textPainter;
 
+  // Currently, computing min/max intrinsic width/height will destroy state
+  // inside the painter. Instead of calling _layout again to get back the correct
+  // state, use a separate TextPainter for intrinsics calculation.
+  //
+  // TODO(abarth): Make computing the min/max intrinsic width/height a
+  //  non-destructive operation.
+  TextPainter? _textIntrinsicsCache;
+  TextPainter get _textIntrinsics {
+    return (_textIntrinsicsCache ??= TextPainter())
+      ..text = _textPainter.text
+      ..textAlign = _textPainter.textAlign
+      ..textDirection = _textPainter.textDirection
+      ..textScaler = _textPainter.textScaler
+      ..maxLines = _textPainter.maxLines
+      ..ellipsis = _textPainter.ellipsis
+      ..locale = _textPainter.locale
+      ..strutStyle = _textPainter.strutStyle
+      ..textWidthBasis = _textPainter.textWidthBasis
+      ..textHeightBehavior = _textPainter.textHeightBehavior;
+  }
+
   List<AttributedString>? _cachedAttributedLabels;
 
   List<InlineSpanSemanticsInformation>? _cachedCombinedSemanticsInfos;
@@ -448,6 +469,7 @@ class RenderParagraph extends RenderBox with ContainerRenderObjectMixin<RenderBo
     _removeSelectionRegistrarSubscription();
     _disposeSelectableFragments();
     _textPainter.dispose();
+    _textIntrinsicsCache?.dispose();
     super.dispose();
   }
 
@@ -630,21 +652,17 @@ class RenderParagraph extends RenderBox with ContainerRenderObjectMixin<RenderBo
     return getOffsetForCaret(position, Rect.zero) + Offset(0, getFullHeightForCaret(position) ?? 0.0);
   }
 
-  List<ui.LineMetrics> _computeLineMetrics() {
-    return _textPainter.computeLineMetrics();
-  }
-
   @override
   double computeMinIntrinsicWidth(double height) {
     if (!_canComputeIntrinsics()) {
       return 0.0;
     }
-    _textPainter.setPlaceholderDimensions(layoutInlineChildren(
+    final List<PlaceholderDimensions> placeholderDimensions = layoutInlineChildren(
       double.infinity,
       (RenderBox child, BoxConstraints constraints) => Size(child.getMinIntrinsicWidth(double.infinity), 0.0),
-    ));
-    _layoutText(); // layout with infinite width.
-    return _textPainter.minIntrinsicWidth;
+    );
+    return (_textIntrinsics..setPlaceholderDimensions(placeholderDimensions)..layout())
+      .minIntrinsicWidth;
   }
 
   @override
@@ -652,23 +670,24 @@ class RenderParagraph extends RenderBox with ContainerRenderObjectMixin<RenderBo
     if (!_canComputeIntrinsics()) {
       return 0.0;
     }
-    _textPainter.setPlaceholderDimensions(layoutInlineChildren(
+    final List<PlaceholderDimensions> placeholderDimensions = layoutInlineChildren(
       double.infinity,
       // Height and baseline is irrelevant as all text will be laid
       // out in a single line. Therefore, using 0.0 as a dummy for the height.
       (RenderBox child, BoxConstraints constraints) => Size(child.getMaxIntrinsicWidth(double.infinity), 0.0),
-    ));
-    _layoutText(); // layout with infinite width.
-    return _textPainter.maxIntrinsicWidth;
+    );
+    return (_textIntrinsics..setPlaceholderDimensions(placeholderDimensions)..layout())
+      .maxIntrinsicWidth;
   }
 
   double _computeIntrinsicHeight(double width) {
     if (!_canComputeIntrinsics()) {
       return 0.0;
     }
-    _textPainter.setPlaceholderDimensions(layoutInlineChildren(width, ChildLayoutHelper.dryLayoutChild));
-    _layoutText(minWidth: width, maxWidth: width);
-    return _textPainter.height;
+    return (_textIntrinsics
+      ..setPlaceholderDimensions(layoutInlineChildren(width, ChildLayoutHelper.dryLayoutChild))
+      ..layout(minWidth: width, maxWidth: _adjustMaxWidth(width)))
+      .height;
   }
 
   @override
@@ -761,14 +780,6 @@ class RenderParagraph extends RenderBox with ContainerRenderObjectMixin<RenderBo
   @visibleForTesting
   bool get debugHasOverflowShader => _overflowShader != null;
 
-  void _layoutText({ double minWidth = 0.0, double maxWidth = double.infinity }) {
-    final bool widthMatters = softWrap || overflow == TextOverflow.ellipsis;
-    _textPainter.layout(
-      minWidth: minWidth,
-      maxWidth: widthMatters ? maxWidth : double.infinity,
-    );
-  }
-
   @override
   void systemFontsDidChange() {
     super.systemFontsDidChange();
@@ -782,9 +793,13 @@ class RenderParagraph extends RenderBox with ContainerRenderObjectMixin<RenderBo
   // restored to the original values before final layout and painting.
   List<PlaceholderDimensions>? _placeholderDimensions;
 
+  double _adjustMaxWidth(double maxWidth) {
+    return softWrap || overflow == TextOverflow.ellipsis ? maxWidth : double.infinity;
+  }
   void _layoutTextWithConstraints(BoxConstraints constraints) {
-    _textPainter.setPlaceholderDimensions(_placeholderDimensions);
-    _layoutText(minWidth: constraints.minWidth, maxWidth: constraints.maxWidth);
+    _textPainter
+      ..setPlaceholderDimensions(_placeholderDimensions)
+      ..layout(minWidth: constraints.minWidth, maxWidth: _adjustMaxWidth(constraints.maxWidth));
   }
 
   @override
@@ -796,9 +811,11 @@ class RenderParagraph extends RenderBox with ContainerRenderObjectMixin<RenderBo
       ));
       return Size.zero;
     }
-    _textPainter.setPlaceholderDimensions(layoutInlineChildren(constraints.maxWidth, ChildLayoutHelper.dryLayoutChild));
-    _layoutText(minWidth: constraints.minWidth, maxWidth: constraints.maxWidth);
-    return constraints.constrain(_textPainter.size);
+    final Size size = (_textIntrinsics
+     ..setPlaceholderDimensions(layoutInlineChildren(constraints.maxWidth, ChildLayoutHelper.dryLayoutChild))
+     ..layout(minWidth: constraints.minWidth, maxWidth: _adjustMaxWidth(constraints.maxWidth)))
+     .size;
+    return constraints.constrain(size);
   }
 
   @override
@@ -876,18 +893,10 @@ class RenderParagraph extends RenderBox with ContainerRenderObjectMixin<RenderBo
 
   @override
   void paint(PaintingContext context, Offset offset) {
-    // Ideally we could compute the min/max intrinsic width/height with a
-    // non-destructive operation. However, currently, computing these values
-    // will destroy state inside the painter. If that happens, we need to get
-    // back the correct state by calling _layout again.
-    //
-    // TODO(abarth): Make computing the min/max intrinsic width/height a
-    //  non-destructive operation.
-    //
-    // If you remove this call, make sure that changing the textAlign still
-    // works properly.
+    // Text alignment only triggers repaint so it's possible the text layout has
+    // been invalidated but performLayout wasn't called at this point. Make sure
+    // the TextPainter has a valid layout.
     _layoutTextWithConstraints(constraints);
-
     assert(() {
       if (debugRepaintTextRainbowEnabled) {
         final Paint paint = Paint()
@@ -1919,7 +1928,7 @@ class _SelectableFragment with Selectable, Diagnosticable, ChangeNotifier implem
   }
 
   MapEntry<TextPosition, SelectionResult> _handleVerticalMovement(TextPosition position, {required double horizontalBaselineInParagraphCoordinates, required bool below}) {
-    final List<ui.LineMetrics> lines = paragraph._computeLineMetrics();
+    final List<ui.LineMetrics> lines = paragraph._textPainter.computeLineMetrics();
     final Offset offset = paragraph.getOffsetForCaret(position, Rect.zero);
     int currentLine = lines.length - 1;
     for (final ui.LineMetrics lineMetrics in lines) {

packages/flutter/test/rendering/editable_intrinsics_test.dart

+// Copyright 2014 The Flutter 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/foundation.dart';
+import 'package:flutter/rendering.dart';
+import 'package:flutter/services.dart';
+import 'package:flutter_test/flutter_test.dart';
+
+void main() {
+  final TextSelectionDelegate delegate = _FakeEditableTextState();
+
+  test('editable intrinsics', () {
+    final RenderEditable editable = RenderEditable(
+      text: const TextSpan(
+        style: TextStyle(height: 1.0, fontSize: 10.0),
+        text: '12345',
+      ),
+      startHandleLayerLink: LayerLink(),
+      endHandleLayerLink: LayerLink(),
+      textDirection: TextDirection.ltr,
+      locale: const Locale('ja', 'JP'),
+      offset: ViewportOffset.zero(),
+      textSelectionDelegate: delegate,
+    );
+    expect(editable.getMinIntrinsicWidth(double.infinity), 50.0);
+    // The width includes the width of the cursor (1.0).
+    expect(editable.getMaxIntrinsicWidth(double.infinity), 52.0);
+    expect(editable.getMinIntrinsicHeight(double.infinity), 10.0);
+    expect(editable.getMaxIntrinsicHeight(double.infinity), 10.0);
+
+    expect(
+      editable.toStringDeep(minLevel: DiagnosticLevel.info),
+      equalsIgnoringHashCodes(
+        'RenderEditable#00000 NEEDS-LAYOUT NEEDS-PAINT NEEDS-COMPOSITING-BITS-UPDATE DETACHED\n'
+        ' │ parentData: MISSING\n'
+        ' │ constraints: MISSING\n'
+        ' │ size: MISSING\n'
+        ' │ cursorColor: null\n'
+        ' │ showCursor: ValueNotifier<bool>#00000(false)\n'
+        ' │ maxLines: 1\n'
+        ' │ minLines: null\n'
+        ' │ selectionColor: null\n'
+        ' │ locale: ja_JP\n'
+        ' │ selection: null\n'
+        ' │ offset: _FixedViewportOffset#00000(offset: 0.0)\n'
+        ' ╘═╦══ text ═══\n'
+        '   ║ TextSpan:\n'
+        '   ║   inherit: true\n'
+        '   ║   size: 10.0\n'
+        '   ║   height: 1.0x\n'
+        '   ║   "12345"\n'
+        '   ╚═══════════\n',
+      ),
+    );
+  });
+
+  test('textScaler affects intrinsics', () {
+    final RenderEditable editable = RenderEditable(
+      text: const TextSpan(
+        style: TextStyle(fontSize: 10),
+        text: 'Hello World',
+      ),
+      textDirection: TextDirection.ltr,
+      startHandleLayerLink: LayerLink(),
+      endHandleLayerLink: LayerLink(),
+      offset: ViewportOffset.zero(),
+      textSelectionDelegate: delegate,
+    );
+
+    expect(editable.getMaxIntrinsicWidth(double.infinity), 110 + 2);
+
+    editable.textScaler = const TextScaler.linear(2);
+    expect(editable.getMaxIntrinsicWidth(double.infinity), 220 + 2);
+  });
+
+  test('maxLines affects intrinsics', () {
+    final RenderEditable editable = RenderEditable(
+      text: TextSpan(
+        style: const TextStyle(fontSize: 10),
+        text: List<String>.filled(5, 'A').join('\n'),
+      ),
+      textDirection: TextDirection.ltr,
+      startHandleLayerLink: LayerLink(),
+      endHandleLayerLink: LayerLink(),
+      offset: ViewportOffset.zero(),
+      textSelectionDelegate: delegate,
+      maxLines: null,
+    );
+
+    expect(editable.getMaxIntrinsicHeight(double.infinity), 50);
+
+    editable.maxLines = 1;
+    expect(editable.getMaxIntrinsicHeight(double.infinity), 10);
+  });
+
+  test('strutStyle affects intrinsics', () {
+    final RenderEditable editable = RenderEditable(
+      text: const TextSpan(
+        style: TextStyle(fontSize: 10),
+        text: 'Hello World',
+      ),
+      textDirection: TextDirection.ltr,
+      startHandleLayerLink: LayerLink(),
+      endHandleLayerLink: LayerLink(),
+      offset: ViewportOffset.zero(),
+      textSelectionDelegate: delegate,
+    );
+
+    expect(editable.getMaxIntrinsicHeight(double.infinity), 10);
+
+    editable.strutStyle = const StrutStyle(fontSize: 100, forceStrutHeight: true);
+    expect(editable.getMaxIntrinsicHeight(double.infinity), 100);
+  }, skip: kIsWeb && !isCanvasKit); // [intended] strut spport for HTML renderer https://github.com/flutter/flutter/issues/32243.
+}
+
+class _FakeEditableTextState with TextSelectionDelegate {
+  @override
+  TextEditingValue textEditingValue = TextEditingValue.empty;
+
+  TextSelection? selection;
+
+  @override
+  void hideToolbar([bool hideHandles = true]) { }
+
+  @override
+  void userUpdateTextEditingValue(TextEditingValue value, SelectionChangedCause cause) {
+    selection = value.selection;
+  }
+
+  @override
+  void bringIntoView(TextPosition position) { }
+
+  @override
+  void cutSelection(SelectionChangedCause cause) { }
+
+  @override
+  Future<void> pasteText(SelectionChangedCause cause) {
+    return Future<void>.value();
+  }
+
+  @override
+  void selectAll(SelectionChangedCause cause) { }
+
+  @override
+  void copySelection(SelectionChangedCause cause) { }
+}

packages/flutter/test/rendering/editable_test.dart

@@ -201,52 +201,6 @@ void main() {
     expect(leaderLayers.single.offset, endpoint + paintOffset, reason: 'offset should respect paintOffset');
   });
 
-  test('editable intrinsics', () {
-    final TextSelectionDelegate delegate = _FakeEditableTextState();
-    final RenderEditable editable = RenderEditable(
-      text: const TextSpan(
-        style: TextStyle(height: 1.0, fontSize: 10.0),
-        text: '12345',
-      ),
-      startHandleLayerLink: LayerLink(),
-      endHandleLayerLink: LayerLink(),
-      textDirection: TextDirection.ltr,
-      locale: const Locale('ja', 'JP'),
-      offset: ViewportOffset.zero(),
-      textSelectionDelegate: delegate,
-    );
-    expect(editable.getMinIntrinsicWidth(double.infinity), 50.0);
-    // The width includes the width of the cursor (1.0).
-    expect(editable.getMaxIntrinsicWidth(double.infinity), 52.0);
-    expect(editable.getMinIntrinsicHeight(double.infinity), 10.0);
-    expect(editable.getMaxIntrinsicHeight(double.infinity), 10.0);
-
-    expect(
-      editable.toStringDeep(minLevel: DiagnosticLevel.info),
-      equalsIgnoringHashCodes(
-        'RenderEditable#00000 NEEDS-LAYOUT NEEDS-PAINT NEEDS-COMPOSITING-BITS-UPDATE DETACHED\n'
-        ' │ parentData: MISSING\n'
-        ' │ constraints: MISSING\n'
-        ' │ size: MISSING\n'
-        ' │ cursorColor: null\n'
-        ' │ showCursor: ValueNotifier<bool>#00000(false)\n'
-        ' │ maxLines: 1\n'
-        ' │ minLines: null\n'
-        ' │ selectionColor: null\n'
-        ' │ locale: ja_JP\n'
-        ' │ selection: null\n'
-        ' │ offset: _FixedViewportOffset#00000(offset: 0.0)\n'
-        ' ╘═╦══ text ═══\n'
-        '   ║ TextSpan:\n'
-        '   ║   inherit: true\n'
-        '   ║   size: 10.0\n'
-        '   ║   height: 1.0x\n'
-        '   ║   "12345"\n'
-        '   ╚═══════════\n',
-      ),
-    );
-  });
-
   // Test that clipping will be used even when the text fits within the visible
   // region if the start position of the text is offset (e.g. during scrolling
   // animation).

packages/flutter/test/rendering/paragraph_intrinsics_test.dart

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'package:flutter/foundation.dart';
 import 'package:flutter/rendering.dart';
 import 'package:flutter_test/flutter_test.dart';
 
@@ -66,4 +67,49 @@ void main() {
     expect(testBlock.getMinIntrinsicHeight(0.0), equals(manyLinesTextHeight));
     expect(testBlock.getMaxIntrinsicHeight(0.0), equals(manyLinesTextHeight));
   });
+
+  test('textScaler affects intrinsics', () {
+    final RenderParagraph paragraph = RenderParagraph(
+      const TextSpan(
+        style: TextStyle(fontSize: 10),
+        text: 'Hello World',
+      ),
+      textDirection: TextDirection.ltr,
+    );
+
+    expect(paragraph.getMaxIntrinsicWidth(double.infinity), 110);
+
+    paragraph.textScaler = const TextScaler.linear(2);
+    expect(paragraph.getMaxIntrinsicWidth(double.infinity), 220);
+  });
+
+  test('maxLines affects intrinsics', () {
+    final RenderParagraph paragraph = RenderParagraph(
+      TextSpan(
+        style: const TextStyle(fontSize: 10),
+        text: List<String>.filled(5, 'A').join('\n'),
+      ),
+      textDirection: TextDirection.ltr,
+    );
+
+    expect(paragraph.getMaxIntrinsicHeight(double.infinity), 50);
+
+    paragraph.maxLines = 1;
+    expect(paragraph.getMaxIntrinsicHeight(double.infinity), 10);
+  });
+
+  test('strutStyle affects intrinsics', () {
+    final RenderParagraph paragraph = RenderParagraph(
+      const TextSpan(
+        style: TextStyle(fontSize: 10),
+        text: 'Hello World',
+      ),
+      textDirection: TextDirection.ltr,
+    );
+
+    expect(paragraph.getMaxIntrinsicHeight(double.infinity), 10);
+
+    paragraph.strutStyle = const StrutStyle(fontSize: 100, forceStrutHeight: true);
+    expect(paragraph.getMaxIntrinsicHeight(double.infinity), 100);
+  }, skip: kIsWeb && !isCanvasKit); // [intended] strut spport for HTML renderer https://github.com/flutter/flutter/issues/32243.
 }

packages/flutter/test/rendering/paragraph_test.dart

@@ -375,6 +375,27 @@ void main() {
     expect(paragraph.size.height, 30.0);
   }, skip: isBrowser); // https://github.com/flutter/flutter/issues/61018
 
+  test('textAlign triggers TextPainter relayout in the paint method', () {
+    final RenderParagraph paragraph = RenderParagraph(
+      const TextSpan(text: 'A', style: TextStyle(fontSize: 10.0)),
+      textDirection: TextDirection.ltr,
+      textAlign: TextAlign.left,
+    );
+
+    Rect getRectForA() => paragraph.getBoxesForSelection(const TextSelection(baseOffset: 0, extentOffset: 1)).single.toRect();
+
+    layout(paragraph, constraints: const BoxConstraints.tightFor(width: 100.0));
+
+    expect(getRectForA(), const Rect.fromLTWH(0, 0, 10, 10));
+
+    paragraph.textAlign = TextAlign.right;
+    expect(paragraph.debugNeedsLayout, isFalse);
+    expect(paragraph.debugNeedsPaint, isTrue);
+
+    paragraph.paint(MockPaintingContext(), Offset.zero);
+    expect(getRectForA(), const Rect.fromLTWH(90, 0, 10, 10));
+  });
+
   group('didExceedMaxLines', () {
     RenderParagraph createRenderParagraph({
       int? maxLines,