close
Skip to content

Ensure TextPainters are disposed when possible.#8902

Merged
srawlins merged 6 commits intoflutter:masterfrom
srawlins:text-painter
Feb 18, 2025
Merged

Ensure TextPainters are disposed when possible.#8902
srawlins merged 6 commits intoflutter:masterfrom
srawlins:text-painter

Conversation

@srawlins
Copy link
Copy Markdown
Contributor

There are a few ways in which TextPainters are used:

  • To get the width of what would be rendered; in this case, we delegate all implementations to calculateTextSpanWidth. We then dispose the TextPainter in that function.
  • To instantiate, layout, and paint exactly once; in this case, we dispose immediately after painting, as the documentation says we should.
  • To be passed to a CustomPainter. in these cases, I think it is not safe to immediately dispose. I'm not sure what the expected workflow is.

Work towards #5888

There are a few ways in which TextPainters are used:

* To get the width of what would be rendered; in this case, we delegate all
  implementations to calculateTextSpanWidth. We then `dispose` the TextPainter
  in that function.
* To instantiate, layout, and paint exactly once; in this case, we `dispose`
  immediately after painting, as the documentation says we should.
* To be passed to a CustomPainter. in these cases, I think it is not safe to
  immediately dispose. I'm not sure what the expected workflow is.
Comment thread packages/devtools_app/lib/src/shared/charts/chart.dart Outdated
Comment on lines -536 to -541
// Draw vertical tick (short or long).
canvas.drawLine(
Offset(xTickCoord, 0),
Offset(xTickCoord, shortTick ? 2 : 6),
axis,
);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did we remove this?

Copy link
Copy Markdown
Contributor Author

@srawlins srawlins Feb 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

displayTime is always passed as true. This was revealed by making the method private.

Comment thread packages/devtools_app/lib/src/shared/charts/chart.dart Outdated
Comment thread packages/devtools_app/lib/src/shared/charts/treemap.dart Outdated
Copy link
Copy Markdown
Contributor Author

@srawlins srawlins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Comment thread packages/devtools_app/lib/src/shared/charts/chart.dart Outdated
Comment thread packages/devtools_app/lib/src/shared/charts/chart.dart Outdated
Comment thread packages/devtools_app/lib/src/shared/charts/treemap.dart Outdated
Comment on lines -536 to -541
// Draw vertical tick (short or long).
canvas.drawLine(
Offset(xTickCoord, 0),
Offset(xTickCoord, shortTick ? 2 : 6),
axis,
);
Copy link
Copy Markdown
Contributor Author

@srawlins srawlins Feb 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

displayTime is always passed as true. This was revealed by making the method private.

@srawlins srawlins requested a review from elliette as a code owner February 18, 2025 22:21
@srawlins srawlins merged commit 5522ddf into flutter:master Feb 18, 2025
@srawlins srawlins deleted the text-painter branch February 18, 2025 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants