Skip to content

Commit

Permalink
Fix text painter longest line resizing logic for `TextWidthBasis.long…
Browse files Browse the repository at this point in the history
…estLine` (#143024)

Fixes flutter/flutter#142309.
  • Loading branch information
LongCatIsLooong authored Feb 10, 2024
1 parent 4fdfe78 commit bc49cf8
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<AppStateModel>(
builder: (BuildContext context, Widget? child, AppStateModel model) {
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions dev/integration_tests/flutter_gallery/lib/gallery/home.dart
Original file line number Diff line number Diff line change
Expand Up @@ -180,15 +180,15 @@ 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,
onPressed: () {
_launchDemo(context);
},
child: Container(
constraints: BoxConstraints(minHeight: _kDemoItemHeight * containerScaingFactor),
constraints: BoxConstraints(minHeight: _kDemoItemHeight * containerScalingFactor),
child: Row(
children: <Widget>[
Container(
Expand Down
79 changes: 46 additions & 33 deletions packages/flutter/lib/src/painting/text_painter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -314,21 +314,32 @@ 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
// paintOffset/contentWidth, as well as some cached text metrics values that
// 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
Expand All @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down
18 changes: 18 additions & 0 deletions packages/flutter/test/painting/text_painter_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down

0 comments on commit bc49cf8

Please sign in to comment.