feat(pin): pinned shots are floating compositor windows - #82
Open
jondkinney wants to merge 10 commits into
Open
Conversation
jondkinney
force-pushed
the
feat/pin-window
branch
from
August 23, 2026 20:10
25b0986 to
eabe2fa
Compare
This was referenced Aug 23, 2026
jondkinney
force-pushed
the
feat/pin-window
branch
3 times, most recently
from
August 27, 2026 04:57
0c8c7a4 to
8f91465
Compare
A pin was an overlay layer surface, positioned by margins and moved by hand-tracking the pointer. It is now a normal frameless window that asks the compositor to float it, keep it on every workspace, and pack it snugly into the bottom-right column, one gap above whatever is already there. The compositor draws the frame and shadow, its own move gesture handles dragging, and the pin appears in window lists like anything else. Placement goes through dispatchers, not window rules, so nothing has to live in the user's config. On a Lua-configured Hyprland the dispatch argument is a Lua expression addressed at this pin's unique title (the classic grammar parses as Lua and fails while reporting success); sway gets the criteria grammar; any other compositor still shows a working pin and just decides the position itself. Positions are read back from the compositor's client list instead of lock files: a count cannot tell a crashed pin from a dragged-away one, and every pin on screen blocks the space it covers, whatever its shape, so a new pin never lands on one the user placed. When a pin closes or is dragged out of the column, the survivors pack back down to close the gap.
A fixed 250x200 frame letterboxed most captures. The frame is now the display's aspect at a fixed width, clamped so a tall pivot or an ultrawide still yields a pin rather than a line, with 16:9 as the guess when the display cannot be asked. The picture fills the whole window, cover-cropped and anchored to the top: a capture's top is its title bar and tabs, which is what identifies the shot, and a mat of our own would be a second frame inside the compositor's. The controls scale to fit.
The old layer surface could not reliably map a tooltip toplevel, so the hint was painted inside the window at a fixed spot, where only the button next to it read as having one. An ordinary window can use real tooltips at the cursor; anchored to each control's rect so the tip stays while the cursor is on the button, and the always-show attribute set because a pin is deliberately never the active window, which is the only kind Qt tips by default.
While a drag hovers the column, the other pins step aside around a hole where the dragged one would land, and it snaps into the hole on release. Any part of the window over the column band joins the stack; the band is the bounding box of every pin and would-be seat, including the open seat on top, so a pin nudged off the top snaps back to its own place until it has been dragged fully past it. Fully outside the band, the drag is just a move and the column packs itself behind it. The dragged pin's place in the order comes from its center against the column as it would pack, not the already-spread live positions, so the preview does not chase its own moves; and each move is dispatched once, against what was last commanded rather than the live mid-animation rect, so the slide plays out instead of restarting every poll.
The end of a compositor-side move has to be inferred: the client never hears it directly. Stillness alone concluded drags early, and a snap dispatched while the button was still down fought the user's grab. Now the release is read three ways, best first: the mouse event devices push the button-up the instant it happens when permissions allow; the first pointer event after the grab began is the release too, since the grab starves the window of them; and a long stillness timeout remains for sessions where neither source exists. The watch polls faster while it lasts, and holding a pin still mid-drag stays a drag.
jondkinney
force-pushed
the
feat/pin-window
branch
from
September 5, 2026 16:46
8f91465 to
93b4929
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Blocking UI-thread IPC, incorrect multi-monitor geometry, racy placement, and the out-of-scope Sway backend must be addressed.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Converts pinned screenshots from layer surfaces into compositor-managed floating windows with dynamic packing and drag-to-stack behavior.
Changes:
- Adds compositor-driven placement, movement, and drag snapping.
- Introduces display-shaped frames and native tooltips.
- Replaces persistent slot locks with compositor geometry queries.
File summaries
| File | Description |
|---|---|
src/main.cpp |
Selects the appropriate Wayland shell integration for pins. |
src/pin.cpp |
Implements floating pin windows and compositor coordination. |
src/pin.hpp |
Updates pinned-window documentation. |
src/pin-layout.cpp |
Adds packing, insertion, sizing, and dispatch helpers. |
src/pin-layout.hpp |
Declares the new layout API. |
src/pin-file.cpp |
Removes slot-lock implementation. |
src/pin-file.hpp |
Removes the slot-lock interface. |
tests/pin-layout-smoke.cpp |
Expands layout and dispatch coverage. |
tests/pin-lifecycle-smoke.cpp |
Removes obsolete slot-lock tests. |
Review details
Suppressed comments (3)
src/pin.cpp:85
runForOutput()blocks the GUI thread for up to one second. After the pin is visible it is called by the 50/80 ms timers and input handlers, often twice per drag poll, so a slowhyprctlcan freeze painting while spawning roughly 25 subprocesses per second. This directly violatesdocs/threading.md:73-82,119-125; move compositor IPC to a worker/asynchronousQProcessand apply completed snapshots on the GUI thread.
QString runForOutput(const QString &program, const QStringList &arguments) {
QProcess process;
process.start(program, arguments);
if (!process.waitForFinished(1000))
return {};
return QString::fromUtf8(process.readAllStandardOutput());
src/pin.cpp:115
- Only returning the focused monitor's size loses its compositor-global origin and ignores Hyprland transforms. Client
atcoordinates and absolute move dispatches are global, so a focused monitor at(1920,0)still generates positions nearx=0and moves pins onto the wrong output; rotated outputs also get the wrong shape. Preserve the logicalQRect(including transformed dimensions, as incapture.cpp:359-368) and make packing/filtering offset-aware.
const double scale =
std::max(0.0001, monitor.value(QStringLiteral("scale")).toDouble(1.0));
return {qRound(monitor.value(QStringLiteral("width")).toDouble() / scale),
qRound(monitor.value(QStringLiteral("height")).toDouble() /
scale)};
src/pin.cpp:224
- Placement is now a read-compute-move sequence shared by independent pin processes, with no reservation or lock. Two pins mapping concurrently can both observe the same free corner and dispatch the same target, recreating the overlap that the removed slot locks prevented. Serialize placement across processes and re-query/reserve the selected spot before moving.
QPoint nextPinPosition(Desktop desktop, const QSize &screen,
const QSize &frame, const QString &ownTitle) {
QVector<QRect> blockers;
for (const CompositorPin &pin : compositorPinRects(desktop)) {
if (pin.title != ownTitle)
blockers.push_back(pin.rect);
}
return pinPackedPosition(blockers, screen, frame, kPinGap,
qRound(kCornerMargin));
- Files reviewed: 9/9 changed files
- Comments generated: 3
- 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 on lines
+21
to
+22
| int x = screenSize.width() - margin - frame.width(); | ||
| for (int column = 0; column < 8; ++column) { |
Comment on lines
+70
to
+76
| enum class Desktop { Hyprland, Sway, Unknown }; | ||
|
|
||
| Desktop detectDesktop() { | ||
| if (qEnvironmentVariableIsSet("HYPRLAND_INSTANCE_SIGNATURE")) | ||
| return Desktop::Hyprland; | ||
| if (qEnvironmentVariableIsSet("SWAYSOCK")) | ||
| return Desktop::Sway; |
Comment on lines
+738
to
+745
| // A normal window, floated and pinned through the compositor, instead of | ||
| // a layer surface: the compositor draws its frame, moves it, and keeps it | ||
| // on every workspace. Not immediately though: showing is this side's word | ||
| // for mapped, and the compositor has not necessarily registered the | ||
| // window under its title yet; dispatches sent then report success and do | ||
| // nothing, which leaves a pin centered and unpinned. Retry until the | ||
| // client list has it, then float first (a tiled window has no position of | ||
| // its own to set), pin it, and drop it into the lowest free slot. |
There was a problem hiding this comment.
🟡 Changes recommended
Valid --pin=<path> invocations use the wrong shell integration, and startup synchronously blocks on hyprctl.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 3
- Review effort level: Balanced
Comment on lines
+106
to
+108
| bool pinInvocation = false; | ||
| for (int index = 1; index < argc; ++index) | ||
| pinInvocation = pinInvocation || qstrcmp(argv[index], "--pin") == 0; |
| } | ||
|
|
||
| PinWindow window(std::move(image), path); | ||
| const QRect screen = compositorScreenRect(); |
Comment on lines
+174
to
+176
| // The dispatch expressions are Lua for a Lua-configured Hyprland and the | ||
| // a placement that silently does | ||
| // nothing is exactly the failure these guard. |
There was a problem hiding this comment.
🟡 Changes recommended
Argument detection, negative-coordinate packing, and failed dispatch handling can produce incorrect pin behavior.
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/pin-layout.cpp:37
- Using
-1as the “no intersection” sentinel breaks packing for any blocker whose top is negative. Such rectangles are valid here when a pin is dragged partly off an output or overlaps from an adjacent output; an intersecting blocker aty < 0leaveslowestTopat-1, so the candidate is incorrectly returned on top of it. Track whether an intersection was found separately from the coordinate value.
src/pin.cpp:187 move()reports success after merely saving the reservation becausehyprDispatch()discards timeout, startup, and non-zero-exit failures. Ifhyprctl dispatchfails, the JSON now reserves a position the window never reached for five seconds, initial placement stops retrying, and later pins pack around that ghost target. Propagate the process result and roll back (or avoid committing) the reservation when the dispatch fails.
- Files reviewed: 14/14 changed files
- Comments generated: 2
- Review effort level: Balanced
Comment on lines
+111
to
+112
| const bool pinInvocation = startupParser.parse(rawArguments) && | ||
| startupParser.isSet(QStringLiteral("pin")); |
Comment on lines
+77
to
+80
| const QCommandLineOption pinOption( | ||
| QStringLiteral("pin"), | ||
| QStringLiteral("Show an image as a pinned always-visible layer."), | ||
| QStringLiteral("path")); |
There was a problem hiding this comment.
🟡 Changes recommended
Wrapped columns are not managed correctly, unrelated windows can be moved by title matching, and notifier cleanup risks a release-time crash.
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/pin.cpp:137
- Matching pins solely by a title prefix allows any unrelated window whose title begins with
omasnap-pinto enter the layout. Compaction or insertion can then dispatch moves to that window. Filterhyprctl clientsby Omasnap's app/class identity as well, and include that identity in dispatch selectors so title collisions cannot target another application.
src/pin.cpp:236 - Only windows in the right-edge column satisfy
pinInColumn; every wrapped column is put inblockers. Consequently, once packing creates a second column, closing or dragging one of its pins never compacts that column, and insertion previews cannot target it. Select the active column from the dragged/excluded pin (or compact each packed column) instead of hard-coding the monitor's right edge.
- Files reviewed: 14/14 changed files
- Comments generated: 1
- Review effort level: Balanced
Comment on lines
+503
to
+506
| void closeButtonWatch() { | ||
| for (const auto &[fd, notifier] : buttonWatches_) { | ||
| delete notifier; | ||
| ::close(fd); |
There was a problem hiding this comment.
🟡 Changes recommended
Free drops can be incorrectly repacked, and the documented non-Lua dispatcher path is absent.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 2
- Review effort level: Balanced
Comment on lines
+164
to
+170
| QString pinFloatDispatch(const QString &address) { | ||
| return QStringLiteral("hl.dsp.window.float({ %1 })") | ||
| .arg(windowSelector(address)); | ||
| } | ||
|
|
||
| QString pinPinDispatch(const QString &address) { | ||
| return QStringLiteral("hl.dsp.window.pin({ %1 })").arg(windowSelector(address)); |
| if (!snapSpot_.isNull()) | ||
| movePin(windowTitle(), snapSpot_); | ||
| else | ||
| compactPinColumn(QString(), dragScreen_); |
There was a problem hiding this comment.
🟡 Changes recommended
Transient compositor failures can leave pins unpinned or leave a dragged stack in an inconsistent layout.
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/pin.cpp:852
- A single failed or timed-out initial
hyprctl monitorscall returns an empty screen and exits here without ever starting the placement retries. The already-visible window then remains an ordinary, unpinned compositor window for its entire lifetime. Retry the monitor lookup (or let each placement attempt resolve the monitor) before abandoning setup.
tests/pin-layout-smoke.cpp:127 - This says half-overlap is required, but
pinInsertionPlan()usesQRect::intersects()and thegrazingcase below intentionally accepts a much smaller overlap. Update the comment so the test documents the actual any-overlap threshold.
- Files reviewed: 14/14 changed files
- Comments generated: 1
- Review effort level: Balanced
Comment on lines
+553
to
+556
| if (!snapSpot_.isNull()) | ||
| movePin(windowTitle(), snapSpot_); | ||
| else | ||
| compactPinColumn(windowTitle(), dragScreen_); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A pinned shot was an overlay layer surface: always on top of everything, invisible to window lists, and moved by hand-tracking the pointer. This makes it an ordinary frameless window that asks the compositor to float it, pin it to every workspace, and place it. The compositor draws the frame and shadow, its own move gesture handles dragging, and pins pack snugly up the bottom-right corner, flowing around anything you have dragged elsewhere so a new pin never covers one you placed.
Placement targets exact compositor window addresses after filtering clients by Omasnap’s application class. It uses Lua dispatch on Omarchy’s Hyprland, with no compositor configuration required. Other compositors and the legacy non-Lua dispatcher are outside this PR’s supported scope. Geometry comes from the compositor, with short-lived target reservations under a shared lock to keep concurrent pins from claiming the same spot while moves animate. Failed dispatches roll back their reservations and retry.
On top of the window change: the pin is shaped like the display at a fixed width, with the picture filling the whole frame, cover-cropped to the top since a capture's top is what identifies it. The controls have real tooltips now that a tooltip can map. And dragging a pin over the stack makes the others step aside around the spot where it would land, snapping in on release; the release is read from the mouse device when permissions allow, with pointer events and a stillness timeout as fallbacks, so the snap is immediate.
The headless smoke suite covers packing, wrapped-column membership and insertion, the display-shaped frame, Qt startup arguments, and dispatcher syntax. Validation:
make checkpasses. Simulated compositor tests also cover concurrent pins on a rotated/scaled offset monitor, failed dispatch rollback and retry, and unrelated applications using matching titles.