Skip to content

[vector_graphics] Add auto RenderingStrategy for better performance #8932

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gaodi-sec
Copy link

Background

Currently, vector_graphics requires manually selecting either raster or picture rendering mode. However, raster mode has limited flexibility, as the generated cache size may not always be appropriate for the current drawing conditions, leading to inefficient rendering. On the other hand, picture mode can introduce additional RenderPass execution overhead.

Changes

This PR adds an auto mode to vector_graphics, which:

  1. Determines the actual rendering resolution using the canvas transformation matrix, ensuring a pixel-perfect raster cache without aliasing.

  2. Includes color and colorFilter effects in the raster cache. Caches are stored globally, preventing redundant creation when identical icons are used.

  3. Switches to picture mode if the calculated rendering resolution exceeds 2048x2048, avoiding excessive memory usage and maintaining performance.

  4. Delays raster cache creation by two frames, using picture mode for drawing in the meantime. If properties change before the cache is created, the delay is extended by another frame. This prevents excessive cache regeneration during scaling animations.

  5. Staggers cache creation across frames when multiple caches need to be generated, ensuring that only one cache is created per frame. For example, if three icons require caching, they will be generated after 2, 3, and 4 frames, respectively. This minimizes frame-time spikes and prevents UI stuttering.

Issue Fixed

This change addresses #166184, allowing flutter_svg to dynamically adjust its rendering strategy for better performance and reduced manual configuration.

Why doesn't Skia have this performance issue?

Because in Skia mode, there is component-level raster caching that automatically stores the result. However, according to #131206, flutter previously decided not to port the raster cache to Impeller.

Test

When a page contains 20 SVGs, its rendering trace for a single frame is as follows:
image

Each small SVG requires a short period of time to render(two RenderPass):
image

Use this auto Strategy, the rendering trace can be this:
image
Each frame saves a significant amount of time spent on executing RenderPass.

Because we have a limit of generating one raster cache per frame, the frame time gradually decreases as the page with 20 SVGs loads, as shown below:
image

The raster cache will be generated with a delay of two frames after the component is drawn. It is created in the raster thread, running concurrently with UI operations before the actual rendering occurs:
image

Once this PR is merged, the changes to integrate this feature into flutter_svg will be proposed.

Pre-Review Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I linked to at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under1.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under1.
  • I updated/added any relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or I have commented below to indicate which test exemption this PR falls under1.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Footnotes

  1. Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. 2 3

@gaodi-sec gaodi-sec force-pushed the auto_strategy_vector_graphics branch 3 times, most recently from 0769349 to c5023db Compare March 29, 2025 09:27
Auto mode generates the appropriate raster cache for the current drawing size to reduce the overhead of subsequent RenderPass executions. If the cache is not created or is too large, it will automatically fall back to picture mode for real-time rendering.
@gaodi-sec gaodi-sec force-pushed the auto_strategy_vector_graphics branch from c5023db to 6a27287 Compare March 29, 2025 11:47
@jonahwilliams
Copy link
Member

I think most of these changes should be made to the existing raster strategy, and we don't need to necessarily introduce a new render object. Also, most of the new RO seems to be copy pasted from the old one which indicates at a minimum some code size savings are possible. Here are my recommendations:

  1. This seems like a bug fix that should be added to the raster approach, ideally in a separate PR.
  2. This also seems like it should be a bug fix.
  3. This feels like a separate configuration option. The numbers maybe should be configurable somehow? Its somehwat arbitrary though, I don't know if this feature makes sense
    4/5. are re-adding what I would consider bugs in the skia raster cache to this package. Staggering/delaying is somewhat pointless as rasterizing a complex picture is just as expensive whether or not you do that as part of a cache or during the frame.

@gaodi-sec
Copy link
Author

I think most of these changes should be made to the existing raster strategy, and we don't need to necessarily introduce a new render object. Also, most of the new RO seems to be copy pasted from the old one which indicates at a minimum some code size savings are possible. Here are my recommendations:

  1. This seems like a bug fix that should be added to the raster approach, ideally in a separate PR.
  2. This also seems like it should be a bug fix.
  3. This feels like a separate configuration option. The numbers maybe should be configurable somehow? Its somehwat arbitrary though, I don't know if this feature makes sense
    4/5. are re-adding what I would consider bugs in the skia raster cache to this package. Staggering/delaying is somewhat pointless as rasterizing a complex picture is just as expensive whether or not you do that as part of a cache or during the frame.

Thank you for the detailed feedback!

1/2. I originally proposed a new approach to avoid affecting existing logic, but I agree they’re bug fixes and should go into the raster strategy. I’ll submit a separate PR for that.

  1. The 2048 threshold was to avoid large memory usage (e.g. 2048x2048 uses 16MB). I picked it based on common screen sizes and texture limits on devices. If you think it’s unnecessary, I can remove it — though making it configurable might still be helpful. What do you think?

4/5. The goal of staggering is not to reduce the cost but to spread it across frames. When many complex images (e.g. 100 SVGs) are cached at once, immediate rasterization can cause long frame stalls (e.g. 500ms). By distributing raster cache generation over multiple frames, long UI stalls caused by generating all caches at once can be avoided. This allows the UI to maintain 60Hz responsiveness during the caching process and reach 120Hz performance after caches are complete.

@gaodi-sec
Copy link
Author

Here is the bug fix for 1/2: #9082

For 4/5, if there is no delay, when a page contains 20 SVGs, the raster mode will cause a ~30ms lag, as shown in the trace below:
image

Each small block in the middle represents the generation of a raster cache (an offscreen snapshot in the raster thread).
image

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