[Bug]: Old Image Stuck in Image Viewer #163
Labels
No labels
Kind/Bug
Kind/Feature
Priority/High
Priority/Medium
Reviewed/Confirmed
Compat/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
ai-collab/bulk-image-organizer#163
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
App version
0.5.0+feature.162 Beta
Platform
Windows
Steps to reproduce
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
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 likeIMG_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_metabefore 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-imagewith 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 tomain— awaiting PR request.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-clearedself._in_viewer_sub = Falsedirectly, 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 theQStackedWidgetto 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 (confirmedcurrentWidget()was still the viewer) and passes with it. Details in DR-040.Pushed to
0.6.1/v0.7.0atf834e80. Let me know if you still see stale images after a directory close on the next build.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 callsself.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 fromcloseEvent()(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 intocloseEvent()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.shstill shows clean app teardown with the relocated call.Pushed to
0.6.1/v0.7.0at7c4d718. 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.