feat(editor): Ctrl+Cut inserts a transparent band - #114
Conversation
With Cut armed, hold Ctrl (or press Ctrl+X) to insert space instead of collapsing it. The toolbar icon swaps to a split-plus, the live band shows a plus, and annotations past the seam shift out. Undo is the same Cut op with insert set.
There was a problem hiding this comment.
🟡 Changes recommended
The insert-band implementation and UI hint logic have confirmed edge-case inconsistencies (insertBand end-clamping and modifier handling) that can produce incorrect behavior or misleading UI.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds an “insert band” variant to the Cut tool in the editor: when Cut is armed, holding Ctrl turns the gesture into inserting a transparent band (expanding the image) instead of removing and collapsing it, with corresponding UI hints, serialization support, and smoke tests.
Changes:
- Extend
CutOpwithinsert, serialize/deserialize it in the operation log, and branch cut replay to insert vs remove. - Implement transparent-band insertion in the cut engine and update annotation/selection shifting on replay.
- Update editor UI (status text, toolbar icon, drag preview) and add smoke coverage for Ctrl+Cut insertion behavior.
File summaries
| File | Description |
|---|---|
| tests/cut-smoke.cpp | Adds unit-style smoke assertions for insert shifting, insertBand, applyCutOp insert, and composedLogicalSize growth. |
| tests/cut-mapping-smoke.cpp | Adds end-to-end editor smoke for Ctrl+X arming cut and Ctrl+drag producing an insert + transparent gap. |
| src/icons.cpp | Adds a dedicated toolbar icon for cut-insert (split-plus). |
| src/editor.hpp | Adds a test accessor for composed source and declares cutInsertHint(). |
| src/editor.cpp | Implements Ctrl-to-insert behavior across status/tooltips/icon swap, drag logging, replay shifting, and preview rendering. |
| src/cut.hpp | Extends CutOp with insert and declares insert/shift helpers. |
| src/cut.cpp | Implements insertBand/applyCutOp/shiftForInsert and updates composeCuts/composedLogicalSize for inserts. |
| src/capture.cpp | Persists cut.insert in JSON only when true; reads it back on load. |
| README.md | Documents Ctrl (or Ctrl+X) to insert a transparent band when using Cut. |
| docs/editing-model.md | Updates the editing model docs to describe cut insertion semantics. |
Review details
Suppressed comments (2)
src/cut.cpp:75
- insertBand() vertical path has the same issue as the horizontal one:
endis never clamped, so the inserted band size can exceed the actual remaining extent whenendis out of bounds (e.g., from ceil/ratio rounding). Clampendand derive the band size from clamped coordinates before allocating the output image.
const int width = image.width();
start = std::clamp(start, 0, width);
QImage out(width + band, image.height(), image.format());
out.setDevicePixelRatio(image.devicePixelRatio());
out.fill(Qt::transparent);
src/editor.cpp:3969
- keyReleaseEvent() also treats Meta as the insert modifier (resetting status on Meta release), but insert detection uses only Qt::ControlModifier. Keeping Meta here can leave the Cut UI hint/status out of sync with actual behavior.
if (tool_ == Tool::Cut &&
(event->key() == Qt::Key_Control || event->key() == Qt::Key_Meta) &&
!cutDragActive_) {
setStatus(QStringLiteral("Cut: drag across a band to remove it"));
- Files reviewed: 10/10 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.
| if (orientation == Qt::Horizontal) { | ||
| const int height = image.height(); | ||
| start = std::clamp(start, 0, height); | ||
| QImage out(image.width(), height + band, image.format()); | ||
| out.setDevicePixelRatio(image.devicePixelRatio()); | ||
| out.fill(Qt::transparent); | ||
| for (int y = 0; y < start; ++y) | ||
| std::memcpy(out.scanLine(y), image.constScanLine(y), image.bytesPerLine()); | ||
| for (int y = start; y < height; ++y) | ||
| std::memcpy(out.scanLine(y + band), image.constScanLine(y), | ||
| image.bytesPerLine()); | ||
| return out; | ||
| } |
| return QGuiApplication::queryKeyboardModifiers().testFlag( | ||
| Qt::ControlModifier); |
| } else if (tool_ == Tool::Cut && | ||
| (event->key() == Qt::Key_Control || | ||
| event->key() == Qt::Key_Meta)) { | ||
| setStatus(QStringLiteral("Insert a band · drag across")); |
Clamp both source endpoints before deriving the transparent band size, keep the insert hint Ctrl-only, and cover out-of-range horizontal and vertical insertions.
tobi
left a comment
There was a problem hiding this comment.
Reviewed and tested locally. Added endpoint clamping, consistent Ctrl-only modifier handling, and out-of-range regression coverage.
There was a problem hiding this comment.
🟡 Changes recommended
Insert currently can’t grow past the bottom/right edge because both the drag coordinates and insertBand() clamp to the current extent, which conflicts with the stated “insert may grow past the current extent” behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 2
- Review effort level: Lite
| const int extent = | ||
| orientation == Qt::Horizontal ? source.height() : source.width(); | ||
| start = std::clamp(start, 0, extent); | ||
| end = std::clamp(end, start, extent); | ||
| const int band = end - start; |
| liveCut_.orientation = std::abs(delta.y()) >= std::abs(delta.x()) | ||
| ? Qt::Horizontal | ||
| : Qt::Vertical; | ||
| liveCut_.insert = | ||
| liveCut_.insert || heldModifiers(event->modifiers()) | ||
| .testFlag(Qt::ControlModifier); |
Independent of clip-out (#111) and Snap defaults (#113). Branched from
tobi/omasnapmain.With Cut armed (
X), hold Ctrl (or press Ctrl+X) to insert space instead of collapsing it.Behavior
insertset.Model
CutOpgainsinsert(JSONinsert: trueonly when set, so old logs stay valid).composeCuts/applyCutOpbranch toinsertBandvsremoveBand. Replay usesshiftForInsertfor annotations and grows the selection.Ctrl at mouse-press arms insert; the 3px drag lock records it (QTest
mouseMovehas no modifiers, so press-time is the source of truth, OR'd with live Ctrl if the real compositor sends it).Tests
runCutSmoke—insertBand,applyCutOp,composedLogicalSizegrowth,shiftForInsertrunCutMappingSmoke— Ctrl+X arms Cut, Ctrl+drag logs an insert, composed source grows with a transparent gapmake checkis green (build, offscreen smoke, clang-tidy).