[Bug]: Old Image Stuck in Image Viewer #163

Closed
opened 2026-08-13 14:18:15 +00:00 by ValleyGeek · 3 comments
Owner

App version

0.5.0+feature.162 Beta

Platform

Windows

Steps to reproduce

  1. Open a directory
  2. Open another directory
  3. Choose "Close directory and delete cache"
  4. Open a third directory
  5. View an image in the viewer

Expected behavior

The active image should display

Actual behavior

The last image viewed from the closed directory is stuck in the viewer. Changing images doesn't refresh it.

Additional context

No response

### App version 0.5.0+feature.162 Beta ### Platform Windows ### Steps to reproduce 1. Open a directory 2. Open another directory 3. Choose "Close directory and delete cache" 4. Open a third directory 5. View an image in the viewer ### Expected behavior The active image should display ### Actual behavior The last image viewed from the closed directory is stuck in the viewer. Changing images doesn't refresh it. ### Additional context _No response_
Member

Root cause: ImageViewerSub.open_session() cleared the display/size/fit caches on a new session but never reset _current_rel_path/_current_meta. _go_to_session_index() has an "already showing this image" fast path that skips reloading when the requested session index equals the current one and its resolved relative path matches _current_rel_path. On a freshly opened session that comparison could spuriously match leftover state from the previously viewed directory (e.g. reused filenames, or common camera defaults like IMG_0001.jpg), so the viewer never issued a load for the new directory's image and the stale pixmap stayed on screen.

Fix: open_session() now resets _current_rel_path/_current_meta before positioning, so the fast path can never fire on the first navigation of a new session.

Pushed to 0.6.1/issue-163-stale-viewer-image with a regression test (tests/test_pr8_viewer.py::test_open_session_reloads_when_relative_path_matches_prior_session) that reproduces the stuck image via two directories sharing a filename at the same session index. Full suite (701 passed) and ruff pass. Not yet merged to main — awaiting PR request.

Root cause: `ImageViewerSub.open_session()` cleared the display/size/fit caches on a new session but never reset `_current_rel_path`/`_current_meta`. `_go_to_session_index()` has an "already showing this image" fast path that skips reloading when the requested session index equals the current one *and* its resolved relative path matches `_current_rel_path`. On a freshly opened session that comparison could spuriously match leftover state from the previously viewed directory (e.g. reused filenames, or common camera defaults like `IMG_0001.jpg`), so the viewer never issued a load for the new directory's image and the stale pixmap stayed on screen. Fix: `open_session()` now resets `_current_rel_path`/`_current_meta` before positioning, so the fast path can never fire on the first navigation of a new session. Pushed to `0.6.1/issue-163-stale-viewer-image` with a regression test (`tests/test_pr8_viewer.py::test_open_session_reloads_when_relative_path_matches_prior_session`) that reproduces the stuck image via two directories sharing a filename at the same session index. Full suite (701 passed) and ruff pass. Not yet merged to `main` — awaiting PR request.
Member

Found a second, distinct bug behind this — different from the open_session() fix already on this branch (which addressed the original repro through a new directory's session picking up a stale cached image). Your report ("closing a directory and deleting cache is keeping the last viewed image in the viewer") pointed at the close flow specifically.

Root cause: _finalize_workspace_close() (the "Close Directory and Delete Cache" handler) force-cleared self._in_viewer_sub = False directly, then called _reset_workspace_to_grid() — which reads that same flag to decide whether the widget stack needs to switch away from the viewer back to the grid. Since the flag was already cleared, that check always evaluated as "wasn't in the viewer," so the actual widget switch never happened whenever you were browsing the grid before opening the viewer (the common case). The internal bookkeeping said "not in viewer" but the screen kept showing the viewer with the last image, since nothing ever told the QStackedWidget to switch back.

Fixed by removing that premature flag clear, so _reset_workspace_to_grid() sees the true prior state and correctly switches back to the grid. Verified with a regression test that fails without the fix (confirmed currentWidget() was still the viewer) and passes with it. Details in DR-040.

Pushed to 0.6.1/v0.7.0 at f834e80. Let me know if you still see stale images after a directory close on the next build.

Found a second, distinct bug behind this — different from the `open_session()` fix already on this branch (which addressed the original repro through a new directory's session picking up a stale cached image). Your report ("closing a directory and deleting cache is keeping the last viewed image in the viewer") pointed at the close flow specifically. Root cause: `_finalize_workspace_close()` (the "Close Directory and Delete Cache" handler) force-cleared `self._in_viewer_sub = False` directly, then called `_reset_workspace_to_grid()` — which reads that same flag to decide whether the widget stack needs to switch away from the viewer back to the grid. Since the flag was already cleared, that check always evaluated as "wasn't in the viewer," so the actual widget switch never happened whenever you were browsing the grid before opening the viewer (the common case). The internal bookkeeping said "not in viewer" but the screen kept showing the viewer with the last image, since nothing ever told the `QStackedWidget` to switch back. Fixed by removing that premature flag clear, so `_reset_workspace_to_grid()` sees the true prior state and correctly switches back to the grid. Verified with a regression test that fails without the fix (confirmed `currentWidget()` was still the viewer) and passes with it. Details in DR-040. Pushed to `0.6.1/v0.7.0` at `f834e80`. Let me know if you still see stale images after a directory close on the next build.
Member

Found the actual root cause — confirmed by live instrumentation of the real async load pipeline, not a guess. Your report ruled out both prior fixes (no filename overlap between the two directories, so no cache-key collision was possible), which pointed me at the async loading path instead of session/state-transition logic.

_release_workspace_db_locks() unconditionally calls self.viewer.shutdown(), which permanently sets the viewer's image-loader signals to inactive — and nothing anywhere in the codebase ever reactivates them. shutdown()'s own docstring says "before window teardown," and it's correctly called from closeEvent() (real app exit) — but _release_workspace_db_locks() is also called from the routine "Close Directory and Delete Cache" path, not just app exit. Once that flag flips, ViewerImageLoader.run()'s very first check silently no-ops every future image load, for any future directory, for the rest of the session. The very first directory close in a session permanently breaks the viewer — exactly matching "even when cycling image, the tiger image never changed," since no image would ever load again after that first close.

Fix: moved viewer.shutdown() out of the shared helper and into closeEvent() directly, so real app teardown is unaffected but routine directory closes no longer touch it.

Verified with a real (non-mocked) reproduction: two directories with real, distinctly-sized JPEG files, loaded through the actual background thread pool — the test fails on the pre-fix code showing the exact symptom (viewer still shows the first directory's image after opening the second) and passes with the fix. Also confirmed smoke-ui-exit.sh still shows clean app teardown with the relocated call.

Pushed to 0.6.1/v0.7.0 at 7c4d718. Full writeup in DR-041, including why the first two fixes on this branch were each real but insufficient. Appreciate you pushing back on my earlier theories with concrete detail each time — that's what actually got to the real cause.

Found the actual root cause — confirmed by live instrumentation of the real async load pipeline, not a guess. Your report ruled out both prior fixes (no filename overlap between the two directories, so no cache-key collision was possible), which pointed me at the async loading path instead of session/state-transition logic. `_release_workspace_db_locks()` unconditionally calls `self.viewer.shutdown()`, which permanently sets the viewer's image-loader signals to inactive — and nothing anywhere in the codebase ever reactivates them. `shutdown()`'s own docstring says "before window teardown," and it's correctly called from `closeEvent()` (real app exit) — but `_release_workspace_db_locks()` is *also* called from the routine "Close Directory and Delete Cache" path, not just app exit. Once that flag flips, `ViewerImageLoader.run()`'s very first check silently no-ops **every future image load, for any future directory, for the rest of the session**. The very first directory close in a session permanently breaks the viewer — exactly matching "even when cycling image, the tiger image never changed," since no image would ever load again after that first close. Fix: moved `viewer.shutdown()` out of the shared helper and into `closeEvent()` directly, so real app teardown is unaffected but routine directory closes no longer touch it. Verified with a real (non-mocked) reproduction: two directories with real, distinctly-sized JPEG files, loaded through the actual background thread pool — the test fails on the pre-fix code showing the *exact* symptom (viewer still shows the first directory's image after opening the second) and passes with the fix. Also confirmed `smoke-ui-exit.sh` still shows clean app teardown with the relocated call. Pushed to `0.6.1/v0.7.0` at `7c4d718`. Full writeup in DR-041, including why the first two fixes on this branch were each real but insufficient. Appreciate you pushing back on my earlier theories with concrete detail each time — that's what actually got to the real cause.
Sign in to join this conversation.
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
ai-collab/bulk-image-organizer#163
No description provided.