Skip to content

fix(selection): drop stale selection indices after undo, redo and clear (0.3.5) - #56

Merged
markm39 merged 1 commit into
mainfrom
fix/stale-selection-after-undo
Oct 2, 2026
Merged

markm39 merged 1 commit into
mainfrom
fix/stale-selection-after-undo

Conversation

@markm39

@markm39 markm39 commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Problem

OpenNotes 1.3.2 App Store reviews report crashes "during work" while drawing. This PR reproduces a deterministic crash in the shipped 0.3.4 engine and fixes it.

Selection (selectedIndices_), the object eraser (pendingDeleteIndices_) and selection transforms store positions into strokes_. Undo, redo, clear, load and object erase rebuild or compact strokes_ without invalidating those positions.

  • Crash: lasso two strokes, tap Undo, then tap Delete on the selection toolbar (it stays visible). remainingStrokes.reserve(strokes.size() - selectedIndices.size()) underflows size_t, reserve throws std::length_error, and the exception reaches the Swift boundary. The result is std::terminate / SIGABRT.
  • Silent data loss: draw A, B, C. Delete A, select C, then Undo. Delete now removes B, which the user never selected. Move and transform can also hit the wrong strokes, and undo during an in-flight transform corrupted geometry.

Fix

  • SkiaDrawingEngine::resetIndexedSelectionState() drops index-based selection, object-eraser and drag state. It runs wherever strokes_ is rebuilt or compacted: undo, redo, clear, deserialize and object erase.
  • An in-flight selection transform is cancelled before history is reverted or snapshotted.
  • reserve() is bounded by the strokes actually removed, so it cannot underflow.
  • deleteSelection no longer commits an empty undo entry.
  • iOS: after undo, redo or clear, the view ends any move or transform gesture and refreshes the selection toolbar and onInkSelectionChange. Android already re-emitted this, and it shares cpp/.

Evidence

The new scripts/selection_history_smoke.cpp (npm run test:native:selection-smoke, added to CI and validate) has 9 assertions.

  • Shipped 0.3.4 engine: 9 assertions fail, including deleteSelection threw after undo: vector (the length_error).
  • This branch: all 9 pass, and also pass under AddressSanitizer.
  • Other checks: test:native:smoke, test:native:eraser-smoke, jest (82/82), typecheck and test:release all pass.
  • Swift: swiftc -typecheck passes for the module against the iOS 26.5 simulator SDK and the React prebuilt framework.

Release

Bumps the version to 0.3.5 with a CHANGELOG entry. Compared with the published 0.3.4, the only shipped-code change is this fix.

Selection, object-eraser and transform state stored positions into the
stroke list. Undo/redo/clear/load and object erase rebuilt that list
without invalidating them, so Delete after Undo underflowed a size_t in
deleteSelection and aborted the app with an uncaught std::length_error,
and Delete/Move could act on strokes the user never selected.

Reset that state wherever the stroke list is rebuilt, cancel an in-flight
transform before reverting, keep reserve() bounded by removed strokes,
skip empty undo entries, and hide the iOS selection toolbar after history
changes. Adds a native regression smoke test run in CI. Release 0.3.5.
@markm39
markm39 merged commit f1bd27a into main Oct 2, 2026
1 check passed
@markm39
markm39 deleted the fix/stale-selection-after-undo branch October 2, 2026 20:38
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