From ec238f3aa4f48b708878e939f49b611fde1856bd Mon Sep 17 00:00:00 2001 From: pax Date: Wed, 8 Apr 2026 20:33:12 -0500 Subject: [PATCH] gui/main_window: replace popout internal access with public methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- booru_viewer/gui/main_window.py | 75 +++++++------- booru_viewer/gui/popout/window.py | 158 ++++++++++++++++++++++++++++++ 2 files changed, 195 insertions(+), 38 deletions(-) diff --git a/booru_viewer/gui/main_window.py b/booru_viewer/gui/main_window.py index 2a1d64e..98e9e31 100644 --- a/booru_viewer/gui/main_window.py +++ b/booru_viewer/gui/main_window.py @@ -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") diff --git a/booru_viewer/gui/popout/window.py b/booru_viewer/gui/popout/window.py index fa9815a..f74d15c 100644 --- a/booru_viewer/gui/popout/window.py +++ b/booru_viewer/gui/popout/window.py @@ -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