Skip to content

perf(import-export): share unchanged paint strokes across history steps - #897

Merged
hm21 merged 2 commits into
stablefrom
perf/history-shared-strokes
Oct 20, 2026
Merged

hm21 merged 2 commits into
stablefrom
perf/history-shared-strokes

Conversation

@hm21

@hm21 hm21 commented Oct 20, 2026 •

Copy link
Copy Markdown
Owner

Description

Problem. Opening a state history that holds a large drawing and a long undo history took hundreds of MB and several seconds, growing with steps × points. In divine-mobile a tester's app was killed by iOS after they had worked with keyframes on two drawings: the editor sat at 2 GB right after reopening the draft, and any further change (adding music, tapping Done) pushed it over the limit. Deleting one of the two drawings was enough to post.

Cause. ImportStateHistory merges each step's layer diff over the step before it and calls Layer.fromMap on the merged map. A step that leaves a paint layer's strokes alone carries no item/items of its own, so the merged map holds the very same raw stroke list as the step before, and PaintLayer.fromMap parsed that list again for every step. Every step got its own Offset objects for every point. Since 14.14.1 (#895) the export writes unchanged strokes only once, so the file stays small (875 KB for two drawings over 1000 steps) while the import grows linearly: about 630 KB per step for 15,000 points. The live history paid a smaller version of the same cost: LayerCopyManager copies every stroke into each step through PaintedModel.copy, which copied the point list (8 bytes per point per step).

Fix.

  • While ImportStateHistory.fromMap runs, PaintLayer.fromMap remembers the strokes it parsed from a raw items list or item map, keyed by the identity of that raw value (PaintStrokeImportCache, internal). A later step with the same raw value gets copies of that parse instead of a new parse. The cached strokes themselves are never handed out, so editing a layer cannot change what later steps receive. The cache is dropped when the import returns, so a map changed in place and imported again (or passed to Layer.fromMap directly) is parsed again; an earlier revision used a global Expando, which returned stale strokes in that case.
  • PaintedModel.copy shares offsets with the original. offsets was already documented as replace-only (assigning a new list is what drops the cached bounds), and nothing in the package changes it in place. erasedOffsets stays a copy, because the partial eraser adds to it in place (paint_canvas.dart).
  • Side effect: drawsSameAs now takes its identical fast path for shared point lists, so an export compares unchanged strokes in constant time instead of point by point.

Not changed / trade-offs.

  • A step that does rewrite strokes (its diff carries items) still parses them, as before.
  • Code outside the package that changes PaintedModel.offsets in place, against its documentation, would now also change the copies held by other history steps. The CHANGELOG entry says so.
  • Payloads from version 4.0.0 and older store full layers in every step, so every step has its own raw list and nothing is shared there.
  • No public API changes, hence a patch release.

Related Issue: Closes #none

Verification

  • New tests: PaintedModel.copy shares the points and keeps erased areas apart; an import shares the points across steps that keep the strokes, still gives each step stroke objects of its own, and reads the new points in a step that rewrites the strokes. The two sharing tests fail without this change. Two more check that a map changed in place is read again, by Layer.fromMap and by a second import.
  • flutter test: 1008 passed on Flutter 3.47.2. On 3.44.0, with the CI's material_ui/cupertino_ui downgrade, flutter analyze lib is clean and the 47 tests of the affected files pass. flutter analyze lib test and dart format are clean.
  • Import of the tester's drawing (28 strokes, 7504 points) present twice, as a divine-mobile draft stores it (Mac, flutter test, RSS growth):
Undo steps Before: memory / time After: memory / time
250 +154 MB / 1.6 s +9 MB / 0.10 s
1000 +615 MB / 6.1 s +19 MB / 0.26 s

The live history (copies through LayerCopyManager) went from 16–59 KB to 7–8 KB per step.

  • divine-mobile on an iPhone 12 Pro (iOS 18.5), profile builds of the same commit with and without this change (path override). A driver adds the drawing twice, makes 900 edits, the app is restarted, reopens the autosaved draft and taps Done:
iPhone 12 Pro 12 steps (control) 903 steps, before 903 steps, after
Editor ready after reopening 0.5 s 8 s 0.5 s
Footprint after reopening ~480 MB ~990–1030 MB ~460–480 MB
Peak on Done (export) 553 MB 1083 MB 578 MB

All 903 steps were restored in both runs.

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

Importing a state history parsed every paint stroke again for every
history step, although each step that leaves the strokes alone receives
the very same raw stroke list. A draft with two drawings of 7504 points
each and 900 undo steps took about 500 MB more memory and 8 s to open on
an iPhone 12 Pro, enough to get a larger draft killed by iOS.

PaintLayer.fromMap now parses a raw stroke list once and hands every step
copies of that parse, and PaintedModel.copy shares the points with the
original instead of copying them. Points are only ever replaced as a
whole, so sharing them is safe; erasedOffsets stays a copy because the
partial eraser adds to it in place.
@hm21 hm21 self-assigned this Oct 20, 2026
The global Expando kept every parse for as long as its raw map lived, so a map
whose strokes were changed in place and imported again (or passed to
Layer.fromMap again) returned the old strokes. The cache now lives only while
ImportStateHistory.fromMap runs; other Layer.fromMap calls parse as before.
@hm21
hm21 marked this pull request as ready for review October 20, 2026 09:08
@hm21
hm21 merged commit 17b9bd5 into stable Oct 20, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant