feat: [flame_svg] Prevent cache thrashing in class Svg by adding cache parameters/properties. by adario · Pull Request #3956 · flame-engine/flame

adario · 2026-07-21T15:25:22Z

Description

Existing behaviour

Currently, class Svg uses the raw clip bounds ratio for the image cache keys; however, when rendering the cached images, the _getImage method obviously creates integer-sized images. This results in cache thrashing (particularly with the default cache size of just 10 slots) when an Svg instance is used by several SvgComponent objects all using different scale/rotation values. Due to floating-point rounding errors, the _getImage method is creating multiple cache entries having the same integral size (hence producing the same output from PictureRecorder).

Proposed changes

The Svg constructor and the static load and loadFromString methods, as well as the SvgLoader extension on class Game adding the loadSvg method, now accept 3 new named parameters:

bool integralSize = false
bool fixedRatio = false
int cacheSize = Svg.defaultCacheSize (defaults to 10)

The integralSize parameter, when true, enforces integer values for the image cache keys, thus preventing useless stalls in the pipeline. Honestly, I don't think this parameter should ever be false... I don't see how it would be useful to use keys that depend on floating-point rounding errors. I would recommend removing this parameter entirely, always behaving as if it were true.

The fixedRatio parameter, when true, ensures that only the actual Svg image size are used for the cache keys, thus ensuring that the image cache always contains a single entry. It may be useful in all situations where a given Svg is only ever scaled down, as in the testbed linked below.

The cacheSize parameter may be fine-tuned where the amount of cache entries, generated by different rotation/scale values, is known or may be estimated in advance.

The new parameters are also available as read-write properties: the related accessors empty the current cache as a side-effect. The defaults ensure that the changes do not break any existing client of class Svg.

Testing

A complete testbed for the changes is available — this displays 200+1 SvgComponent objects all with different rotation/scale values. For comparison purposes, the testbed starts with the default implementation; the Mode and Size buttons cycle through several combinations of the new parameters.

Notes

I understand that having read-write properties for the new parameters might not adhere to the coding conventions; they were necessary to demonstrate the issue in the testbed.

Checklist

I have followed the Contributor Guide when preparing my PR.
I have updated/added tests for ALL new/updated/fixed functionality.
I have updated/added relevant documentation in docs and added dartdoc comments with ///.
I have updated/added relevant examples in examples or docs.

Breaking Change?

Yes, this PR is a breaking change.
No, this PR is not a breaking change.

Related Issues

…aching.

These are available at construction/loading, as well as at runtime; the latter is probably overkill.

spydon

Lgtm!
What do you think about adding a follow-up where we remove bool integralSize = false, completely and just have it as true, but marking that PR a breaking change?

spydon · 2026-07-21T20:43:43Z

 resolution: workspace
 description: Package to add SVG rendering support for the Flame game engine
-version: 1.12.2
+version: 1.13.0


Don't change anything regarding this version, it's automatically handled by Melos

Don't change anything regarding this version, it's automatically handled by Melos

Sorry about that: I've reverted commit d419623

Copilot

Pull request overview

This PR updates flame_svg to reduce image-cache thrashing in Svg by adding cache-configuration parameters and exposing them as mutable properties, along with a package version/dependency bump.

Changes:

Add integralSize, fixedRatio, and cacheSize parameters to Svg (constructor + load/loadFromString) and to Game.loadSvg.
Rework cache key generation in _getImage and add cache emptying behavior on configuration changes / dispose().
Bump flame_svg version and update the flutter_svg dependency.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
packages/flame_svg/lib/svg.dart	Adds cache configuration parameters/properties and adjusts cache keying + cache lifecycle handling.
packages/flame_svg/pubspec.yaml	Bumps package version to `1.13.0` and updates `flutter_svg` constraint.

Comments suppressed due to low confidence (1)

packages/flame_svg/lib/svg.dart:198

MemoryCache evicts entries by removing keys but doesn't dispose the removed Image. With the new configurable cacheSize (and the existing default of 10), this can lead to unbounded native memory growth during normal operation as entries churn. It would be safer to explicitly dispose the image that will be evicted before inserting a new entry.

      picture.dispose();
      _imageCache.setValue(cacheKey, image);
      return image;

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

spydon · 2026-07-21T22:26:58Z

+    final width = size.width * widthRatio;
+    final height = size.height * heightRatio;
+    Size cacheKey;
+    if (fixedRatio || integralSize) {
+      cacheKey = Size(width.roundToDouble(), height.roundToDouble());


@adario does this make sense?

Ah nvm, saw that you replied in the other comment.

Ah nvm, saw that you replied in the other comment.

I've fixed the first two Copilot issues in commit 4895884.

Please let me know what I should do about the integralSize property — thank you.

This reverts commit d419623.

adario · 2026-07-21T22:06:13Z

Lgtm! What do you think about adding a follow-up where we remove bool integralSize = false, completely and just have it as true, but marking that PR a breaking change?

Sure, whatever you deem best; should I modify the class and create a new branch/PR, or just modify Svg and mark this PR as breaking?
(Although behaving as if integralSize were always true would not truly break existing clients, only reduce the pressure on their _imageCache: always setting fixedRatio to true would be breaking... ;-)

In any case, I'll address the first two issues raised by Copilot.

…review.

adario · 2026-07-22T11:13:53Z

@spydon I've sync'd the fork with the latest changes from main.
I've seen you're extremely busy with the (excellent) performance work... should you find a minute, please let me know what your preference is regarding the integralSize property, and I'll apply whatever changes you deem fit.

spydon · 2026-07-22T11:20:16Z

@spydon I've sync'd the fork with the latest changes from main.
I've seen you're extremely busy with the (excellent) performance work... should you find a minute, please let me know what your preference is regarding the integralSize property, and I'll apply whatever changes you deem fit.

Thanks! Let's merge this first and I'll do a release of it, and then you can submit another breaking change PR that removes theintegralSize property and only keep the code that ran when it was true. What do you think about that?

adario · 2026-07-22T12:22:40Z

@spydon I've sync'd the fork with the latest changes from main.
I've seen you're extremely busy with the (excellent) performance work... should you find a minute, please let me know what your preference is regarding the integralSize property, and I'll apply whatever changes you deem fit.

Thanks! Let's merge this first and I'll do a release of it, and then you can submit another breaking change PR that removes theintegralSize property and only keep the code that ran when it was true. What do you think about that?

Sounds good: thank you for merging.
May I reuse the same branch for the next PR, or should I create a new branch?

spydon · 2026-07-22T12:37:31Z

Sounds good: thank you for merging.
May I reuse the same branch for the next PR, or should I create a new branch?

It'll be easier if you create a new branch from main, because the other branch was squash merged.

adario added 12 commits July 15, 2026 15:50

[flame_svg] Added support in class Svg to switch between memory/map c…

62ed89b

…aching.

Fixed erroneous cache emptying toggling between modes.

4b4d5f2

[flame_svg] Added Svg.integralSize, .fixedRatio and .useMap properties.

10e1351

These are available at construction/loading, as well as at runtime; the latter is probably overkill.

[flame_svg] Refactored unlimited caching in Svg to use MemoryCache.

21ab3d7

Merge branch 'main' into feat/extended-svg

a79c50d

Merge branch 'main' into feat/extended-svg

be7791d

[flame_svg] Updated 'flutter_svg' package to latest release.

3194bb9

Merge branch 'main' into feat/extended-svg

3771935

[flame_svg] Added missing assertion message in class Svg.

a9d5952

[flame_svg] Refactored class Svg constructor for melos compliance.

924e276

[flame_svg] Bumped up package version.

d419623

[flame_svg] Made cache emptying method private.

fbb0f2b

adario changed the title ~~feat: Prevent cache thrashing in class Svg by adding cache parameters/properties.~~ feat: [flame_svg] Prevent cache thrashing in class Svg by adding cache parameters/properties. Jul 21, 2026

spydon requested a review from Copilot July 21, 2026 20:41

Copilot started reviewing on behalf of spydon July 21, 2026 20:41 View session

spydon approved these changes Jul 21, 2026

View reviewed changes

Copilot AI reviewed Jul 21, 2026

View reviewed changes

Revert "[flame_svg] Bumped up package version."

907fc80

This reverts commit d419623.

adario added 2 commits July 22, 2026 01:35

[flame_svg] Fixed issues raised by Copilot in the PR flame-engine#3956 …

4895884

…review.

Merge branch 'main' into feat/extended-svg

6089ef7

spydon merged commit cc47ed4 into flame-engine:main Jul 22, 2026
8 checks passed

adario mentioned this pull request Jul 22, 2026

feat!: Remove integralSize property from class Svg, enforcing 'always true' behaviour #3969

Open

6 tasks

Uh oh!

Uh oh!

Conversation

adario commented Jul 21, 2026

Description

Checklist

Breaking Change?

Related Issues

Uh oh!

spydon left a comment

Choose a reason for hiding this comment

Uh oh!

spydon Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

adario Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

spydon Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

spydon Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

adario Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

adario commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adario commented Jul 22, 2026

Uh oh!

spydon commented Jul 22, 2026

Uh oh!

Uh oh!

adario commented Jul 22, 2026

Uh oh!

spydon commented Jul 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

adario commented Jul 21, 2026 •

edited

Loading