pax/booru-viewer
1
0
Fork 0
Code Issues Releases 18 Activity

gui/main_window: replace popout internal access with public methods

Browse Source 
Drops every direct popout._underscore access from main_window in favor
of nine new public methods on FullscreenPreview. The legacy private
fields (_video, _viewer, _stack, _bookmark_btn, etc.) stay in place —
this is a clean public wrapper layer, not a re-architecture. Going
through public methods makes the popout's interface explicit and
prevents future code from reaching into popout internals.

New public methods on FullscreenPreview:

  is_video_active() -> bool
    Replaces popout._stack.currentIndex() == 1 checks. Used to gate
    video-only operations.

  set_toolbar_visibility(*, bookmark, save, bl_tag, bl_post)
    Replaces 4-line popout._bookmark_btn.setVisible(...) etc block.
    Per-tab toolbar gating.

  sync_video_state(*, volume, mute, autoplay, loop_state)
    Replaces 4-line popout._video.volume = ... etc block. Called by
    main_window's _open_fullscreen_preview to push embedded preview
    state into the popout.

  get_video_state() -> dict
    Returns volume / mute / autoplay / loop_state / position_ms in
    one read. Replaces 5 separate popout._video.* attribute reads
    in main_window's _on_fullscreen_closed reverse sync.

  seek_video_to(ms)
    Wraps VideoPlayer.seek_to_ms (which uses 'absolute+exact' since
    the 609066c drag-back fix). Used by the seek-after-load pattern.

  connect_media_ready_once(callback)
    One-shot callback wiring with auto-disconnect. Replaces the
    manual lambda + try/except disconnect dance in main_window.

  pause_media()
    Wraps VideoPlayer.pause(). Replaces 3 sites of direct
    popout._video.pause() calls in privacy-screen / external-open
    paths.

  force_mpv_pause()
    Direct mpv.pause = True without button text update. Replaces
    the legacy popout._video._mpv.pause = True deep attribute access
    in main_window's _on_post_activated. Used to prevent the OLD
    video from reaching natural EOF during the new post's async
    download.

  stop_media()
    Stops the video and clears the image viewer. Replaces 2 sites
    of the popout._viewer.clear() + popout._video.stop() sequence
    in blacklist-removal flow.

main_window.py call sites updated:

  Line 1122-1130 (_on_post_activated):
    popout._video._mpv.pause = True → popout.force_mpv_pause()

  Line 1339-1342 (_update_fullscreen):
    4 popout._*.setVisible(...) → popout.set_toolbar_visibility(...)

  Line 1798, 1811, 2731:
    popout._video.pause() → popout.pause_media()

  Line 2151-2166 (_open_fullscreen_preview sync block):
    sv = popout._video; sv.volume = ...; ...
    + manual seek-when-ready closure
    → popout.sync_video_state(...) + popout.connect_media_ready_once(...)

  Line 2196-2207 (_on_fullscreen_closed reverse sync):
    sv = popout._video; pv.volume = sv.volume; ...; popout._stack.currentIndex...
    → popout.get_video_state() returning a dict

  Line 2393-2394, 2421-2423 (blacklist removal):
    popout._viewer.clear() + popout._video.stop()
    → popout.stop_media()

After this commit, main_window has ZERO direct popout._underscore
accesses. The popout's public method surface is the only way for
main_window to interact with the popout's internals.

The popout's public method surface is now:

  Lifecycle:
    - set_media (existing — keeps the kind, info, width, height contract)
    - update_state (existing — bookmarked/saved button labels)
    - close (Qt builtin — triggers closeEvent)

  Wiring:
    - set_post_tags
    - set_bookmark_folders_callback
    - set_folders_callback

  Privacy:
    - privacy_hide / privacy_show (existing)

  New in commit 15:
    - is_video_active
    - set_toolbar_visibility
    - sync_video_state
    - get_video_state
    - seek_video_to
    - connect_media_ready_once
    - pause_media
    - force_mpv_pause
    - stop_media

  Outbound signals (unchanged from refactor start):
    - navigate / play_next_requested / closed
    - bookmark_requested / bookmark_to_folder
    - save_to_folder / unsave_requested
    - blacklist_tag_requested / blacklist_post_requested
    - privacy_requested

Tests passing after this commit: 81 / 81 (16 Phase A + 65 state).
Phase A still green.

Verification:
- Imports clean
- Pure-Python state machine + tests unchanged
- main_window's popout interaction goes through public methods only

Test cases for commit 16 (final shim cleanup):
- Drop the hyprland re-export shim methods from popout/window.py
- Have callers use popout.hyprland directly
This commit is contained in:
pax 2026-04-08 20:33:12 -05:00
parent 69d25b325e
commit ec238f3aa4
2 changed files with 195 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

75
booru_viewer/gui/main_window.py
View File

@ -1120,9 +1120,7 @@ class BooruApp(QMainWindow):
# or already stopped — pause is a no-op there.
try:
if self._fullscreen_window:
fmpv = self._fullscreen_window._video._mpv
if fmpv is not None:
fmpv.pause = True
self._fullscreen_window.force_mpv_pause()
pmpv = self._preview._video_player._mpv
if pmpv is not None:
pmpv.pause = True
@ -1336,10 +1334,12 @@ class BooruApp(QMainWindow):
# visible — it acts as Unsave for the library file currently
# being viewed, matching the embedded preview's library mode.
show_full = self._stack.currentIndex() != 2
self._fullscreen_window._bookmark_btn.setVisible(show_full)
self._fullscreen_window._save_btn.setVisible(True)
self._fullscreen_window._bl_tag_btn.setVisible(show_full)
self._fullscreen_window._bl_post_btn.setVisible(show_full)
self._fullscreen_window.set_toolbar_visibility(
bookmark=show_full,
save=True,
bl_tag=show_full,
bl_post=show_full,
)
self._update_fullscreen_state()
def _update_fullscreen_state(self) -> None:
@ -1795,7 +1795,7 @@ class BooruApp(QMainWindow):
if path is not None:
self._preview._video_player.pause()
if self._fullscreen_window and self._fullscreen_window.isVisible():
self._fullscreen_window._video.pause()
self._fullscreen_window.pause_media()
QDesktopServices.openUrl(QUrl.fromLocalFile(str(path)))
else:
self._status.showMessage("Bookmark not cached — open it first to download")
@ -1808,7 +1808,7 @@ class BooruApp(QMainWindow):
if current and Path(current).exists():
self._preview._video_player.pause()
if self._fullscreen_window and self._fullscreen_window.isVisible():
self._fullscreen_window._video.pause()
self._fullscreen_window.pause_media()
QDesktopServices.openUrl(QUrl.fromLocalFile(current))
return
# Browse: original path. Removed the "open random cache file"
@ -2148,22 +2148,21 @@ class BooruApp(QMainWindow):
post = self._preview._current_post
if post:
self._fullscreen_window.set_post_tags(post.tag_categories, post.tag_list)
# Sync video player state from preview to popout
# Sync video player state from preview to popout via the
# popout's public sync_video_state method (replaces direct
# popout._video.* attribute writes).
pv = self._preview._video_player
sv = self._fullscreen_window._video
sv.volume = pv.volume
sv.is_muted = pv.is_muted
sv.autoplay = pv.autoplay
sv.loop_state = pv.loop_state
self._fullscreen_window.sync_video_state(
volume=pv.volume,
mute=pv.is_muted,
autoplay=pv.autoplay,
loop_state=pv.loop_state,
)
# Connect seek-after-load BEFORE set_media so we don't miss media_ready
if video_pos > 0:
def _seek_when_ready():
sv.seek_to_ms(video_pos)
try:
sv.media_ready.disconnect(_seek_when_ready)
except RuntimeError:
pass
sv.media_ready.connect(_seek_when_ready)
self._fullscreen_window.connect_media_ready_once(
lambda: self._fullscreen_window.seek_video_to(video_pos)
)
# Pre-fit dimensions for the popout video pre-fit optimization
# — `post` is the same `self._preview._current_post` referenced
# at line 2164 (set above), so reuse it without an extra read.
@ -2193,18 +2192,20 @@ class BooruApp(QMainWindow):
# Clear the popout-active flag now that the right splitter is back
# in its real shape — future splitterMoved events should persist.
self._popout_active = False
# Sync video player state from popout back to preview
if self._fullscreen_window:
sv = self._fullscreen_window._video
pv = self._preview._video_player
pv.volume = sv.volume
pv.is_muted = sv.is_muted
pv.autoplay = sv.autoplay
pv.loop_state = sv.loop_state
# Grab video position before cleanup
# Sync video player state from popout back to preview via
# the popout's public get_video_state method (replaces direct
# popout._video.* attribute reads + popout._stack.currentIndex
# check). The dict carries volume / mute / autoplay / loop_state
# / position_ms in one read.
video_pos = 0
if self._fullscreen_window and self._fullscreen_window._stack.currentIndex() == 1:
video_pos = self._fullscreen_window._video.get_position_ms()
if self._fullscreen_window:
vstate = self._fullscreen_window.get_video_state()
pv = self._preview._video_player
pv.volume = vstate["volume"]
pv.is_muted = vstate["mute"]
pv.autoplay = vstate["autoplay"]
pv.loop_state = vstate["loop_state"]
video_pos = vstate["position_ms"]
# Restore preview with current media
path = self._preview._current_path
info = self._preview._info_label.text()
@ -2390,8 +2391,7 @@ class BooruApp(QMainWindow):
if cp == self._preview._current_path:
self._preview.clear()
if self._fullscreen_window and self._fullscreen_window.isVisible():
self._fullscreen_window._viewer.clear()
self._fullscreen_window._video.stop()
self._fullscreen_window.stop_media()
self._status.showMessage(f"Blacklisted: {tag}")
self._remove_blacklisted_from_grid(tag=tag)
elif action == bl_post_action:
@ -2419,8 +2419,7 @@ class BooruApp(QMainWindow):
if cp == self._preview._current_path:
self._preview.clear()
if self._fullscreen_window and self._fullscreen_window.isVisible():
self._fullscreen_window._viewer.clear()
self._fullscreen_window._video.stop()
self._fullscreen_window.stop_media()
break
# Remove from posts list (reverse order to keep indices valid)
@ -2728,7 +2727,7 @@ class BooruApp(QMainWindow):
# Pause any playing video before opening externally
self._preview._video_player.pause()
if self._fullscreen_window and self._fullscreen_window.isVisible():
self._fullscreen_window._video.pause()
self._fullscreen_window.pause_media()
QDesktopServices.openUrl(QUrl.fromLocalFile(str(path)))
else:
self._status.showMessage("Image not cached yet — double-click to download first")

158
booru_viewer/gui/popout/window.py
View File

@ -615,6 +615,164 @@ class FullscreenPreview(QMainWindow):
self._is_saved = saved
self._save_btn.setText("Unsave" if saved else "Save")
# ------------------------------------------------------------------
# Public method interface (commit 15)
# ------------------------------------------------------------------
#
# The methods below replace direct underscore access from
# main_window.py. They wrap the existing private fields so
# main_window doesn't have to know about VideoPlayer / ImageViewer
# / QStackedWidget internals. The legacy private fields stay in
# place — these are clean public wrappers, not a re-architecture.
def is_video_active(self) -> bool:
"""True if the popout is currently showing a video (vs image).
Replaces direct `popout._stack.currentIndex() == 1` checks
from main_window. Used to gate per-tab video-only operations
(volume scroll, seek, pause).
"""
return self._stack.currentIndex() == 1
def set_toolbar_visibility(
self,
*,
bookmark: bool,
save: bool,
bl_tag: bool,
bl_post: bool,
) -> None:
"""Per-tab toolbar gating.
Replaces direct `popout._bookmark_btn.setVisible(...)` etc
from main_window's `_update_fullscreen` method. Library tab
hides Bookmark / BL Tag / BL Post (no site/post id to act
on) but keeps Save (acts as Unsave for the file currently
being viewed).
"""
self._bookmark_btn.setVisible(bookmark)
self._save_btn.setVisible(save)
self._bl_tag_btn.setVisible(bl_tag)
self._bl_post_btn.setVisible(bl_post)
def sync_video_state(
self,
*,
volume: int,
mute: bool,
autoplay: bool,
loop_state: int,
) -> None:
"""Push state from the embedded preview into the popout's
video player.
Called by main_window's `_open_fullscreen_preview` after the
popout is constructed. Replaces direct `popout._video.volume
= ...` etc writes. Uses VideoPlayer's existing setters which
handle the lazy-mpv pending-state pattern (mute survives
first-load via _pending_mute, volume survives via the slider
widget acting as persistent storage).
"""
self._video.volume = volume
self._video.is_muted = mute
self._video.autoplay = autoplay
self._video.loop_state = loop_state
def get_video_state(self) -> dict:
"""Read video player state for the reverse sync at popout close.
Returns a dict with `volume`, `mute`, `autoplay`, `loop_state`,
and `position_ms` (current playback position in milliseconds,
0 if the popout isn't currently on the video stack). Called
by main_window's `_on_fullscreen_closed` to push the state
back into the embedded preview's video player.
"""
return {
"volume": self._video.volume,
"mute": self._video.is_muted,
"autoplay": self._video.autoplay,
"loop_state": self._video.loop_state,
"position_ms": (
self._video.get_position_ms()
if self.is_video_active()
else 0
),
}
def seek_video_to(self, ms: int) -> None:
"""Seek the video to a specific position in milliseconds.
Used by main_window's seek-after-load pattern when restoring
video position across popout open/close cycles. Wraps
`VideoPlayer.seek_to_ms` (which uses `'absolute+exact'` for
frame-accurate landing — same as the slider's `_seek` after
the 609066c drag-back fix).
"""
self._video.seek_to_ms(ms)
def connect_media_ready_once(self, callback) -> None:
"""Wire a one-shot callback to the video player's media_ready
signal. The callback fires once when the next loaded video
becomes ready, then disconnects itself.
Replaces main_window's manual lambda + try/except disconnect
dance for the seek-when-ready pattern (open popout → wait for
the new mpv instance to load → restore the embedded preview's
playback position).
"""
def _wrapper():
try:
callback()
finally:
try:
self._video.media_ready.disconnect(_wrapper)
except (TypeError, RuntimeError):
pass
self._video.media_ready.connect(_wrapper)
def pause_media(self) -> None:
"""Pause the active video player. No-op if no video is loaded.
Replaces direct `popout._video.pause()` calls from main_window
in privacy-screen / blacklist / video-end paths. Goes through
VideoPlayer.pause() which handles the play-button text update
and respects the lazy-mpv state.
"""
self._video.pause()
def force_mpv_pause(self) -> None:
"""Set mpv.pause = True directly without going through Qt
property setters or button text updates.
Used by main_window's `_on_post_activated` to prevent the OLD
video from reaching natural EOF during the new post's async
download (which would auto-advance past the post the user
clicked). Different from `pause_media` because this writes
the mpv property directly — no eof-reached side effect, no
button text flicker mid-load.
Replaces the legacy `popout._video._mpv.pause = True` deep
attribute access from main_window line ~1125. The
`_mpv is None` guard handles the pre-first-load case where
`_ensure_mpv` hasn't run yet.
"""
if self._video._mpv is not None:
try:
self._video._mpv.pause = True
except Exception:
pass
def stop_media(self) -> None:
"""Stop the video and clear the image viewer.
Used by main_window's blacklist-removal flow when the post
being viewed gets blacklisted. Replaces the
`popout._viewer.clear() + popout._video.stop()` sequence from
main_window with a single call.
"""
self._video.stop()
self._viewer.clear()
def set_bookmark_folders_callback(self, callback) -> None:
"""Wire the bookmark folder list source. Called once from app.py
right after the popout is constructed; matches the embedded