Unverified Commit 62e78bf1 authored by LongCatIsLooong's avatar LongCatIsLooong Committed by GitHub

Improve `TextPainter.layout` caching (#118128)

Improves `TextPainter.layout` caching when only the input constraints change: 
- removes the double layout calls in `TextPainter._layoutParagraph`: now double layout is only needed when `TextAlign` is not left, and the input `maxWidth == double.infinity`.  
- skip calls to `ui.Paragraph.layout` when it's guaranteed that there's no soft line breaks before/after the layout call.

This doesn't introduce new APIs but may slightly shift text rendered on screen.
This reduces the number of `layout` calls but since shaping results are already cached so it only skips the relatively cheap line-breaking process when possible.

528 scuba failures but all of them seem reasonable.
parent 4d1c6a43
...@@ -1129,6 +1129,7 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix ...@@ -1129,6 +1129,7 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
} }
void _updateLabelPainter(Thumb thumb) { void _updateLabelPainter(Thumb thumb) {
final RangeLabels? labels = this.labels;
if (labels == null) { if (labels == null) {
return; return;
} }
...@@ -1137,14 +1138,13 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix ...@@ -1137,14 +1138,13 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
final TextPainter labelPainter; final TextPainter labelPainter;
switch (thumb) { switch (thumb) {
case Thumb.start: case Thumb.start:
text = labels!.start; text = labels.start;
labelPainter = _startLabelPainter; labelPainter = _startLabelPainter;
case Thumb.end: case Thumb.end:
text = labels!.end; text = labels.end;
labelPainter = _endLabelPainter; labelPainter = _endLabelPainter;
} }
if (labels != null) {
labelPainter labelPainter
..text = TextSpan( ..text = TextSpan(
style: _sliderTheme.valueIndicatorTextStyle, style: _sliderTheme.valueIndicatorTextStyle,
...@@ -1153,9 +1153,6 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix ...@@ -1153,9 +1153,6 @@ class _RenderRangeSlider extends RenderBox with RelayoutWhenSystemFontsChangeMix
..textDirection = textDirection ..textDirection = textDirection
..textScaleFactor = textScaleFactor ..textScaleFactor = textScaleFactor
..layout(); ..layout();
} else {
labelPainter.text = null;
}
// Changing the textDirection can result in the layout changing, because the // Changing the textDirection can result in the layout changing, because the
// bidi algorithm might line up the glyphs differently which can result in // bidi algorithm might line up the glyphs differently which can result in
// different ligatures, different shapes, etc. So we always markNeedsLayout. // different ligatures, different shapes, etc. So we always markNeedsLayout.
......
...@@ -5692,8 +5692,15 @@ void main() { ...@@ -5692,8 +5692,15 @@ void main() {
expect( expect(
find.text(longStringB), find.text(longStringB),
paints..something((Symbol methodName, List<dynamic> arguments) {
if (methodName != #clipRect) {
return false;
}
final Rect clipRect = arguments[0] as Rect;
// 133.3 is approximately 100 / 0.75 (_kFinalLabelScale) // 133.3 is approximately 100 / 0.75 (_kFinalLabelScale)
paints..clipRect(rect: const Rect.fromLTWH(0, 0, 133.0, 16.0)), expect(clipRect, rectMoreOrLessEquals(const Rect.fromLTWH(0, 0, 133.0, 16.0)));
return true;
}),
); );
}, skip: isBrowser); // TODO(yjbanov): https://github.com/flutter/flutter/issues/44020 }, skip: isBrowser); // TODO(yjbanov): https://github.com/flutter/flutter/issues/44020
......
...@@ -382,9 +382,9 @@ void main() { ...@@ -382,9 +382,9 @@ void main() {
test('TextPainter requires textDirection', () { test('TextPainter requires textDirection', () {
final TextPainter painter1 = TextPainter(text: const TextSpan(text: '')); final TextPainter painter1 = TextPainter(text: const TextSpan(text: ''));
expect(() { painter1.layout(); }, throwsAssertionError); expect(painter1.layout, throwsStateError);
final TextPainter painter2 = TextPainter(text: const TextSpan(text: ''), textDirection: TextDirection.rtl); final TextPainter painter2 = TextPainter(text: const TextSpan(text: ''), textDirection: TextDirection.rtl);
expect(() { painter2.layout(); }, isNot(throwsException)); expect(painter2.layout, isNot(throwsStateError));
}); });
test('TextPainter size test', () { test('TextPainter size test', () {
...@@ -1457,8 +1457,67 @@ void main() { ...@@ -1457,8 +1457,67 @@ void main() {
painter.dispose(); painter.dispose();
}); });
test('TextPainter infinite width - centered', () {
final TextPainter painter = TextPainter()
..textAlign = TextAlign.center
..textDirection = TextDirection.ltr;
painter.text = const TextSpan(text: 'A', style: TextStyle(fontSize: 10));
MockCanvasWithDrawParagraph mockCanvas = MockCanvasWithDrawParagraph();
painter.layout(minWidth: double.infinity);
expect(painter.width, double.infinity);
expect(() => painter.paint(mockCanvas = MockCanvasWithDrawParagraph(), Offset.zero), returnsNormally);
expect(mockCanvas.centerX, isNull);
painter.layout();
expect(painter.width, 10);
expect(() => painter.paint(mockCanvas = MockCanvasWithDrawParagraph(), Offset.zero), returnsNormally);
expect(mockCanvas.centerX, 5);
painter.layout(minWidth: 100);
expect(painter.width, 100);
expect(() => painter.paint(mockCanvas = MockCanvasWithDrawParagraph(), Offset.zero), returnsNormally);
expect(mockCanvas.centerX, 50);
painter.dispose();
});
test('TextPainter infinite width - LTR justified', () {
final TextPainter painter = TextPainter()
..textAlign = TextAlign.justify
..textDirection = TextDirection.ltr;
painter.text = const TextSpan(text: 'A', style: TextStyle(fontSize: 10));
MockCanvasWithDrawParagraph mockCanvas = MockCanvasWithDrawParagraph();
painter.layout(minWidth: double.infinity);
expect(painter.width, double.infinity);
expect(() => painter.paint(mockCanvas = MockCanvasWithDrawParagraph(), Offset.zero), returnsNormally);
expect(mockCanvas.offsetX, 0);
painter.layout();
expect(painter.width, 10);
expect(() => painter.paint(mockCanvas = MockCanvasWithDrawParagraph(), Offset.zero), returnsNormally);
expect(mockCanvas.offsetX, 0);
painter.layout(minWidth: 100);
expect(painter.width, 100);
expect(() => painter.paint(mockCanvas = MockCanvasWithDrawParagraph(), Offset.zero), returnsNormally);
expect(mockCanvas.offsetX, 0);
painter.dispose();
});
} }
class MockCanvas extends Fake implements Canvas { class MockCanvas extends Fake implements Canvas {
}
class MockCanvasWithDrawParagraph extends Fake implements Canvas {
double? centerX;
double? offsetX;
@override
void drawParagraph(ui.Paragraph paragraph, Offset offset) {
offsetX = offset.dx;
centerX = offset.dx + paragraph.width / 2;
}
} }
...@@ -1225,9 +1225,11 @@ void main() { ...@@ -1225,9 +1225,11 @@ void main() {
width: 400, width: 400,
child: Center( child: Center(
child: RichText( child: RichText(
// 400 is not wide enough for this string. The part after the
// whitespace is going to be broken into a 2nd line.
text: const TextSpan(text: 'fwefwefwewfefewfwe fwfwfwefweabcdefghijklmnopqrstuvwxyz'), text: const TextSpan(text: 'fwefwefwewfefewfwe fwfwfwefweabcdefghijklmnopqrstuvwxyz'),
textWidthBasis: TextWidthBasis.longestLine, textWidthBasis: TextWidthBasis.longestLine,
textDirection: TextDirection.ltr, textDirection: TextDirection.rtl,
), ),
), ),
), ),
...@@ -1239,11 +1241,12 @@ void main() { ...@@ -1239,11 +1241,12 @@ void main() {
return false; return false;
} }
final ui.Paragraph paragraph = arguments[0] as ui.Paragraph; final ui.Paragraph paragraph = arguments[0] as ui.Paragraph;
if (paragraph.longestLine > paragraph.width) { final Offset offset = arguments[1] as Offset;
throw 'paragraph width (${paragraph.width}) greater than its longest line (${paragraph.longestLine}).'; final List<ui.LineMetrics> lines = paragraph.computeLineMetrics();
for (final ui.LineMetrics line in lines) {
if (line.left + offset.dx + line.width >= 400) {
throw 'line $line is greater than the max width constraints';
} }
if (paragraph.width >= 400) {
throw 'paragraph.width (${paragraph.width}) >= 400';
} }
return true; return true;
})); }));
......
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