[TextPainter] Don't invalidate layout cache for paint only changes (#89515)

packages/flutter/lib/src/painting/text_painter.dart

@@ -3,7 +3,7 @@
 // found in the LICENSE file.
 
 import 'dart:math' show min, max;
-import 'dart:ui' as ui show Paragraph, ParagraphBuilder, ParagraphConstraints, ParagraphStyle, PlaceholderAlignment, LineMetrics, TextHeightBehavior, BoxHeightStyle, BoxWidthStyle;
+import 'dart:ui' as ui show Paragraph, ParagraphBuilder, ParagraphConstraints, ParagraphStyle, PlaceholderAlignment, LineMetrics, TextHeightBehavior, TextStyle, BoxHeightStyle, BoxWidthStyle;
 
 import 'package:flutter/foundation.dart';
 import 'package:flutter/services.dart';
@@ -185,8 +185,18 @@ class TextPainter {
        _textWidthBasis = textWidthBasis,
        _textHeightBehavior = textHeightBehavior;
 
+  // _paragraph being null means the text needs layout because of style changes.
+  // Setting _paragraph to null invalidates all the layout cache.
+  //
+  // The TextPainter class should not aggressively invalidate the layout as long
+  // as `markNeedsLayout` is not called (i.e., the layout cache is still valid).
+  // See: https://github.com/flutter/flutter/issues/85108
   ui.Paragraph? _paragraph;
-  bool _needsLayout = true;
+  // Whether _paragraph contains outdated paint information and needs to be
+  // rebuilt before painting.
+  bool _rebuildParagraphForPaint = true;
+
+  bool get _debugNeedsLayout => _paragraph == null;
 
   /// Marks this text painter's layout information as dirty and removes cached
   /// information.
@@ -196,7 +206,6 @@ class TextPainter {
   /// in framework will automatically invoke this method.
   void markNeedsLayout() {
     _paragraph = null;
-    _needsLayout = true;
     _previousCaretPosition = null;
     _previousCaretPrototype = null;
   }
@@ -219,8 +228,21 @@ class TextPainter {
       return;
     if (_text?.style != value?.style)
       _layoutTemplate = null;
+
+    final RenderComparison comparison = value == null
+      ? RenderComparison.layout
+      : _text?.compareTo(value) ?? RenderComparison.layout;
+
     _text = value;
+
+    if (comparison.index >= RenderComparison.layout.index) {
       markNeedsLayout();
+    } else if (comparison.index >= RenderComparison.paint.index) {
+      // Don't clear the _paragraph instance variable just yet. It still
+      // contains valid layout information.
+      _rebuildParagraphForPaint = true;
+    }
+    // Neither relayout or repaint is needed.
   }
 
   /// How the text should be aligned horizontally.
@@ -378,8 +400,6 @@ class TextPainter {
     markNeedsLayout();
   }
 
-  ui.Paragraph? _layoutTemplate;
-
   /// An ordered list of [TextBox]es that bound the positions of the placeholders
   /// in the paragraph.
   ///
@@ -454,6 +474,18 @@ class TextPainter {
     );
   }
 
+  ui.Paragraph? _layoutTemplate;
+  ui.Paragraph _createLayoutTemplate() {
+    final ui.ParagraphBuilder builder = ui.ParagraphBuilder(
+      _createParagraphStyle(TextDirection.rtl),
+    ); // direction doesn't matter, text is just a space
+    final ui.TextStyle? textStyle = text?.style?.getTextStyle(textScaleFactor: textScaleFactor);
+    if (textStyle != null)
+      builder.pushStyle(textStyle);
+    builder.addText(' ');
+    return builder.build()
+      ..layout(const ui.ParagraphConstraints(width: double.infinity));
+  }
   /// The height of a space in [text] in logical pixels.
   ///
   /// Not every line of text in [text] will have this height, but this height
@@ -466,19 +498,7 @@ class TextPainter {
   /// that contribute to the [preferredLineHeight]. If [text] is null or if it
   /// specifies no styles, the default [TextStyle] values are used (a 10 pixel
   /// sans-serif font).
-  double get preferredLineHeight {
-    if (_layoutTemplate == null) {
-      final ui.ParagraphBuilder builder = ui.ParagraphBuilder(
-        _createParagraphStyle(TextDirection.rtl),
-      ); // direction doesn't matter, text is just a space
-      if (text?.style != null)
-        builder.pushStyle(text!.style!.getTextStyle(textScaleFactor: textScaleFactor));
-      builder.addText(' ');
-      _layoutTemplate = builder.build()
-        ..layout(const ui.ParagraphConstraints(width: double.infinity));
-    }
-    return _layoutTemplate!.height;
-  }
+  double get preferredLineHeight => (_layoutTemplate ??= _createLayoutTemplate()).height;
 
   // Unfortunately, using full precision floating point here causes bad layouts
   // because floating point math isn't associative. If we add and subtract
@@ -496,7 +516,7 @@ class TextPainter {
   ///
   /// Valid only after [layout] has been called.
   double get minIntrinsicWidth {
-    assert(!_needsLayout);
+    assert(!_debugNeedsLayout);
     return _applyFloatingPointHack(_paragraph!.minIntrinsicWidth);
   }
 
@@ -504,7 +524,7 @@ class TextPainter {
   ///
   /// Valid only after [layout] has been called.
   double get maxIntrinsicWidth {
-    assert(!_needsLayout);
+    assert(!_debugNeedsLayout);
     return _applyFloatingPointHack(_paragraph!.maxIntrinsicWidth);
   }
 
@@ -512,7 +532,7 @@ class TextPainter {
   ///
   /// Valid only after [layout] has been called.
   double get width {
-    assert(!_needsLayout);
+    assert(!_debugNeedsLayout);
     return _applyFloatingPointHack(
       textWidthBasis == TextWidthBasis.longestLine ? _paragraph!.longestLine : _paragraph!.width,
     );
@@ -522,7 +542,7 @@ class TextPainter {
   ///
   /// Valid only after [layout] has been called.
   double get height {
-    assert(!_needsLayout);
+    assert(!_debugNeedsLayout);
     return _applyFloatingPointHack(_paragraph!.height);
   }
 
@@ -530,7 +550,7 @@ class TextPainter {
   ///
   /// Valid only after [layout] has been called.
   Size get size {
-    assert(!_needsLayout);
+    assert(!_debugNeedsLayout);
     return Size(width, height);
   }
 
@@ -539,7 +559,7 @@ class TextPainter {
   ///
   /// Valid only after [layout] has been called.
   double computeDistanceToActualBaseline(TextBaseline baseline) {
-    assert(!_needsLayout);
+    assert(!_debugNeedsLayout);
     assert(baseline != null);
     switch (baseline) {
       case TextBaseline.alphabetic:
@@ -561,38 +581,29 @@ class TextPainter {
   ///
   /// Valid only after [layout] has been called.
   bool get didExceedMaxLines {
-    assert(!_needsLayout);
+    assert(!_debugNeedsLayout);
     return _paragraph!.didExceedMaxLines;
   }
 
   double? _lastMinWidth;
   double? _lastMaxWidth;
 
-  /// Computes the visual position of the glyphs for painting the text.
-  ///
-  /// The text will layout with a width that's as close to its max intrinsic
-  /// width as possible while still being greater than or equal to `minWidth` and
-  /// less than or equal to `maxWidth`.
-  ///
-  /// The [text] and [textDirection] properties must be non-null before this is
-  /// called.
-  void layout({ double minWidth = 0.0, double maxWidth = double.infinity }) {
-    assert(text != null, 'TextPainter.text must be set to a non-null value before using the TextPainter.');
-    assert(textDirection != null, 'TextPainter.textDirection must be set to a non-null value before using the TextPainter.');
-    if (!_needsLayout && minWidth == _lastMinWidth && maxWidth == _lastMaxWidth)
-      return;
-    _needsLayout = false;
-    if (_paragraph == null) {
+  // Creates a ui.Paragraph using the current configurations in this class and
+  // assign it to _paragraph.
+  void _createParagraph() {
+    assert(_paragraph == null || _rebuildParagraphForPaint);
+    final InlineSpan? text = this.text;
+    if (text == null) {
+      throw StateError('TextPainter.text must be set to a non-null value before using the TextPainter.');
+    }
     final ui.ParagraphBuilder builder = ui.ParagraphBuilder(_createParagraphStyle());
-      _text!.build(builder, textScaleFactor: textScaleFactor, dimensions: _placeholderDimensions);
+    text.build(builder, textScaleFactor: textScaleFactor, dimensions: _placeholderDimensions);
     _inlinePlaceholderScales = builder.placeholderScales;
     _paragraph = builder.build();
+    _rebuildParagraphForPaint = false;
   }
-    _lastMinWidth = minWidth;
-    _lastMaxWidth = maxWidth;
-    // A change in layout invalidates the cached caret metrics as well.
-    _previousCaretPosition = null;
-    _previousCaretPrototype = null;
+
+  void _layoutParagraph(double minWidth, double maxWidth) {
     _paragraph!.layout(ui.ParagraphConstraints(width: maxWidth));
     if (minWidth != maxWidth) {
       double newWidth;
@@ -614,6 +625,32 @@ class TextPainter {
         _paragraph!.layout(ui.ParagraphConstraints(width: newWidth));
       }
     }
+  }
+
+  /// Computes the visual position of the glyphs for painting the text.
+  ///
+  /// The text will layout with a width that's as close to its max intrinsic
+  /// width as possible while still being greater than or equal to `minWidth` and
+  /// less than or equal to `maxWidth`.
+  ///
+  /// The [text] and [textDirection] properties must be non-null before this is
+  /// called.
+  void layout({ double minWidth = 0.0, double maxWidth = double.infinity }) {
+    assert(text != null, 'TextPainter.text must be set to a non-null value before using the TextPainter.');
+    assert(textDirection != null, 'TextPainter.textDirection must be set to a non-null value before using the TextPainter.');
+    // Return early if the current layout information is not outdated, even if
+    // _needsPaint is true (in which case _paragraph will be rebuilt in paint).
+    if (_paragraph != null && minWidth == _lastMinWidth && maxWidth == _lastMaxWidth)
+      return;
+
+    if (_rebuildParagraphForPaint || _paragraph == null)
+      _createParagraph();
+    _lastMinWidth = minWidth;
+    _lastMaxWidth = maxWidth;
+    // A change in layout invalidates the cached caret metrics as well.
+    _previousCaretPosition = null;
+    _previousCaretPrototype = null;
+    _layoutParagraph(minWidth, maxWidth);
     _inlinePlaceholderBoxes = _paragraph!.getBoxesForPlaceholders();
   }
 
@@ -630,15 +667,30 @@ class TextPainter {
   /// To set the text style, specify a [TextStyle] when creating the [TextSpan]
   /// that you pass to the [TextPainter] constructor or to the [text] property.
   void paint(Canvas canvas, Offset offset) {
-    assert(() {
-      if (_needsLayout) {
-        throw FlutterError(
+    final double? minWidth = _lastMinWidth;
+    final double? maxWidth = _lastMaxWidth;
+    if (_paragraph == null || minWidth == null || maxWidth == null) {
+      throw StateError(
         'TextPainter.paint called when text geometry was not yet calculated.\n'
         'Please call layout() before paint() to position the text before painting it.',
       );
     }
+
+    if (_rebuildParagraphForPaint) {
+      Size? debugSize;
+      assert(() {
+        debugSize = size;
         return true;
       }());
+
+      _createParagraph();
+      // Unfortunately we have to redo the layout using the same constraints,
+      // since we've created a new ui.Paragraph. But there's no extra work being
+      // done: if _needsPaint is true and _paragraph is not null, the previous
+      // `layout` call didn't invoke _layoutParagraph.
+      _layoutParagraph(minWidth, maxWidth);
+      assert(debugSize == size);
+    }
     canvas.drawParagraph(_paragraph!, offset);
   }
 
@@ -775,7 +827,7 @@ class TextPainter {
   }
 
   Offset get _emptyOffset {
-    assert(!_needsLayout); // implies textDirection is non-null
+    assert(!_debugNeedsLayout); // implies textDirection is non-null
     assert(textAlign != null);
     switch (textAlign) {
       case TextAlign.left:
@@ -836,7 +888,7 @@ class TextPainter {
   // Checks if the [position] and [caretPrototype] have changed from the cached
   // version and recomputes the metrics required to position the caret.
   void _computeCaretMetrics(TextPosition position, Rect caretPrototype) {
-    assert(!_needsLayout);
+    assert(!_debugNeedsLayout);
     if (position == _previousCaretPosition && caretPrototype == _previousCaretPrototype)
       return;
     final int offset = position.offset;
@@ -884,7 +936,7 @@ class TextPainter {
     ui.BoxHeightStyle boxHeightStyle = ui.BoxHeightStyle.tight,
     ui.BoxWidthStyle boxWidthStyle = ui.BoxWidthStyle.tight,
   }) {
-    assert(!_needsLayout);
+    assert(!_debugNeedsLayout);
     assert(boxHeightStyle != null);
     assert(boxWidthStyle != null);
     return _paragraph!.getBoxesForRange(
@@ -897,7 +949,7 @@ class TextPainter {
 
   /// Returns the position within the text for the given pixel offset.
   TextPosition getPositionForOffset(Offset offset) {
-    assert(!_needsLayout);
+    assert(!_debugNeedsLayout);
     return _paragraph!.getPositionForOffset(offset);
   }
 
@@ -911,7 +963,7 @@ class TextPainter {
   /// <http://www.unicode.org/reports/tr29/#Word_Boundaries>.
   /// {@endtemplate}
   TextRange getWordBoundary(TextPosition position) {
-    assert(!_needsLayout);
+    assert(!_debugNeedsLayout);
     return _paragraph!.getWordBoundary(position);
   }
 
@@ -919,7 +971,7 @@ class TextPainter {
   ///
   /// The newline (if any) is not returned as part of the range.
   TextRange getLineBoundary(TextPosition position) {
-    assert(!_needsLayout);
+    assert(!_debugNeedsLayout);
     return _paragraph!.getLineBoundary(position);
   }
 
@@ -939,7 +991,7 @@ class TextPainter {
   /// to repeatedly call this. Instead, cache the results. The cached results
   /// should be invalidated upon the next successful [layout].
   List<ui.LineMetrics> computeLineMetrics() {
-    assert(!_needsLayout);
+    assert(!_debugNeedsLayout);
     return _paragraph!.computeLineMetrics();
   }
 }

packages/flutter/test/painting/text_painter_test.dart

@@ -152,7 +152,16 @@ void main() {
 
   test('TextPainter error test', () {
     final TextPainter painter = TextPainter(textDirection: TextDirection.ltr);
-    expect(() { painter.paint(MockCanvas(), Offset.zero); }, anyOf(throwsFlutterError, throwsAssertionError));
+    Object? e;
+    try {
+      painter.paint(MockCanvas(), Offset.zero);
+    } catch (exception) {
+      e = exception;
+    }
+    expect(
+      e.toString(),
+      contains('TextPainter.paint called when text geometry was not yet calculated'),
+    );
   });
 
   test('TextPainter requires textDirection', () {

packages/flutter/test/widgets/text_test.dart

@@ -1261,7 +1261,7 @@ void main() {
   testWidgets('Text uses TextStyle.overflow', (WidgetTester tester) async {
     const TextOverflow overflow = TextOverflow.fade;
 
-    await tester.pumpWidget( const Text(
+    await tester.pumpWidget(const Text(
       'Hello World',
       textDirection: TextDirection.ltr,
       style: TextStyle(overflow: overflow),
@@ -1272,6 +1272,42 @@ void main() {
     expect(richText.overflow, overflow);
     expect(richText.text.style!.overflow, overflow);
   });
+
+  testWidgets(
+    'Text can be hit-tested without layout or paint being called in a frame',
+    (WidgetTester tester) async {
+      // Regression test for https://github.com/flutter/flutter/issues/85108.
+      await tester.pumpWidget(
+        const Opacity(
+          opacity: 1.0,
+          child: Text(
+            'Hello World',
+            textDirection: TextDirection.ltr,
+            style: TextStyle(color: Color(0xFF123456)),
+          ),
+        ),
+      );
+
+      // The color changed and the opacity is set to 0:
+      //  * 0 opacity will prevent RenderParagraph.paint from being called.
+      //  * Only changing the color will prevent RenderParagraph.performLayout
+      //    from being called.
+      //  The underlying TextPainter should not evict its layout cache in this
+      //  case, for hit-testing.
+      await tester.pumpWidget(
+        const Opacity(
+          opacity: 0.0,
+          child: Text(
+            'Hello World',
+            textDirection: TextDirection.ltr,
+            style: TextStyle(color: Color(0x87654321)),
+          ),
+        ),
+      );
+
+      await tester.tap(find.text('Hello World'));
+      expect(tester.takeException(), isNull);
+  });
 }
 
 Future<void> _pumpTextWidget({