feat(editor): edit in a normal window, not only the overlay - #94
feat(editor): edit in a normal window, not only the overlay#94jondkinney wants to merge 9 commits into
Conversation
23a8bb2 to
6809853
Compare
…red form The edit-phase key guide entries move out of the paint call into editorHotkeyEntries(), and the legend gains a second layout: anchored above a rect, it spreads wide and a few rows tall instead of the corner card, with hotkeyLegendAnchoredSize() reporting the height that form needs at a given width. Nothing draws the anchored form yet; the overlay renders exactly as before.
A capture is often annotated next to the thing it is about, which a fullscreen overlay cannot do. [editor] mode = window opens file edits as a normal compositor window instead, and --editor window|overlay overrides the config either way. The shell integration is chosen before Qt connects, so the process scans its arguments and the config up front and drops the layer-shell env for a windowed run. The window hugs the capture at 100% plus its chrome, capped at 90% of the screen and floored where the toolbar still reads, with a minimum size the compositor holds during floating resizes. By default the compositor is asked, through a window rule registered before the window maps, to float and center it at that size, so it never tiles for a frame first; [editor] window = tiled skips that and re-registers the rule disabled. A settle fallback dispatches the same result on compositors that ignored the rule. The backdrop is a solid mat with a double drop shadow under the capture ([editor] backdrop = translucent keeps the overlay dim), and an opacity rule keeps the window solid under desktop-wide transparency rules. Windowed, the key guide spreads in its anchored form pinned above the toolbar, with a reserved row between them for the tool hint pill and another below the toolbar for the color dropdown and its popover peers, so neither covers the guide or the canvas. The toolbar and every popover anchored to it share one toolbarTop(), so the palette, shape menu, and text size panel follow the toolbar in both presentations. The canvas centers in the remaining band and the chrome stays pinned when the window resizes.
Zoomed past fit, the content clip that keeps the image inside its band was being discarded: the content and annotation draws set their own clips with the default ReplaceClip, so zoomed content overdrew the key guide, the toolbar, and the status areas (the OCR sweep escaped the same way). They intersect the band now. The crop outline, its handles, and the drop shadow were still drawn around the full zoomed rect, whose edges sit far off screen, so the outline stopped surrounding the image and the shadow vanished. They now frame visibleEditImageRect(), the zoomed rect clipped to the band, with the outline and handles drawn above the band clip. Panning limits also followed hardcoded overlay margins; they clamp against the actual band, windowed or not. This applies to the overlay's zoomed view as much as the windowed one.
The presentation is a property of the process, chosen before Qt connects, so switching means handing the working document to a fresh process and closing this one. W does that from either side: the crash snapshot is flushed, copied with its op log to a private handoff path, and the new process opens it as a file edit, so the selection, the layers, and the undo history all carry over. The initial selection is committed as a leading crop for the same reason. With [editor] mode = window a fresh capture still selects on the fullscreen overlay, where the whole screen is visible, and hands itself to a window when it enters the edit phase. Stale handoff documents are pruned a day after the processes that used them are gone.
6809853 to
de531b5
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Handoff can lose deferred edits and block the UI, while CLI parsing and compositor integration contain correctness and scope issues.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a normal-window presentation for the annotation editor while retaining fullscreen capture selection.
Changes:
- Adds configurable window/overlay presentation and live handoff.
- Introduces window-specific sizing, chrome, clipping, and compositor rules.
- Documents and tests the new behavior.
File summaries
| File | Description |
|---|---|
README.md |
Documents editor modes and shortcut. |
src/capture.cpp |
Adds handoff paths and window sizing. |
src/capture.hpp |
Declares handoff and sizing helpers. |
src/editor.cpp |
Implements window layout and handoff behavior. |
src/editor.hpp |
Adds window-presentation state and APIs. |
src/main.cpp |
Selects shell integration and configures windows. |
src/output-config.cpp |
Loads editor configuration and rules. |
src/output-config.hpp |
Declares editor configuration APIs. |
src/overlay-chrome.cpp |
Implements anchored hotkey legends. |
src/overlay-chrome.hpp |
Declares anchored legend APIs. |
tests/editor-smoke.cpp |
Adds sizing, layout, and persistence checks. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 8
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| QProcess::execute(QStringLiteral("hyprctl"), | ||
| {QStringLiteral("eval"), | ||
| editorFloatRuleScript(floatingWindow)}); |
| if (!QFile::copy(snapshotPath_, path) || | ||
| !QFile::copy(workingLogPath(), operationLogPath(path))) { |
| void CaptureEditor::handOffEditor(bool toWindow) { | ||
| if (busy_) | ||
| return; | ||
| QString path; |
| if (qstrcmp(arg, "--editor") == 0 && index + 1 < argc) { | ||
| editorWindowArg = editorWindowArg || qstrcmp(argv[index + 1], "window") == 0; | ||
| editorOverlayArg = | ||
| editorOverlayArg || qstrcmp(argv[index + 1], "overlay") == 0; | ||
| ++index; |
| QProcess probe; | ||
| probe.start(QStringLiteral("hyprctl"), | ||
| {QStringLiteral("-j"), QStringLiteral("clients")}); | ||
| const bool hyprland = probe.waitForFinished(500); |
| if (!hyprland && qEnvironmentVariableIsSet("SWAYSOCK")) { | ||
| settle->stop(); | ||
| QProcess::execute( | ||
| QStringLiteral("swaymsg"), | ||
| {QStringLiteral("[pid=%1] floating enable, resize set %2 %3, " | ||
| "move position center") | ||
| .arg(pid) | ||
| .arg(naturalSize.width()) | ||
| .arg(naturalSize.height())}); | ||
| return; | ||
| } |
| QFont anchoredLegendFont() { | ||
| QFont font = QFontDatabase::systemFont(QFontDatabase::GeneralFont); | ||
| font.setPixelSize(11); | ||
| return font; | ||
| } |
| CaptureEditor window(reopened, CaptureEditor::CaptureMode::File, | ||
| QuickOutputMode::None, log); |
There was a problem hiding this comment.
🟡 Changes recommended
Fullscreen and scrolling captures bypass configured window handoff, and zoomed content remains interactive behind window chrome.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 13/13 changed files
- Comments generated: 3
- Review effort level: Balanced
| QRectF CaptureEditor::visibleEditImageRect() const { | ||
| const QRectF image = editImageRect(); | ||
| if (viewZoom_ <= 1.0) | ||
| return image; | ||
| const qreal bandTop = | ||
| windowedPresentation_ ? contentBandTop() : imageTopMargin(); | ||
| const qreal bandBottom = windowedPresentation_ ? 64 : 58; | ||
| return image.intersected(QRectF( | ||
| 0, bandTop, width(), std::max<qreal>(1, height() - bandTop - bandBottom))); |
| if (editorWindowMode && !editingImage) | ||
| editor.setWindowedHandoffOnEdit(captureMode != | ||
| CaptureEditor::CaptureMode::Scroll); |
| acceptText(); | ||
| busy_ = true; | ||
| setEnabled(false); | ||
| setStatus(QStringLiteral("Preparing editor window…")); |
There was a problem hiding this comment.
🟡 Changes recommended
Handoffs can time out during snapshot persistence, leak temporary captures, and mis-detect window mode when Qt CLI options are present.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/main.cpp:141
- The pre-
QApplicationparser rejects Qt's own command-line options (for example the supported-platformtheme gtk3mentioned below). In that casestartupParsedis false, so a valid--file … --editor windowlaunch keeps the layer-shell integration and silently opens as an overlay afterQApplicationremoves the Qt option. The early shell-role scan needs to tolerate/filter Qt options rather than treating any unknown pre-Qt option as a reason to disable window mode.
- Files reviewed: 13/13 changed files
- Comments generated: 2
- Review effort level: Balanced
| if (windowedHandoffOnEdit_) { | ||
| // Fullscreen can enter Edit in the constructor. Defer launching until | ||
| // construction and the caller's surface setup have completed. | ||
| QTimer::singleShot(0, this, [this] { | ||
| if (windowedHandoffOnEdit_ && phase_ == Phase::Edit) { | ||
| windowedHandoffOnEdit_ = false; | ||
| handOffEditor(true); |
| const bool launched = launcher ? launcher(program, arguments) | ||
| : QProcess::startDetached(program, arguments); | ||
| if (launched) | ||
| return QString(); |
There was a problem hiding this comment.
🟡 Changes recommended
Window scrolling, monitor placement, shadow toggling, and pointer-path performance have unresolved defects.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
Previously missed (3) — in code that hasn't changed since the last review.
src/editor.cpp:1806
toolbarTop()now measures every legend string each time it is called. This method is reached repeatedly fromtoolbarButtons()duringmouseMoveEvent()and pointer-damage calculation, so high-rate pointer input performs multiple font-metric passes and allocations per sample—the exact hot path the editor otherwise keeps measurement-free. Cache the anchored legend size when the widget width/font changes and have bothtoolbarTop()andcontentBandTop()read that cached value.
src/editor.cpp:1883- The new window viewport dimensions are not used by
wheelEvent(): its overflow checks still compare againstheight() - 126andwidth() - 60. In window mode the actual vertical viewport starts much lower, so at modest zoom a 16:9 image can overflow vertically while the old checks classify it as horizontal-only; a vertical wheel gesture is then remapped to horizontal movement and the image does not scroll. Derive both slack checks fromeditViewportRect().size().
src/editor.cpp:6285 - This window-specific shadow ignores the existing
imageShadow_toggle, soShift+Bcannot turn the capture shadow off in the default windowed/no-background presentation even though the same setting controls every other shadow path. IncludeimageShadow_in this condition.
- Files reviewed: 13/13 changed files
- Comments generated: 1
- Review effort level: Balanced
| const QImage source = pristineSource_; | ||
| const OperationLog log{ops_, opIndex_, nextAnnotationId_, nextMarker_, | ||
| pristineLogicalSize_}; |
There was a problem hiding this comment.
🟡 Changes recommended
Clipped crop handles can jump unexpectedly, and OCR painting can escape the windowed viewport into editor chrome.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
src/editor.cpp:1973
- Clipping the source rectangle creates crop handles on viewport edges that are not actual crop edges.
mousePressEvent()still stores the un-clippedsourceFrameWidgetRect()incropDragImageRect_, so the first movement of one of these synthetic handles maps from the real off-screen edge and makes the crop jump by the hidden distance. Either omit handles for clipped-off edges or make the drag mapping use a matching visible-source range.
src/editor.hpp:285 - The existing layer-surface documentation now sits immediately before
setWindowedPresentation(), so generated documentation associates the keyboard-interactivity/input-mask behavior with the wrong setter whilesetLayerWindow()loses that explanation. Move those two lines back abovesetLayerWindow().
- Files reviewed: 13/13 changed files
- Comments generated: 1
- Review effort level: Balanced
| if (clipViewport) { | ||
| painter.save(); | ||
| painter.setClipRect(editViewportRect()); |
There was a problem hiding this comment.
🟡 Changes recommended
Resizing, handoff cleanup, and compositor fallback contain unresolved correctness and reliability issues.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
src/editor.cpp:1915
- The new editor window can be resized, but this clamp is only called when zooming, panning, or refreshing the canvas. If a user pans a zoomed image and then resizes the floating window (or the compositor changes a tiled window's size),
baseImageRect()and the viewport change whileviewOffset_remains valid only for the old geometry, which can leave blank gaps or make content unreachable. Add a resize handler that reclamps the view offset (and refreshes any geometry-dependent editor state).
src/main.cpp:483 - Finding a tiled client is treated as success even though all float/resize/center command results are discarded. The watcher then stops its retry loop, so a transient or permanent dispatch failure leaves the editor tiled or incorrectly sized despite the ten-attempt fallback. Keep probing until
clientsreports the expected floating state, and propagate/checkhyprctlexit status before considering placement complete.
- Files reviewed: 13/13 changed files
- Comments generated: 1
- Review effort level: Balanced
| static const QRegularExpression name(QStringLiteral("^edit-[0-9]+-[0-9a-f]{16}\\.png$")); | ||
| if (runtime.isEmpty() || file.absolutePath() != runtime || | ||
| !name.match(file.fileName()).hasMatch()) |
Adds a normal-window presentation so annotations can sit beside the document, chat, or app they came from, while capture selection remains fullscreen.
[editor] mode = windowopens file edits in a normal compositor window,--editor window|overlayoverrides it per launch, andWhands a live edit between presentations. The working document and operation log preserve the selection, layers, and undo history. Fresh captures still select in the overlay, then hand off when editing begins.[editor] window = tiledopts out. The default mat and capture shadow separate image from chrome,[editor] backdrop = translucentkeeps the overlay dim, and an opacity rule avoids desktop-wide transparency affecting the editor.Smoke coverage includes window sizing, config and compositor rules, popover anchoring, pixel-level zoom framing, and a full handoff round trip covering selection, annotations, and undo. This overlaps #82 only in the early
argvscan that chooses shell integration; whichever merges second should fold in the other flag.