atagen

monolith

public
Star0
Watch0
96942b9aee390d3dbfc8426db36fd966fe0aef84
1
ssh://brewery@pls.lobotomise.me:33/atagen/monolith.git

Blame: design/review-2026-10-19-followup.md

7c3251ae504eatagen2026-10-19 10:16 UTC1
# Follow-up review of the 2026-10-19 session
7c3251ae504eatagen2026-10-19 10:16 UTC2
7c3251ae504eatagen2026-10-19 10:16 UTC3
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.
7c3251ae504eatagen2026-10-19 10:16 UTC4
7c3251ae504eatagen2026-10-19 10:16 UTC5
## Backend, power and pacing
7c3251ae504eatagen2026-10-19 10:16 UTC6
7c3251ae504eatagen2026-10-19 10:16 UTC7
High
7c3251ae504eatagen2026-10-19 10:16 UTC8
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.
7c3251ae504eatagen2026-10-19 10:16 UTC9
7c3251ae504eatagen2026-10-19 10:16 UTC10
Medium
7c3251ae504eatagen2026-10-19 10:16 UTC11
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.
7c3251ae504eatagen2026-10-19 10:16 UTC12
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.
7c3251ae504eatagen2026-10-19 10:16 UTC13
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.
7c3251ae504eatagen2026-10-19 10:16 UTC14
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`.
7c3251ae504eatagen2026-10-19 10:16 UTC15
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.
7c3251ae504eatagen2026-10-19 10:16 UTC16
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.
7c3251ae504eatagen2026-10-19 10:16 UTC17
7c3251ae504eatagen2026-10-19 10:16 UTC18
Low
7c3251ae504eatagen2026-10-19 10:16 UTC19
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.
7c3251ae504eatagen2026-10-19 10:16 UTC20
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.
7c3251ae504eatagen2026-10-19 10:16 UTC21
10. `frame_cpu_ns` grows without bound when frames render without vblanks (empty-frame loops, failing retries).
7c3251ae504eatagen2026-10-19 10:16 UTC22
11. `shown_windows` runs twice per frame and `render_surface` arranges a third time. Compute once and pass it down.
7c3251ae504eatagen2026-10-19 10:16 UTC23
12. The signal handler does not save errno. SIGHUP and a repeated SIGTERM are not handled.
7c3251ae504eatagen2026-10-19 10:16 UTC24
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.
7c3251ae504eatagen2026-10-19 10:16 UTC25
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.
7c3251ae504eatagen2026-10-19 10:16 UTC26
15. Stray tracked files in the repo root, `monolith.log` and `write`, are old run logs from 2026-10-17.
7c3251ae504eatagen2026-10-19 10:16 UTC27
7c3251ae504eatagen2026-10-19 10:16 UTC28
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.
7c3251ae504eatagen2026-10-19 10:16 UTC29
7c3251ae504eatagen2026-10-19 10:16 UTC30
## Windows, layout, rules and config
7c3251ae504eatagen2026-10-19 10:16 UTC31
7c3251ae504eatagen2026-10-19 10:16 UTC32
High
7c3251ae504eatagen2026-10-19 10:16 UTC33
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.
7c3251ae504eatagen2026-10-19 10:16 UTC34
7c3251ae504eatagen2026-10-19 10:16 UTC35
Medium
7c3251ae504eatagen2026-10-19 10:16 UTC36
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.
7c3251ae504eatagen2026-10-19 10:16 UTC37
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.
7c3251ae504eatagen2026-10-19 10:16 UTC38
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.
7c3251ae504eatagen2026-10-19 10:16 UTC39
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.
7c3251ae504eatagen2026-10-19 10:16 UTC40
6. Hide to tray leaves no close ghost, because smithay clears the textures on a null-buffer commit before `last_frame` runs.
7c3251ae504eatagen2026-10-19 10:16 UTC41
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.
7c3251ae504eatagen2026-10-19 10:16 UTC42
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.
7c3251ae504eatagen2026-10-19 10:16 UTC43
9. Startup config warnings are only logged, and IPC reports warnings only for an IPC-issued reload, not a reload from a bind.
7c3251ae504eatagen2026-10-19 10:16 UTC44
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.
7c3251ae504eatagen2026-10-19 10:16 UTC45
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()`.
7c3251ae504eatagen2026-10-19 10:16 UTC46
7c3251ae504eatagen2026-10-19 10:16 UTC47
Low
7c3251ae504eatagen2026-10-19 10:16 UTC48
- `floats()` floats any window with a max size even when the max exceeds the output. Compare against the tile rect. (verify)
7c3251ae504eatagen2026-10-19 10:16 UTC49
- Fixed-size dialogs use their current geometry rather than the fixed size, so one that has not drawn gets the 60% default.
7c3251ae504eatagen2026-10-19 10:16 UTC50
- Reclassified windows never get `rule.size`.
7c3251ae504eatagen2026-10-19 10:16 UTC51
- A session lock during a resize drag clears the hold without `apply_layout`, leaving tiles stretched until a later layout pass.
7c3251ae504eatagen2026-10-19 10:16 UTC52
- A drag can outlive its window or output and write a float rect in a stale coordinate space.
7c3251ae504eatagen2026-10-19 10:16 UTC53
- `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.
7c3251ae504eatagen2026-10-19 10:16 UTC54
- The duplicate-bind warning says an earlier bind wins, but the table is a BTreeMap, so earlier means alphabetical.
7c3251ae504eatagen2026-10-19 10:16 UTC55
- `realign` is dead since the Space origin fix, and its doc is stale. A redundant `placed_by_rule = false` in `release_hold`.
7c3251ae504eatagen2026-10-19 10:16 UTC56
- `WindowState`'s six booleans form an implicit state machine with unreachable combinations. An enum would make it explicit.
7c3251ae504eatagen2026-10-19 10:16 UTC57
- `windows.rs` mixes mapping, natural-size negotiation, rule placement and ghost capture.
7c3251ae504eatagen2026-10-19 10:16 UTC58
- The fullscreen toggle exists in three diverging copies (xdg, X11, toplevels).
7c3251ae504eatagen2026-10-19 10:16 UTC59
- Some doc comments restate their item.
7c3251ae504eatagen2026-10-19 10:16 UTC60
- `x11_drag` lacks the same-client check the xdg path has.
7c3251ae504eatagen2026-10-19 10:16 UTC61
- Scratchpad drags are janky, since arrange recentres them every pass. Moves do nothing and resizes track at half speed.
7c3251ae504eatagen2026-10-19 10:16 UTC62
- Pre-existing: xdg-activation tokens are never removed and accumulate.
7c3251ae504eatagen2026-10-19 10:16 UTC63
7c3251ae504eatagen2026-10-19 10:16 UTC64
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.
7c3251ae504eatagen2026-10-19 10:16 UTC65
7c3251ae504eatagen2026-10-19 10:16 UTC66
## Scale, scene and renderer
7c3251ae504eatagen2026-10-19 10:16 UTC67
7c3251ae504eatagen2026-10-19 10:16 UTC68
Medium
7c3251ae504eatagen2026-10-19 10:16 UTC69
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`.
7c3251ae504eatagen2026-10-19 10:16 UTC70
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.
7c3251ae504eatagen2026-10-19 10:16 UTC71
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.
7c3251ae504eatagen2026-10-19 10:16 UTC72
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.
7c3251ae504eatagen2026-10-19 10:16 UTC73
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.
7c3251ae504eatagen2026-10-19 10:16 UTC74
7c3251ae504eatagen2026-10-19 10:16 UTC75
Low
7c3251ae504eatagen2026-10-19 10:16 UTC76
- 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.
7c3251ae504eatagen2026-10-19 10:16 UTC77
- `follow_pointer_scale` clones an `Output` and allocates its name on every motion event.
7c3251ae504eatagen2026-10-19 10:16 UTC78
- `send_scales` skips popups, which keep a stale scale after a scale change or output move until they next commit.
7c3251ae504eatagen2026-10-19 10:16 UTC79
- 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.
7c3251ae504eatagen2026-10-19 10:16 UTC80
- (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.
7c3251ae504eatagen2026-10-19 10:16 UTC81
- (verify) The DnD icon is drawn over the lock screen, and locking does not cancel the DnD grab.
7c3251ae504eatagen2026-10-19 10:16 UTC82
- Readback staging returns to the pool after a fence-wait timeout while a hung GPU may still write it. Keep it in `Pending`.
7c3251ae504eatagen2026-10-19 10:16 UTC83
- Non-coherent flush and invalidate use `WHOLE_SIZE`, covering neighbouring suballocations. Use the size rounded to the atom.
7c3251ae504eatagen2026-10-19 10:16 UTC84
- 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.
7c3251ae504eatagen2026-10-19 10:16 UTC85
- `reap` runs per shm commit and again after every dispatch, each time walking texture maps and locking the staging pool.
7c3251ae504eatagen2026-10-19 10:16 UTC86
- `malloc_trim(0)` runs on the main thread before `flush_clients`, delaying client events by a few milliseconds when it fires.
7c3251ae504eatagen2026-10-19 10:16 UTC87
- The CSD shadow inset and drawn size round separately, so the crop or strip decision can be off by a pixel.
7c3251ae504eatagen2026-10-19 10:16 UTC88
- `capture.rs` says a window is captured with its popups, but `render_elements` returns only the surface tree.
7c3251ae504eatagen2026-10-19 10:16 UTC89
- `window_elements` and `effect_elements` take scale and opacity positionally under clippy expects. A small placement struct would remove both.
7c3251ae504eatagen2026-10-19 10:16 UTC90
7c3251ae504eatagen2026-10-19 10:16 UTC91
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.
7c3251ae504eatagen2026-10-19 10:16 UTC92
7c3251ae504eatagen2026-10-19 10:16 UTC93
## Input, popups and selections
7c3251ae504eatagen2026-10-19 10:16 UTC94
7c3251ae504eatagen2026-10-19 10:16 UTC95
High
7c3251ae504eatagen2026-10-19 10:16 UTC96
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.
7c3251ae504eatagen2026-10-19 10:16 UTC97
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.
7c3251ae504eatagen2026-10-19 10:16 UTC98
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.
7c3251ae504eatagen2026-10-19 10:16 UTC99
7c3251ae504eatagen2026-10-19 10:16 UTC100
Medium
7c3251ae504eatagen2026-10-19 10:16 UTC101
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`.
7c3251ae504eatagen2026-10-19 10:16 UTC102
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.
7c3251ae504eatagen2026-10-19 10:16 UTC103
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.
7c3251ae504eatagen2026-10-19 10:16 UTC104
7. Mod+mouse ignores shortcut inhibition, so a VM that inhibited Super still loses Super+click.
7c3251ae504eatagen2026-10-19 10:16 UTC105
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.
7c3251ae504eatagen2026-10-19 10:16 UTC106
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.
7c3251ae504eatagen2026-10-19 10:16 UTC107
10. (decision) The selection-access setting gates reads only. Any X11 client can overwrite the Wayland clipboard.
7c3251ae504eatagen2026-10-19 10:16 UTC108
7c3251ae504eatagen2026-10-19 10:16 UTC109
Low
7c3251ae504eatagen2026-10-19 10:16 UTC110
- `realign` is dead since the Space origin fix and runs on every commit.
7c3251ae504eatagen2026-10-19 10:16 UTC111
- (verify) An Xwayland crash leaves a stale X11-owned selection.
7c3251ae504eatagen2026-10-19 10:16 UTC112
- (verify) A popup holding keyboard focus releases an active pointer lock, because the constraint check compares root surfaces.
7c3251ae504eatagen2026-10-19 10:16 UTC113
- The tiled split drag uses incremental deltas, so clamping makes it drift from the pointer.
7c3251ae504eatagen2026-10-19 10:16 UTC114
- Locking mid-drag clears the resize hold without `apply_layout`, leaving windows stretched.
7c3251ae504eatagen2026-10-19 10:16 UTC115
- A window closing mid-drag leaves the drag and hold set until release.
7c3251ae504eatagen2026-10-19 10:16 UTC116
- The hover resize cursor can replace the DnD cursor over a ring.
7c3251ae504eatagen2026-10-19 10:16 UTC117
- Popups straddling an output edge are clickable where they are not drawn.
7c3251ae504eatagen2026-10-19 10:16 UTC118
- The cursor cache keeps every size and frame of 17 shapes, a few MB. A failed import is cached forever.
7c3251ae504eatagen2026-10-19 10:16 UTC119
- `Edges::from_xdg` and `from_x11` duplicate one table. `fullscreen_at` is a layout query living in `popups.rs`. `input.rs` is 736 lines.
7c3251ae504eatagen2026-10-19 10:16 UTC120
- A 130-column comment in `xwayland.rs`, and several modules over the comment budget.
7c3251ae504eatagen2026-10-19 10:16 UTC121
- The nested backend's keyboard leave does not end a Mod drag.
7c3251ae504eatagen2026-10-19 10:16 UTC122
7c3251ae504eatagen2026-10-19 10:16 UTC123
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.
7c3251ae504eatagen2026-10-19 10:16 UTC124
7c3251ae504eatagen2026-10-19 10:16 UTC125
## Also found while merging
7c3251ae504eatagen2026-10-19 10:16 UTC126
- 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.