Skip to content

screenshots via keyboard only nvim motions - #118

Open
ilyaZar wants to merge 8 commits into
omacom:mainfrom
ilyaZar:feat-keyboard-region-capture
Open

screenshots via keyboard only nvim motions #118
ilyaZar wants to merge 8 commits into
omacom:mainfrom
ilyaZar:feat-keyboard-region-capture

Conversation

@ilyaZar

@ilyaZar ilyaZar commented Sep 6, 2026

Copy link
Copy Markdown

hey guys thank you for the tool

given a feasability check, I'd like to work on this feature; the annoyance is that I have to leave my fingers from the keyboard and take the mouse to do screenshots, and I dont think this is necessary

this adds keyboard control to the capture overlay, using the existing Wayland virtual pointer and output path.

  • sttart the pointer at the focused monitor's center
  • hold hjkl to glide, with acceleration and shift for fine movement
  • pess enter to capture the window under the pointer
  • hold ctrl with hjkl to draw from a fixed corner
  • release ctrl to capture; releasing direction keys only pauses movement

Esc cancels the rectangle. Focus loss cancels keyboard movement and selection. Auto-repeat, a lone ctrl release, and zero-width or zero-height regions do not trigger a capture. --copy --save exports immediately; normal launches open
the editor after selection.

Validation: make check, plus live Hyprland checks on landscape and rotated
portrait monitors for pointer centering, movement, window capture, and region
capture. Saved PNGs were compared byte-for-byte with the clipboard output.

Copilot AI balanced review requested due to automatic review settings September 6, 2026 17:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It adds a new threaded Wayland pointer component and reworks the editor's core input path, which warrants human verification against live compositor behavior beyond the headless smoke coverage.

Pull request overview

This PR adds full keyboard control to the capture overlay so a region or window can be selected without a mouse. It reuses the existing output-bound zwlr_virtual_pointer_v1 (previously used only by scroll capture) to warp the real pointer, and drives selection through the editor's existing mouse-event path via synthetic events. The pointer starts at the focused monitor's center; hjkl glides with acceleration (Shift for fine motion), Enter captures the window under the pointer, and Ctrl+hjkl draws a region committed on Ctrl release. This fits Omasnap's "fast capture" principle and keeps the main thread non-blocking by running the Wayland pointer on a worker thread.

Changes:

  • New CapturePointer (declared in capture-pointer.hpp, implemented in scroll-inject.cpp) drives an output-bound virtual pointer on a dedicated worker thread with a mutex/condition-variable handoff.
  • Editor gains keyboard motion (timer-driven glide + acceleration), window-under-pointer Enter capture, and Ctrl+hjkl fixed-anchor region drawing, with cancellation on Esc/focus loss and guards against auto-repeat and zero-size regions.
  • Documentation (README) and a comprehensive headless smoke test (runKeyboardCaptureSmoke) cover the new behavior.
File summaries
File Description
src/capture-pointer.hpp New RAII interface wrapping an output-bound Wayland virtual pointer.
src/scroll-inject.cpp Implements CapturePointer worker thread and thread-safe moveTo/shutdown.
src/editor.hpp New members, timers, and overrides (showEvent/hideEvent/focusOutEvent) for keyboard capture.
src/editor.cpp Key/motion handling, pointer centering, region/window selection, cancellation, and hotkey legend.
tests/editor-smoke.cpp Adds runKeyboardCaptureSmoke covering glide, window capture, region draw, and edge cases.
README.md Documents the new keyboard capture controls.
Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/editor.cpp
update();
}
pointerKeys_.insert(event->key());
keyboardFineMotion_ = event->modifiers().testFlag(Qt::ShiftModifier);
@ilyaZar

ilyaZar commented Sep 6, 2026

Copy link
Copy Markdown
Author

so atm, it blocks my mouse, but you get the idea

likely this stuff will feel rather different on each machine:

  • I would add it as an optional setting that can be invoked
  • let a couple of users maybe do feedback on it, so to probe a larger amount of different devices
  • a couple of redesign / refactor stuff to fine tune and get out the edges of the implementaion

currently it feels good on my machine, but this thing is likely the case where "feels good on my machine" will require some fine tuning

I very much like the feature, bc. I would want to get rid of the damn mouse fully, and for screenshots I cant

@ilyaZar ilyaZar changed the title feat: capture screenshots with hjkl and ctrl screenshots via keyboard only nvim motions Sep 6, 2026
Copilot AI review requested due to automatic review settings September 7, 2026 02:50
@ilyaZar
ilyaZar marked this pull request as ready for review September 7, 2026 02:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It is a substantial new input-handling feature that spawns a Wayland pointer thread, synthesizes mouse events, warps the physical cursor on every launch, and changes existing Enter/window-capture semantics, which warrants human review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

tests/editor-smoke.cpp:7703

  • This is the only smoke check in main() that logs its failure with qCritical(); every other check (e.g. runAreaLastRegionSmoke at line 7707, runChromeFontCheck at line 7727, and ~50 others) uses qWarning().noquote(). For consistency with the surrounding harness, prefer qWarning() here.
  • Files reviewed: 6/6 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Copilot AI review requested due to automatic review settings September 7, 2026 03:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It introduces a threaded Wayland input path with a concurrency concern (UI thread can block on the worker's mutex during wl_display_flush) that needs human judgment on the intended locking model.

Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread src/scroll-inject.cpp
Comment on lines +456 to +470
const QPointF position = state->position;
state->pending = false;
// Fractions map to the bound output, including its scale and rotation.
constexpr uint32_t extent = 1000000;
zwlr_virtual_pointer_v1_motion_absolute(
pointer->pointer, pointer->timeMs(),
static_cast<uint32_t>(std::clamp(position.x(), 0.0, 1.0) * extent),
static_cast<uint32_t>(std::clamp(position.y(), 0.0, 1.0) * extent),
extent, extent);
zwlr_virtual_pointer_v1_frame(pointer->pointer);
if (wl_display_flush(pointer->display) < 0) {
qWarning() << "capture pointer: Wayland motion failed";
return;
}
}
Copilot AI review requested due to automatic review settings September 7, 2026 14:03
@ilyaZar

ilyaZar commented Sep 7, 2026

Copy link
Copy Markdown
Author

@tobi @jondkinney whoever reads the code here

the clanker is hallucinating bs. as far as I can tell

https://wayland.freedesktop.org/docs/html/apb.html says explicitly says wl_display_flush() never blocks

if it cannot send everything immediately, it returns EAGAIN (try again) instead of waiting for the desktop; the interface can still wait for that shared lock so releasing it before sending movement is fine

releasing the shared lock before sending the pointer movement is correct tho, see 671b53c

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It combines worker-thread Wayland pointer injection, a nontrivial keyboard input state machine, and a broad behavioral change (warping the physical cursor to monitor center on every capture) that warrants human verification on real Hyprland hardware.

Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants