From fea561301b1143990a522d3f3eb601541d16ba59 Mon Sep 17 00:00:00 2001 From: Kostia Sokolovskyi Date: Sat, 28 Oct 2023 06:29:02 +0200 Subject: [PATCH] TextPainter should dispatch creation and disposal events. (#137416) --- .../flutter/lib/src/material/snack_bar.dart | 1 + packages/flutter/lib/src/material/switch.dart | 13 ++--- .../lib/src/painting/flutter_logo.dart | 7 ++- .../lib/src/painting/text_painter.dart | 19 ++++++- .../flutter/lib/src/rendering/proxy_box.dart | 6 +++ packages/flutter/lib/src/widgets/banner.dart | 51 ++++++++++++------- .../test/painting/text_painter_test.dart | 8 +++ 7 files changed, 80 insertions(+), 25 deletions(-) diff --git a/packages/flutter/lib/src/material/snack_bar.dart b/packages/flutter/lib/src/material/snack_bar.dart index 85df795b31..520abc225a 100644 --- a/packages/flutter/lib/src/material/snack_bar.dart +++ b/packages/flutter/lib/src/material/snack_bar.dart @@ -664,6 +664,7 @@ class _SnackBarState extends State { final double actionAndIconWidth = actionTextPainter.size.width + (widget.action != null ? actionHorizontalMargin : 0) + (showCloseIcon ? (iconButton?.iconSize ?? 0 + iconHorizontalMargin) : 0); + actionTextPainter.dispose(); final EdgeInsets margin = widget.margin?.resolve(TextDirection.ltr) ?? snackBarTheme.insetPadding ?? defaults.insetPadding!; diff --git a/packages/flutter/lib/src/material/switch.dart b/packages/flutter/lib/src/material/switch.dart index 975bbb54c8..7bef0d2401 100644 --- a/packages/flutter/lib/src/material/switch.dart +++ b/packages/flutter/lib/src/material/switch.dart @@ -1309,6 +1309,7 @@ class _SwitchPainter extends ToggleablePainter { notifyListeners(); } + final TextPainter _textPainter = TextPainter(); Color? _cachedThumbColor; ImageProvider? _cachedThumbImage; ImageErrorListener? _cachedThumbErrorListener; @@ -1594,16 +1595,15 @@ class _SwitchPainter extends ToggleablePainter { shadows: iconShadows, ), ); - final TextPainter textPainter = TextPainter( - textDirection: textDirection, - text: textSpan, - ); - textPainter.layout(); + _textPainter + ..textDirection = textDirection + ..text = textSpan; + _textPainter.layout(); final double additionalHorizontalOffset = (thumbSize.width - iconSize) / 2; final double additionalVerticalOffset = (thumbSize.height - iconSize) / 2; final Offset offset = thumbPaintOffset + Offset(additionalHorizontalOffset, additionalVerticalOffset); - textPainter.paint(canvas, offset); + _textPainter.paint(canvas, offset); } } finally { _isPainting = false; @@ -1612,6 +1612,7 @@ class _SwitchPainter extends ToggleablePainter { @override void dispose() { + _textPainter.dispose(); _cachedThumbPainter?.dispose(); _cachedThumbPainter = null; _cachedThumbColor = null; diff --git a/packages/flutter/lib/src/painting/flutter_logo.dart b/packages/flutter/lib/src/painting/flutter_logo.dart index 81f08eeeb0..ecbdffd061 100644 --- a/packages/flutter/lib/src/painting/flutter_logo.dart +++ b/packages/flutter/lib/src/painting/flutter_logo.dart @@ -215,10 +215,15 @@ class _FlutterLogoPainter extends BoxPainter { final FlutterLogoDecoration _config; // these are configured assuming a font size of 100.0. - // TODO(dnfield): Figure out how to dispose this https://github.com/flutter/flutter/issues/110601 late TextPainter _textPainter; late Rect _textBoundingRect; + @override + void dispose() { + _textPainter.dispose(); + super.dispose(); + } + void _prepareText() { const String kLabel = 'Flutter'; _textPainter = TextPainter( diff --git a/packages/flutter/lib/src/painting/text_painter.dart b/packages/flutter/lib/src/painting/text_painter.dart index 955e716ed0..e8a4d64009 100644 --- a/packages/flutter/lib/src/painting/text_painter.dart +++ b/packages/flutter/lib/src/painting/text_painter.dart @@ -438,6 +438,8 @@ final class _EmptyLineCaretMetrics implements _CaretMetrics { final double lineVerticalOffset; } +const String _flutterPaintingLibrary = 'package:flutter/painting.dart'; + /// An object that paints a [TextSpan] tree into a [Canvas]. /// /// To use a [TextPainter], follow these steps: @@ -498,7 +500,17 @@ class TextPainter { _locale = locale, _strutStyle = strutStyle, _textWidthBasis = textWidthBasis, - _textHeightBehavior = textHeightBehavior; + _textHeightBehavior = textHeightBehavior { + // TODO(polina-c): stop duplicating code across disposables + // https://github.com/flutter/flutter/issues/137435 + if (kFlutterMemoryAllocationsEnabled) { + MemoryAllocations.instance.dispatchObjectCreated( + library: _flutterPaintingLibrary, + className: '$TextPainter', + object: this, + ); + } + } /// Computes the width of a configured [TextPainter]. /// @@ -1598,6 +1610,11 @@ class TextPainter { _disposed = true; return true; }()); + // TODO(polina-c): stop duplicating code across disposables + // https://github.com/flutter/flutter/issues/137435 + if (kFlutterMemoryAllocationsEnabled) { + MemoryAllocations.instance.dispatchObjectDisposed(object: this); + } _layoutTemplate?.dispose(); _layoutTemplate = null; _layoutCache?.paragraph.dispose(); diff --git a/packages/flutter/lib/src/rendering/proxy_box.dart b/packages/flutter/lib/src/rendering/proxy_box.dart index c6f1f0c900..fdee268f29 100644 --- a/packages/flutter/lib/src/rendering/proxy_box.dart +++ b/packages/flutter/lib/src/rendering/proxy_box.dart @@ -2263,6 +2263,12 @@ class RenderDecoratedBox extends RenderProxyBox { markNeedsPaint(); } + @override + void dispose() { + _painter?.dispose(); + super.dispose(); + } + @override bool hitTestSelf(Offset position) { return _decoration.hitTest(size, position, textDirection: configuration.textDirection); diff --git a/packages/flutter/lib/src/widgets/banner.dart b/packages/flutter/lib/src/widgets/banner.dart index 33a11fd828..d1118394b1 100644 --- a/packages/flutter/lib/src/widgets/banner.dart +++ b/packages/flutter/lib/src/widgets/banner.dart @@ -231,7 +231,7 @@ class BannerPainter extends CustomPainter { /// /// * [CheckedModeBanner], which the [WidgetsApp] widget includes by default in /// debug mode, to show a banner that says "DEBUG". -class Banner extends StatelessWidget { +class Banner extends StatefulWidget { /// Creates a banner. const Banner({ super.key, @@ -288,31 +288,48 @@ class Banner extends StatelessWidget { /// The style of the text shown on the banner. final TextStyle textStyle; + @override + State createState() => _BannerState(); +} + +class _BannerState extends State { + BannerPainter? _painter; + + @override + void dispose() { + _painter?.dispose(); + super.dispose(); + } + @override Widget build(BuildContext context) { - assert((textDirection != null && layoutDirection != null) || debugCheckHasDirectionality(context)); + assert((widget.textDirection != null && widget.layoutDirection != null) || debugCheckHasDirectionality(context)); + + _painter?.dispose(); + _painter = BannerPainter( + message: widget.message, + textDirection: widget.textDirection ?? Directionality.of(context), + location: widget.location, + layoutDirection: widget.layoutDirection ?? Directionality.of(context), + color: widget.color, + textStyle: widget.textStyle, + ); + return CustomPaint( - foregroundPainter: BannerPainter( - message: message, - textDirection: textDirection ?? Directionality.of(context), - location: location, - layoutDirection: layoutDirection ?? Directionality.of(context), - color: color, - textStyle: textStyle, - ), - child: child, + foregroundPainter: _painter, + child: widget.child, ); } @override void debugFillProperties(DiagnosticPropertiesBuilder properties) { super.debugFillProperties(properties); - properties.add(StringProperty('message', message, showName: false)); - properties.add(EnumProperty('textDirection', textDirection, defaultValue: null)); - properties.add(EnumProperty('location', location)); - properties.add(EnumProperty('layoutDirection', layoutDirection, defaultValue: null)); - properties.add(ColorProperty('color', color, showName: false)); - textStyle.debugFillProperties(properties, prefix: 'text '); + properties.add(StringProperty('message', widget.message, showName: false)); + properties.add(EnumProperty('textDirection', widget.textDirection, defaultValue: null)); + properties.add(EnumProperty('location', widget.location)); + properties.add(EnumProperty('layoutDirection', widget.layoutDirection, defaultValue: null)); + properties.add(ColorProperty('color', widget.color, showName: false)); + widget.textStyle.debugFillProperties(properties, prefix: 'text '); } } diff --git a/packages/flutter/test/painting/text_painter_test.dart b/packages/flutter/test/painting/text_painter_test.dart index 8dec069945..a3ce3c1f26 100644 --- a/packages/flutter/test/painting/text_painter_test.dart +++ b/packages/flutter/test/painting/text_painter_test.dart @@ -7,6 +7,7 @@ import 'dart:ui' as ui; import 'package:flutter/foundation.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart'; void _checkCaretOffsetsLtrAt(String text, List boundaries) { expect(boundaries.first, 0); @@ -1525,6 +1526,13 @@ void main() { expect(metrics, hasLength(1)); } }, skip: kIsWeb && !isCanvasKit); // [intended] Browsers seem to always round font/glyph metrics. + + test('TextPainter dispatches memory events', () async { + await expectLater( + await memoryEvents(() => TextPainter().dispose(), TextPainter), + areCreateAndDispose, + ); + }); } class MockCanvas extends Fake implements Canvas {