Lays down the data shapes for the popout state machine ahead of any
transition logic. Pure Python — does not import PySide6, mpv, httpx,
subprocess, or any module that does. The Phase B test suite (commit 3)
will exercise this purity by importing it directly without standing
up a QApplication; the test suite is the forcing function that keeps
the file pure as transitions land in commits 4-11.
Module structure follows docs/POPOUT_ARCHITECTURE.md exactly.
States (6, target ≤10):
AwaitingContent — popout exists, no current media (initial OR mid-nav)
DisplayingImage — static image or GIF on screen
LoadingVideo — set_media called for video, awaiting first frame
PlayingVideo — video active (paused or playing)
SeekingVideo — user-initiated seek pending
Closing — closeEvent received, terminal
Events (17, target ≤20):
Open / ContentArrived / NavigateRequested
VideoStarted / VideoEofReached / VideoSizeKnown
SeekRequested / SeekCompleted
MuteToggleRequested / VolumeSet / LoopModeSet / TogglePlayRequested
FullscreenToggled
WindowMoved / WindowResized / HyprlandDriftDetected
CloseRequested
Effects (14, target ≤15):
LoadImage / LoadVideo / StopMedia
ApplyMute / ApplyVolume / ApplyLoopMode
SeekVideoTo / TogglePlay
FitWindowToContent / EnterFullscreen / ExitFullscreen
EmitNavigate / EmitPlayNextRequested / EmitClosed
Frozen dataclasses for events and effects, Enum for State / MediaKind /
LoopMode. Dispatch uses Python 3.10+ structural pattern matching to
route by event type.
StateMachine fields cover the full inventory:
- Lifecycle: state, is_first_content_load
- Persistent (orthogonal): fullscreen, mute, volume, loop_mode
- Geometry: viewport, pre_fullscreen_viewport, last_dispatched_rect
- Seek: seek_target_ms
- Content snapshot: current_path, current_info, current_kind,
current_width, current_height
- Open-event payload: saved_geo, saved_fullscreen, monitor
- Nav: grid_cols
Read-path query implemented even at the skeleton stage:
compute_slider_display_ms(mpv_pos_ms) returns seek_target_ms while
in SeekingVideo, mpv_pos_ms otherwise. This is the structural
replacement for the 500ms _seek_pending_until timestamp window —
no timestamp, just the SeekingVideo state.
Every per-event handler is a stub that returns []. Real transitions
land in commits 4-11 (priority order: PlayingVideo + LoadingVideo +
EOF race fix → Navigating + AwaitingContent + double-load fix →
SeekingVideo + slider pin → Fullscreen + F11 → viewport + drift →
mute/volume/loop persistence → DisplayingImage + Closing → illegal
transition handler).
Closing is treated as terminal at the dispatch entry — once we're
there, every event returns [] regardless of type. Same property the
current closeEvent has implicitly.
Verification:
- Phase A test suite (16 tests) still passes
- state.py imports cleanly with PySide6/mpv/httpx blocked at the
meta_path level (purity gate)
- StateMachine() constructs with all fields initialized to sensible
defaults
- Stub dispatch returns [] for every event type
- 6 states / 17 events / 14 effects all under budget (≤10/≤20/≤15)
Test cases for state machine tests (Prompt 3 commit 3):
- Construct StateMachine, assert initial state == AwaitingContent
- Assert is_first_content_load is True at construction
- Assert all stub dispatches return []
- Assert compute_slider_display_ms returns mpv_pos when not seeking
Adds a Qt Signal that mirrors mpv's `playback-restart` event. The
upcoming popout state machine refactor (Prompt 3) needs a clean,
event-driven "seek/load completed" edge to drive its SeekingVideo →
PlayingVideo and LoadingVideo → PlayingVideo transitions, replacing
the current 500ms timestamp suppression window in `_seek_pending_until`.
mpv's `playback-restart` fires once after each loadfile (when playback
actually starts producing frames) and once after each completed seek.
Empirically verified by the pre-commit-1 probe in
docs/POPOUT_REFACTOR_PLAN.md: a load + 2 seeks produces exactly 3
events, with `seeking=False` at every event (the event represents the
completion edge, not the in-progress state).
The state machine adapter will distinguish "load done" from "seek done"
by checking the state machine's current state at dispatch time:
- `playback-restart` while in LoadingVideo → VideoStarted event
- `playback-restart` while in SeekingVideo → SeekCompleted event
Implementation:
- One Signal added near the existing play_next / media_ready /
video_size definitions, with a doc comment explaining what fires
it and which state machine consumes it.
- One event_callback registration in `_ensure_mpv` (alongside the
existing observe_property calls). The callback runs on mpv's
event thread; emitting a Qt Signal is thread-safe and the
receiving slot runs on the GUI thread via Qt's default
AutoConnection (sender and receiver in the same thread by the
time the popout adapter wires the connection).
- The decorator-based `@self._mpv.event_callback(...)` form is used
to match the rest of the python-mpv idioms in the file. The inner
function name `_emit_playback_restart` is local-scoped — mpv keeps
its own reference, so there's no leak from re-creation across
popout open/close cycles (each popout gets a fresh VideoPlayer
with its own _ensure_mpv call).
This is the only commit in the popout state machine refactor that
touches `media/video_player.py`. All subsequent commits land in
`popout/` (state.py, effects.py, hyprland.py, window.py) or
`gui/main_window.py` interface updates. 21 lines added, 0 removed.
Verification:
- Phase A test suite (16 tests) still passes
- Module imports cleanly with the new Signal in place
- App launches without errors (smoke test)
Test cases for state machine adapter (Prompt 3 popout/state.py):
- VideoPlayer.playback_restart fires once on play_file completion
- VideoPlayer.playback_restart fires once per _seek call
First regression-test layer for booru-viewer. Pure Python — no Qt, no
mpv, no network, no real filesystem outside tmp_path. Locks in the
security and concurrency invariants from the 54ccc40 + eb58d76 hardening
commits ahead of the upcoming popout state machine refactor (Prompt 3),
which needs a stable baseline to refactor against.
16 tests across five files mirroring the source layout under
booru_viewer/core/:
- tests/core/test_db.py (4):
- _validate_folder_name rejection rules (.., /foo, \\foo, .hidden,
~user, empty) and acceptance categories (unicode, spaces, parens)
- add_bookmark INSERT OR IGNORE collision returns the existing row
id (locks in the lastrowid=0 fix)
- get_bookmarks LIKE escaping (literal cat_ear does not match catear)
- tests/core/test_cache.py (7):
- _referer_for hostname suffix matching (gelbooru.com / donmai.us
apex rewrite, both exact-match and subdomain)
- _referer_for rejects substring attackers
(imgblahgelbooru.attacker.com does NOT pick up the booru referer)
- ugoira frame-count and uncompressed-size caps refuse zip bombs
before any decompression
- _do_download MAX_DOWNLOAD_BYTES enforced both at the
Content-Length pre-check AND in the chunk-loop running total
- _is_valid_media returns True on OSError (no delete + redownload
loop on transient EBUSY)
- tests/core/test_config.py (2):
- saved_folder_dir rejects literal .. and ../escape
- find_library_files walks root + 1 level, filters by
MEDIA_EXTENSIONS, exact post-id stem match
- tests/core/test_concurrency.py (2):
- get_app_loop raises RuntimeError before set_app_loop is called
- run_on_app_loop round-trips a coroutine result from a worker
thread loop back to the test thread
- tests/core/api/test_base.py (1):
- BooruClient._shared_client lazy singleton constructor-once under
10-thread first-call race
Plus tests/conftest.py with fixtures: tmp_db, tmp_library,
reset_app_loop, reset_shared_clients. All fixtures use tmp_path or
reset module-level globals around the test so the suite is parallel-
safe.
pyproject.toml:
- New [project.optional-dependencies] test extra: pytest>=8.0,
pytest-asyncio>=0.23
- New [tool.pytest.ini_options]: asyncio_mode = "auto",
testpaths = ["tests"]
README.md:
- Linux install section gains "Run tests" with the
pip install -e ".[test]" + pytest tests/ invocation
Phase B (post-sweep VideoPlayer regression tests for the seek slider
pin, _pending_mute lazy replay, and volume replay) is deferred to
Prompt 3's state machine work — VideoPlayer cannot be instantiated
without QApplication and a real mpv, which is out of scope for a
unit test suite. Once the state machine carves the pure-Python state
out of VideoPlayer, those tests become trivial against the helper
module.
Suite runs in 0.07s (16 tests). Independent of Qt/mpv/network/ffmpeg.
Test cases for Prompt 3:
- (already covered) — this IS the test suite Prompt 3 builds on top of
The popout's VideoPlayer is constructed with no mpv attached — mpv
gets wired up in _ensure_mpv on the first set_media call. main_window's
_open_fullscreen_preview syncs preview→popout state right after the
popout is constructed, so it writes is_muted *before* mpv exists. The
old setter only forwarded to mpv if mpv was set:
@is_muted.setter
def is_muted(self, val: bool) -> None:
if self._mpv:
self._mpv.mute = val
self._mute_btn.setText("Unmute" if val else "Mute")
For the popout's pre-mpv VideoPlayer this updated the button text but
silently dropped the value. _ensure_mpv then created the mpv instance
later with default mute=False, so the popout always opened unmuted
even when the embedded preview was muted (or when a previous popout
session had muted and then closed).
Fix: introduce a Python-side _pending_mute field that survives the
lazy mpv creation. The setter writes to _pending_mute unconditionally
and forwards to mpv if it exists. The getter returns _mpv.mute when
mpv is set, otherwise _pending_mute. _ensure_mpv replays _pending_mute
into the freshly-created mpv instance after applying the volume from
the slider, mirroring the existing volume-from-slider replay pattern
that already worked because the slider widget exists from construction
and acts as the volume's persistent storage.
Also threaded _pending_mute through _toggle_mute so the click-driven
toggle path stays consistent with the setter path — without it, a
mute toggle inside the popout would update mpv but not _pending_mute,
and the next sync round-trip via the setter would clobber it.
Verified manually:
- popout video, click mute, close popout, reopen on same video →
mute persisted (button shows "Unmute", audio silent)
- toggle to unmute, close, reopen → unmuted persisted
- embedded preview video mute → close popout → state propagates
correctly via _on_fullscreen_closed's reverse sync
Note #3 in REFACTOR_NOTES.md (search result count + end-of-results
flag mismatch) reproduced once during the refactor verification sweep
and not again at later commits, so it's intermittent — likely
scenario-dependent (specific tag, blacklist hit rate, page-size /
limit interaction). The bug is real but not reliably repro-able, so
the right move is to add logging now and capture real data on the
next reproduction instead of guessing at a fix.
Both _do_search (paginated) and _on_reached_bottom (infinite scroll
backfill) now log a `do_search:` / `on_reached_bottom:` line with the
following fields:
- limit the configured page_size
- api_returned_total raw count of posts the API returned across all
fetched pages (sum of every batch the loop saw)
- kept post-filter, post-clamp count actually emitted
- drops_bl_tags posts dropped by the blacklist-tags filter
- drops_bl_posts posts dropped by the blacklist-posts filter
- drops_dedup posts dropped by the dedup-against-seen filter
- api_short_signal (do_search only) whether the LAST batch came
back smaller than limit — the implicit "API ran
out" hint
- api_exhausted (on_reached_bottom only) the explicit
api_exhausted flag the loop sets when len(batch)
falls short
- last_page (on_reached_bottom only) the highest page index
the backfill loop touched
_on_search_done also gets a one-liner with displayed_count, limit,
and the at_end decision so the user-visible "(end)" flag can be
correlated with the upstream numbers.
Implementation note: the per-filter drop counters live in a closure-
captured `drops` dict that the `_filter` closure mutates as it walks
its three passes (bl_tags → bl_posts → dedup). Same dict shape in
both `_do_search` and `_on_reached_bottom` so the two log lines are
directly comparable. Both async closures also accumulate `raw_total`
across the loop iterations to capture the API's true return count,
since the existing locals only kept the last batch's length.
All logging is `log.debug` so it's off at default INFO level. To
capture: bump booru_viewer logger level (or run with debug logging
enabled in main_window.py:440 — already DEBUG by default per the
existing setLevel call).
This commit DOES NOT fix#3 — the symptom is still intermittent and
the root cause is unknown. It just makes the next reproduction
diagnosable in one shot instead of requiring a second instrumented
run.
Two related improvements to the Ctrl+P privacy screen flow.
1. Resume video on un-hide
Pre-fix: Ctrl+P paused any playing video in the embedded preview and
the popout, but the second Ctrl+P only hid the privacy overlay — the
videos stayed paused. The user had to manually click Play to resume.
Fix: in _toggle_privacy's privacy-off branch, mirror the privacy-on
pause logic with resume() calls on the embedded preview's video player
and the popout's video. Unconditional resume — if the user manually
paused before Ctrl+P, the auto-resume on un-hide is a tiny annoyance,
but the common case (privacy hides → user comes back → video should
be playing again) wins.
2. Popout privacy uses an in-place overlay instead of hide()
Pre-fix attempt: privacy-on called self._fullscreen_window.hide() and
privacy-off called .show(). On Wayland (Hyprland) the hide→show round
trip drops the window's position because the compositor unmaps the
window on hide and remaps it at the default tile position on show.
A first attempt at restoring the position via a deferred
hyprctl_resize_and_move dispatch in privacy_show didn't take — by
the time the dispatch landed, the window had already been re-tiled
and the move was gated by `if not win.get("floating"): return`.
Cleaner fix: don't hide the popout window at all. FullscreenPreview
gains its own _privacy_overlay (a black QWidget child of central,
parallel to the existing toolbar / controls bar children) that
privacy_hide raises over the media stack. The popout window stays
mapped, position is preserved automatically because nothing moves,
and the overlay covers the content visually.
privacy_hide / privacy_show methods live in FullscreenPreview, not
in main_window — popout-internal state belongs to the popout module.
_toggle_privacy in main_window just calls them. This also makes
adding more popout-side privacy state later (e.g. fullscreen save)
a one-method change inside the popout class.
Also added a _popout_was_visible flag in BooruApp._toggle_privacy so
privacy-off only restores the popout if it was actually visible at
privacy-on time. Without the gate, privacy-off would inappropriately
re-show a popout the user had closed before triggering privacy.
Verified manually:
- popout open + drag to non-default pos + Ctrl+P + Ctrl+P → popout
still at the dragged position, content visible again
- popout open + video playing + Ctrl+P + Ctrl+P → video resumes
- popout closed + Ctrl+P + Ctrl+P → popout stays closed
- embedded preview video + Ctrl+P + Ctrl+P → resumes
- Ctrl+P with no video on screen → no errors
Pax requested the keyboard shortcut be removed — too easy to fat-finger
when navigating with the keyboard, and "Open in Default App" still
ships an external process that may steal focus from the app. The
right-click menu's Open in Default App action stays, both on browse
thumbnails and in the preview pane right-click — only the bare-key
shortcut goes away.
The deleted block was the only Key_O handler in BooruApp.keyPressEvent,
so no other behavior changes.
Verified manually:
- press O on a selected thumbnail → nothing happens
- right-click thumbnail → "Open in Default App" still present and opens
- right-click preview pane → same
Two related preservation bugs around the popout's F11 fullscreen
toggle, both surfaced during the post-refactor verification sweep.
1. ImageViewer zoom/pan loss on resize
ImageViewer.resizeEvent unconditionally called _fit_to_view() on every
resize event. F11 enter resizes the widget to the full screen, F11
exit resizes it back to the windowed size — both fired _fit_to_view,
clobbering any explicit user zoom and offset. Same problem for manual
window drags and splitter moves.
Fix: in resizeEvent, compute the previous-size fit-to-view zoom from
event.oldSize() and compare to current _zoom. Only re-fit if the user
was at fit-to-view at the previous size (within a 0.001 epsilon —
tighter than any wheel/key zoom step). Otherwise leave _zoom and
_offset alone.
The first-resize case (no valid oldSize, e.g. initial layout) still
defaults to fit, matching the original behavior for fresh widgets.
2. Popout window position lost on F11 round-trip
FullscreenPreview._enter_fullscreen captured _windowed_geometry but
the F11-exit restore goes through `_viewport` (the persistent center +
long_side that drives _fit_to_content). The drift detection in
_derive_viewport_for_fit only updates _viewport when
_last_dispatched_rect is set AND a fit is being computed — neither
path catches the "user dragged the popout with Super+drag and then
immediately pressed F11" sequence:
- Hyprland Super+drag does NOT fire Qt's moveEvent (xdg-toplevel
doesn't expose absolute screen position to clients on Wayland),
so Qt-side drift detection is dead on Hyprland.
- The Hyprland-side drift detection in _derive_viewport_for_fit
only fires inside a fit, and no fit is triggered between a drag
and F11.
- Result: _viewport still holds whatever it had before the drag —
typically the saved-from-last-session geometry seeded by the
first-fit one-shot at popout open.
When F11 exits, the deferred _fit_to_content reads the stale viewport
and restores the popout to the *previously seeded* position instead of
where the user actually had it.
Fix: in _enter_fullscreen, after capturing _windowed_geometry, also
write the current windowed state into self._viewport directly. The
viewport then holds the actual pre-fullscreen position regardless of
how it got there (drag, drag+nav, drag+F11, etc.), and F11 exit's
restore reads it correctly.
Bundled into one commit because both fixes are "F11 round-trip should
preserve where the user was" — the image fix preserves content state
(zoom/pan), the popout fix preserves window state (position). Same
theme, related root cause class. Bisecting one without the other
would be misleading.
Verified manually:
- image: scroll-zoom + drag pan + F11 + F11 → zoom and pan preserved
- image: untouched zoom + F11 + F11 → still fits to view
- image: scroll-zoom + manual window resize → zoom preserved
- popout: Super+drag to a new position + F11 + F11 → lands at the
dragged position, not at the saved-from-last-session position
- popout: same sequence on a video post → same result (videos don't
have zoom/pan, but the window-position fix applies to all media)
The seek slider snapped visually backward after a click for the first
~tens to hundreds of ms — long enough to be obvious. Race trace:
user clicks slider at target T
→ _ClickSeekSlider.mousePressEvent fires
→ setValue(T) lands the visual at the click position
→ clicked_position emits → _seek dispatches mpv.seek(T) async
→ mpv processes the seek on its own thread
meanwhile the 100ms _poll timer keeps firing
→ reads mpv.time_pos (still the OLD position, mpv hasn't caught up)
→ calls self._seek_slider.setValue(pos_ms)
→ slider visually snaps backward to the pre-seek position
→ mpv finishes seeking, next poll tick writes the new position
→ slider jumps forward to settle near T
The isSliderDown() guard at the existing setValue site (around line
425) only suppresses writebacks during a *drag* — fast clicks never
trigger isSliderDown, so the guard didn't help here.
Fix: pin the slider to the user's target throughout a 500ms post-seek
window. Mirror the existing _eof_ignore_until pattern (stale-eof
suppression in play_file → _on_eof_reached) — it's the same shape:
"after this dispatch, ignore poll-driven writebacks for N ms because
mpv hasn't caught up yet."
- _seek now records _seek_target_ms and arms _seek_pending_until
- _poll forces _seek_slider.setValue(_seek_target_ms) on every tick
inside the window, instead of mpv's lagging time_pos
- After the window expires, normal mpv-driven writes resume
Pin window is 500ms (vs the eof window's 250ms) because network and
streaming seeks take noticeably longer than local-cache seeks. Inside
the window the slider is forced to the target every tick, so mpv lag
is invisible no matter how long it takes within the window.
First attempt used a smaller 250ms window with a "close enough"
early-release condition (release suppression once mpv reports a
position within 250ms of the target). That still showed minor
track-back because the "close enough" threshold permitted writing
back a position slightly less than the target, producing a small
visible jump. The pin-to-target approach is robust against any
mpv interim position.
The time_label keeps updating to mpv's actual position throughout —
only the slider value is pinned, so the user can still see the
seek progressing in the time text.
Verified manually: clicks at start / middle / end of a video slider
all hold position cleanly. Drag still works (the isSliderDown path
is untouched). Normal playback advances smoothly (the pin window
only affects the post-seek window, not steady-state playback).
Two related fixes for the File → Batch Download Page (Ctrl+D) flow.
1. Saved-dot refresh
Pre-fix: when the user picked a destination inside the library, the
batch wrote files to disk but the browse grid's saved-dots stayed dark
until the next refresh. The grid was lying about local state.
Fix: stash the chosen destination as self._batch_dest at the dispatch
site, then in _on_batch_progress (which already fires per-file via
the existing batch_progress signal) check whether dest is inside
saved_dir(); if so, find the just-finished post in self._posts by id
and light its grid thumb's saved-locally dot. Dots appear incrementally
as each file lands, not all at once at the end.
The batch_progress signal grew a third int param (post_id of the
just-finished item). It's a single-consumer signal — only
_on_batch_progress connects to it — so the shape change is local.
Both batch download paths (the file menu's _batch_download and the
multi-select menu's _batch_download_posts) pass post.id through.
When the destination is OUTSIDE the library, dots stay dark — the
saved-dot means "in library", not "downloaded somewhere". The check
uses Path.is_relative_to (Python 3.11+).
self._batch_dest is cleared in _on_batch_done after the batch finishes
so a subsequent non-batch save doesn't accidentally see a stale dest.
2. Tab gating
Pre-fix: File → Batch Download Page... was enabled on Bookmarks and
Library tabs, where it makes no sense (those tabs already show local
files). Ctrl+D fired regardless of active tab.
Fix: store the QAction as self._batch_action instead of a local var
in _setup_menu, then toggle setEnabled(index == 0) from _switch_view.
Disabling the QAction also disables its keyboard shortcut, so Ctrl+D
becomes a no-op on non-browse tabs without a separate guard.
Verified manually:
- Browse tab → menu enabled, Ctrl+D works
- Bookmarks/Library tabs → menu grayed out, Ctrl+D no-op
- Batch dl into ~/.local/share/booru-viewer/saved → dots light up
one-by-one as files land
- Batch dl into /tmp → files written, dots stay dark
The infinite-scroll backfill loop in _on_reached_bottom accumulates
results from up to 9 follow-up API pages until len(collected) >= limit,
but the break condition is >= not ==, so the very last full batch
would push collected past the configured page_size. The non-infinite
search path in _do_search already slices collected[:limit] before
emitting search_done at line 805 — the infinite path was emitting the
unclamped list. Result: a single backfill round occasionally appended
more than page_size posts, producing irregular batch sizes the user
could see.
Fix: one-character change at the search_append.emit call site to mirror
the non-infinite path's slice.
Why collected[:limit] over the alternative break-early-with-clamp:
1. Consistency — the non-infinite path in _do_search already does
the same slice before emit. One pattern, both branches.
2. Trivially fewer lines than restructuring the loop break.
3. The slight wasted download work (the over-fetched final batch is
already on disk by the time we slice) is acceptable. It's at most
one extra page's worth, only happens at the boundary, only on
infinite scroll, and the next backfill round picks up from where
the visible slice ends — nothing is *lost*, just briefly redundant.
Verified manually on a high-volume tag with infinite scroll enabled
and page_size=40: pre-fix appended >40 posts in one round, post-fix
appended exactly 40.
The browse grid's multi-select right-click menu collapsed library and
bookmark actions into a single "Remove All Bookmarks" entry that did
*both* — it called delete_from_library and remove_bookmark per post,
and was unconditionally visible regardless of selection state. Two
problems:
1. There was no way to bulk-unsave files from the library without
also stripping the bookmarks. Saved-but-not-bookmarked posts had
no bulk-unsave path at all.
2. The single misleadingly-named action didn't match the single-post
right-click menu's clean separation of "Save to Library / Unsave
from Library" vs. "Bookmark as / Remove Bookmark".
Reshape: split into four distinct actions, each with symmetric
conditional visibility:
- Save All to Library → shown only if any post is unsaved
- Unsave All from Library → shown only if any post is saved (NEW)
- Bookmark All → shown only if any post is unbookmarked
- Remove All Bookmarks → shown only if any post is bookmarked
Mixed selections show whichever subset of the four is relevant. The
new Unsave All from Library calls a new _bulk_unsave method that
mirrors the _bulk_save shape but synchronously (delete_from_library
is a filesystem op, no httpx round-trip). Remove All Bookmarks now
*only* removes bookmarks — it no longer touches the library, matching
the single-post Remove Bookmark action's scope.
Always-shown actions (Download All, Copy All URLs) stay below a
separator at the bottom.
Verified:
- Multi-select unbookmarked+unsaved posts → only Save All / Bookmark All
- Multi-select saved-not-bookmarked → only Unsave All / Bookmark All
- Multi-select bookmarked+saved → only Unsave All / Remove All Bookmarks
- Mixed selection → all four appear
- Unsave All from Library removes files, leaves bookmarks
- Remove All Bookmarks removes bookmarks, leaves files
Final commit of the gui/app.py + gui/preview.py structural refactor.
Updates the four call sites that were importing through the
preview.py / app.py shims to import from each entity's canonical
sibling module instead, then deletes the now-empty shim files.
Edits:
- main_gui.py:38 from booru_viewer.gui.app import run
→ from booru_viewer.gui.app_runtime import run
- main_window.py:44 from .preview import ImagePreview
→ from .preview_pane import ImagePreview
- main_window.py:1133 from .preview import VIDEO_EXTENSIONS
→ from .media.constants import VIDEO_EXTENSIONS
- main_window.py:2061 from .preview import FullscreenPreview
→ from .popout.window import FullscreenPreview
- main_window.py:2135 from .preview import FullscreenPreview
→ from .popout.window import FullscreenPreview
Deleted:
- booru_viewer/gui/app.py
- booru_viewer/gui/preview.py
Final gui/ tree:
gui/
__init__.py (unchanged, empty)
app_runtime.py entry point + style loader
main_window.py BooruApp QMainWindow
preview_pane.py ImagePreview embedded preview
info_panel.py InfoPanel widget
log_handler.py LogHandler (Qt-aware logger adapter)
async_signals.py AsyncSignals signal hub
search_state.py SearchState dataclass
media/
__init__.py
constants.py VIDEO_EXTENSIONS, _is_video
image_viewer.py ImageViewer (zoom/pan)
mpv_gl.py _MpvGLWidget, _MpvOpenGLSurface
video_player.py VideoPlayer + _ClickSeekSlider
popout/
__init__.py
viewport.py Viewport NamedTuple, _DRIFT_TOLERANCE
window.py FullscreenPreview popout window
grid.py, bookmarks.py, library.py, search.py, sites.py,
settings.py, dialogs.py (all untouched)
Net result for the refactor: 2 god-files (app.py 3608 lines +
preview.py 2273 lines = 5881 lines mixing every concern) replaced
by 12 small clean modules + 2 oversize-by-design god-class files
(main_window.py and popout/window.py — see docs/REFACTOR_PLAN.md
for the indivisible-class rationale).
Followups discovered during execution are recorded in
docs/REFACTOR_NOTES.md (gitignored, local-only).
Step 13 of the gui/app.py + gui/preview.py structural refactor —
final move out of app.py. The four entry-point helpers move together
because they're a tightly-coupled cluster: run() calls all three of
the others (_apply_windows_dark_mode, _load_user_qss,
_BASE_POPOUT_OVERLAY_QSS). Splitting them across commits would just
add bookkeeping overhead with no bisect benefit.
app_runtime.py imports BooruApp from main_window for run()'s
instantiation site, plus Qt at module level (the nested
_DarkArrowStyle class inside run() needs Qt.PenStyle.NoPen at call
time). Otherwise the four helpers are byte-identical to their
app.py originals.
After this commit app.py is just the original imports header + log
+ the shim block — every entity that used to live in it now lives
in its canonical module. main_gui.py still imports from
booru_viewer.gui.app via the shim (`from .app_runtime import run`
re-exports it). Commit 14 swaps main_gui.py to the canonical path
and deletes app.py.
Step 12 of the gui/app.py + gui/preview.py structural refactor — the
biggest single move out of app.py. The entire ~3020-line BooruApp
QMainWindow class moves to its own module under gui/. The class body
is byte-identical: every method, every signal connection, every
private attribute access stays exactly as it was.
main_window.py imports the helper classes that already moved out of
app.py (SearchState, LogHandler, AsyncSignals, InfoPanel) directly
from their canonical sibling modules at the top of the file, so the
bare-name lookups inside BooruApp method bodies (`SearchState(...)`,
`LogHandler(self._log_text)`, `AsyncSignals()`, `InfoPanel()`) keep
resolving to the same class objects. Same package depth as app.py
was, so no relative-import depth adjustment is needed for any of
the lazy `..core.X` or `.preview` imports inside method bodies —
they keep working through the preview.py shim until commit 14
swaps them to canonical paths.
app.py grows the BooruApp re-export shim line. After this commit
app.py is just imports + log + the four helpers (run,
_apply_windows_dark_mode, _load_user_qss, _BASE_POPOUT_OVERLAY_QSS)
+ the shim block. Commit 13 carves the helpers out, commit 14
deletes the shims and the file.
VERIFICATION: full method-cluster sweep (see docs/REFACTOR_PLAN.md
"Commit 12 expanded verification" section), not the 7-item smoke test.
Step 11 of the gui/app.py + gui/preview.py structural refactor. Pure
copy: the toggleable info panel widget with category-coloured tag
list moves to its own module. The new module gets its own
`log = logging.getLogger("booru")` at module level — same logger
instance the rest of the app uses (logging.getLogger is idempotent
by name), matching the existing per-module convention used by
grid.py / bookmarks.py / library.py. All six tag-color Qt Properties
preserved verbatim. app.py grows another shim line. Shim removed
in commit 14.
Step 10 of the gui/app.py + gui/preview.py structural refactor. Pure
copy: the QObject signal hub that BooruApp uses to marshal async
worker results back to the GUI thread moves to its own module. All
14 signals are preserved verbatim. app.py grows another shim line
so internal `AsyncSignals()` references in BooruApp keep working.
Shim removed in commit 14.
Step 9 of the gui/app.py + gui/preview.py structural refactor. Pure
copy: the Qt-aware logging.Handler that bridges the booru logger to
the in-app QTextEdit log panel moves to its own module. app.py grows
another shim line so any internal `LogHandler(...)` reference (the
single one in BooruApp._setup_ui) keeps resolving through the module
namespace. Shim removed in commit 14.
Step 8 of the gui/app.py + gui/preview.py structural refactor — first
move out of app.py. Pure copy: the SearchState dataclass moves to its
own module. app.py grows its first re-export shim block at the bottom
so any internal `SearchState(...)` reference in BooruApp keeps working
through the module-namespace lookup. Shim removed in commit 14.
Step 7 of the gui/app.py + gui/preview.py structural refactor. Pure
move: the embedded preview pane class (the one that lives in the
right column of the main window and combines image+video+toolbar)
is now in its own module. preview_pane.py is at the same package
depth as preview.py was, so no relative-import depth adjustment is
needed inside the class body.
preview.py grows the final preview-side re-export shim line. After
this commit preview.py is just the original imports + _log + shim
block — every class that used to live in it now lives in its
canonical module under media/ or popout/ or as preview_pane. The
file gets deleted entirely in commit 14.
Step 6 of the gui/app.py + gui/preview.py structural refactor — the
biggest single move in the sequence. The entire 1046-line popout
window class moves to its own module under popout/, alongside the
viewport NamedTuple it depends on. The popout overlay styling
documentation comment that lived above the class moves with it
since it's about the popout, not about ImagePreview.
Address-only adjustment: the lazy `from ..core.config import` lines
inside `_hyprctl_resize` and `_hyprctl_resize_and_move` become
`from ...core.config import` because the new module sits one package
level deeper. Same target module, different relative-import depth —
no behavior change.
preview.py grows another re-export shim so app.py's two lazy
`from .preview import FullscreenPreview` call sites (in
_open_fullscreen_preview and _on_fullscreen_closed) keep working
unchanged. Shim removed in commit 14, where the call sites move
to the canonical `from .popout.window import FullscreenPreview`.
Step 5 of the gui/app.py + gui/preview.py structural refactor. Moves
the click-to-seek QSlider variant and the mpv-backed transport-control
widget into their own module under media/. The new module imports
_MpvGLWidget from .mpv_gl (sibling) instead of relying on the bare
name in the old preview.py namespace.
Address-only adjustment: the lazy `from ..core.cache import _referer_for`
inside `play_file` becomes `from ...core.cache import _referer_for`
because the new module sits one package level deeper. Same target
module, different relative-import depth — no behavior change.
preview.py grows another re-export shim line so ImagePreview (still
in preview.py) and FullscreenPreview can keep constructing
VideoPlayer unchanged. Shim removed in commit 14.
Step 4 of the gui/app.py + gui/preview.py structural refactor. Pure
move: the OpenGL render-context host and its concrete QOpenGLWidget
companion are now in their own module under media/. The mid-file
`from PySide6.QtOpenGLWidgets import QOpenGLWidget as _QOpenGLWidget`
import that used to sit between the two classes moves with them to
the new module's import header. preview.py grows another re-export
shim line so VideoPlayer (still in preview.py) can keep constructing
_MpvGLWidget unchanged. Shim removed in commit 14.
Step 3 of the gui/app.py + gui/preview.py structural refactor. Pure
move: the zoom/pan image viewer class is now in its own module under
media/. preview.py grows another re-export shim line so ImagePreview
and FullscreenPreview (both still in preview.py) can keep constructing
ImageViewer instances unchanged. Shim removed in commit 14.
Step 2 of the gui/app.py + gui/preview.py structural refactor. Pure
move: the popout viewport NamedTuple and the drift-tolerance constant
are now in their own module under popout/. preview.py grows another
re-export shim line so FullscreenPreview's method bodies (which
reference Viewport and _DRIFT_TOLERANCE by bare name) keep working
unchanged. Shim removed in commit 14. See docs/REFACTOR_PLAN.md.
Step 1 of the gui/app.py + gui/preview.py structural refactor. Pure
move: the constant and predicate are now in their own module, and
preview.py grows a re-export shim at the bottom so existing imports
(app.py:1351 and the in-file class methods) keep working unchanged.
Shim is removed in commit 14 once importers update to the canonical
path. See docs/REFACTOR_PLAN.md for the full migration order.
Pre-existing bug in `_navigate_preview` that surfaced after the
preceding perf round shifted timing enough to expose the race. For
every tab, `_navigate_preview` was calling `grid._select(idx)`
followed by an explicit activate-handler call:
self._grid._select(idx)
self._on_post_activated(idx) # ← redundant
`grid._select(idx)` ends with `self.post_selected.emit(index)`,
which is wired to `_on_post_selected` (or the bookmark/library
equivalents), which already calls `_on_post_activated` after a
multi-select length check that's always 1 here because `_select`
calls `_clear_multi` first. So the activation handler ran TWICE per
keyboard nav.
Each `_on_post_activated` schedules an async `_load`, which fires
`image_done` → `_on_image_done` → `_update_fullscreen` →
`set_media` → `_video.stop()` + `_video.play_file(path)`. Two
activations produced two `set_media` cycles in quick succession.
The stale-eof suppression race:
1. First `play_file` opens window A: `_eof_ignore_until = T+250ms`
2. Second `play_file` runs ~10-50ms later
3. Inside the second `play_file`: `_eof_pending = False` runs
BEFORE `_eof_ignore_until` is reset
4. Window A may have already expired by this point if the load
was slow
5. An async `eof-reached=True` event from the second
`_video.stop()` lands in the un-armed gap
6. The gate check `monotonic() < _eof_ignore_until` fails (window A
expired, window B not yet open)
7. `_eof_pending = True` sticks
8. Next `_poll` cycle: `_handle_eof` sees Loop=Next, emits
`play_next` → `_on_video_end_next` → `_navigate_preview(1, wrap=True)`
→ ANOTHER post advance
9. User pressed Right once, popout skipped a post
Random and timing-dependent. Hard to reproduce manually but happens
often enough to be visible during normal browsing.
Fix: stop calling the activation handler directly after `_select`.
The signal chain handles it. Applied to all five sites in
`_navigate_preview`:
- browse view (line 2046-2047)
- bookmarks view normal nav (line 2024-2025)
- bookmarks view wrap-edge (line 2028-2029)
- library view normal nav (line 2036-2037)
- library view wrap-edge (line 2040-2041)
The wrap-edge cases were called out in the original plan as "leave
alone for scope creep" but they have the same duplicate-call shape
and the same race exposure during auto-advance from EOF. Fixing
them keeps the code consistent and removes a latent bug from a
less-traveled path.
Verified by reading: `_grid._select(idx)` calls `_clear_multi()`
first, so by the time `post_selected` fires, `selected_indices`
returns `[idx]` (length 1), `_on_post_selected`'s multi-select
early-return doesn't fire, and `_on_post_activated(index)` is
always called. Same for the bookmark/library `_on_selected` slots
which have no early-return at all.
Net: ~5 lines deleted, ~25 lines of comments added explaining the
race and the trust-the-signal-chain rule for future contributors.
A bundle of popout video performance work plus three layered race
fixes that were uncovered as the perf round shifted timing. Lands
together because the defensive layers depend on each other and
splitting them would create commits that don't cleanly verify in
isolation.
## Perf wins
**mpv URL streaming for uncached videos.** Click an uncached video
and mpv now starts playing the remote URL directly instead of waiting
for the entire file to download. New `video_stream` signal +
`_on_video_stream` slot route the URL to mpv via `play_file`'s new
`http://`/`https://` branch, which sets the per-file `referrer`
option from the booru's hostname (reuses `cache._referer_for`).
`download_image` continues running in parallel to populate the cache
for next time. The `image_done` emit is suppressed in the streaming
case so the eventual cache-write completion doesn't re-call set_media
mid-playback. Result: first frame in 1-2 seconds on uncached videos
instead of waiting for the full multi-MB transfer.
**mpv fast-load options.** `vd_lavc_fast="yes"` and
`vd_lavc_skiploopfilter="nonkey"` added to the MPV() constructor.
Saves ~50-100ms on first-frame decode for h264/hevc by skipping
bitstream-correctness checks and the in-loop filter on non-keyframes.
Documented mpv "fast load" use case — artifacts only on the first
few frames before steady state and only on degraded sources.
**GL pre-warm at popout open.** New `showEvent` override on
`FullscreenPreview` calls `_video._gl_widget.ensure_gl_init()` as
soon as the popout is mapped. The first video click after open no
longer pays the ~100-200ms one-time GL render context creation
cost. `ensure_gl_init` is idempotent so re-shows after close are
cheap no-ops.
**Identical-rect skip in `_fit_to_content`.** If the computed
window rect matches `_last_dispatched_rect`, the function early-
returns without dispatching to hyprctl or `setGeometry`. The window
is already in that state per the previous dispatch, the persistent
viewport's drift detection already ran above and would have changed
the computed rect if Hyprland reported real drift. Saves the
subprocess.Popen + Hyprland's processing of the redundant resize on
back-to-back same-aspect navs (very common with multi-video posts
from the same source).
## Race-defense layers
**Pause-on-activate at top of `_on_post_activated`.** The first
thing every post activation does now is `mpv.pause = True` on both
the popout's and the embedded preview's mpv. Prevents the previous
video from naturally reaching EOF during a long async download —
without this, an in-flight EOF would fire `play_next` in
Loop=Next mode and auto-advance past the post the user wanted.
Uses pause (property change, no eof side effect) instead of
stop (which emits eof-reached).
**250ms stale-eof suppression window in VideoPlayer.** New
`_eof_ignore_until` field, set in `play_file` to
`monotonic() + 0.25`. `_on_eof_reached` drops events arriving while
`monotonic() < _eof_ignore_until`. Closes the race where mpv's
`command('stop')` (called by `set_media` before `play_file`)
generates an async eof event that lands AFTER `play_file`'s
`_eof_pending = False` reset and sticks the bool back to True,
causing the next `_poll` cycle to fire `play_next` for a video
the user just navigated away from.
**Removed redundant `_update_fullscreen` calls** from
`_navigate_fullscreen` and `_on_video_end_next`. Those calls used
the still-stale `_preview._current_path` (the previous post's path,
because async _load hasn't completed yet) and produced a stop+reload
of the OLD video in the popout. Each redundant reload was another
trigger for the eof race above. Bookmark and library navigation
already call `_update_fullscreen` from inside their downstream
`_on_*_activated` handlers with the correct path; browse navigation
goes through the async `_on_image_done` flow which also calls it
with the correct new path.
## Plumbing
**Pre-fit signature on `FullscreenPreview.set_media`** — `width`
and `height` params accepted but currently unused. Pre-fit was
tried (call `_fit_to_content(width, height)` immediately on video
set_media) and reverted because the redundant second hyprctl
dispatch when mpv's `video_size` callback fires produced a visible
re-settle. The signature stays so call sites can pass dimensions
without churn if pre-fit is re-enabled later under different
conditions.
**`_update_fullscreen` reads dimensions** from
`self._preview._current_post` and passes them to `set_media`.
Same plumbing for the popout-open path at app.py:2183.
**dl_progress auto-hide** on `downloaded == total` in
`_on_download_progress`. The streaming path suppresses
`_on_image_done` (which is the normal place dl_progress is hidden),
so without this the bar would stay visible forever after the
parallel cache download completes. Harmlessly redundant on the
non-streaming path.
## Files
`booru_viewer/gui/app.py`, `booru_viewer/gui/preview.py`.
Group B of the popout viewport work. The v0.2.2 viewport compute swap
fixed the big aspect-ratio failures (width-anchor ratchet, asymmetric
clamps, manual-resize destruction) but kept a minor "recompute from
current state every nav" shortcut that accumulated 1-2px of downward
drift across long navigation sessions. This commit replaces that
shortcut with a true persistent viewport that's only updated by
explicit user action, not by reading our own dispatch output back.
The viewport (center_x, center_y, long_side) is now stored as a
field on FullscreenPreview, seeded from `_pending_*` on first fit
after open or F11 exit, and otherwise preserved across navigations.
External moves/resizes are detected via a `_last_dispatched_rect`
cache: at the start of each fit, the current `hyprctl clients -j`
position is compared against the last rect we dispatched, and if
they differ by more than `_DRIFT_TOLERANCE` (2px) the user is
treated as having moved the window externally and the viewport
adopts the new state. Sub-pixel rounding stays inside the tolerance
and the viewport stays put.
`_exit_fullscreen` is simplified — no more re-arming the
`_first_fit_pending` one-shots. The persistent viewport already
holds the pre-fullscreen center+long_side (fullscreen entry/exit
runs no fits, so nothing overwrites it), and the deferred fit after
`showNormal()` reads it directly. Side benefit: this fixes the
legacy F11-walks-toward-saved-top-left bug 1f as a free byproduct.
## The moveEvent/resizeEvent gate (load-bearing — Group B v1 broke
## without it)
First implementation of Group B added moveEvent/resizeEvent handlers
to capture user drags/resizes into the persistent viewport on the
non-Hyprland Qt path. They were guarded with a `_applying_dispatch`
reentrancy flag set around the dispatch call. **This broke every
navigation, F11 round-trip, and external drag on Hyprland**, sending
the popout to the top-left corner.
Two interacting reasons:
1. On Wayland (Hyprland included), `self.geometry()` returns
`QRect(0, 0, w, h)` for top-level windows. xdg-toplevel doesn't
expose absolute screen position to clients, and Qt6's wayland
plugin reflects that by always reporting `x=0, y=0`. So the
handlers wrote viewport center = `(w/2, h/2)` — small positive
numbers far from the actual screen center.
2. The `_applying_dispatch` reentrancy guard works for the
synchronous non-Hyprland `setGeometry()` path (moveEvent fires
inside the try-block) but does NOT work for the async hyprctl
dispatch path. `subprocess.Popen` returns instantly, the
`try/finally` clears the guard, THEN Hyprland processes the
dispatch and sends a configure event back to Qt, THEN Qt fires
moveEvent — at which point the guard is already False. So the
guard couldn't suppress the bogus updates that Wayland's
geometry handling produces.
Fix: gate both moveEvent and resizeEvent's viewport-update branches
with `if os.environ.get("HYPRLAND_INSTANCE_SIGNATURE"): return` at
the top. On Hyprland, the cur-vs-last-dispatched comparison in
`_derive_viewport_for_fit` is the sole external-drag detector,
which is what it was designed to be. The non-Hyprland branch stays
unchanged so X11/Windows users still get drag-and-resize tracking
via Qt events (where `self.geometry()` is reliable).
## Verification
All seven manual tests pass on the user's Hyprland session:
1. Drift fix (P↔L navigation cycles): viewport stays constant, no
walking toward any corner
2. Super+drag externally then nav: new dragged position picked up
by the cur-vs-last-dispatched comparison and preserved
3. Corner-resize externally then nav: same — comparison branch
adopts the new long_side
4. F11 same-aspect round-trip: window lands at pre-fullscreen center
5. F11 across-aspect round-trip: window lands at pre-fullscreen
center with the new aspect's shape
6. First-open from saved geometry: works (untouched first-fit path)
7. Restart persistence across app sessions: works (untouched too)
## Files
`booru_viewer/gui/preview.py` only. ~239 added, ~65 removed:
- `_DRIFT_TOLERANCE = 2` constant at module top
- `_viewport`, `_last_dispatched_rect`, `_applying_dispatch` fields
in `FullscreenPreview.__init__`
- `_build_viewport_from_current` helper (extracted from old
`_derive_viewport_for_fit`)
- `_derive_viewport_for_fit` rewritten with three branches:
first-fit seed, defensive build, persistent + drift check
- `_fit_to_content` wraps dispatch with `_applying_dispatch` guard,
caches `_last_dispatched_rect` after dispatch
- `_exit_fullscreen` simplified (no more `_first_fit_pending`
re-arm), invalidates `_last_dispatched_rect` so the post-F11 fit
doesn't false-positive on "user moved during fullscreen"
- `moveEvent` added (gated to non-Hyprland)
- `resizeEvent` extended with viewport update (gated to non-Hyprland)
Two fixes that surfaced from daily use after the v0.2.2 popout polish round 1.
1. Show download progress on the active thumbnail when the
embedded preview is hidden (gui/app.py)
After the previous fix to suppress the dl_progress widget when
the popout is open, the user lost all visible feedback about
the active download in the main app. The grid had no indicator,
the dl_progress widget was hidden, and the only signal was the
status bar text "Loading #X..." at the bottom edge.
`_on_post_activated` now decides per call whether to use the
dl_progress widget at the bottom of the right splitter or fall
back to drawing the download progress on the active thumbnail
in the main grid via the existing prefetch-progress paint path.
The decision is captured at function entry as
`preview_hidden = not (self._preview.isVisible() and
self._preview.width() > 0)` and closed over by the `_progress`
callback and the `_load` coroutine, so the indicator that
starts on a download stays on the same target even if the user
opens or closes the popout mid-download.
The thumbnail bar uses the same paint path as prefetch
indicators (`set_prefetch_progress(0.0..1.0)` for fill,
`set_prefetch_progress(-1)` for clear), so the visual is
identical and no new widget code was added. `_load`'s finally
block emits the clear when `preview_hidden` was true at start.
Generalizes to any reason the preview is hidden, not just the
popout-open case: a user who has dragged the main splitter to
collapse the preview also gets the thumbnail indicator now,
even with the popout closed.
2. Stop auto-showing the popout overlay on every navigation
(gui/preview.py)
`FullscreenPreview.set_media` ended with an unconditional
`self._show_overlay()` call, which meant the floating toolbar
and video controls bar popped back into view on every left/
right/hjkl navigation between posts. Visually noisy and not
what the user wants once they've started navigating — the
overlay is supposed to be a hover-triggered surface, not a
per-post popup.
Removed the call. The overlay is still shown by:
- `__init__` default state (`_ui_visible = True`), so the
user sees it for ~2 seconds on first popout open and the
auto-hide timer hides it after that
- `eventFilter` mouse-move-into-top/bottom-edge zone (the
intended hover trigger, unchanged)
- Volume scroll on video stack (unchanged)
- Ctrl+H toggle (unchanged)
After this, the only way the overlay appears mid-session is
hover or Ctrl+H. Navigation through posts no longer flashes it
back into view.
Three independent fixes accumulated since the v0.2.2 viewport
compute swap. Bundled because they all touch preview.py and
app.py and the staging surface doesn't split cleanly.
1. Suppress dl_progress flash when popout is open (gui/app.py)
The QProgressBar at the bottom of the right splitter was
unconditionally show()'d on every post click via _on_post_activated
and _on_download_progress, including when the popout was open.
With the popout open, the right splitter is set to [0, 0, 1000]
and the user typically has the main splitter dragged to give the
grid full width — the show() call then forces a layout pass on
the right splitter that briefly compresses the main grid before
the download finishes (often near-instant for cached files) and
hide() fires. Visible flash on every grid click, including
clicks on the same post that's already loaded, because
download_image still runs against the cache and the show/hide
cycle still fires.
Three callsites now skip the dl_progress widget entirely when
the popout is visible. The status bar message ("Loading #X...")
still updates so the user has feedback in the main window. With
the popout closed, behavior is unchanged.
2. Cache hyprctl_get_window across one fit call (gui/preview.py)
_fit_to_content was calling _hyprctl_get_window three times per
fit:
- At the top, to determine the floating state
- Inside _derive_viewport_for_fit, to read at/size for the
viewport derivation
- Inside _hyprctl_resize_and_move, to look up the window
address for the dispatch
Each call is a ~3ms subprocess.run that blocks the Qt event
loop. ~9ms of UI freeze per navigation, perceptible as
"slow/glitchy" especially on rapid clicking.
Added optional `win=None` parameter to _derive_viewport_for_fit
and _hyprctl_resize_and_move. _fit_to_content now fetches `win`
once at the top and threads it down. Per-fit subprocess count
drops from 3 to 1 (~6ms saved per navigation).
3. Discord screen-share audio capture works (gui/preview.py)
mpv defaults to ao=pipewire on Linux, which is the native
PipeWire audio output. Discord's screen-share-with-audio
capture on Linux only enumerates clients connected via the
libpulse API; native PipeWire clients are invisible to it.
The visible symptom: video plays locally fine but audio is
silently dropped from any Discord screen share. Firefox works
because Firefox uses libpulse to talk to PipeWire's pulseaudio
compat layer.
Verified by inspection: with ao=pipewire, mpv's sink-input had
`module-stream-restore.id = "sink-input-by-application-id:..."`
(the native-pipewire form). With ao=pulse, the same client
shows `"sink-input-by-application-name:..."` (the pulseaudio
protocol form, identical to Firefox's entry). wireplumber
literally renames the restore key to indicate the protocol.
Fix is one mpv option. Set `ao="pulse,wasapi,"` in the MPV
constructor: comma-separated priority list, mpv tries each in
order. `pulse` works on Linux via the pipewire pulseaudio compat
layer; `wasapi` is the Windows audio API; trailing empty falls
through to the compiled-in default. No platform branch needed
in the constructor — mpv silently skips audio outputs that
aren't available on the current platform.
Also added `audio_client_name="booru-viewer"` so the client
shows up in pulseaudio/pipewire introspection tools as
booru-viewer rather than the default "mpv Media Player". Sets
application.name, application.id, application.icon_name,
node.name, and device.description to "booru-viewer". Cosmetic
on its own but groups mpv's audio under the same identity as
the Qt application.
References for the Discord audio bug:
https://github.com/mpv-player/mpv/issues/11100https://github.com/edisionnano/Screenshare-with-audio-on-Discord-with-Linuxhttps://bbs.archlinux.org/viewtopic.php?id=307698
The old _fit_to_content was width-anchored with an asymmetric height
clamp, so every portrait nav back-derived a smaller width and P>L>P
loops progressively shrunk landscape. Replaced with a viewport-keyed
compute (long_side + center), symmetric across aspect flips. The
non-Hyprland branch now uses setGeometry instead of self.resize() to
stop top-left drift.
Three fixes that all surfaced from the bookmark/library decoupling
shake-out:
- Popout first-image aspect-lock race: _fit_to_content used to call
_is_hypr_floating which returned None for both "not Hyprland" and
"Hyprland but the window isn't visible to hyprctl yet". The latter
happens on the very first popout open because the wm:openWindow
event hasn't been processed when set_media fires. The method then
fell through to a plain Qt resize and skipped the
keep_aspect_ratio setprop, so the first image always opened
unlocked and only subsequent navigations got the right shape. Now
we inline the env-var check, distinguish the two None cases, and
retry on Hyprland with a 40ms backoff (capped at 5 attempts /
200ms total) when the window isn't registered yet.
- Image fill in popout (and embedded preview): ImageViewer._fit_to_view
used min(scale_w, scale_h, 1.0) which clamped the zoom at native
pixel size, so a smaller image in a larger window centered with
letterbox space around it. Dropped the 1.0 cap so images scale up
to fill the available view, matching how the video player fills
its widget. Combined with the popout's keep_aspect_ratio, the
window matches the image's aspect AND the image fills it cleanly.
Tiled popouts with mismatched aspect still letterbox (intentional —
the layout owns the window shape).
- Combo + button padding tightening across all 12 bundled themes
and Library sort combo: QPushButton padding 2px 8px → 2px 6px,
QComboBox padding 2px 6px → 2px 4px, QComboBox::drop-down width
18px → 14px. Saves 8px non-text width per combo and 4px per
button, so the new "Post ID" sort entry fits in 75px instead of
needing 90. Library sort combo bumped from "Name" (lexicographic)
to "Post ID" with a numeric stem sort that handles non-digit
stems gracefully.
Bookmark folders and library folders used to share identity through
_db.get_folders() — the same string was both a row in favorite_folders
and a directory under saved_dir. They look like one concept but they're
two stores, and the cross-bleed produced a duplicate-on-move bug and
made "Save to Library" silently re-file the bookmark too.
Now they're independent name spaces:
- library_folders() in core.config reads filesystem subdirs of
saved_dir; the source of truth for every Save-to-Library menu
- find_library_files(post_id) walks the library shallowly and is the
new "is this saved?" / delete primitive
- bookmark folders stay DB-backed and are only used for bookmark
organization (filter combo, Move to Folder)
- delete_from_library no longer takes a folder hint — walks every
library folder by post id and deletes every match (also cleans up
duplicates left by the old save-to-folder copy bug)
- _save_to_library is move-aware: if the post is already in another
library folder, atomic Path.rename() into the destination instead
of re-copying from cache (the duplicate bug fix)
- bookmark "Move to Folder" no longer also calls _copy_to_library;
Save to Library no longer also calls move_bookmark_to_folder
- settings export/import unchanged; favorite_folders table preserved
so no migration
UI additions:
- Library tab right-click: Move to Folder submenu (single + multi),
uses Path.rename for atomic moves
- Bookmarks tab: − Folder button next to + Folder for deleting the
selected bookmark folder (DB-only, library filesystem untouched)
- Browse tab right-click: "Bookmark" replaced with "Bookmark as"
submenu when not yet bookmarked (Unfiled / folders / + New); flat
"Remove Bookmark" when already bookmarked
- Embedded preview Bookmark button: same submenu shape via new
bookmark_to_folder signal + set_bookmark_folders_callback
- Popout Bookmark button: same shape — works in both browse and
bookmarks tab modes
- Popout Save button: Save-to-Library submenu via new save_to_folder
+ unsave_requested signals (drops save_toggle_requested + the
_save_toggle_from_popout indirection)
- Popout in library mode: Save button stays visible as Unsave; the
rest of the toolbar (Bookmark / BL Tag / BL Post) is hidden
State plumbing:
- _update_fullscreen_state mirrors the embedded preview's
_is_bookmarked / _is_saved instead of re-querying DB+filesystem,
eliminating the popout state drift during async bookmark adds
- Library tab Save button reads "Unsave" the entire time; Save
button width bumped 60→75 so the label doesn't clip on tight themes
- Embedded preview tracks _is_bookmarked alongside _is_saved so the
new Bookmark-as submenu can flip to a flat unbookmark when active
Naming:
- "Unsorted" renamed to "Unfiled" everywhere user-facing — library
Unfiled and bookmarks Unfiled now share one label. Internal
comparison in library.py:_scan_files updated to match the combo.
- Replace tagline with positioning ("for people who keep what they save and rice what they run") and fold backend names into the factual sub-line in strict alphabetical order
- Add Why booru-viewer section between Screenshots and Features: names ahoviewer / Grabber / Hydrus, lays out the labor axis (who does the filing) and the desktop axis (Hyprland/Wayland targeting)
- Split intertwined Bookmarks & Library section into two distinct sections. Bookmarks gets the browser-star framing with the bookmark-folder vs library-folder separation noted; Library absorbs save/promotion/folder content and gets the tag-search bullet
- Add three-tab callout at the top of Features mapping Browse/Bookmarks/Library to commitment levels
- Browsing thumbnail grid bullet absorbs grid-wide features (multi-select, bulk context menus, drag-out)
- Theming: note that each bundled theme ships in rounded and square variants
- Data Locations: backup recipe explaining the saved/ + booru.db split and recovery path
Mixing `threading.Thread + asyncio.run` workers with the long-lived
asyncio loop in gui/app.py is a real loop-affinity bug: the first worker
thread to call `asyncio.run` constructs a throwaway loop, which the
shared httpx clients then attach to, and the next call from the
persistent loop fails with "Event loop is closed" / "attached to a
different loop". This commit eliminates the pattern across the GUI and
adds the locking + cleanup that should have been there from the start.
Persistent loop accessor (core/concurrency.py — new)
- set_app_loop / get_app_loop / run_on_app_loop. BooruApp registers the
one persistent loop at startup; everything that wants to schedule
async work calls run_on_app_loop instead of spawning a thread that
builds its own loop. Three functions, ~30 lines, single source of
truth for "the loop".
Lazy-init lock + cleanup on shared httpx clients (core/api/base.py,
core/api/e621.py, core/cache.py)
- Each shared singleton (BooruClient._shared_client, E621Client._e621_client,
cache._shared_client) now uses fast-path / locked-slow-path lazy init.
Concurrent first-callers from the same loop can no longer both build
a client and leak one (verified: 10 racing callers => 1 httpx instance).
- Each module exposes an aclose helper that BooruApp.closeEvent runs via
run_coroutine_threadsafe(...).result(timeout=5) BEFORE stopping the
loop. The connection pool, keepalive sockets, and TLS state finally
release cleanly instead of being abandoned at process exit.
- E621Client tracks UA-change leftovers in _e621_to_close so the old
client doesn't leak when api_user changes — drained in aclose_shared.
GUI workers routed through the persistent loop (gui/sites.py,
gui/bookmarks.py)
- SiteDialog._on_detect / _on_test: replaced
`threading.Thread(target=lambda: asyncio.run(...))` with
run_on_app_loop. Results marshaled back through Qt Signals connected
with QueuedConnection. Added _closed flag + _inflight futures list:
closeEvent cancels pending coroutines and shorts out the result emit
if the user closes the dialog mid-detect (no use-after-free on
destroyed QObject).
- BookmarksView._load_thumb_async: same swap. The existing thumb_ready
signal already used QueuedConnection so the marshaling side was
already correct.
DB write serialization (core/db.py)
- Database._write_lock = threading.RLock() — RLock not Lock so a
writing method can call another writing method on the same thread
without self-deadlocking.
- New _write() context manager composes the lock + sqlite3's connection
context manager (the latter handles BEGIN / COMMIT / ROLLBACK
atomically). Every write method converted: add_site, update_site,
delete_site, add_bookmark, add_bookmarks_batch, remove_bookmark,
update_bookmark_cache_path, add_folder, remove_folder, rename_folder,
move_bookmark_to_folder, add/remove_blacklisted_tag,
add/remove_blacklisted_post, save_library_meta, remove_library_meta,
set_setting, add_search_history, clear_search_history,
remove_search_history, add_saved_search, remove_saved_search.
- _migrate keeps using the lock + raw _conn context manager because
it runs from inside the conn property's lazy init (where _write()
would re-enter conn).
- Reads stay lock-free and rely on WAL for reader concurrency. Verified
under contention: 5 threads × 50 add_bookmark calls => 250 rows,
zero corruption, zero "database is locked" errors.
Smoke-tested with seven scenarios: get_app_loop raises before set,
run_on_app_loop round-trips, lazy init creates exactly one client,
10 concurrent first-callers => 1 httpx, aclose_shared cleans up,
RLock allows nested re-acquire, multi-threaded write contention.
Sweep of defensive hardening across the core layers plus a related popout
overlay regression that surfaced during verification.
Database integrity (core/db.py)
- Wrap delete_site, add_search_history, remove_folder, rename_folder,
and _migrate in `with self.conn:` so partial commits can't leave
orphan rows on a crash mid-method.
- add_bookmark re-SELECTs the existing id when INSERT OR IGNORE
collides on (site_id, post_id). Was returning Bookmark(id=0)
silently, which then no-op'd update_bookmark_cache_path the next
time the post was bookmarked.
- get_bookmarks LIKE clauses now ESCAPE '%', '_', '\\' so user search
literals stop acting as SQL wildcards (cat_ear no longer matches
catear).
Path traversal (core/db.py + core/config.py)
- Validate folder names at write time via _validate_folder_name —
rejects '..', os.sep, leading '.' / '~'. Permits Unicode/spaces/
parens so existing folders keep working.
- saved_folder_dir() resolves the candidate path and refuses anything
that doesn't relative_to the saved-images base. Defense in depth
against folder strings that bypass the write-time validator.
- gui/bookmarks.py and gui/app.py wrap add_folder calls in try/except
ValueError and surface a QMessageBox.warning instead of crashing.
Download safety (core/cache.py)
- New _do_download(): payloads >=50MB stream to a tempfile in the
destination dir and atomically os.replace into place; smaller
payloads keep the existing buffer-then-write fast path. Both
enforce a 500MB hard cap against the advertised Content-Length AND
the running total inside the chunk loop (servers can lie).
- Per-URL asyncio.Lock coalesces concurrent downloads of the same
URL so two callers don't race write_bytes on the same path.
- Image.MAX_IMAGE_PIXELS = 256M with DecompressionBombError handling
in both converters.
- _convert_ugoira_to_gif checks frame count + cumulative uncompressed
size against UGOIRA_MAX_FRAMES / UGOIRA_MAX_UNCOMPRESSED_BYTES from
ZipInfo headers BEFORE decompressing — defends against zip bombs.
- _convert_animated_to_gif writes a .convfailed sentinel sibling on
failure to break the re-decode-on-every-paint loop for malformed
animated PNGs/WebPs.
- _is_valid_media returns True (don't delete) on OSError so a
transient EBUSY/permissions hiccup no longer triggers a delete +
re-download loop on every access.
- _referer_for() uses proper hostname suffix matching, not substring
`in` (imgblahgelbooru.attacker.com no longer maps to gelbooru.com).
- PIL handles wrapped in `with` blocks for deterministic cleanup.
API client retry + visibility (core/api/*)
- base.py: _request retries on httpx.NetworkError + ConnectError in
addition to TimeoutException. test_connection no longer echoes the
HTTP response body in the error string (it was an SSRF body-leak
gadget when used via detect_site_type's redirect-following client).
- detect.py + danbooru.py + e621.py + gelbooru.py + moebooru.py:
every previously-swallowed exception in search/autocomplete/probe
paths now logs at WARNING with type, message, and (where relevant)
the response body prefix. Debugging "the site isn't working" used
to be a total blackout.
main_gui.py
- file_dialog_platform DB probe failure prints to stderr instead of
vanishing.
Popout overlay (gui/preview.py + gui/app.py)
- preview.py:79,141 — setAttribute(WA_StyledBackground, True) on
_slideshow_toolbar and _slideshow_controls. Plain QWidget parents
silently ignore QSS `background:` declarations without this
attribute, which is why the popout overlay strip was rendering
fully transparent (buttons styled, bar behind them showing the
letterbox color).
- app.py: bake _BASE_POPOUT_OVERLAY_QSS as a fallback prepended
before the user's custom.qss in the loader. Custom themes that
don't define overlay rules now still get a translucent black
bar with white text + hairline borders. Bundled themes win on
tie because their identical-specificity rules come last in the
prepended string.
