diff --git a/docs/PIPELINE.md b/docs/PIPELINE.md index 3b9cce60..41308c91 100644 --- a/docs/PIPELINE.md +++ b/docs/PIPELINE.md @@ -308,7 +308,9 @@ When the render intent is **Linear**, the entire darkroom pipeline is bypassed. ### Peek Negative **Code**: `AppController.toggle_negative_peek` / `_paint_negative_peek` -The canvas equivalent of Linear Output, and not a pipeline stage of its own. The decoded source buffer (`AppState.preview_raw`, pre-bake, so before flat-field, sensor unmix and every defect repair) runs through `GeometryProcessor` and `CropProcessor`, the same two the base and crop stages use, so the frame keeps the user's rotation, flip, straighten, keystone and crop. It then takes `working_oetf_encode`, a display encode rather than an edit, since a linear buffer would otherwise show as near-black. Everything between those two is skipped: nothing is inverted, metered or normalized, so the negative is on screen as the loader read it. `content_rect` is cleared, no border stage having run. The buffer is in camera or scanner primaries, so it is marked `splash` and the working-to-display matrix and the soft proof are left off. The state is transient, mutually exclusive with the flat peek and the before/after split, and any render with no config override drops it. +The canvas equivalent of Linear Output, and not a pipeline stage of its own. The decoded source buffer (`AppState.preview_raw`, pre-bake, so before flat-field, sensor unmix and every defect repair) runs through `GeometryProcessor` and `CropProcessor`, the same two the base and crop stages use, so the frame keeps the user's rotation, flip, straighten, keystone and crop. It then takes `working_oetf_encode`, a display encode rather than an edit, since a linear buffer would otherwise show as near-black. Everything between those two is skipped: nothing is inverted, metered or normalized, so the negative is on screen as the loader read it. `content_rect` is cleared, no border stage having run. The state is transient, mutually exclusive with the flat peek and the before/after split, and any render with no config override drops it. + +The buffer arrives in camera primaries, so the peek applies `camera_to_working_matrix` itself before the encode; a source that carries no matrix (scanner TIFF, JPEG) is already profiled and passes through. The as-shot multipliers fold into that matrix when the decode skipped them (Linear RAW, transparency transfer), which makes the peek independent of how the source was decoded — the same reason the transfer path folds them. Without the matrix the canvas takes camera RGB for display RGB, and the film base loses a large part of its chroma: a C-41 mask reads much weaker than the file's own. The frame is therefore *not* marked `splash`; it takes the normal working-to-display conversion with `proof` set False, since a paper simulation describes a print and this is a scan. --- diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index dcabd551..118cf0cd 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -24,7 +24,7 @@ The split stays up while you work, so a slider moves the after side against a fi ### Peek Negative -The canvas toolbar's film button (or `N`) shows the scan as it was loaded: the negative, un-inverted, with no metering, no film-base normalization and none of your edits. Use it to judge the scan rather than the print — whether the frame is thin or dense, what colour the mask really is, whether the scanner clipped. It changes nothing and closes as soon as you touch a control. Your crop, rotation and flip still apply, so the frame stays where you put it. Because it is the file's own numbers, it gets no colour management or soft proof, so expect the raw orange cast rather than an accurate one. +The canvas toolbar's film button (or `N`) shows the scan as it was loaded: the negative, un-inverted, with no metering, no film-base normalization and none of your edits. Use it to judge the scan rather than the print — whether the frame is thin or dense, what color the mask really is, whether the scanner clipped. It changes nothing and closes as soon as you touch a control. Your crop, rotation and flip still apply, so the frame stays where you put it. The frame is color managed, so a C-41 mask is as orange here as in the file, and Linear RAW does not change how it looks. The soft proof stays off: this is the scan, not a print. ### The workflow (and the order things happen) diff --git a/negpy/desktop/controller.py b/negpy/desktop/controller.py index 29c98bc6..20c180f1 100644 --- a/negpy/desktop/controller.py +++ b/negpy/desktop/controller.py @@ -46,6 +46,7 @@ from negpy.desktop.workers.hdr import HdrTask, HdrWorker from negpy.desktop.workers.stitch import StitchTask, StitchWorker from negpy.features.hdr.models import ANCHOR_EV_UNSET, hdr_frame_paths, hdr_hash, hdr_name +from negpy.features.process.capture_color import apply_camera_matrix, camera_to_working_matrix from negpy.features.process.logic import effective_linear_raw, narrowband_profile_active from negpy.features.stitch.models import stitch_hash, stitch_name from negpy.desktop.workers.capture_worker import ( @@ -1498,6 +1499,7 @@ def load_file(self, file_path: str, preserve_zoom: bool = False, force_detect: b self.state.last_metrics["base_positive"] = memo["base_positive"] self.state.last_metrics["content_rect"] = memo.get("content_rect") self.state.last_metrics["splash"] = False + self.state.last_metrics["proof"] = True # These pixels are this frame's own last render. Leaving the outgoing # frame's hash next to them would file them under it on the next # thumbnail refresh, which reads whatever last_metrics holds. @@ -3634,7 +3636,7 @@ def effective_input_icc(self, process: Optional[ProcessConfig] = None) -> Option return get_resource_path("icc/RGBScan.icc") return None - def display_transform_params(self, splash: bool = False) -> tuple[str, Optional[bytes], Optional[tuple]]: + def display_transform_params(self, splash: bool = False, proofed: bool = True) -> tuple[str, Optional[bytes], Optional[tuple]]: """Everything the display transform needs for the current render, as ``(color_space, monitor_icc_bytes, proof)``. @@ -3644,10 +3646,13 @@ def display_transform_params(self, splash: bool = False) -> tuple[str, Optional[ proof is *not* baked into them, it is folded into the display LUT here (see ``get_display_lut``), which is what lets a GPU texture go to the shader untouched. ``splash`` marks the embedded camera thumbnail, already sRGB. + ``proofed`` is False for a working-space buffer that is not a print: the + negative peek shows the scan, which a paper simulation would misdescribe. """ if splash: return ColorSpace.SRGB.value, self.state.monitor_icc_bytes, None - return self.state.workspace_color_space, self.state.monitor_icc_bytes, self.proof_profiles() + proof = self.proof_profiles() if proofed else None + return self.state.workspace_color_space, self.state.monitor_icc_bytes, proof def proof_profiles(self) -> Optional[tuple]: """``(input_icc, output_icc)`` for the preview proof, or None when off. @@ -3952,9 +3957,15 @@ def _paint_negative_peek(self) -> None: the base and crop stages use, so the negative sits at the orientation and framing the user set. Everything after it is skipped: no metering, no inversion, no look. Only the working OETF follows, or a linear buffer shows as - near-black. The source is in camera primaries, so it is marked splash to keep - the working-to-display matrix and the proof out of it, and content_rect is - cleared because no border stage ran to inset the picture. + near-black. ``content_rect`` is cleared because no border stage ran to inset the + picture. + + The source is in camera primaries, so the camera matrix runs here: painting those + numbers as display RGB flattens the film base, which on a C-41 negative reads as a + mask that is far weaker than the one in the file. The multipliers fold into the + matrix when the decode skipped them, so the peek looks the same either way and + Linear RAW does not change what the mask looks like. The proof stays off — this is + the scan, not a print. """ source = self.state.preview_raw if source is None: @@ -3973,10 +3984,19 @@ def _paint_negative_peek(self) -> None: img = GeometryProcessor(geometry, flatfield.k1 if flatfield.apply else 0.0).process(source, context) if not context.crop_preview_full: img = CropProcessor(geometry).process(img, context) + decoded_without_wb = effective_linear_raw(self.state.config.process, self.state.config.exposure.render_intent) + img = apply_camera_matrix( + img, + camera_to_working_matrix( + self.state.preview_cam_xyz, + self.state.preview_camera_wb if decoded_without_wb else None, + ), + ) with self.state.metrics_lock: self.state.last_metrics["base_positive"] = working_oetf_encode(img) self.state.last_metrics["content_rect"] = None - self.state.last_metrics["splash"] = True + self.state.last_metrics["splash"] = False + self.state.last_metrics["proof"] = False self.image_updated.emit() def toggle_negative_peek(self, force: Optional[bool] = None) -> None: @@ -4809,6 +4829,9 @@ def _on_render_finished(self, _result: Any, metrics: Dict[str, Any]) -> None: with self.state.metrics_lock: self.state.last_metrics.update(metrics) self.state.last_metrics["splash"] = False + # last_metrics carries over between frames, so a peek's suppressed proof must + # not outlive it onto the next render. + self.state.last_metrics["proof"] = True self._freeze_resolved_auto_crop(metrics) @@ -5025,7 +5048,9 @@ def _update_thumbnail_from_state(self, persist: bool = True) -> None: # The same transform the canvas used for this buffer, so the filmstrip and the canvas # cannot disagree about the frame's color. - display_cs, monitor_bytes, proof = self.display_transform_params(splash=bool(metrics.get("splash"))) + display_cs, monitor_bytes, proof = self.display_transform_params( + splash=bool(metrics.get("splash")), proofed=bool(metrics.get("proof", True)) + ) # The asset's own key, so the batch (source) path re-serves this rendered positive # instead of the uninverted source merge it would decode itself. self.thumbnail_update_requested.emit( diff --git a/negpy/desktop/view/main_window.py b/negpy/desktop/view/main_window.py index 097e688e..55a3cda8 100644 --- a/negpy/desktop/view/main_window.py +++ b/negpy/desktop/view/main_window.py @@ -579,7 +579,9 @@ def _on_image_updated(self) -> None: # Shared with the filmstrip thumbnail, so the same frame cannot render two different # colors in the two places (see display_transform_params). - display_cs, monitor_bytes, proof = self.controller.display_transform_params(splash=bool(metrics.get("splash"))) + display_cs, monitor_bytes, proof = self.controller.display_transform_params( + splash=bool(metrics.get("splash")), proofed=bool(metrics.get("proof", True)) + ) self.canvas.update_buffer(buffer, display_cs, content_rect=content_rect, monitor_icc_bytes=monitor_bytes, proof=proof) def _refresh_image_info(self) -> None: diff --git a/tests/test_controller.py b/tests/test_controller.py index a8d62958..7379ac57 100644 --- a/tests/test_controller.py +++ b/tests/test_controller.py @@ -2454,6 +2454,124 @@ def test_thumbnail_task_carries_the_same_params_as_the_canvas(self): # next to a proofed canvas. self.assertIsNotNone(task.proof) + def test_unproofed_working_space_buffer_keeps_the_conversion(self): + """The negative peek wants the working->display conversion without the paper + simulation. Reporting sRGB to get rid of the proof would lose both.""" + self.controller.state.soft_proof_enabled = True + cs, monitor, proof = self.controller.display_transform_params(proofed=False) + self.assertEqual(cs, self.controller.state.workspace_color_space) + self.assertEqual(monitor, b"fake-monitor-profile") + self.assertIsNone(proof) + + +class TestNegativePeekColor(unittest.TestCase): + """The peek paints camera-native pixels, so it owns the camera matrix itself. + + Without it the buffer goes to the canvas as though it were already display RGB, + which drains the film base: a C-41 mask reads far weaker than the file's own. + """ + + def setUp(self): + self.mock_session_manager = MagicMock(spec=DesktopSessionManager) + self.mock_session_manager.state = AppState() + self.mock_session_manager.repo = MagicMock() + with ( + patch("negpy.desktop.controller.RenderWorker") as mock_rw_class, + patch("negpy.desktop.controller.PreviewManager") as mock_pm_class, + ): + mock_rw_class.return_value = MagicMock() + mock_pm_class.return_value = MagicMock(spec=PreviewManager) + mock_pm_class.return_value.load_linear_preview.return_value = (None, (0, 0), {}) + self.controller = AppController(self.mock_session_manager) + + def tearDown(self): + import gc + + for thread in [ + self.controller.render_thread, + self.controller.export_thread, + self.controller.thumb_thread, + self.controller.norm_thread, + self.controller.discovery_thread, + self.controller.preview_load_thread, + self.controller.scan_thread, + ]: + if thread is not None and thread.isRunning(): + thread.quit() + thread.wait() + del self.controller + gc.collect() + + # A real decoder matrix (Nikon D3300), so the test fails on a plausible transform + # rather than only on an artificial one. + D3300 = [ + [0.6988000273704529, -0.13840000331401825, -0.0714000016450882], + [-0.5630999803543091, 1.340999960899353, 0.24469999969005585], + [-0.148499995470047, 0.22040000557899475, 0.7318000197410583], + ] + + def _paint(self, cam_xyz, camera_wb=None): + import numpy as np + + state = self.controller.state + # An orange-mask film base: red passes, blue is held back. + state.preview_raw = np.full((8, 8, 3), 0.1, dtype=np.float32) * np.array([1.0, 0.45, 0.2], dtype=np.float32) + state.original_res = (8, 8) + state.preview_cam_xyz = cam_xyz + state.preview_camera_wb = camera_wb + self.controller._paint_negative_peek() + return state.last_metrics + + def test_the_peek_applies_the_camera_matrix(self): + import numpy as np + + from negpy.features.process.capture_color import apply_camera_matrix, camera_to_working_matrix + from negpy.kernel.image.logic import working_oetf_encode + + metrics = self._paint(self.D3300) + painted = metrics["base_positive"] + + source = self.controller.state.preview_raw + expected = working_oetf_encode(apply_camera_matrix(source, camera_to_working_matrix(self.D3300, None))) + np.testing.assert_allclose(painted, expected, atol=1e-5) + # And it is not the un-matrixed buffer, which is what shipped the weak mask. + self.assertFalse(np.allclose(painted, working_oetf_encode(source), atol=1e-3)) + + def test_the_peek_is_color_managed_but_never_proofed(self): + metrics = self._paint(self.D3300) + self.assertFalse(metrics["splash"], "a camera-matrixed buffer is in the working space") + self.assertFalse(metrics["proof"], "the peek shows the scan, not a print") + + def test_a_source_with_no_matrix_passes_through(self): + """Scanner TIFF and JPEG carry no camera matrix; they are already profiled.""" + import numpy as np + + from negpy.kernel.image.logic import working_oetf_encode + + metrics = self._paint(None) + np.testing.assert_allclose(metrics["base_positive"], working_oetf_encode(self.controller.state.preview_raw), atol=1e-6) + + def test_linear_raw_folds_the_multipliers_back_in(self): + """The decode's white balance must not change what the mask looks like, or + Linear RAW silently restyles the negative instead of leaving it alone.""" + import numpy as np + + wb = [1.891, 1.0, 1.578] + with_wb = np.array(self._paint(self.D3300, camera_wb=wb)["base_positive"]) + + state = self.controller.state + state.config = replace(state.config, process=replace(state.config.process, linear_raw=True)) + # The Linear RAW decode skips the multipliers, so its buffer is the unbalanced one. + raw_unbalanced = np.full((8, 8, 3), 0.1, dtype=np.float32) * np.array([1.0, 0.45, 0.2], dtype=np.float32) + state.preview_raw = (raw_unbalanced / np.array(wb, dtype=np.float32)).astype(np.float32) + state.original_res = (8, 8) + state.preview_cam_xyz = self.D3300 + state.preview_camera_wb = wb + self.controller._paint_negative_peek() + without_wb = np.array(state.last_metrics["base_positive"]) + + np.testing.assert_allclose(with_wb, without_wb, atol=1e-5) + class TestCompareFlatPeekInteraction(unittest.TestCase): """Before/After and flat-peek are mutually exclusive overlays; a geometry op must @@ -2546,9 +2664,12 @@ def test_negative_peek_paints_the_source_with_only_the_oetf(self): self.assertTrue(self.controller.state.negative_peek) self.assertTrue(painted) metrics = self.controller.state.last_metrics + # No camera matrix on this source, so the encode is all that separates it from the + # buffer the loader read. See TestNegativePeekColor for the camera-native path. np.testing.assert_allclose(metrics["base_positive"], working_oetf_encode(source)) - # Camera primaries, so the working-to-display matrix and the proof stay out. - self.assertTrue(metrics["splash"]) + # Working space, so the display conversion runs; the proof does not. + self.assertFalse(metrics["splash"]) + self.assertFalse(metrics["proof"]) def test_leaving_the_negative_peek_re_renders_the_edit(self): self.controller.state.negative_peek = True diff --git a/tests/test_thumbnail_readback_thread.py b/tests/test_thumbnail_readback_thread.py index bf8c012b..a638ba8e 100644 --- a/tests/test_thumbnail_readback_thread.py +++ b/tests/test_thumbnail_readback_thread.py @@ -41,7 +41,7 @@ def _controller_stub(self, metrics): last_metrics=metrics, metrics_lock=MagicMock(__enter__=lambda s: None, __exit__=lambda s, *a: None), ), - display_transform_params=lambda splash=False: ("Adobe RGB", None, None), + display_transform_params=lambda splash=False, proofed=True: ("Adobe RGB", None, None), thumbnail_update_requested=MagicMock(), ) # Attributing a render to its own frame is the caller's first step.