Repository navigation
feat(canvas): restore eraser pressure width, layer quick switch, crop tools and middle-click open - #9808
Conversation
…iddle-click open Eraser honours pen pressure for width through its own Dynamics switch; Alt+[ / Alt+] cycle layers when nothing is staged and Q jumps to a bookmarked or previous layer via engine-held memory and a layer-menu bookmark. Reference crop dialog gains aspect-ratio presets, wheel zoom, Space/middle-button pan, view buttons and Save to Uploads; Gallery gets an opt-in middle-click-opens-in-new-tab setting on image and video tiles.
…rops free Reference crop: Space pans only when no button or field has focus, the aspect preset is restored only for a saved partial crop, the ratio Select carries an accessible name, Save to Uploads targets the Gallery auto-add board, and the crop geometry lives in the reference image core so the editor startup graph is unchanged. Canvas: Alt+] enters a selected group at its first leaf and Alt+[ leaves it upward.
lstein
left a comment
There was a problem hiding this comment.
Reviewed at 233aa343e9. I found no blockers. It merges cleanly with current main. On the PR head, pnpm run lint passes (format, oxlint, tsc, architecture). The unit tests under src/workbench, src/features/gallery and src/features/generation pass (7,609), and so do the 6 browser test files this PR touches (203 tests). I didn't run the full check:release, the performance or accessibility gates, or any hands-on browser testing.
Each finding below is non-blocking and checked against the code. #1 is the one I'd most like fixed before merge.
1. Q stops working after the selected layer is deleted or merged
canvas-engine/engine.ts:1593 (onSelectionChanged) saves lastSelectedLayerId as previousLayerId without checking that the layer still exists.
Steps: select A, select B, then delete (or merge down) B. The reducer moves the selection to a neighbour N in the same update, and previousLayerId becomes B, which no longer exists. With no bookmark, quickSwitchLayerId (canvasHotkeyCommands.ts:78) finds B is gone and returns null, so Q does nothing until the user selects a layer by hand. You'd expect Q to go back to A, or at least to a layer that exists.
Fix: only save the outgoing id as previous if it still exists in the new document (or keep the old previous when it doesn't).
2. Selection changes made by the engine overwrite the user's "previous" layer
Selection changes that the engine makes, not the user, also pass through onSelectionChanged and replace previousLayerId:
- Paint-stroke rollback (
tools/paintTool.ts:181-187). Previous = B, A selected. A stroke creates layer N and is then cancelled (fully clipped, refused, or the tool is switched mid-drag). N is added (previous = A), removed so the selection falls to a neighbour X (previous = N), then restored to A (previous = X). Q now jumps to X, a layer the user never picked. - Opening a text edit (
engine.ts:2310) and SAM'sselectLayer(engine.ts:2869) do the same.
Only selections the user makes should update previous.
3. Ratio-locked crop: a box touching an image edge can't grow from its side handles
core/referenceImage.ts:325 (resolveRatioDrag). For e/w handles, maxWidth is capped at 2 * Math.min(cy, 100 - cy) * pctRatio, and the n/s handles use the matching 2 * Math.min(cx, 100 - cx) cap. When the box touches the top or bottom edge, that cap equals the box's current size.
Steps: 800×600 image, pick 16:9, shrink the box with a corner and move it to the top-left (x=0, y=0, w=50, h=37.5, so cy=18.75), then drag the e handle outward. The cap is 2·18.75·(4/3) = 50, the current width, so the box doesn't move. Only the corner handles still work. You'd expect the box to grow along the edge it touches, or move away from it. No test covers this case.
4. Ratio-locked corner drags can't shrink along one axis
core/referenceImage.ts:351: width = Math.max(free.width, free.height * pctRatio). If you drag se straight left, free.height * pctRatio stays at the old width, so the box doesn't change. A corner only shrinks when both axes move inward. Using the axis that moved more, or a projection onto the diagonal, would feel more natural.
5. Wheel zoom: horizontal scrolling zooms out, and trackpads overshoot
ReferenceImageCropDialog.tsx:289: factor = event.deltaY < 0 ? ZOOM_WHEEL_FACTOR : 1 / ZOOM_WHEEL_FACTOR.
- A sideways trackpad swipe, or Shift+wheel in Chromium on Windows, sends
deltaY === 0, which counts as zoom-out. - Every event is a fixed 10% step. A trackpad gesture sends dozens of small-delta events, so it reaches the 0.25× or 16× limit almost at once.
Ignore deltaY === 0, and scale the factor by deltaY (normalising deltaMode).
6. Crop overlay and "Actual size" are wrong when the height cap applies
The viewport is w="full" with aspectRatio={ratio} and maxH="60vh" (ReferenceImageCropDialog.tsx:548), and the <img> uses object-fit: contain. When maxH caps the height, the width stays at 100%, so the image doesn't fill the stage: there are empty bars at the sides. Crop percentages and getPointerPct are measured against the stage, not the image. As a result:
- The full-frame box extends past the image into the empty bars.
- A 1:1 box doesn't look square on screen.
- "Actual size" (
:334,original.width / viewport.clientWidth) misses 1:1, because the displayed width isclientHeight * ratio.
On a 768px-tall laptop window, even a 4:3 image hits the cap, and most portrait images do on any screen. The PR description notes this limitation, but it affects more images than "very tall" suggests. The browser tests only use a 512×512 image, so they never hit it. Fix: size the stage to the image's contained box (width = min(full, 60vh·ratio)).
7. Alt+[ / Alt+] can select a layer the Layers panel doesn't show
canvasHotkeyCommands.ts:55-73. Cycling can land on a leaf inside a collapsed group, or one hidden by the panel filter. LayersTree.tsx (722-766) only expands and scrolls to a row on properties or focus requests, so nothing is highlighted and long lists don't scroll to the selection. The properties-request path already calls setLayerGroupExpanded; cycling could reuse it.
8. Docs say the bookmark lasts longer than it does
docs/.../07.Canvas/03.layers-panel.mdx:85 says "The bookmark lasts for the session." The bookmark is stored on the engine, and engineRegistry disposes an engine 30 s after its last lease is released. So switching to a project or preset without this canvas for more than 30 s clears it, along with the eraser Dynamics setting. That fits "transient per engine", but the docs should say so.
Nits
- Deselect then reselect breaks Q. Select A, select B, deselect, reselect B: previous == current == B, so Q does nothing even though A still exists (
engine.ts:1596).engine.test.tsasserts this behaviour on purpose, but it's surprising for users. - Space + any mouse button pans.
beginPan(ReferenceImageCropDialog.tsx:359) only checkspanState !== 'ready' && event.button !== 1, so holding Space and right-clicking starts a pan and opens the context menu. - Wheel during a pan jumps. Zooming during a middle-drag pan makes the image jump:
movePointerresets the pan frompanRefas captured at pan start (:394), which throws away the zoom's cursor-anchored correction. - No pan bounds. The image can be panned fully out of view (Fit brings it back).
Test gaps
- No test runs the real
handleToggleQuickSwitchBookmark(LayerContextMenu.tsx:842);layerContextActions.test.tsreplaces it with a mock. A regression that never clears the bookmark, or dropspreviousLayerIdfrom the spread, would pass every test. - The crop browser tests don't cover non-square images, Actual size, the Zoom in/out buttons,
deltaY === 0, or ratio drags against an edge. - The gallery middle-click test dispatches synthetic
mousedown/auxclickevents. It confirms the handlers are attached, not that autoscroll is suppressed.
Checked and working as claimed
- Eraser: pressure changes width only (
pressureOpacitystays false). Mouse input keeps its normal width (pressure 0.5). The eraser and brush settings are independent, and nothing is persisted. - Staging: the staging check for Alt+[ / Alt+] still runs first, before the lock check and the new branch, so behaviour with staged results is unchanged.
- Q hotkey: it doesn't fire in editable fields and doesn't clash with any other shortcut. Alt+[ / Alt+] match on
BracketLeft/BracketRight, so they work with the macOS Option key. - Cycling: it follows the panel's row order (
LAYER_STACKS_TOP_FIRSTpreorder), wraps, handles empty documents and single layers, and adds no undo history. - Save to Uploads: it uploads at full resolution as a
userimage that isn't intermediate, goes to the auto-add board (or none), can't be submitted twice, and never triggersonApply. - Middle-click setting: old persisted settings without the key load as off. Middle mousedown doesn't start a drag or select the tile. With the setting off, behaviour is unchanged.
Preserve quick-switch return targets and reveal navigated layers. Correct crop geometry, zoom and pan behavior with regression coverage.
Distinguish actual-size zoom from Fit image while preserving its tooltip and accessible name.
lstein
left a comment
There was a problem hiding this comment.
Re-reviewed at ee20983043. The fixes are in a55d61cf49 and 664bd93f48. The main merge resolved one conflict, in canvas-engine/ARCHITECTURE.md, and the resolution is correct.
Every finding from my previous review is fixed. I checked each fix against the code:
- Q after delete or merge:
onSelectionChangednow only records a layer that still exists. The engine edit counter is wrapped intry/finallyand the store notifies synchronously, so the counter can't leak or fall into an async gap. Paint rollback, text-edit open and SAM no longer move the return target. I mutation-tested both guards inengine.ts: dropping the existence check fails 1 quick-switch test, and dropping theengineMutationDepthguard fails 3. - Ratio crop: the side-handle growth and single-axis corner shrink now work. Reverting either change fails its new unit and browser cases.
- Crop stage: the stage is sized to the image's contained box, so there is no letterbox, and Actual size is correct.
- Crop pan and zoom: the wheel now scales with the scroll delta and its unit. The pan updates incrementally and is bounded. A Space drag needs the primary button.
- Panel reveal: the reveal on navigation works.
- Docs and tests: the bookmark docs are corrected. The bookmark test now drives the real menu handler, and the middle-click tests use trusted events.
Gates on ee20983043:
pnpm run lint: clean.- Unit tests (
src/workbench,src/features/gallery,src/features/generation): 7,670 passed. - The 8 touched browser test files: 263 passed.
I found no blockers. Three non-blocking items are new in this round:
1. An engine edit that selects a new layer now makes Q skip the layer the user was on
engine.ts:1609-1621. Steps: select A, then select B. Duplicate B (or add a layer, create a text layer, or paint into an auto-created layer). Each of these commits with engineMutationDepth > 0, so previousLayerId stays A and only lastSelectedLayerId becomes the new layer N. Pressing Q on N now goes to A, not B. Before this round it went to B. The new test returns from a bookmark to the automatically selected new text layer asserts previousLayerId === 'a', so it locks this behaviour in.
This matches the new ARCHITECTURE.md contract. But I think the contract draws the line in the wrong place: layers an edit creates and keeps aren't temporary selections. Only the paint rollback, text-edit open and SAM needed suppressing. "Duplicate, then Q back to the original" is a natural use of Q and now lands two layers back.
2. Repairing the selection clears the Layers panel filter
layerPanelState.ts:215-228. Whenever the primary changes to a layer the filter hides, reconcileLayerPanelStates clears the filter. It runs on every dispatch, engine dispatches included, so it fires on selection changes the user never navigated to:
- Filter for "sky", then delete a matching layer.
prepareRemovemoves the selection to the nearest neighbour, which usually doesn't match, so the filter is wiped. Deleting the matches one by one is a normal filtered workflow. - With a filter active, a brush pointerdown on a non-paintable selection creates the temporary "Layer N" and wipes the filter, even if the stroke is then cancelled and rolled back.
Suggest clearing the filter only for navigation (Q, Alt+[ / Alt+], an external select), not for structural repair or temporary engine selections.
3. Horizontal wheel events are no longer prevented in the crop viewport
ReferenceImageCropDialog.tsx:303-305. deltaY === 0 now returns before preventDefault(), and nothing sets overscroll-behavior on the dialog. A horizontal two-finger swipe over the viewport can therefore reach the browser, and on macOS Chrome or Safari that can trigger swipe-back navigation. Suggest keeping preventDefault() for those events and only skipping the zoom. I haven't checked this in a browser.
Nits
- Pinch zoom is slow. A trackpad pinch sends Ctrl+wheel events with deltas of about 1–10 px, which is about 0.1–1% zoom per event at
ln(1.1)/100per px. Most pinch handlers use about 1% per px. ActrlKeymultiplier would help. - A flush corner drag can shrink while pulling outward.
referenceImage.ts:351-354picks the dominant axis from the displacements afterresolveFreeDraghas clamped them. With the box flush right, ansedrag of dx=+50 and dy=−3 clamps Δw to 0, so the vertical jitter wins and the box shrinks. Comparing the raw pointer deltas would avoid it.
Summary
Four interaction behaviors from the legacy frontend were missing from webv2.
Related Issues / Discussions
Follows up the legacy parity audit behind #9803.
QA Instructions
From
invokeai/frontend/webv2(Node 22.14, pnpm 10.12): allpnpm check:releasegates passed, with no baseline changes. The initial run reached a busy performance-test port; the performance, project-file and accessibility browser gates passed when rerun after the port was free. After the final selection fix, 712 engine/hotkey tests andpnpm lintpassed. Hands-on browser checks verified crop geometry, portrait Actual size, hidden-layer navigation, deselect/reselect, and Q in both directions after deletion.Mock backend (representative profile):
Review
Resolved selection-memory loss after deletion/merge and automatic tool selections, deselect/reselect breaking Q, and navigation into collapsed or filtered layers. Corrected ratio crop edge growth and single-axis corner shrinking, fitted stage dimensions and Actual size, wheel magnitude/delta-mode handling, right-button Space drag, zoom during pan, and pan bounds. Bookmark documentation now describes the canvas engine lifetime. Regression coverage exercises real bookmark handlers and trusted middle-click events for images and videos. Earlier keyboard activation and crop accessibility fixes remain covered.
Remaining verification limit: native Windows autoscroll was not exercised on Linux; Chromium tests verify trusted mouse-event cancellation when the setting is enabled.
Compatibility / Rollout
The new Gallery setting is optional and off by default; bookmark and previous-selection memory are never persisted. No API changes.
Checklist
What's Newcopy (if doing a release after this PR)Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.