Repository navigation
Review/389 fixes - #392
Merged
Merged
Review/389 fixes#392
Conversation
Whole-stroke erasing deletes a stroke the moment the disc touches it, which is the wrong tool for shortening a stroke or opening a gap in a long line. This adds EraserMode::Partial: the swept capsule is subtracted from each touched stroke and whatever it missed stays in the layer, so the eraser cuts where it passes instead of taking the whole stroke. VectorStroke::subtractEraserSweep() does the clipping. Reach is measured to the stroke centre line with no half-width term: a cut is a topological change, so letting a graze of a thick stroke's edge sever it looks wrong. Segment boundaries are refined by bisection, so the cut lands where the disc actually ends rather than at the nearest sample. VectorLayer::replaceStroke() swaps a stroke for its fragments in place. Appending them would lift a cut stroke above everything drawn after it, which is visible when overlapping strokes differ in colour. The mode is stored as EraserMode::Partial between Normal and Lasso, so the toggle's mode index is the enum value. Saved modes are stamped with eraserModeVersion: without it, a pre-partial "1" (Lasso) would be read back as Partial. ModeToggleButton now cycles an arbitrary number of modes; the two-mode API is unchanged, so the insert and action toggles are untouched. Untested on hardware as of this commit - pending manual verification.
Erasing at high zoom left a thin background-coloured line along the edge of
the erased area (a square "ring", since the patch is a rectangle).
The focus-cache patch cleared its rectangle with an *antialiased* painter
(beginFocusPainter enables AA for the redraw), so the clear bled roughly half
a pixel past the rect edge. The redraw that followed was clipped to the exact
same edge, so the bled pixels were neither restored nor repainted - they kept
the cleared (transparent) value and showed as paper colour. The capped patch
had the same structure and only escaped it because a fresh QPainter defaults
to AA off.
Both patches now share clearAndRepaintPatch(), which:
- clears with AA disabled, so the clear cannot bleed past the rect, and
- grows the rect (clear AND clip) by CACHE_PATCH_MARGIN, so the two edges
stay consistent even where they land on fractional device pixels.
Tested by comparing a patched focus cache against a rebuilt one at fractional
coordinates: 157 differing pixels before, none after. The test is kept as a
regression guard (testFocusCachePatchHasNoSeam), alongside a general check
that a region-limited repaint after a partial erase matches a full repaint.
The eraser runs on press and on every pointer move, and each of those calls pushed its own undo action. A single sweep therefore filled the 100-entry history, so undoing it took dozens of Ctrl+Z and any genuinely earlier work had already been evicted. Partial erasing made it worse, since one pass cuts the same stroke repeatedly. Eraser undo actions are now buffered while a gesture is open and become one entry when the pen lifts (begin/commitEraserGesture). Partial erasing cannot simply concatenate its per-event actions: move N cuts the fragments move N-1 produced, so the recorded segments reference intermediate strokes that no longer exist and undoing the batch would restore a half-cut state. It instead snapshots the layer before the first cut and diffs against it at commit, which yields exactly the strokes that disappeared and the ones that are there now. Whole-stroke erasing keeps its per-event segments — it only ever removes strokes, so they stay valid — and its granularity is now a setting (Settings > Tools > Eraser): one step per erase pass (default) or one step per stroke. Tests drive a real press/move/release sequence and assert that partial erasing undoes in one step back to the original stroke, and that the whole-stroke setting produces one step versus three for the same sweep.
The eraser mode was a single button that cycled whole -> partial -> lasso. That hides the alternatives, gives no sense of position, and reaching a previous mode means clicking through the others; at 24 px it was easy to miss that it was a control at all. It is now a row of three preset buttons, the same visual language the Pen subtoolbar uses for its colour and thickness presets: round 24 px buttons on the subtoolbar background, with an accent ring on the active one. Clicking the already-active mode does nothing, so the row cannot be toggled into an "nothing selected" state. IconPresetButton is the small widget that provides that look; it packs an icon instead of a colour swatch or a thickness dot, and reuses the _reversed icon convention for dark mode. ModeToggleButton goes back to its original two-state form: its N-mode generalisation existed only for the eraser cycle and would now be unused surface. The insert- and action-mode toggles are unaffected. Translations: lupdate picked up the seven new strings (three mode tooltips, the undo-grouping label, its two options and the explanatory text). They are translated into de/es/fr/pt/zh and the .qm files rebuilt, so the new UI is not English-only in a localized build. The pre-existing backlog of untranslated strings (15 per non-zh language, 89 in zh, mostly CLI help) is untouched. A button test asserts the three-way picker: exactly one selection at a time, signals carrying the enum value, no-op on re-click, and external mode changes moving the ring.
The three size presets were shared by every eraser mode, so switching from whole-stroke to partial carried the same small radii across - exactly the ones that are too small to cut a stroke. The two modes want different magnitudes: a whole-stroke eraser only has to touch ink, while the partial one should span the stroke it is cutting. Each disc-based mode now owns three presets plus its own selection, and switching modes swaps the whole set. Defaults are the values this bar already shipped with for whole-stroke (5/15/40) and 8/15/25 for partial. Lasso mode removes everything inside the drawn region and has no size at all, so the size row and its separator are hidden there instead of offering a control that changes nothing. cycleSize() is a no-op in that mode. The buttons keep the last disc-based values while hidden, so currentSize() stays meaningful and switching back is immediate. Settings gain a version stamp (3): older layouts had a single shared set, which is migrated to the whole-stroke slot while the partial slot starts from its defaults. Per-tab state now carries both modes' sets. The test drives the real widget: per-mode values and selections survive round trips, the row hides in lasso, and it snapshots/restores the eraser QSettings group so a test run cannot wipe a developer's presets.
Two things, both in the lasso eraser. 1. The lasso eraser left stale ink behind. It deletes every stroke that *intersects* the drawn region, but repaired the stroke cache over the lasso bounds only. A stroke sticking out of the region therefore kept its outside ink in the cached pixmap; since the stroke was already gone from the layer it could never be erased again, and it only disappeared once something unrelated rebuilt the cache (scrolling). The cache is now repaired over the union of the lasso and each removed stroke's bounds, in both page and tile storage. 2. New EraserMode::LassoPartial: draw a region and cut strokes at its edge, keeping everything outside. It reuses the whole-stroke lasso's region test, so both lasso modes agree on what the region contains. The cutting itself is VectorStroke::subtractPolygon(), which shares the fragment stitching with the disc eraser. The per-segment work was factored into subtractWhere()/survivingIntervalsWhere(): the disc eraser passes a distance predicate, the polygon version a point-in-polygon test, and both get the same sample-then-bisect boundary refinement. Cuts therefore land on the polygon edge rather than at the nearest sample. LassoPartial is appended to the enum so stored modes keep their meaning. It has no size (it works on a region), so it hides the size row exactly like the whole-stroke lasso; eraserModeUsesDisc()/eraserModeIsLasso() replace the scattered comparisons so the new mode cannot be forgotten in one place. Tests: a lasso erase now repaints identically to a cache rebuild (this failed with 2673 stale pixels before the fix), the partial lasso splits a crossing stroke at the region edges while leaving other strokes untouched, its repaint matches a rebuild, and one undo restores the two originals. Icons: rope_partial follows the eraser_partial layout - a dashed region ring above the same broken bar - so the four mode buttons read as one family. Translations updated for the four mode tooltips (de/es/fr/pt/zh).
Tests cover the behaviours the new modes introduced:
- a region-limited repaint after a partial erase matches a full repaint
- patching the focus cache leaves no hairline seam at fractional zoom
- one erase pass costs one undo entry (partial), and the whole-stroke
granularity setting yields one entry versus one per stroke
- lasso erasing repaints the strokes it removed
- the partial lasso cuts at the region edge and keeps the outside
Translations: the four mode tooltips, the Eraser settings group and its
undo-grouping rows are translated into de/es/fr/pt/zh and the .qm files
rebuilt, so a localized build does not show English in the new controls.
Co-authored-by: Cursor <cursoragent@cursor.com>
The eraser gesture snapshot kept a raw Page*. Document::removeTileIfEmpty() frees an emptied tile in the middle of the gesture, so commitEraserGesture() read freed memory on pen-up (AddressSanitizer: heap-use-after-free). Snapshots are now identified by coordinates and the container is resolved at commit; a freed tile counts as an empty layer, so everything it held is recorded as removed and undo recreates the tile. Co-authored-by: Cursor <cursoragent@cursor.com>
- UndoAction::StrokeSegment gains layerIndex and zIndex. A drag across pages with different active layers used to restore every stroke into the first action's layer. - ErasePartial records each original's and each fragment's slot in the layer (new VectorLayer::insertStroke), so undo/redo no longer lift cut strokes above everything drawn after them. The lasso variant now builds its undo entry with the same snapshot diff as the disc eraser. - erasePartialAt opens its own gesture when called outside a pointer press. Co-authored-by: Cursor <cursoragent@cursor.com>
- VectorStroke caches its widest half-width (O(1) broad phase) - subtractEraserSweep/subtractPolygon report changedBounds (Catmull-Rom aware) - VectorLayer::replaceStroke takes a dirty hint; viewport update() uses the same tight rects - cache patch margin scales with cache resolution (fixes seam at low zoom) - test: patched cache matches rebuild at several zooms Co-authored-by: Cursor <cursoragent@cursor.com>
…actWhere Co-authored-by: Cursor <cursoragent@cursor.com>
…-space sweep origin Co-authored-by: Cursor <cursoragent@cursor.com>
…r, fix stale comments, toolbar slot follows eraserModeUsesDisc Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
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.
No description provided.