diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 7be1ac38..afa267de 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- Fix: **A frame no longer wears another frame's thumbnail after switching folders.** #800 stopped a finished render being filed under whatever frame was selected, but only when the session had no frames at all. With a folder open, a render whose own frame had since left the list — a different folder opened, or a merge swapping frames for a composite — still fell back to the selection, and the write is persisted, so the wrong picture survived restarts until that frame was rendered again. Found on disk: a slide-08 frame's render cached under a slide-07 frame, correlating 0.10 with its own file. A render that cannot be attributed to an open frame now updates nothing; the next render of that frame writes it. - Fix: **A long status message now wraps instead of running off the canvas.** The toast was a single unwrapped line, which was fine for "merging exposures" but cut off anything that had to explain itself — such as why a file will not open. It now wraps within a share of the canvas width, using the room it has before folding; short toasts are unchanged. - Fix: **A Nikon High Efficiency NEF now says why it cannot be opened.** The Z 8 and Z 9 can record raw in HE or HE*, which is intoPIX TicoRAW under a licensed codec. Such a file keeps the `.NEF` extension and the same TIFF compression tag as an ordinary lossless NEF, so it parses cleanly and reports full sensor dimensions, and only fails when the pixels are unpacked — as "Unsupported file format or not RAW file", which reads as a corrupt file to someone whose other NEFs all work. NegPy now recognises the payload and says what it is and what to do: re-shoot in Lossless Compressed, or convert to DNG. Decoding these is not possible without the codec. The check runs only after a decode has already failed, so files that work pay nothing for it. - New: **Multi-core CPU rendering is now a setting** (canvas toolbar → **»**). It spreads the CPU rendering kernels across your cores and applies immediately, since every kernel is compiled both ways and picked per call. The kernels run 5-8x faster; a whole HDR merge comes down by about 10%, because most of a merge is decoding the RAW files and that is untouched. **Nothing changes for anyone who does not touch it**: Windows and Linux stay on, macOS stays off. macOS remains off pending wider evidence — Numba's threading layer terminates the process on concurrent entry, and although every parallel call is serialised behind one lock (verified on macOS: the unguarded path aborts within a second, the guarded one survived 68,720 concurrent invocations across 8 threads), that is one machine rather than the range of them. Because such an abort leaves no exception and no log entry, NegPy now records whether each session exited cleanly, and offers to turn the setting back off after a session that did not. diff --git a/negpy/desktop/controller.py b/negpy/desktop/controller.py index 69d7aab7..ef40f5b3 100644 --- a/negpy/desktop/controller.py +++ b/negpy/desktop/controller.py @@ -4489,13 +4489,18 @@ def _asset_for_render(self, metrics: Dict[str, Any]) -> Optional[Dict[str, Any]] active_file_changing caller — there the outgoing file is still selected. """ source_hash = metrics.get("source_hash") - files = self.state.uploaded_files - if source_hash: - for asset in files: - if asset.get("hash") == source_hash: - return asset - idx = self.state.selected_file_idx - return files[idx] if 0 <= idx < len(files) else None + if not source_hash: + return None + for asset in self.state.uploaded_files: + if asset.get("hash") == source_hash: + return asset + # No fallback to the selected frame. This runs on file switch, on save and after an + # export as well as from the render itself, so last_metrics can hold a render whose + # frame has since left the list — a different folder was opened, or a merge swapped + # frames for a composite. Guessing files that buffer under whatever is selected now, + # and persists it, so one frame wears another's picture until it is rendered again. + # A skipped refresh costs nothing; the next render of that frame writes it. + return None def _update_thumbnail_from_state(self, persist: bool = True) -> None: if not self.state.current_file_path or not self.state.current_file_hash: diff --git a/tests/test_controller.py b/tests/test_controller.py index 50397a2b..98066ea0 100644 --- a/tests/test_controller.py +++ b/tests/test_controller.py @@ -260,7 +260,7 @@ def test_rendered_thumbnail_keys_by_asset_identity_not_filename(self): state.config = dc_replace( state.config, rgbscan=RgbScanConfig(enabled=True, green_path="/tmp/_DSC1317.NEF", blue_path="/tmp/_DSC1318.NEF") ) - state.last_metrics = {"base_positive": np.zeros((2, 2, 3), dtype=np.float32)} + state.last_metrics = {"base_positive": np.zeros((2, 2, 3), dtype=np.float32), "source_hash": "h1"} captured = {} self.controller.thumbnail_update_requested.connect(lambda task: captured.setdefault("task", task)) @@ -1957,7 +1957,7 @@ def test_thumbnail_task_carries_the_same_params_as_the_canvas(self): state.selected_file_idx = 0 state.current_file_path = "/tmp/frame.cr2" state.current_file_hash = "hash-1" - state.last_metrics = {"base_positive": np.zeros((4, 4, 3), dtype=np.float32)} + state.last_metrics = {"base_positive": np.zeros((4, 4, 3), dtype=np.float32), "source_hash": "hash-1"} emitted = [] # Drop the real worker connection first: emitting would otherwise hand the diff --git a/tests/test_thumbnail_attribution.py b/tests/test_thumbnail_attribution.py index f3b1c796..105de71e 100644 --- a/tests/test_thumbnail_attribution.py +++ b/tests/test_thumbnail_attribution.py @@ -57,15 +57,19 @@ def test_the_ordinary_case_is_unchanged(self): self.assertEqual(_emitted_key(controller), asset_thumbnail_key(B)) - def test_without_a_source_hash_it_falls_back_to_the_selection(self): - """The active_file_changing caller snapshots the outgoing frame, which is still - the selected one, so the selection is the right answer there.""" + def test_without_a_source_hash_nothing_is_written(self): + """Originally this fell back to the selection, on the grounds that the + active_file_changing caller snapshots the outgoing frame, which is still selected. + That cannot tell "outgoing frame still selected" from "a different folder is open + now", and the second case files one frame's picture under another's — found on + disk, correlating 0.10 with its own file. The render worker always sets + source_hash, so refusing here costs nothing real.""" metrics = {"base_positive": np.zeros((4, 4, 3), np.float32)} controller = _controller(selected_idx=0, metrics=metrics) AppController._update_thumbnail_from_state(controller) - self.assertEqual(_emitted_key(controller), asset_thumbnail_key(A)) + controller.thumbnail_update_requested.emit.assert_not_called() def test_a_render_of_an_unloaded_frame_updates_nothing(self): """Its asset is gone from the session — better to skip than to key by the diff --git a/tests/test_thumbnail_attribution_fallback.py b/tests/test_thumbnail_attribution_fallback.py new file mode 100644 index 00000000..073e4e39 --- /dev/null +++ b/tests/test_thumbnail_attribution_fallback.py @@ -0,0 +1,49 @@ +"""A render whose frame is no longer open must not be filed under whatever is selected now. + +`_update_thumbnail_from_state` runs on file switch, on save, and after an export — not only +from the render that produced the buffer. So `state.last_metrics` can hold a render whose +frame has since left `uploaded_files`: opening a different folder replaces the list, and a +merge or unmerge swaps frames for a composite. Resolving that by falling back to +`selected_file_idx` files one frame's picture under another frame's key, and the write is +persisted, so it survives restarts until that frame is rendered again. +""" + +from unittest.mock import MagicMock + +from negpy.desktop.controller import AppController + + +def _controller(files, selected): + ctrl = MagicMock() + ctrl.state.uploaded_files = files + ctrl.state.selected_file_idx = selected + return ctrl + + +A = {"hash": "hash-a", "path": "/x/a.nef"} +B = {"hash": "hash-b", "path": "/x/b.nef"} + + +def test_a_render_is_attributed_to_its_own_frame(): + ctrl = _controller([A, B], 1) + assert AppController._asset_for_render(ctrl, {"source_hash": "hash-a"}) is A + + +def test_a_render_whose_frame_has_closed_is_not_attributed_to_anyone(): + """The reported bug: a slide-08 frame's render cached under a slide-07 frame's key, + correlation 0.10 against its own file, persisted to disk.""" + ctrl = _controller([A, B], 1) + assert AppController._asset_for_render(ctrl, {"source_hash": "hash-of-a-closed-folder"}) is None + + +def test_metrics_without_an_identity_are_not_attributed(): + ctrl = _controller([A, B], 1) + assert AppController._asset_for_render(ctrl, {}) is None + + +def test_a_composite_is_matched_by_its_own_suffixed_hash(): + """Merges and half-frames carry suffixed hashes; source_hash is current_file_hash, so + they match directly and must not be mistaken for a miss.""" + composite = {"hash": "digest#hdr", "path": "/x/a.nef", "hdr_paths": ("/x/b.nef",)} + ctrl = _controller([composite], 0) + assert AppController._asset_for_render(ctrl, {"source_hash": "digest#hdr"}) is composite diff --git a/tests/test_thumbnail_readback_thread.py b/tests/test_thumbnail_readback_thread.py index 3c11703f..bf8c012b 100644 --- a/tests/test_thumbnail_readback_thread.py +++ b/tests/test_thumbnail_readback_thread.py @@ -50,7 +50,7 @@ def _controller_stub(self, metrics): def test_gpu_texture_is_not_read_back_on_the_ui_thread(self): tex = _FakeTexture(np.zeros((8, 8, 4), dtype=np.float32)) - fn, stub = self._controller_stub({"base_positive": tex}) + fn, stub = self._controller_stub({"base_positive": tex, "source_hash": "h1"}) with patch("negpy.desktop.controller.GPUTexture", _FakeTexture): fn(stub) self.assertEqual(tex.readbacks, 0, "the UI thread must not read back the render texture") @@ -59,7 +59,7 @@ def test_gpu_texture_is_not_read_back_on_the_ui_thread(self): def test_uses_the_host_copy_the_worker_attached(self): tex = _FakeTexture(np.zeros((8, 8, 4), dtype=np.float32)) host = np.full((8, 8, 3), 0.25, dtype=np.float32) - fn, stub = self._controller_stub({"base_positive": tex, "thumbnail_source": host}) + fn, stub = self._controller_stub({"base_positive": tex, "thumbnail_source": host, "source_hash": "h1"}) with patch("negpy.desktop.controller.GPUTexture", _FakeTexture): fn(stub) self.assertEqual(tex.readbacks, 0)