feat(editor): clip a screenshot region from Select (rect / ellipse / lasso + Snap) - #111
feat(editor): clip a screenshot region from Select (rect / ellipse / lasso + Snap)#111lukejmorrison wants to merge 7 commits into
Conversation
Select already grows the canvas when you drag a layer off the screenshot; this lets the screenshot itself be the source of that layer. An empty marquee locks a pixel clip; drag it to lift those pixels, punch a hole (backdrop shows through), and commit one Clip operation. Repeat to pull another region. Ctrl+Z undoes the hole and the layer together. Covered by runClipSmoke (copy/punch/mapping) and runClipMappingSmoke (empty marquee, lift, hole, undo, repeat). The suite fails without the change. make check is green.
There was a problem hiding this comment.
🟡 Changes recommended
There are a couple of correctness issues in the new code paths (JSON parsing symmetry for clip PNG data and cursor state after canceling a clip lift) that should be fixed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a new “clip-out” interaction to the Select tool, letting users marquee a pixel region of the screenshot, lift it into a new layer, and punch a transparent hole in the source while keeping the operation-log model (undo restores both hole + layer).
Changes:
- Introduces a new clip engine (
copyRect,punchRect, logical→native mapping, snap threshold) and a newOperation::Type::Clip/Annotation::Kind::Clip. - Wires Select-mode empty-marquee → locked pixel rect → lift preview → commit/undo into
CaptureEditor, including replay behavior and persistence. - Adds smoke coverage for both the clip engine and editor mapping, plus documentation/build wiring updates.
File summaries
| File | Description |
|---|---|
| tests/editor-smoke.cpp | Registers the new clip smoke tests in the overall smoke runner. |
| tests/clip-smoke.hpp | Declares the clip engine smoke test entry point. |
| tests/clip-smoke.cpp | Adds focused unit-style smoke coverage for clip engine primitives and mapping. |
| tests/clip-mapping-smoke.hpp | Declares editor-level clip mapping smoke test entry point. |
| tests/clip-mapping-smoke.cpp | Adds editor-driven smoke coverage for marquee→lift→hole→undo and repeated clips. |
| src/editor.hpp | Adds clip-related test hooks and new internal helpers/state for pixel clip + lift. |
| src/editor.cpp | Implements Select-mode pixel clip interaction, commit logic, replay support, and preview drawing. |
| src/clip.hpp | Defines ClipOp and clip engine APIs. |
| src/clip.cpp | Implements rectangular copy/punch and logical→native mapping + snapping helpers. |
| src/capture.hpp | Extends Annotation::Kind / Operation::Type to include clip and stores clip tile pixels on annotations. |
| src/capture.cpp | Adds drawing + JSON (de)serialization support for clip annotations and clip operations. |
| README.md | Documents the new clip-out user behavior and keybinding description. |
| docs/editing-model.md | Updates the editing model doc to describe Clip as an operation-log primitive. |
| CMakeLists.txt | Adds new source/test files to the build. |
| AGENTS.md | Updates repository layout docs to include the new clip module. |
Review details
- Files reviewed: 15/16 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const QByteArray png = QByteArray::fromBase64( | ||
| object.value(QStringLiteral("png")).toString().toLatin1()); | ||
| if (!png.isEmpty()) | ||
| annotation.image.loadFromData(png, "PNG"); |
| if (clipLiftActive_) { | ||
| cancelClipLift(); | ||
| dragging_ = false; | ||
| setStatus(QStringLiteral("Clip cancelled")); | ||
| } |
| QImage tile = copyRect(composed, op.clip.sourceRect); | ||
| punchRect(composed, op.clip.sourceRect); | ||
| Annotation layer; |
After an empty Select marquee locks a region, a swatch row appears under the rect: transparent (checkerboard, default), the palette colours, and custom. 1–8 and the eyedropper set the infill while the clip is armed. The fill is stored on Operation::Clip so undo/replay keep the hole.
Ctrl+Z during a lift cancels only the in-progress drag — QTest (and real keyboards) send Control before Z, so a modifier-only press must not drop the lift and then let Z undo the previous op. Empty marquees need a 12px floor, crop chrome stays until lift, and the hole-fill fly-out docks to the side. Replay keeps a present clip tile, PNG loads only on Clip, and live lift paints a downsampled pixmap with snap hysteresis. Co-authored-by: Luke Morrison <lukejmorrison@users.noreply.github.com>
Rectangle, ellipse, and lasso masks with click-to-snap, keyboard hole fill, and drag-to-lift. Spec for the follow-on to rectangular clip-out.
Select cycles clip shape with V and a chip strip. Empty-canvas drags that miss layers still lock a mask; R/E/F keep arming drawing tools. Snap ray-casts to an axis-aligned ellipse. Lifted tiles carry alpha outside the path. Fable and Sol P0s applied to the key layer.
|
Follow-up on this branch: clip-out now supports rectangle, ellipse, and lasso masks plus click-to-snap (axis-aligned ellipse from a ray-cast), after a Fable + GPT Sol review of the key layer.
Happy to split this into a stacked PR if you want the rectangular clip-out merged first. |
Cycling clip shape now updates the path already on screen (rect to ellipse, and Snap ray-casts from the mask centre). Installed to ~/.local/bin so Print no longer launches last morning's build.
…e hole Snap treats the drag as a search window: 8-connected flood with a 1px close, an enclosing circle for Ellipse (pins stay in), and a rounded rect for Rect when two corners agree on a radius. Crop chrome hides while a mask is locked. The hole-fill fly-out adds match-surroundings and the same Sample from image eyedropper as the toolbar.
0842f55 to
5e51400
Compare
Select already grows the canvas when you drag a layer off the screenshot; this lets the screenshot itself be the source of that layer — lock a mask, fill the hole, lift a copy, repeat.
Split (pick what you want)
This PR is clip-out only. Two follow-ups, so each can land on its own:
Tcycles fill while the mask is lockedKitchen-sink daily use (all three) stays on
lukejmorrison/omasnapmain.Behavior
In Select (
V):Vcycles clip shape: rectangle → ellipse → lasso. A chip strip under the toolbar shows those plus a Snap toggle.1–8/ Sample from imageI/#while the mask is locked).R/E/Fstill arm the drawing tools.This is not Cut (
X). Cut removes a band and collapses the gap. Clip copies a region, leaves the rest in place, and does not shift other annotations.Model
One
Operation::Type::Clip(shape + radius + fill + clip annotation). Replay walks frompristineSource_, applying cuts and clip punches in order, and copies the tile from the composed image as it existed at that op so later cuts cannot rewrite an already-torn piece. Live drag is editor-only preview; the log is touched only on release.Engine lives in
src/clip.cpp. No new tool, no config key.Tests
runClipSmoke— copy, punch, hidpi mapping. Fails withoutclip.cpp.runClipMappingSmoke— empty marquee, lift, hole alpha, undo, two sequential clips. Fails without the editor glue.make checkis green (build, offscreen smoke, clang-tidy).Pitch
Select already grows the canvas when you drag a layer off the screenshot; this lets the screenshot itself be the source of that layer. Then you can select any part of the image, pull the clip, copy the clip, repeat.