From 74c2813e5ef76453bb6fc5f5d75fea462532bfc8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 19:52:34 +0000 Subject: [PATCH] perf(thumbnails): reuse a decode the live preview cache already holds Update Thumbnails decodes through its own PreviewManager, so a frame the canvas or the neighbor prefetch had just decoded was read from the source again; on a slow network share that is a 15-20 s read per frame (marcinz606/NegPy#1232). The gain is bounded: the live cache holds at most preview_cache_max_entries (8) decodes and the open frame is excluded, so at most ~7 of a 49-frame roll. - PreviewBufferCache.peek: a lookup that leaves the LRU order unchanged. - _linear_preview_key: every plain-frame put and lookup in PreviewManager builds its key here (pure refactor of six inline copies). - PreviewManager.peek_linear_preview: never decodes, never puts. - ThumbnailRenderWorker takes the live service and peeks it first for plain frames (not stitch, HDR or RGB triplet), copying the buffer and metadata arrays so the pipeline never writes into the navigation cache; a miss falls back to the private cache as before. Hits and misses log at debug. - Half-frame assets and roll forks key differently on the live path and always miss; decode params that differ between saved settings and the resolved roll config miss correctly. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W1MEH5L17tDmpmz1H47dap --- negpy/desktop/controller.py | 2 +- negpy/desktop/workers/render.py | 43 +++++- negpy/services/rendering/preview_cache.py | 6 + negpy/services/rendering/preview_manager.py | 151 +++++++++++++------- tests/test_preview_cache.py | 32 +++++ tests/test_preview_manager.py | 22 +++ tests/test_thumbnail_refresh_controller.py | 39 ++++- tests/test_thumbnail_refresh_worker.py | 59 ++++++++ 8 files changed, 299 insertions(+), 55 deletions(-) diff --git a/negpy/desktop/controller.py b/negpy/desktop/controller.py index 20337bbf2..677cacfba 100644 --- a/negpy/desktop/controller.py +++ b/negpy/desktop/controller.py @@ -591,7 +591,7 @@ def __init__(self, session_manager: DesktopSessionManager): self.norm_worker.moveToThread(self.norm_thread) self.batch_autocrop_worker = BatchAutoCropWorker(self.batch_autocrop_preview_service) self.batch_autocrop_worker.moveToThread(self.norm_thread) - self.thumbnail_render_worker = ThumbnailRenderWorker(self.thumbnail_render_preview_service) + self.thumbnail_render_worker = ThumbnailRenderWorker(self.thumbnail_render_preview_service, self.preview_service) self.thumbnail_render_worker.moveToThread(self.norm_thread) self.norm_thread.start() diff --git a/negpy/desktop/workers/render.py b/negpy/desktop/workers/render.py index ca555f376..a61e4d062 100644 --- a/negpy/desktop/workers/render.py +++ b/negpy/desktop/workers/render.py @@ -1591,9 +1591,10 @@ class ThumbnailRenderWorker(QObject): cancelled = pyqtSignal() error = pyqtSignal(str) - def __init__(self, preview_service) -> None: + def __init__(self, preview_service, live_preview_service=None) -> None: super().__init__() self._preview_service = preview_service + self._live_preview_service = live_preview_service self._processor = ImageProcessor(use_gpu=False) self._cancel_lock = threading.RLock() self._cancelled_generations: set[int] = set() @@ -1619,6 +1620,42 @@ def _cancel_requested(self, generation: int) -> bool: with self._cancel_lock: return generation in self._cancelled_generations + def _peek_live_preview(self, frame: ThumbnailRenderInput, workspace_color_space: str) -> Optional[tuple[np.ndarray, dict]]: + """A plain frame's decode from the navigation cache, copied, or None; never writes or + reorders it. The key is the whole-scan one `_decode_asset_preview_with_meta` slices, so + a half-frame asset or roll fork the live path cached under its own hash and half slice + is not found.""" + from negpy.services.assets.half_frame import base_hash, slice_for_asset + + config = frame.config + if self._live_preview_service is None or stitch_active(config.stitch) or hdr_active(config.hdr) or is_rgb_triplet(config.rgbscan): + return None + hit = self._live_preview_service.peek_linear_preview( + frame.file_info["path"], + workspace_color_space, + use_camera_wb=not effective_linear_raw(config.process), + file_hash=base_hash(frame.file_info.get("hash")), + demosaic=config.process.demosaic_preview, + positive_source=config.process.positive_source, + highlight_mode=effective_highlight_reconstruction(config.process), + bake_camera_wb=highlight_reconstruction_bakes_wb(config.process), + lens_corrections=metadata_lens_corrections(config), + lens_flatfield=config.flatfield, + ) + logger.debug("thumbnail refresh live cache %s: %s", "miss" if hit is None else "hit", frame.file_info["path"]) + if hit is None: + return None + raw, _dims, meta = hit + # The navigation cache still serves these arrays, so the pipeline gets its own. + meta = {k: np.copy(v) if isinstance(v, np.ndarray) else v for k, v in meta.items()} + return slice_for_asset(np.copy(raw), frame.file_info), meta + + def _decode(self, frame: ThumbnailRenderInput, workspace_color_space: str) -> tuple[np.ndarray, dict]: + hit = self._peek_live_preview(frame, workspace_color_space) + if hit is not None: + return hit + return _decode_asset_preview_with_meta(self._preview_service, frame.file_info, frame.config, workspace_color_space) + def _emit_finished_unless_cancelled(self, generation: int, rendered_count: int) -> None: """Atomically choose the terminal signal for a generation, so a `cancel()` racing the last frame's completion cannot land between the check and the emit.""" @@ -1653,9 +1690,7 @@ def process(self, task: ThumbnailRenderTask) -> None: break name = str(frame.file_info.get("name") or frame.file_info.get("path") or done) try: - buffer, meta = _decode_asset_preview_with_meta( - self._preview_service, frame.file_info, frame.config, task.workspace_color_space - ) + buffer, meta = self._decode(frame, task.workspace_color_space) cam_xyz = meta.get("cam_xyz") if frame.icc_input_active: cam_xyz = wb_only_cam_xyz(cam_xyz) diff --git a/negpy/services/rendering/preview_cache.py b/negpy/services/rendering/preview_cache.py index 97f5b3500..d46d1fee6 100644 --- a/negpy/services/rendering/preview_cache.py +++ b/negpy/services/rendering/preview_cache.py @@ -102,6 +102,12 @@ def get(self, key: PreviewCacheKey) -> Optional[tuple[ImageBuffer, Dimensions, d self._order.append(t) return ent.buffer, ent.dims, ent.metadata + def peek(self, key: PreviewCacheKey) -> Optional[tuple[ImageBuffer, Dimensions, dict]]: + """Like ``get``, but leaves the LRU order as it is.""" + with self._lock: + ent = self._data.get(key.as_tuple()) + return None if ent is None else (ent.buffer, ent.dims, ent.metadata) + def put( self, key: PreviewCacheKey, diff --git a/negpy/services/rendering/preview_manager.py b/negpy/services/rendering/preview_manager.py index 15335f1a9..d8f6ccf8c 100644 --- a/negpy/services/rendering/preview_manager.py +++ b/negpy/services/rendering/preview_manager.py @@ -82,6 +82,38 @@ def _output_dimensions_from_raw(raw: Any, postprocessed_h: int, postprocessed_w: del _warmup +def _linear_preview_key( + file_hash: str, + *, + color_space: str, + use_camera_wb: bool, + full_resolution: bool, + half_slice: tuple[int, float, tuple[float, float, float, float] | None, float] | None, + demosaic: str, + positive_source: bool, + highlight_mode: int, + bake_camera_wb: bool, + lens_corrections: LensCorrections, + lens_flatfield: FlatFieldConfig, +) -> PreviewCacheKey: + """The cache key of one plain-frame linear decode; every put and lookup builds it here.""" + return PreviewCacheKey( + file_hash=file_hash, + use_camera_wb=use_camera_wb, + workspace_color_space=color_space, + full_resolution=full_resolution, + demosaic=demosaic, + lens_token=lens_decode_token(lens_corrections, lens_flatfield), + half=half_slice[0] if half_slice else 0, + split_x=half_slice[1] if half_slice else 0.5, + crop_rect=half_slice[2] if half_slice else None, + gutter_thickness=half_slice[3] if half_slice else 0.0, + positive_source=positive_source, + highlight_mode=highlight_mode, + bake_camera_wb=bake_camera_wb, + ) + + class PreviewManager: """ Loads RAW (and other) files for UI preview, with in-memory LRU and fast decode. @@ -111,20 +143,18 @@ def prefetch_linear_preview( """Warm one preview when its cache and system-memory budgets both admit it.""" if not file_hash: return False - key = PreviewCacheKey( - file_hash=file_hash, + key = _linear_preview_key( + file_hash, + color_space=color_space, use_camera_wb=use_camera_wb, - workspace_color_space=color_space, full_resolution=False, + half_slice=half_slice, demosaic=demosaic, - lens_token=lens_decode_token(lens_corrections, lens_flatfield), - half=half_slice[0] if half_slice else 0, - split_x=half_slice[1] if half_slice else 0.5, - crop_rect=half_slice[2] if half_slice else None, - gutter_thickness=half_slice[3] if half_slice else 0.0, positive_source=positive_source, highlight_mode=highlight_mode, bake_camera_wb=bake_camera_wb, + lens_corrections=lens_corrections, + lens_flatfield=lens_flatfield, ) if self._cache.contains(key): return True @@ -175,6 +205,41 @@ def prefetch_linear_preview( ) return True + def peek_linear_preview( + self, + file_path: str, + color_space: str, + *, + use_camera_wb: bool, + file_hash: str | None, + full_resolution: bool = False, + half_slice: tuple[int, float, tuple[float, float, float, float] | None, float] | None = None, + demosaic: str = DemosaicMode.AUTO, + positive_source: bool = False, + highlight_mode: int = 0, + bake_camera_wb: bool = False, + lens_corrections: LensCorrections = LensCorrections(), + lens_flatfield: FlatFieldConfig = FlatFieldConfig(), + ) -> Optional[Tuple[ImageBuffer, Dimensions, dict]]: + """The cached decode ``load_linear_preview`` would return for these arguments, or None. + Never decodes and never reorders the LRU. The buffer is shared: do not mutate it.""" + if not file_hash: + return None + key = _linear_preview_key( + file_hash, + color_space=color_space, + use_camera_wb=use_camera_wb, + full_resolution=full_resolution, + half_slice=half_slice, + demosaic=demosaic, + positive_source=positive_source, + highlight_mode=highlight_mode, + bake_camera_wb=bake_camera_wb, + lens_corrections=lens_corrections, + lens_flatfield=lens_flatfield, + ) + return self._cache.peek(key) + # Internal helpers. They operate on an already-open raw object, so a caller that needs # both splash and linear shares one file open. @@ -446,20 +511,18 @@ def _load_from_open_raw( if should_cancel is not None and should_cancel(): raise InterruptedError("preview load cancelled") if file_hash: - ck = PreviewCacheKey( - file_hash=file_hash, + ck = _linear_preview_key( + file_hash, + color_space=color_space, use_camera_wb=use_camera_wb, - workspace_color_space=color_space, full_resolution=full_resolution, + half_slice=half_slice, demosaic=demosaic, - lens_token=lens_decode_token(lens_corrections, lens_flatfield), - half=half_slice[0] if half_slice else 0, - split_x=half_slice[1] if half_slice else 0.5, - crop_rect=half_slice[2] if half_slice else None, - gutter_thickness=half_slice[3] if half_slice else 0.0, positive_source=positive_source, highlight_mode=highlight_mode, bake_camera_wb=bake_camera_wb, + lens_corrections=lens_corrections, + lens_flatfield=lens_flatfield, ) # The cache entry aliases the returned buffer, under the same read-only contract as # a cache hit, so there is no defensive copy. On HQ loads that copy was a large part @@ -531,20 +594,18 @@ def load_linear_preview( # Fast path: skip file open entirely when all cache-key params are known upfront. if file_hash and color_space is not None: - ck = PreviewCacheKey( - file_hash=file_hash, + ck = _linear_preview_key( + file_hash, + color_space=color_space, use_camera_wb=use_camera_wb, - workspace_color_space=color_space, full_resolution=full_resolution, + half_slice=half_slice, demosaic=demosaic, - lens_token=lens_decode_token(lens_corrections, lens_flatfield), - half=half_slice[0] if half_slice else 0, - split_x=half_slice[1] if half_slice else 0.5, - crop_rect=half_slice[2] if half_slice else None, - gutter_thickness=half_slice[3] if half_slice else 0.0, positive_source=positive_source, highlight_mode=highlight_mode, bake_camera_wb=bake_camera_wb, + lens_corrections=lens_corrections, + lens_flatfield=lens_flatfield, ) hit = self._cache.get(ck) if hit is not None: @@ -563,20 +624,18 @@ def load_linear_preview( color_space = metadata.get("color_space") or WORKING_COLOR_SPACE # Re-check now that color_space is resolved from metadata. if file_hash: - ck = PreviewCacheKey( - file_hash=file_hash, + ck = _linear_preview_key( + file_hash, + color_space=color_space, use_camera_wb=use_camera_wb, - workspace_color_space=color_space, full_resolution=full_resolution, + half_slice=half_slice, demosaic=demosaic, - lens_token=lens_decode_token(lens_corrections, lens_flatfield), - half=half_slice[0] if half_slice else 0, - split_x=half_slice[1] if half_slice else 0.5, - crop_rect=half_slice[2] if half_slice else None, - gutter_thickness=half_slice[3] if half_slice else 0.0, positive_source=positive_source, highlight_mode=highlight_mode, bake_camera_wb=bake_camera_wb, + lens_corrections=lens_corrections, + lens_flatfield=lens_flatfield, ) hit = self._cache.get(ck) if hit is not None: @@ -929,20 +988,18 @@ def load_splash_and_linear( # Fast path: skip file open entirely when all cache-key params are known upfront. if file_hash and color_space is not None: - ck = PreviewCacheKey( - file_hash=file_hash, + ck = _linear_preview_key( + file_hash, + color_space=color_space, use_camera_wb=use_camera_wb, - workspace_color_space=color_space, full_resolution=full_resolution, + half_slice=half_slice, demosaic=demosaic, - lens_token=lens_decode_token(lens_corrections, lens_flatfield), - half=half_slice[0] if half_slice else 0, - split_x=half_slice[1] if half_slice else 0.5, - crop_rect=half_slice[2] if half_slice else None, - gutter_thickness=half_slice[3] if half_slice else 0.0, positive_source=positive_source, highlight_mode=highlight_mode, bake_camera_wb=bake_camera_wb, + lens_corrections=lens_corrections, + lens_flatfield=lens_flatfield, ) hit = self._cache.get(ck) if hit is not None: @@ -965,20 +1022,18 @@ def load_splash_and_linear( color_space = metadata.get("color_space") or WORKING_COLOR_SPACE # Re-check now that color_space is resolved from metadata. if file_hash: - ck = PreviewCacheKey( - file_hash=file_hash, + ck = _linear_preview_key( + file_hash, + color_space=color_space, use_camera_wb=use_camera_wb, - workspace_color_space=color_space, full_resolution=full_resolution, + half_slice=half_slice, demosaic=demosaic, - lens_token=lens_decode_token(lens_corrections, lens_flatfield), - half=half_slice[0] if half_slice else 0, - split_x=half_slice[1] if half_slice else 0.5, - crop_rect=half_slice[2] if half_slice else None, - gutter_thickness=half_slice[3] if half_slice else 0.0, positive_source=positive_source, highlight_mode=highlight_mode, bake_camera_wb=bake_camera_wb, + lens_corrections=lens_corrections, + lens_flatfield=lens_flatfield, ) hit = self._cache.get(ck) if hit is not None: diff --git a/tests/test_preview_cache.py b/tests/test_preview_cache.py index 6913e2c71..44302874c 100644 --- a/tests/test_preview_cache.py +++ b/tests/test_preview_cache.py @@ -434,3 +434,35 @@ def navigate(*_args, should_cancel, **_kwargs): ) assert seen == [True, False] + + +def test_cache_peek_leaves_lru_order_unchanged() -> None: + c = PreviewBufferCache(_small_cfg()) + a = np.zeros((4, 4, 3), dtype=np.float32) + first, second, third = (PreviewCacheKey(h, False, "Adobe RGB", False) for h in ("h1", "h2", "h3")) + c.put(first, a, (4, 4), {}) + c.put(second, a.copy(), (4, 4), {}) + + assert c.peek(first)[0] is a + c.put(third, a.copy(), (4, 4), {}) + + assert c.peek(first) is None # still the oldest, so evicted + assert c.peek(second) is not None + + +def test_cache_get_still_reorders() -> None: + c = PreviewBufferCache(_small_cfg()) + a = np.zeros((4, 4, 3), dtype=np.float32) + first, second, third = (PreviewCacheKey(h, False, "Adobe RGB", False) for h in ("h1", "h2", "h3")) + c.put(first, a, (4, 4), {}) + c.put(second, a.copy(), (4, 4), {}) + + assert c.get(first) is not None + c.put(third, a.copy(), (4, 4), {}) + + assert c.peek(first) is not None + assert c.peek(second) is None + + +def test_cache_peek_misses_an_absent_key() -> None: + assert PreviewBufferCache(_small_cfg()).peek(PreviewCacheKey("none", False, "Adobe RGB", False)) is None diff --git a/tests/test_preview_manager.py b/tests/test_preview_manager.py index f684cbef5..7159cc316 100644 --- a/tests/test_preview_manager.py +++ b/tests/test_preview_manager.py @@ -622,3 +622,25 @@ def __exit__(self, *args: object) -> None: _, kwargs = raw.postprocess.call_args assert kwargs["demosaic_algorithm"] == rawpy.DemosaicAlgorithm.VNG assert "half_size" not in kwargs + + +def test_peek_linear_preview_hits_a_decode_load_linear_preview_cached() -> None: + ctx = NonStandardFileWrapper(np.full((120, 160, 3), 0.25, dtype=np.float32)) + mgr = PreviewManager() + with patch("negpy.services.rendering.preview_manager.loader_factory") as lf: + lf.get_loader.return_value = (ctx, {"color_space": "Adobe RGB"}) + buf, dims, _meta = mgr.load_linear_preview("/fake/path.tif", "Adobe RGB", use_camera_wb=True, file_hash="peek-hash") + lf.reset_mock() + + hit = mgr.peek_linear_preview("/fake/path.tif", "Adobe RGB", use_camera_wb=True, file_hash="peek-hash") + wrong_wb = mgr.peek_linear_preview("/fake/path.tif", "Adobe RGB", use_camera_wb=False, file_hash="peek-hash") + absent = mgr.peek_linear_preview("/fake/path.tif", "Adobe RGB", use_camera_wb=True, file_hash="other-hash") + + lf.get_loader.assert_not_called() + assert hit is not None and hit[0] is buf and hit[1] == dims + assert wrong_wb is None + assert absent is None + + +def test_peek_linear_preview_without_a_hash_is_a_miss() -> None: + assert PreviewManager().peek_linear_preview("/fake/path.tif", "Adobe RGB", use_camera_wb=True, file_hash=None) is None diff --git a/tests/test_thumbnail_refresh_controller.py b/tests/test_thumbnail_refresh_controller.py index 249dbf0a8..5dd328ec6 100644 --- a/tests/test_thumbnail_refresh_controller.py +++ b/tests/test_thumbnail_refresh_controller.py @@ -6,9 +6,9 @@ from negpy.desktop.controller import _THUMBNAIL_REFRESH_MEMORY_RETRY_MS, AppController from negpy.desktop.session import AppState, DesktopSessionManager -from negpy.desktop.workers.render import ThumbnailUpdateTask +from negpy.desktop.workers.render import ThumbnailRenderWorker, ThumbnailUpdateTask from negpy.domain.models import WorkspaceConfig -from negpy.services.rendering.preview_manager import PreviewManager +from negpy.services.rendering.preview_manager import PreviewManager, _linear_preview_key class TestThumbnailRefreshController: @@ -480,3 +480,38 @@ def test_cancel_thumbnail_refresh_is_a_noop_with_nothing_running(self) -> None: self.controller.cancel_thumbnail_refresh() self.controller.thumbnail_render_worker.cancel.assert_not_called() + + def test_live_worker_reads_the_navigation_preview_service(self) -> None: + assert self.controller.thumbnail_render_worker._live_preview_service is self.controller.preview_service + + def test_neighbor_prefetch_key_matches_the_thumbnail_refresh_key(self) -> None: + """A plain whole frame the neighbor prefetch cached is a live-cache hit for the refresh.""" + asset = self.files[1] + self.session.repo.load_file_settings.return_value = WorkspaceConfig() + task = self.controller._neighbor_prefetch_task(asset, 0, ()) + assert task is not None + live = PreviewManager() + live._cache.put( + _linear_preview_key( + task.file_hash, + color_space=task.workspace_color_space, + use_camera_wb=task.use_camera_wb, + full_resolution=task.full_resolution, + half_slice=task.half_slice, + demosaic=task.demosaic, + positive_source=task.positive_source, + highlight_mode=task.highlight_mode, + bake_camera_wb=task.bake_camera_wb, + lens_corrections=task.lens_corrections, + lens_flatfield=task.lens_flatfield, + ), + np.zeros((4, 6, 3), dtype=np.float32), + (6, 4), + {}, + ) + self.controller.refresh_thumbnails_for([asset["hash"]]) + frame = self.tasks[0].frames[0] + worker = ThumbnailRenderWorker(MagicMock(), live) + + assert worker._peek_live_preview(frame, self.tasks[0].workspace_color_space) is not None + self.controller._on_thumbnail_render_cancelled() diff --git a/tests/test_thumbnail_refresh_worker.py b/tests/test_thumbnail_refresh_worker.py index cc5a4eb67..72b2465c6 100644 --- a/tests/test_thumbnail_refresh_worker.py +++ b/tests/test_thumbnail_refresh_worker.py @@ -292,3 +292,62 @@ def test_thumbnail_render_cancel_does_not_poison_next_generation(qapp, monkeypat assert rendered == ["key-kept"] assert finished == [1] + + +class _LivePreviewService: + def __init__(self, hit=None) -> None: + self.hit = hit + self.peek_calls: list[dict] = [] + + def peek_linear_preview(self, file_path, color_space, **kwargs): + self.peek_calls.append({"file_path": file_path, "color_space": color_space, **kwargs}) + return self.hit + + +def test_thumbnail_render_uses_a_live_cache_hit_without_decoding(qapp, monkeypatch) -> None: + processor = _StubProcessor() + monkeypatch.setattr(render_workers, "ImageProcessor", lambda use_gpu=True: processor) + cached = np.full((4, 6, 3), 0.6, dtype=np.float32) + meta = _meta("live") + live = _LivePreviewService(hit=(cached, (6, 4), meta)) + private = _PreviewService() + worker = ThumbnailRenderWorker(private, live) + + worker.process(_task(_input("a", WorkspaceConfig()))) + + assert private.linear_calls == [] + assert [call["file_hash"] for call in live.peek_calls] == ["hash-a"] + buffer = processor.run_calls[0]["buffer"] + np.testing.assert_array_equal(buffer, cached) + # The pipeline gets its own copies; the navigation cache's arrays stay untouched. + assert not np.shares_memory(buffer, cached) + assert not np.shares_memory(processor.run_calls[0]["ir_buffer"], meta["ir_preview"]) + + +def test_thumbnail_render_falls_back_to_the_private_cache_on_a_live_miss(qapp, monkeypatch) -> None: + processor = _StubProcessor() + monkeypatch.setattr(render_workers, "ImageProcessor", lambda use_gpu=True: processor) + live = _LivePreviewService(hit=None) + private = _PreviewService() + worker = ThumbnailRenderWorker(private, live) + + worker.process(_task(_input("a", WorkspaceConfig()))) + + assert len(live.peek_calls) == 1 + assert [call["file_hash"] for call in private.linear_calls] == ["hash-a"] + assert len(processor.run_calls) == 1 + + +def test_thumbnail_render_skips_the_live_peek_for_a_stitch(qapp, monkeypatch) -> None: + processor = _StubProcessor() + monkeypatch.setattr(render_workers, "ImageProcessor", lambda use_gpu=True: processor) + live = _LivePreviewService(hit=(np.zeros((4, 6, 3), dtype=np.float32), (6, 4), {})) + private = _PreviewService() + worker = ThumbnailRenderWorker(private, live) + stitch = StitchConfig(stitch_enabled=True, stitch_paths=("/a.dng", "/b.dng")) + config = replace(WorkspaceConfig(), stitch=stitch) + + worker.process(_task(_input("panorama", config))) + + assert live.peek_calls == [] + assert len(private.stitch_calls) == 1