atagen

monolith

public
Star0
Watch0
e27a31fac80d6c681920e4212a42b8cc3105dc0b
1
ssh://brewery@pls.lobotomise.me:33/atagen/monolith.git
design/review-2026-10-19-followup.mdMarkdown126 lines14.9 KB

Follow-up review of the 2026-10-19 session

Four reviewers read the session’s diff (189b5d01..a1f375a8) by area. Items are as reported, lightly edited. Those marked (verify) rest on runtime behaviour the reviewer could not check. Confirmed items were rechecked against the code by the lead.

Backend, power and pacing

High

  1. (confirmed) Powering outputs on after an idle blank re-blanks them about 1 ms later. set_output_power(on) calls wake_idle, which re-arms the idle timer without touching last_activity. This hits msg power on and wlr-output-power clients. Touch activity when powering on.

Medium 2. A closed lid leaves the eDP in the layout and Space, just dark. The pointer can enter it, windows can map there and mouse-follows-focus can warp onto it. Remove and restore the layout output on lid transitions, as sway does. 3. Pointer warps count as user activity (input.rs, activity() in pointer_motion for Motion::Warp). Background maps and constraint hint warps wake blanked outputs and reset idle-notify. Skip activity() for warps. 4. (verify) ActivateSession resets queued without frame_submitted(). A flip in flight at pause can leave a stale pending_frame in DrmCompositor, so the first frame after resume waits for the 100 ms watchdog. Drain with frame_submitted() on activate. 5. (verify) FIFO barriers are latched only for windows and layers. Lock surfaces, the DnD icon and cursor surfaces fall back to the 100 ms sweep, about 7 to 10 fps for a fifo-using locker. Use fifo::scanout in lock::send_frames. 6. DrmScanEvent::Changed, which only fires on mode list changes, does a full disconnect and connect. That makes a new wl_output, drops the lock surface on a locked session, re-places windows and forgets an explicit power-off. Update the mode in place, or at least keep power state. 7. Nested treats an idle blank as real. Outputs report off over wlr-output-power and confirm_lock counts them blanked. Skip the idle blank when not on DRM.

Low 8. DPMS wake keeps the stale last_vblank_ns, so the first frame counts as missed and adds 0.5 ms slack for 30 s on every wake. Reset pacing state on power-on. 9. lid_acted is never reset while the lid stays shut, so a wake with the lid closed never suspends again. At boot with the lid closed, the action can run before external outputs are scanned. 10. frame_cpu_ns grows without bound when frames render without vblanks (empty-frame loops, failing retries). 11. shown_windows runs twice per frame and render_surface arranges a third time. Compute once and pass it down. 12. The signal handler does not save errno. SIGHUP and a repeated SIGTERM are not handled. 13. logind: a system bus restart silently drops the inhibitor with no reconnect. Logind::connect can block startup for 2 s. The systemctl fallback is written twice. 14. drm.rs is 1265 lines, Surface has 24 fields, render_surface about 175 lines, and the surfaces.get_mut(&crtc) lookup repeats 9 times. power.rs mixes idle, DPMS and lid. fifo::unblock and idle::root_surface are generic helpers living in feature modules. A few comments restate their field. 15. Stray tracked files in the repo root, monolith.log and write, are old run logs from 2026-10-17.

Checked and fine: dropped presentation feedback sends discarded, timing release cannot loop or spin, hook order for commit timing, VRR re-applied after DPMS wake, timer tokens removed on disconnect and the watchdog reschedules correctly, hotplug retry logic and the 5 s global removal, presentation coverage and clock, fifo release after empty frames, confirm_lock with dark outputs, wlr-output-power second-control failed, dbus build inputs.

Windows, layout, rules and config

High

  1. (confirmed) Launches::find has no age check, since the 120 s TTL is only applied when a new record is added. A terminal spawned in the morning places every window launched from it at the morning’s tag and output, and the owner-ancestor tier added today is never reached. Expire in find, and prefer the owning window once it is mapped.

Medium 2. Transients ignore their parent for output and tag. map_window uses launch_of(pid), whose owner lookup takes the first window of the pid in slot order. A dialog can land on a hidden tag or another output while drawn over its parent. Inherit the parent’s output and tag first. 3. A taskbar activate or an xdg-activation request can focus a window covered by a fullscreen window (activation.rs, toplevels.rs Activate, Layout::summon). Add one focus_unless_covered and mark urgent instead. 4. focus_if_covering lets an unfocused client take the output and the keyboard by requesting fullscreen. Only move focus when the requester already held it, otherwise mark it urgent. 5. Hide to tray calls remove_window, losing floating, sticky, scratchpad, the user’s tag and the float rect, breaking children’s parent links and recreating the foreign-toplevel handle. Keep the layout entry marked unmapped, or stash and restore. 6. Hide to tray leaves no close ghost, because smithay clears the textures on a null-buffer commit before last_frame runs. 7. arrange only recognises a fullscreen window that is in the focus history. A rule fullscreen = true with focus = false, or a fullscreen window mapped on a hidden tag, is drawn tiled while IPC says fullscreen. Make arrange independent of history. 8. (verify) The lenient TOML parse can drop a broken [[rules] or [binds header line, attaching its keys to the previous table, so options silently land in the wrong rule. The reload doc saying a non-TOML file leaves the config untouched is now stale. Never drop header lines, fail or drop the whole section instead. 9. Startup config warnings are only logged, and IPC reports warnings only for an IPC-issued reload, not a reload from a bind. 10. parent_changed after map re-runs rules but not the layout, so a dialog that gains a fullscreen parent through xdg-foreign stays hidden and does not float. 11. float_rect_for and place() do not clamp. A dialog centred on a parent near an edge goes off screen or under the bar. Centring uses area while clamp and scratchpads use usable().

Low

  • floats() floats any window with a max size even when the max exceeds the output. Compare against the tile rect. (verify)
  • Fixed-size dialogs use their current geometry rather than the fixed size, so one that has not drawn gets the 60% default.
  • Reclassified windows never get rule.size.
  • A session lock during a resize drag clears the hold without apply_layout, leaving tiles stretched until a later layout pass.
  • A drag can outlive its window or output and write a float rect in a stale coordinate space.
  • rematch re-runs facts, inhibit updates and a redraw on every title change once effects have been followed. Compare only the fields that can differ.
  • The duplicate-bind warning says an earlier bind wins, but the table is a BTreeMap, so earlier means alphabetical.
  • realign is dead since the Space origin fix, and its doc is stale. A redundant placed_by_rule = false in release_hold.
  • WindowState’s six booleans form an implicit state machine with unreachable combinations. An enum would make it explicit.
  • windows.rs mixes mapping, natural-size negotiation, rule placement and ghost capture.
  • The fullscreen toggle exists in three diverging copies (xdg, X11, toplevels).
  • Some doc comments restate their item.
  • x11_drag lacks the same-client check the xdg path has.
  • Scratchpad drags are janky, since arrange recentres them every pass. Moves do nothing and resizes track at half speed.
  • Pre-existing: xdg-activation tokens are never removed and accumulate.

Checked and fine: stale timers are safe through slotmap generations and serials, natural-size negotiation and glide-once, null-buffer unmap ordering, fullscreen before map, transients above fullscreen and cycle rejection, reclassify before release, locked binds intercept order, autotile fallbacks, float rescale and clamp with zero areas, urgency clearing, rule error handling, the Space origin fix.

Scale, scene and renderer

Medium

  1. A hung GPU still blocks exit. The renderer’s drop skips device_wait_idle when hung, but Device::drop, which runs with the last Arc<Shared>, waits unconditionally. Gate it on hung() and skip destroy_device.
  2. The overshoot strips in window_elements ignore the opacity rule, so a dimmed window with a one-pixel size mismatch gets opaque lines on its right and bottom edge.
  3. PlacedTexture gets a new Id and a constant commit counter every frame, so the strips damage their area every frame at fractional scales and an idle screen keeps flipping. Keep a stable id per window and take the commit from the surface.
  4. (verify) Fullscreen at a fractional scale whose logical size is not an integer (2560 px at 1.5) cannot be exact. The client draws one pixel over, the crop path runs, and that likely blocks direct scanout.
  5. Queued uploads can pin up to 64 MiB (UPLOAD_BATCH_BYTES) of host-visible staging, twice the pool cap, while no frame submits (scanout, DPMS off, VT switch). Each flush then frees half, bringing back the NVIDIA allocation stall the pool avoids. Flush at the pool size or on idle.

Low

  • Cursor size jumps between outputs. At scale 1 the nearest theme image is drawn unstretched (32 px for size 24) while other scales stretch exactly, so the decision to skip the stretch at scale 1 is reversed. Always stretch to the requested size.
  • follow_pointer_scale clones an Output and allocates its name on every motion event.
  • send_scales skips popups, which keep a stale scale after a scale change or output move until they next commit.
  • The scale is sent after the initial configure of toplevels, popups and layers, so a first buffer can be drawn at scale 1 and redrawn. Send it before the configure.
  • (verify) A surface is marked told even when no output was found, so a layer or lock surface on a new output waits for an unrelated layout pass.
  • (verify) The DnD icon is drawn over the lock screen, and locking does not cancel the DnD grab.
  • Readback staging returns to the pool after a fence-wait timeout while a hung GPU may still write it. Keep it in Pending.
  • Non-coherent flush and invalidate use WHOLE_SIZE, covering neighbouring suballocations. Use the size rounded to the atom.
  • The pipeline cache is only saved on drop, so a crash loses the warm-up, and the hung exit path reads cache data from a bad device. Save once after warm-up.
  • reap runs per shm commit and again after every dispatch, each time walking texture maps and locking the staging pool.
  • malloc_trim(0) runs on the main thread before flush_clients, delaying client events by a few milliseconds when it fires.
  • The CSD shadow inset and drawn size round separately, so the crop or strip decision can be off by a pixel.
  • capture.rs says a window is captured with its popups, but render_elements returns only the surface tree.
  • window_elements and effect_elements take scale and opacity positionally under clippy expects. A small placement struct would remove both.

Checked and fine: barrier scopes and foreign queue transfers, LayoutLog commit after submit and requeue on failure, upload batching order and texture lifetimes, blend-off only for truly opaque draws, y-invert, device-lost bookkeeping, the driver environment filter and its thread-safety claim, fractional-scale deduplication and viewport mapping, single-pixel buffers, rotation and capture sizes, layer inset rounding, dmabuf default feedback, import worker join order.

Input, popups and selections

High

  1. (confirmed, security) Locking with a popup grab open leaves the keyboard on the app that owns the menu. lock() never ends existing popup grabs, and smithay’s popup keyboard grab ignores focus changes until it ends, so the password goes to the client. The pointer grab hides the lock surface from the pointer too. End the keyboard and pointer grabs and send popup_done on lock.
  2. The same grab survives binds, tag switches and window closes, so typing keeps going to the old, possibly hidden, client, and sync_focus runs uselessly every dispatch. End the grab when focus moves to another root.
  3. A floating window’s resize ring takes clicks over layer surfaces, popups and X11 menus, including a bar the float sits against. Only take the ring over a tile or nothing.

Medium 4. Popups of transients drawn above a fullscreen window are neither drawn nor hit-tested. Skip only windows that are neither the fullscreen window nor in arr.over. 5. x11_drag accepts any active pointer grab, so an X11 client can start a compositor drag while the user holds a button elsewhere. Require the grab to start on that window. 6. The Latin-1 relay thread blocks forever on a source that never closes, buffers without a cap, and thread::spawn panics on EAGAIN. Cap it and move it to a nonblocking calloop source. 7. Mod+mouse ignores shortcut inhibition, so a VM that inhibited Super still loses Super+click. 8. A Mod drag ends on any button release, so pressing and releasing a second button mid-drag ends it and the later release reaches the client unmatched. 9. (verify) Per motion, fullscreen_at arranges the layout three or four times, window_popup_under allocates per window, and drag_to runs apply_layout including send_scales over every window. Arrange once per event and skip unchanged scale sends. 10. (decision) The selection-access setting gates reads only. Any X11 client can overwrite the Wayland clipboard.

Low

  • realign is dead since the Space origin fix and runs on every commit.
  • (verify) An Xwayland crash leaves a stale X11-owned selection.
  • (verify) A popup holding keyboard focus releases an active pointer lock, because the constraint check compares root surfaces.
  • The tiled split drag uses incremental deltas, so clamping makes it drift from the pointer.
  • Locking mid-drag clears the resize hold without apply_layout, leaving windows stretched.
  • A window closing mid-drag leaves the drag and hold set until release.
  • The hover resize cursor can replace the DnD cursor over a ring.
  • Popups straddling an output edge are clickable where they are not drawn.
  • The cursor cache keeps every size and frame of 17 shapes, a few MB. A failed import is cached forever.
  • Edges::from_xdg and from_x11 duplicate one table. fullscreen_at is a layout query living in popups.rs. input.rs is 736 lines.
  • A 130-column comment in xwayland.rs, and several modules over the comment budget.
  • The nested backend’s keyboard leave does not end a Mod drag.

Checked and fine: popup frame and unconstrain coordinates under scale and rotation, popup grab serials, the Space origin fix and hit-test agreement, hit-test order, xdg drag serial and client checks, resize clamping precedence, pointer constraint relative motion and hint warps, bind interception order, the selection bridge echo and replay, STRING mapping, DnD icon cleanup.

Also found while merging

  • When a modal maps and takes focus by itself, its parent is not put just below it in the focus history, so closing the modal can return focus to an unrelated window.