From bc49cf809163bc3a8f99a8da59a1f3f29b9cfad0 Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Sat, 10 Feb 2024 15:39:20 -0800 Subject: [PATCH] Fix text painter longest line resizing logic for `TextWidthBasis.longestLine` (#143024) Fixes https://github.com/flutter/flutter/issues/142309. --- .../shrine/supplemental/product_card.dart | 4 +- .../flutter_gallery/lib/gallery/home.dart | 4 +- .../lib/src/painting/text_painter.dart | 79 +++++++++++-------- .../test/painting/text_painter_test.dart | 18 +++++ 4 files changed, 68 insertions(+), 37 deletions(-) diff --git a/dev/integration_tests/flutter_gallery/lib/demo/shrine/supplemental/product_card.dart b/dev/integration_tests/flutter_gallery/lib/demo/shrine/supplemental/product_card.dart index 6b2f611c4cb..194177010f6 100644 --- a/dev/integration_tests/flutter_gallery/lib/demo/shrine/supplemental/product_card.dart +++ b/dev/integration_tests/flutter_gallery/lib/demo/shrine/supplemental/product_card.dart @@ -35,7 +35,7 @@ class ProductCard extends StatelessWidget { // The fontSize to use for computing the heuristic UI scaling factor. 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( builder: (BuildContext context, Widget? child, AppStateModel model) { @@ -56,7 +56,7 @@ class ProductCard extends StatelessWidget { child: imageWidget, ), SizedBox( - height: kTextBoxHeight * containerScaingFactor, + height: kTextBoxHeight * containerScalingFactor, width: 121.0, child: Column( mainAxisAlignment: MainAxisAlignment.end, diff --git a/dev/integration_tests/flutter_gallery/lib/gallery/home.dart b/dev/integration_tests/flutter_gallery/lib/gallery/home.dart index 756f2c73215..a6ceab8850b 100644 --- a/dev/integration_tests/flutter_gallery/lib/gallery/home.dart +++ b/dev/integration_tests/flutter_gallery/lib/gallery/home.dart @@ -180,7 +180,7 @@ class _DemoItem extends StatelessWidget { final bool isDark = theme.brightness == Brightness.dark; // The fontSize to use for computing the heuristic UI scaling factor. 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( splashColor: theme.primaryColor.withOpacity(0.12), highlightColor: Colors.transparent, @@ -188,7 +188,7 @@ class _DemoItem extends StatelessWidget { _launchDemo(context); }, child: Container( - constraints: BoxConstraints(minHeight: _kDemoItemHeight * containerScaingFactor), + constraints: BoxConstraints(minHeight: _kDemoItemHeight * containerScalingFactor), child: Row( children: [ Container( diff --git a/packages/flutter/lib/src/painting/text_painter.dart b/packages/flutter/lib/src/painting/text_painter.dart index e03f5fa407b..c0d9406fb1f 100644 --- a/packages/flutter/lib/src/painting/text_painter.dart +++ b/packages/flutter/lib/src/painting/text_painter.dart @@ -314,6 +314,13 @@ class _TextLayout { 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 @@ -321,14 +328,18 @@ class _TextLayout { // depends on the current text layout, which will be invalidated as soon as the // text layout is invalidated. class _TextPainterLayoutCacheWithOffset { - _TextPainterLayoutCacheWithOffset(this.layout, this.textAlignment, double minWidth, double maxWidth, TextWidthBasis widthBasis) - : contentWidth = _contentWidthFor(minWidth, maxWidth, widthBasis, layout), - assert(textAlignment >= 0.0 && textAlignment <= 1.0); + _TextPainterLayoutCacheWithOffset(this.layout, this.textAlignment, this.layoutMaxWidth, this.contentWidth) + : assert(textAlignment >= 0.0 && textAlignment <= 1.0), + assert(!layoutMaxWidth.isNaN), + assert(!contentWidth.isNaN); 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. - // This is also used to compute `paintOffset` + // This is also used to compute `paintOffset`. double contentWidth; // The effective text alignment in the TextPainter's canvas. The value is @@ -352,20 +363,14 @@ class _TextPainterLayoutCacheWithOffset { 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 // adjusting the paint offset (so no line-breaking changes needed). // - // Returns false if the new constraints require re-computing the line breaks, - // in which case no side effects will occur. + // Returns false if the new constraints require the text layout library to + // re-compute the line breaks. bool _resizeToFit(double minWidth, double maxWidth, TextWidthBasis widthBasis) { assert(layout.maxIntrinsicLineExtent.isFinite); + assert(minWidth <= maxWidth); // The assumption here is that if a Paragraph's width is already >= 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 @@ -377,21 +382,30 @@ class _TextPainterLayoutCacheWithOffset { // of double.infinity, and to make the text visible the paintOffset.dx is // bound to be double.negativeInfinity, which invalidates all arithmetic // 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; } - 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) { assert(paintOffset.dx == double.infinity); assert(paragraph.width == double.infinity); return false; } + final double maxIntrinsicWidth = paragraph.maxIntrinsicWidth; - if ((paragraph.width - maxIntrinsicWidth) > -precisionErrorTolerance && (maxWidth - maxIntrinsicWidth) > -precisionErrorTolerance) { - // Adjust the paintOffset and contentWidth to the new input constraints. - contentWidth = newContentWidth; + // Skip line breaking if the input width remains the same, of there will be + // no soft breaks. + 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 false; @@ -631,10 +645,6 @@ class TextPainter { // recreated. The caller may not call `layout` again after text color is // updated. See: https://github.com/flutter/flutter/issues/85108 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 { assert(!debugDisposed); @@ -1127,7 +1137,7 @@ class TextPainter { // infinite paint offset. final bool adjustMaxWidth = !maxWidth.isFinite && paintOffsetAlignment != 0; 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 // `_rebuildParagraphForPaint` is true. It's best to not eagerly rebuild @@ -1137,18 +1147,21 @@ class TextPainter { // 2. the user could be measuring the text layout so `paint` will never be // called. final ui.Paragraph paragraph = (cachedLayout?.paragraph ?? _createParagraph(text)) - ..layout(ui.ParagraphConstraints(width: _inputWidth)); - final _TextPainterLayoutCacheWithOffset newLayoutCache = _TextPainterLayoutCacheWithOffset( - _TextLayout._(paragraph), paintOffsetAlignment, minWidth, maxWidth, textWidthBasis, - ); + ..layout(ui.ParagraphConstraints(width: layoutMaxWidth)); + final _TextLayout layout = _TextLayout._(paragraph); + final double contentWidth = layout._contentWidthFor(minWidth, maxWidth, textWidthBasis); + + final _TextPainterLayoutCacheWithOffset newLayoutCache; // Call layout again if newLayoutCache had an infinite paint offset. // This is not as expensive as it seems, line breaking is relatively cheap // as compared to shaping. if (adjustedMaxWidth == null && minWidth.isFinite) { assert(maxWidth.isInfinite); - final double newInputWidth = newLayoutCache.layout.maxIntrinsicLineExtent; + final double newInputWidth = layout.maxIntrinsicLineExtent; paragraph.layout(ui.ParagraphConstraints(width: newInputWidth)); - _inputWidth = newInputWidth; + newLayoutCache = _TextPainterLayoutCacheWithOffset(layout, paintOffsetAlignment, newInputWidth, contentWidth); + } else { + newLayoutCache = _TextPainterLayoutCacheWithOffset(layout, paintOffsetAlignment, layoutMaxWidth, contentWidth); } _layoutCache = newLayoutCache; } @@ -1189,8 +1202,8 @@ class TextPainter { // 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 // and re-laid out. - assert(!_inputWidth.isNaN); - layoutCache.layout._paragraph = _createParagraph(text!)..layout(ui.ParagraphConstraints(width: _inputWidth)); + assert(!layoutCache.layoutMaxWidth.isNaN); + layoutCache.layout._paragraph = _createParagraph(text!)..layout(ui.ParagraphConstraints(width: layoutCache.layoutMaxWidth)); assert(paragraph.width == layoutCache.layout._paragraph.width); paragraph.dispose(); assert(debugSize == size); diff --git a/packages/flutter/test/painting/text_painter_test.dart b/packages/flutter/test/painting/text_painter_test.dart index 5d14a1f3b42..8e6558d1ec9 100644 --- a/packages/flutter/test/painting/text_painter_test.dart +++ b/packages/flutter/test/painting/text_painter_test.dart @@ -1510,6 +1510,24 @@ void main() { 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', () { const double fontSize = 1.25; const String text = '12345';