Unverified Commit 80ee3c1e authored by LongCatIsLooong's avatar LongCatIsLooong Committed by GitHub

TextPainter throw with stack trace to help track down read-before-layout (#108571)

parent eb2b0acc
...@@ -206,7 +206,18 @@ class TextPainter { ...@@ -206,7 +206,18 @@ class TextPainter {
// rebuilt before painting. // rebuilt before painting.
bool _rebuildParagraphForPaint = true; bool _rebuildParagraphForPaint = true;
bool get _debugNeedsLayout => _paragraph == null; bool get _debugAssertTextLayoutIsValid {
if (_paragraph == null) {
throw FlutterError.fromParts(<DiagnosticsNode>[
ErrorSummary('Text layout not available'),
if (_debugMarkNeedsLayoutCallStack != null) DiagnosticsStackTrace('The calls that first invalidated the text layout were', _debugMarkNeedsLayoutCallStack)
else ErrorDescription('The TextPainter has never been laid out.')
]);
}
return true;
}
StackTrace? _debugMarkNeedsLayoutCallStack;
/// Marks this text painter's layout information as dirty and removes cached /// Marks this text painter's layout information as dirty and removes cached
/// information. /// information.
...@@ -215,6 +226,12 @@ class TextPainter { ...@@ -215,6 +226,12 @@ class TextPainter {
/// layout changes in engine. In most cases, updating text painter properties /// layout changes in engine. In most cases, updating text painter properties
/// in framework will automatically invoke this method. /// in framework will automatically invoke this method.
void markNeedsLayout() { void markNeedsLayout() {
assert(() {
if (_paragraph != null) {
_debugMarkNeedsLayoutCallStack ??= StackTrace.current;
}
return true;
}());
_paragraph = null; _paragraph = null;
_lineMetricsCache = null; _lineMetricsCache = null;
_previousCaretPosition = null; _previousCaretPosition = null;
...@@ -540,7 +557,7 @@ class TextPainter { ...@@ -540,7 +557,7 @@ class TextPainter {
/// ///
/// Valid only after [layout] has been called. /// Valid only after [layout] has been called.
double get minIntrinsicWidth { double get minIntrinsicWidth {
assert(!_debugNeedsLayout); assert(_debugAssertTextLayoutIsValid);
return _applyFloatingPointHack(_paragraph!.minIntrinsicWidth); return _applyFloatingPointHack(_paragraph!.minIntrinsicWidth);
} }
...@@ -548,7 +565,7 @@ class TextPainter { ...@@ -548,7 +565,7 @@ class TextPainter {
/// ///
/// Valid only after [layout] has been called. /// Valid only after [layout] has been called.
double get maxIntrinsicWidth { double get maxIntrinsicWidth {
assert(!_debugNeedsLayout); assert(_debugAssertTextLayoutIsValid);
return _applyFloatingPointHack(_paragraph!.maxIntrinsicWidth); return _applyFloatingPointHack(_paragraph!.maxIntrinsicWidth);
} }
...@@ -556,7 +573,7 @@ class TextPainter { ...@@ -556,7 +573,7 @@ class TextPainter {
/// ///
/// Valid only after [layout] has been called. /// Valid only after [layout] has been called.
double get width { double get width {
assert(!_debugNeedsLayout); assert(_debugAssertTextLayoutIsValid);
return _applyFloatingPointHack( return _applyFloatingPointHack(
textWidthBasis == TextWidthBasis.longestLine ? _paragraph!.longestLine : _paragraph!.width, textWidthBasis == TextWidthBasis.longestLine ? _paragraph!.longestLine : _paragraph!.width,
); );
...@@ -566,7 +583,7 @@ class TextPainter { ...@@ -566,7 +583,7 @@ class TextPainter {
/// ///
/// Valid only after [layout] has been called. /// Valid only after [layout] has been called.
double get height { double get height {
assert(!_debugNeedsLayout); assert(_debugAssertTextLayoutIsValid);
return _applyFloatingPointHack(_paragraph!.height); return _applyFloatingPointHack(_paragraph!.height);
} }
...@@ -574,7 +591,7 @@ class TextPainter { ...@@ -574,7 +591,7 @@ class TextPainter {
/// ///
/// Valid only after [layout] has been called. /// Valid only after [layout] has been called.
Size get size { Size get size {
assert(!_debugNeedsLayout); assert(_debugAssertTextLayoutIsValid);
return Size(width, height); return Size(width, height);
} }
...@@ -583,7 +600,7 @@ class TextPainter { ...@@ -583,7 +600,7 @@ class TextPainter {
/// ///
/// Valid only after [layout] has been called. /// Valid only after [layout] has been called.
double computeDistanceToActualBaseline(TextBaseline baseline) { double computeDistanceToActualBaseline(TextBaseline baseline) {
assert(!_debugNeedsLayout); assert(_debugAssertTextLayoutIsValid);
assert(baseline != null); assert(baseline != null);
switch (baseline) { switch (baseline) {
case TextBaseline.alphabetic: case TextBaseline.alphabetic:
...@@ -605,7 +622,7 @@ class TextPainter { ...@@ -605,7 +622,7 @@ class TextPainter {
/// ///
/// Valid only after [layout] has been called. /// Valid only after [layout] has been called.
bool get didExceedMaxLines { bool get didExceedMaxLines {
assert(!_debugNeedsLayout); assert(_debugAssertTextLayoutIsValid);
return _paragraph!.didExceedMaxLines; return _paragraph!.didExceedMaxLines;
} }
...@@ -623,6 +640,10 @@ class TextPainter { ...@@ -623,6 +640,10 @@ class TextPainter {
final ui.ParagraphBuilder builder = ui.ParagraphBuilder(_createParagraphStyle()); final ui.ParagraphBuilder builder = ui.ParagraphBuilder(_createParagraphStyle());
text.build(builder, textScaleFactor: textScaleFactor, dimensions: _placeholderDimensions); text.build(builder, textScaleFactor: textScaleFactor, dimensions: _placeholderDimensions);
_inlinePlaceholderScales = builder.placeholderScales; _inlinePlaceholderScales = builder.placeholderScales;
assert(() {
_debugMarkNeedsLayoutCallStack = null;
return true;
}());
_paragraph = builder.build(); _paragraph = builder.build();
_rebuildParagraphForPaint = false; _rebuildParagraphForPaint = false;
} }
...@@ -859,7 +880,7 @@ class TextPainter { ...@@ -859,7 +880,7 @@ class TextPainter {
} }
Offset get _emptyOffset { Offset get _emptyOffset {
assert(!_debugNeedsLayout); // implies textDirection is non-null assert(_debugAssertTextLayoutIsValid); // implies textDirection is non-null
assert(textAlign != null); assert(textAlign != null);
switch (textAlign) { switch (textAlign) {
case TextAlign.left: case TextAlign.left:
...@@ -920,7 +941,7 @@ class TextPainter { ...@@ -920,7 +941,7 @@ class TextPainter {
// Checks if the [position] and [caretPrototype] have changed from the cached // Checks if the [position] and [caretPrototype] have changed from the cached
// version and recomputes the metrics required to position the caret. // version and recomputes the metrics required to position the caret.
void _computeCaretMetrics(TextPosition position, Rect caretPrototype) { void _computeCaretMetrics(TextPosition position, Rect caretPrototype) {
assert(!_debugNeedsLayout); assert(_debugAssertTextLayoutIsValid);
if (position == _previousCaretPosition && caretPrototype == _previousCaretPrototype) { if (position == _previousCaretPosition && caretPrototype == _previousCaretPrototype) {
return; return;
} }
...@@ -969,7 +990,7 @@ class TextPainter { ...@@ -969,7 +990,7 @@ class TextPainter {
ui.BoxHeightStyle boxHeightStyle = ui.BoxHeightStyle.tight, ui.BoxHeightStyle boxHeightStyle = ui.BoxHeightStyle.tight,
ui.BoxWidthStyle boxWidthStyle = ui.BoxWidthStyle.tight, ui.BoxWidthStyle boxWidthStyle = ui.BoxWidthStyle.tight,
}) { }) {
assert(!_debugNeedsLayout); assert(_debugAssertTextLayoutIsValid);
assert(boxHeightStyle != null); assert(boxHeightStyle != null);
assert(boxWidthStyle != null); assert(boxWidthStyle != null);
return _paragraph!.getBoxesForRange( return _paragraph!.getBoxesForRange(
...@@ -982,7 +1003,7 @@ class TextPainter { ...@@ -982,7 +1003,7 @@ class TextPainter {
/// Returns the position within the text for the given pixel offset. /// Returns the position within the text for the given pixel offset.
TextPosition getPositionForOffset(Offset offset) { TextPosition getPositionForOffset(Offset offset) {
assert(!_debugNeedsLayout); assert(_debugAssertTextLayoutIsValid);
return _paragraph!.getPositionForOffset(offset); return _paragraph!.getPositionForOffset(offset);
} }
...@@ -996,7 +1017,7 @@ class TextPainter { ...@@ -996,7 +1017,7 @@ class TextPainter {
/// <http://www.unicode.org/reports/tr29/#Word_Boundaries>. /// <http://www.unicode.org/reports/tr29/#Word_Boundaries>.
/// {@endtemplate} /// {@endtemplate}
TextRange getWordBoundary(TextPosition position) { TextRange getWordBoundary(TextPosition position) {
assert(!_debugNeedsLayout); assert(_debugAssertTextLayoutIsValid);
return _paragraph!.getWordBoundary(position); return _paragraph!.getWordBoundary(position);
} }
...@@ -1004,7 +1025,7 @@ class TextPainter { ...@@ -1004,7 +1025,7 @@ class TextPainter {
/// ///
/// The newline (if any) is not returned as part of the range. /// The newline (if any) is not returned as part of the range.
TextRange getLineBoundary(TextPosition position) { TextRange getLineBoundary(TextPosition position) {
assert(!_debugNeedsLayout); assert(_debugAssertTextLayoutIsValid);
return _paragraph!.getLineBoundary(position); return _paragraph!.getLineBoundary(position);
} }
...@@ -1021,7 +1042,7 @@ class TextPainter { ...@@ -1021,7 +1042,7 @@ class TextPainter {
/// ///
/// Valid only after [layout] has been called. /// Valid only after [layout] has been called.
List<ui.LineMetrics> computeLineMetrics() { List<ui.LineMetrics> computeLineMetrics() {
assert(!_debugNeedsLayout); assert(_debugAssertTextLayoutIsValid);
return _lineMetricsCache ??= _paragraph!.computeLineMetrics(); return _lineMetricsCache ??= _paragraph!.computeLineMetrics();
} }
} }
...@@ -8,6 +8,7 @@ import 'dart:ui' as ui show BoxHeightStyle, BoxWidthStyle, Gradient, Placeholder ...@@ -8,6 +8,7 @@ import 'dart:ui' as ui show BoxHeightStyle, BoxWidthStyle, Gradient, Placeholder
import 'package:flutter/foundation.dart'; import 'package:flutter/foundation.dart';
import 'package:flutter/gestures.dart'; import 'package:flutter/gestures.dart';
import 'package:flutter/scheduler.dart';
import 'package:flutter/semantics.dart'; import 'package:flutter/semantics.dart';
import 'box.dart'; import 'box.dart';
...@@ -640,11 +641,38 @@ class RenderParagraph extends RenderBox ...@@ -640,11 +641,38 @@ class RenderParagraph extends RenderBox
); );
} }
bool _systemFontsChangeScheduled = false;
@override @override
void systemFontsDidChange() { void systemFontsDidChange() {
final SchedulerPhase phase = SchedulerBinding.instance.schedulerPhase;
switch (phase) {
case SchedulerPhase.idle:
case SchedulerPhase.postFrameCallbacks:
if (_systemFontsChangeScheduled) {
return;
}
_systemFontsChangeScheduled = true;
SchedulerBinding.instance.scheduleFrameCallback((Duration timeStamp) {
assert(_systemFontsChangeScheduled);
_systemFontsChangeScheduled = false;
assert(
attached || (debugDisposed ?? true),
'$this is detached during $phase but not disposed.',
);
if (attached) {
super.systemFontsDidChange(); super.systemFontsDidChange();
_textPainter.markNeedsLayout(); _textPainter.markNeedsLayout();
} }
});
break;
case SchedulerPhase.transientCallbacks:
case SchedulerPhase.midFrameMicrotasks:
case SchedulerPhase.persistentCallbacks:
super.systemFontsDidChange();
_textPainter.markNeedsLayout();
break;
}
}
// Placeholder dimensions representing the sizes of child inline widgets. // Placeholder dimensions representing the sizes of child inline widgets.
// //
......
...@@ -2,9 +2,12 @@ ...@@ -2,9 +2,12 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:async';
import 'package:flutter/cupertino.dart'; import 'package:flutter/cupertino.dart';
import 'package:flutter/foundation.dart'; import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/services.dart'; import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
...@@ -15,6 +18,8 @@ void main() { ...@@ -15,6 +18,8 @@ void main() {
home: Text('text widget'), home: Text('text widget'),
), ),
); );
final RenderObject renderObject = tester.renderObject(find.text('text widget'));
const Map<String, dynamic> data = <String, dynamic>{ const Map<String, dynamic> data = <String, dynamic>{
'type': 'fontsChange', 'type': 'fontsChange',
}; };
...@@ -23,8 +28,39 @@ void main() { ...@@ -23,8 +28,39 @@ void main() {
SystemChannels.system.codec.encodeMessage(data), SystemChannels.system.codec.encodeMessage(data),
(ByteData? data) { }, (ByteData? data) { },
); );
final RenderObject renderObject = tester.renderObject(find.text('text widget'));
expect(renderObject.debugNeedsLayout, isTrue); final Completer<bool> animation = Completer<bool>();
tester.binding.scheduleFrameCallback((Duration timeStamp) {
animation.complete(renderObject.debugNeedsLayout);
});
expect(renderObject.debugNeedsLayout, isFalse);
await tester.pump();
expect(await animation.future, isTrue);
});
testWidgets('Safe to query RenderParagraph for text layout after system fonts changes', (WidgetTester tester) async {
await tester.pumpWidget(
const MaterialApp(
home: Text('text widget'),
),
);
const Map<String, dynamic> data = <String, dynamic>{
'type': 'fontsChange',
};
await ServicesBinding.instance.defaultBinaryMessenger.handlePlatformMessage(
'flutter/system',
SystemChannels.system.codec.encodeMessage(data),
(ByteData? data) { },
);
final RenderParagraph paragraph = tester.renderObject<RenderParagraph>(find.text('text widget'));
Object? exception;
try {
paragraph.getPositionForOffset(Offset.zero);
paragraph.hitTest(BoxHitTestResult(), position: Offset.zero);
} catch (e) {
exception = e;
}
expect(exception, isNull);
}); });
testWidgets('RenderEditable relayout upon system fonts changes', (WidgetTester tester) async { testWidgets('RenderEditable relayout upon system fonts changes', (WidgetTester tester) async {
......
...@@ -1034,6 +1034,41 @@ void main() { ...@@ -1034,6 +1034,41 @@ void main() {
lines = painter.computeLineMetrics(); lines = painter.computeLineMetrics();
expect(lines.length, 1); expect(lines.length, 1);
}, skip: kIsWeb && !isCanvasKit); // https://github.com/flutter/flutter/issues/62819 }, skip: kIsWeb && !isCanvasKit); // https://github.com/flutter/flutter/issues/62819
test('TextPainter throws with stack trace when accessing text layout', () {
final TextPainter painter = TextPainter()
..text = const TextSpan(text: 'TEXT')
..textDirection = TextDirection.ltr;
FlutterError? exception;
try {
painter.getPositionForOffset(Offset.zero);
} on FlutterError catch (e) {
exception = e;
}
expect(exception?.message, contains('The TextPainter has never been laid out.'));
exception = null;
try {
painter.layout();
painter.getPositionForOffset(Offset.zero);
} on FlutterError catch (e) {
exception = e;
}
expect(exception, isNull);
exception = null;
try {
painter.markNeedsLayout();
painter.getPositionForOffset(Offset.zero);
} on FlutterError catch (e) {
exception = e;
}
expect(exception?.message, contains('The calls that first invalidated the text layout were:'));
exception = null;
});
} }
class MockCanvas extends Fake implements Canvas { class MockCanvas extends Fake implements Canvas {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment