Unverified Commit bc49cf80 authored by LongCatIsLooong's avatar LongCatIsLooong Committed by GitHub

Fix text painter longest line resizing logic for `TextWidthBasis.longestLine` (#143024)

Fixes https://github.com/flutter/flutter/issues/142309.
parent 4fdfe785
...@@ -35,7 +35,7 @@ class ProductCard extends StatelessWidget { ...@@ -35,7 +35,7 @@ class ProductCard extends StatelessWidget {
// The fontSize to use for computing the heuristic UI scaling factor. // The fontSize to use for computing the heuristic UI scaling factor.
const double defaultFontSize = 14.0; const double defaultFontSize = 14.0;
final double containerScaingFactor = MediaQuery.textScalerOf(context).scale(defaultFontSize) / defaultFontSize; final double containerScalingFactor = MediaQuery.textScalerOf(context).scale(defaultFontSize) / defaultFontSize;
return ScopedModelDescendant<AppStateModel>( return ScopedModelDescendant<AppStateModel>(
builder: (BuildContext context, Widget? child, AppStateModel model) { builder: (BuildContext context, Widget? child, AppStateModel model) {
...@@ -56,7 +56,7 @@ class ProductCard extends StatelessWidget { ...@@ -56,7 +56,7 @@ class ProductCard extends StatelessWidget {
child: imageWidget, child: imageWidget,
), ),
SizedBox( SizedBox(
height: kTextBoxHeight * containerScaingFactor, height: kTextBoxHeight * containerScalingFactor,
width: 121.0, width: 121.0,
child: Column( child: Column(
mainAxisAlignment: MainAxisAlignment.end, mainAxisAlignment: MainAxisAlignment.end,
......
...@@ -180,7 +180,7 @@ class _DemoItem extends StatelessWidget { ...@@ -180,7 +180,7 @@ class _DemoItem extends StatelessWidget {
final bool isDark = theme.brightness == Brightness.dark; final bool isDark = theme.brightness == Brightness.dark;
// The fontSize to use for computing the heuristic UI scaling factor. // The fontSize to use for computing the heuristic UI scaling factor.
const double defaultFontSize = 14.0; const double defaultFontSize = 14.0;
final double containerScaingFactor = MediaQuery.textScalerOf(context).scale(defaultFontSize) / defaultFontSize; final double containerScalingFactor = MediaQuery.textScalerOf(context).scale(defaultFontSize) / defaultFontSize;
return RawMaterialButton( return RawMaterialButton(
splashColor: theme.primaryColor.withOpacity(0.12), splashColor: theme.primaryColor.withOpacity(0.12),
highlightColor: Colors.transparent, highlightColor: Colors.transparent,
...@@ -188,7 +188,7 @@ class _DemoItem extends StatelessWidget { ...@@ -188,7 +188,7 @@ class _DemoItem extends StatelessWidget {
_launchDemo(context); _launchDemo(context);
}, },
child: Container( child: Container(
constraints: BoxConstraints(minHeight: _kDemoItemHeight * containerScaingFactor), constraints: BoxConstraints(minHeight: _kDemoItemHeight * containerScalingFactor),
child: Row( child: Row(
children: <Widget>[ children: <Widget>[
Container( Container(
......
...@@ -314,6 +314,13 @@ class _TextLayout { ...@@ -314,6 +314,13 @@ class _TextLayout {
TextBaseline.ideographic => _paragraph.ideographicBaseline, TextBaseline.ideographic => _paragraph.ideographicBaseline,
}; };
} }
double _contentWidthFor(double minWidth, double maxWidth, TextWidthBasis widthBasis) {
return switch (widthBasis) {
TextWidthBasis.longestLine => clampDouble(longestLine, minWidth, maxWidth),
TextWidthBasis.parent => clampDouble(maxIntrinsicLineExtent, minWidth, maxWidth),
};
}
} }
// This class stores the current text layout and the corresponding // This class stores the current text layout and the corresponding
...@@ -321,14 +328,18 @@ class _TextLayout { ...@@ -321,14 +328,18 @@ class _TextLayout {
// depends on the current text layout, which will be invalidated as soon as the // depends on the current text layout, which will be invalidated as soon as the
// text layout is invalidated. // text layout is invalidated.
class _TextPainterLayoutCacheWithOffset { class _TextPainterLayoutCacheWithOffset {
_TextPainterLayoutCacheWithOffset(this.layout, this.textAlignment, double minWidth, double maxWidth, TextWidthBasis widthBasis) _TextPainterLayoutCacheWithOffset(this.layout, this.textAlignment, this.layoutMaxWidth, this.contentWidth)
: contentWidth = _contentWidthFor(minWidth, maxWidth, widthBasis, layout), : assert(textAlignment >= 0.0 && textAlignment <= 1.0),
assert(textAlignment >= 0.0 && textAlignment <= 1.0); assert(!layoutMaxWidth.isNaN),
assert(!contentWidth.isNaN);
final _TextLayout layout; final _TextLayout layout;
// The input width used to lay out the paragraph.
final double layoutMaxWidth;
// The content width the text painter should report in TextPainter.width. // The content width the text painter should report in TextPainter.width.
// This is also used to compute `paintOffset` // This is also used to compute `paintOffset`.
double contentWidth; double contentWidth;
// The effective text alignment in the TextPainter's canvas. The value is // The effective text alignment in the TextPainter's canvas. The value is
...@@ -352,20 +363,14 @@ class _TextPainterLayoutCacheWithOffset { ...@@ -352,20 +363,14 @@ class _TextPainterLayoutCacheWithOffset {
ui.Paragraph get paragraph => layout._paragraph; ui.Paragraph get paragraph => layout._paragraph;
static double _contentWidthFor(double minWidth, double maxWidth, TextWidthBasis widthBasis, _TextLayout layout) {
return switch (widthBasis) {
TextWidthBasis.longestLine => clampDouble(layout.longestLine, minWidth, maxWidth),
TextWidthBasis.parent => clampDouble(layout.maxIntrinsicLineExtent, minWidth, maxWidth),
};
}
// Try to resize the contentWidth to fit the new input constraints, by just // Try to resize the contentWidth to fit the new input constraints, by just
// adjusting the paint offset (so no line-breaking changes needed). // adjusting the paint offset (so no line-breaking changes needed).
// //
// Returns false if the new constraints require re-computing the line breaks, // Returns false if the new constraints require the text layout library to
// in which case no side effects will occur. // re-compute the line breaks.
bool _resizeToFit(double minWidth, double maxWidth, TextWidthBasis widthBasis) { bool _resizeToFit(double minWidth, double maxWidth, TextWidthBasis widthBasis) {
assert(layout.maxIntrinsicLineExtent.isFinite); assert(layout.maxIntrinsicLineExtent.isFinite);
assert(minWidth <= maxWidth);
// The assumption here is that if a Paragraph's width is already >= its // The assumption here is that if a Paragraph's width is already >= its
// maxIntrinsicWidth, further increasing the input width does not change its // maxIntrinsicWidth, further increasing the input width does not change its
// layout (but may change the paint offset if it's not left-aligned). This is // layout (but may change the paint offset if it's not left-aligned). This is
...@@ -377,21 +382,30 @@ class _TextPainterLayoutCacheWithOffset { ...@@ -377,21 +382,30 @@ class _TextPainterLayoutCacheWithOffset {
// of double.infinity, and to make the text visible the paintOffset.dx is // of double.infinity, and to make the text visible the paintOffset.dx is
// bound to be double.negativeInfinity, which invalidates all arithmetic // bound to be double.negativeInfinity, which invalidates all arithmetic
// operations. // operations.
final double newContentWidth = _contentWidthFor(minWidth, maxWidth, widthBasis, layout);
if (newContentWidth == contentWidth) { if (maxWidth == contentWidth && minWidth == contentWidth) {
contentWidth = layout._contentWidthFor(minWidth, maxWidth, widthBasis);
return true; return true;
} }
assert(minWidth <= maxWidth);
// Always needsLayout when the current paintOffset and the paragraph width are not finite. // Special case:
// When the paint offset and the paragraph width are both +∞, it's likely
// that the text layout engine skipped layout because there weren't anything
// to paint. Always try to re-compute the text layout.
if (!paintOffset.dx.isFinite && !paragraph.width.isFinite && minWidth.isFinite) { if (!paintOffset.dx.isFinite && !paragraph.width.isFinite && minWidth.isFinite) {
assert(paintOffset.dx == double.infinity); assert(paintOffset.dx == double.infinity);
assert(paragraph.width == double.infinity); assert(paragraph.width == double.infinity);
return false; return false;
} }
final double maxIntrinsicWidth = paragraph.maxIntrinsicWidth; final double maxIntrinsicWidth = paragraph.maxIntrinsicWidth;
if ((paragraph.width - maxIntrinsicWidth) > -precisionErrorTolerance && (maxWidth - maxIntrinsicWidth) > -precisionErrorTolerance) { // Skip line breaking if the input width remains the same, of there will be
// Adjust the paintOffset and contentWidth to the new input constraints. // no soft breaks.
contentWidth = newContentWidth; final bool skipLineBreaking = maxWidth == layoutMaxWidth // Same input max width so relayout is unnecessary.
|| ((paragraph.width - maxIntrinsicWidth) > -precisionErrorTolerance && (maxWidth - maxIntrinsicWidth) > -precisionErrorTolerance);
if (skipLineBreaking) {
// Adjust the content width in case the TextWidthBasis changed.
contentWidth = layout._contentWidthFor(minWidth, maxWidth, widthBasis);
return true; return true;
} }
return false; return false;
...@@ -631,10 +645,6 @@ class TextPainter { ...@@ -631,10 +645,6 @@ class TextPainter {
// recreated. The caller may not call `layout` again after text color is // recreated. The caller may not call `layout` again after text color is
// updated. See: https://github.com/flutter/flutter/issues/85108 // updated. See: https://github.com/flutter/flutter/issues/85108
bool _rebuildParagraphForPaint = true; bool _rebuildParagraphForPaint = true;
// `_layoutCache`'s input width. This is only needed because there's no API to
// create paint only updates that don't affect the text layout (e.g., changing
// the color of the text), on ui.Paragraph or ui.ParagraphBuilder.
double _inputWidth = double.nan;
bool get _debugAssertTextLayoutIsValid { bool get _debugAssertTextLayoutIsValid {
assert(!debugDisposed); assert(!debugDisposed);
...@@ -1127,7 +1137,7 @@ class TextPainter { ...@@ -1127,7 +1137,7 @@ class TextPainter {
// infinite paint offset. // infinite paint offset.
final bool adjustMaxWidth = !maxWidth.isFinite && paintOffsetAlignment != 0; final bool adjustMaxWidth = !maxWidth.isFinite && paintOffsetAlignment != 0;
final double? adjustedMaxWidth = !adjustMaxWidth ? maxWidth : cachedLayout?.layout.maxIntrinsicLineExtent; final double? adjustedMaxWidth = !adjustMaxWidth ? maxWidth : cachedLayout?.layout.maxIntrinsicLineExtent;
_inputWidth = adjustedMaxWidth ?? maxWidth; final double layoutMaxWidth = adjustedMaxWidth ?? maxWidth;
// Only rebuild the paragraph when there're layout changes, even when // Only rebuild the paragraph when there're layout changes, even when
// `_rebuildParagraphForPaint` is true. It's best to not eagerly rebuild // `_rebuildParagraphForPaint` is true. It's best to not eagerly rebuild
...@@ -1137,18 +1147,21 @@ class TextPainter { ...@@ -1137,18 +1147,21 @@ class TextPainter {
// 2. the user could be measuring the text layout so `paint` will never be // 2. the user could be measuring the text layout so `paint` will never be
// called. // called.
final ui.Paragraph paragraph = (cachedLayout?.paragraph ?? _createParagraph(text)) final ui.Paragraph paragraph = (cachedLayout?.paragraph ?? _createParagraph(text))
..layout(ui.ParagraphConstraints(width: _inputWidth)); ..layout(ui.ParagraphConstraints(width: layoutMaxWidth));
final _TextPainterLayoutCacheWithOffset newLayoutCache = _TextPainterLayoutCacheWithOffset( final _TextLayout layout = _TextLayout._(paragraph);
_TextLayout._(paragraph), paintOffsetAlignment, minWidth, maxWidth, textWidthBasis, final double contentWidth = layout._contentWidthFor(minWidth, maxWidth, textWidthBasis);
);
final _TextPainterLayoutCacheWithOffset newLayoutCache;
// Call layout again if newLayoutCache had an infinite paint offset. // Call layout again if newLayoutCache had an infinite paint offset.
// This is not as expensive as it seems, line breaking is relatively cheap // This is not as expensive as it seems, line breaking is relatively cheap
// as compared to shaping. // as compared to shaping.
if (adjustedMaxWidth == null && minWidth.isFinite) { if (adjustedMaxWidth == null && minWidth.isFinite) {
assert(maxWidth.isInfinite); assert(maxWidth.isInfinite);
final double newInputWidth = newLayoutCache.layout.maxIntrinsicLineExtent; final double newInputWidth = layout.maxIntrinsicLineExtent;
paragraph.layout(ui.ParagraphConstraints(width: newInputWidth)); paragraph.layout(ui.ParagraphConstraints(width: newInputWidth));
_inputWidth = newInputWidth; newLayoutCache = _TextPainterLayoutCacheWithOffset(layout, paintOffsetAlignment, newInputWidth, contentWidth);
} else {
newLayoutCache = _TextPainterLayoutCacheWithOffset(layout, paintOffsetAlignment, layoutMaxWidth, contentWidth);
} }
_layoutCache = newLayoutCache; _layoutCache = newLayoutCache;
} }
...@@ -1189,8 +1202,8 @@ class TextPainter { ...@@ -1189,8 +1202,8 @@ class TextPainter {
// Unfortunately even if we know that there is only paint changes, there's // Unfortunately even if we know that there is only paint changes, there's
// no API to only make those updates so the paragraph has to be recreated // no API to only make those updates so the paragraph has to be recreated
// and re-laid out. // and re-laid out.
assert(!_inputWidth.isNaN); assert(!layoutCache.layoutMaxWidth.isNaN);
layoutCache.layout._paragraph = _createParagraph(text!)..layout(ui.ParagraphConstraints(width: _inputWidth)); layoutCache.layout._paragraph = _createParagraph(text!)..layout(ui.ParagraphConstraints(width: layoutCache.layoutMaxWidth));
assert(paragraph.width == layoutCache.layout._paragraph.width); assert(paragraph.width == layoutCache.layout._paragraph.width);
paragraph.dispose(); paragraph.dispose();
assert(debugSize == size); assert(debugSize == size);
......
...@@ -1510,6 +1510,24 @@ void main() { ...@@ -1510,6 +1510,24 @@ void main() {
painter.dispose(); painter.dispose();
}); });
test('LongestLine TextPainter properly relayout when maxWidth changes.', () {
// Regression test for https://github.com/flutter/flutter/issues/142309.
final TextPainter painter = TextPainter()
..textAlign = TextAlign.justify
..textWidthBasis = TextWidthBasis.longestLine
..textDirection = TextDirection.ltr
..text = TextSpan(text: 'A' * 100, style: const TextStyle(fontSize: 10));
painter.layout(maxWidth: 1000);
expect(painter.width, 1000);
painter.layout(maxWidth: 100);
expect(painter.width, 100);
painter.layout(maxWidth: 1000);
expect(painter.width, 1000);
});
test('TextPainter line breaking does not round to integers', () { test('TextPainter line breaking does not round to integers', () {
const double fontSize = 1.25; const double fontSize = 1.25;
const String text = '12345'; const String text = '12345';
......
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