perf: cache parsed flags and let the raster cache retain them - #1
Draft
ik-make wants to merge 2 commits into
Draft
perf: cache parsed flags and let the raster cache retain them#1ik-make wants to merge 2 commits into
ik-make wants to merge 2 commits into
Conversation
ScalableImageWidget.fromSISource falls back to a zero-size cache when none is supplied, so every flag scrolling out of view is evicted immediately and re-parsed on the way back. In a long scrolling list this also releases the native Path objects that already-queued pictures still reference, and Flutter web (Skwasm) then traps with "memory access out of bounds". Share one ScalableImageCache across all _FlagImage instances, sized to hold the whole flag pack. Co-authored-by: Cursor <cursoragent@cursor.com>
CustomPaint.isComplex defaults to false, so the raster cache never retains a flag and the whole compact image is replayed on every frame. Several flags carry a coat of arms (rs.si is ~500KB, pe.si ~127KB), which makes that replay expensive during a scroll and churns a large number of short-lived native Path objects. Co-authored-by: Cursor <cursoragent@cursor.com>
|
IMHO it's unlikely to be a jovial_svg bug. Rather, it appears to be a bug in the Skwasm renderer that you were able to work around (partially?) in jovial_svg. See zathras/jovial_svg#143 . |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Found while chasing a Skwasm renderer crash in a 145-row language picker on Flutter 3.47.2 (
--wasm, single-threaded skwasm). The crash itself turned out to be a jovial_svg bug, not acountry_flagsbug — it is fixed separately in zathras/jovial_svg#142. These two changes are pure performance work, verified not to be required for the crash fix.1. Share a
ScalableImageCache_FlagImagecallsScalableImageWidget.fromSISourcewithout acache:. Per jovial_svg's docs on that constructor:So every flag that scrolls out of view is evicted immediately and re-parsed when it scrolls back. Some assets in this pack are large (
rs.si~500KB,pe.si~127KB,es.si~77KB), so a scroll repeatedly re-parses hundreds of KB on the main thread.A single static cache, sized to hold the whole pack, removes that.
2.
isComplex: trueCustomPaint.isComplexdefaults to false, so the raster cache never retains a painted flag and the whole compact image is replayed every frame. Flags are detailed vector art (several carry a coat of arms), which is exactly the caseisComplexexists for.Notes
No API change, no asset change. Both are opt-out-free improvements for any consumer rendering many flags at once.