diff --git a/negpy/desktop/view/widgets/export_settings_form.py b/negpy/desktop/view/widgets/export_settings_form.py index 0f7b7d20..8d96c0c6 100644 --- a/negpy/desktop/view/widgets/export_settings_form.py +++ b/negpy/desktop/view/widgets/export_settings_form.py @@ -23,6 +23,7 @@ from negpy.desktop.view.styles.theme import THEME from negpy.desktop.view.widgets.sliders import CompactSlider from negpy.domain.models import ( + EXPORT_COLOR_SPACES, JXL_TAGGABLE_SPACES, AspectRatio, ColorSpace, @@ -246,7 +247,7 @@ def _build_color(self, root: QVBoxLayout) -> None: cs_row = QHBoxLayout() cs_row.addWidget(self._row_label("Colour space")) self.color_space_combo = QComboBox() - self.color_space_combo.addItems([cs.value for cs in ColorSpace]) + self.color_space_combo.addItems(EXPORT_COLOR_SPACES) constrain_combo(self.color_space_combo) self.color_space_combo.currentTextChanged.connect(self._on_changed) self.color_space_combo.currentTextChanged.connect(self._refresh_jxl_warning) diff --git a/negpy/desktop/workers/export.py b/negpy/desktop/workers/export.py index c96abd14..2a6b580c 100644 --- a/negpy/desktop/workers/export.py +++ b/negpy/desktop/workers/export.py @@ -5,15 +5,24 @@ import tempfile import threading from PyQt6.QtCore import QObject, pyqtSignal, pyqtSlot -from negpy.domain.models import WorkspaceConfig, ExportConfig, ExportFormat, ExportPreset, ExportPresetOutputMode +from negpy.domain.models import ColorSpace, WorkspaceConfig, ExportConfig, ExportFormat, ExportPreset, ExportPresetOutputMode from negpy.features.metadata.writer import embed_metadata, preserve_source_metadata from negpy.features.metadata.models import MetadataConfig -from negpy.infrastructure.display.color_spaces import WORKING_COLOR_SPACE +from negpy.infrastructure.display.color_spaces import WORKING_COLOR_SPACE, ColorSpaceRegistry from negpy.services.rendering.image_processor import ImageProcessor from negpy.services.export.templating import render_export_filename from negpy.services.export.contact_sheet import ContactSheetService +def _srgb_icc_bytes() -> Optional[bytes]: + """Bundled sRGB profile for tagging contact sheets (tiles are display/sRGB).""" + path = ColorSpaceRegistry.get_icc_path(ColorSpace.SRGB.value) + if path and os.path.exists(path): + with open(path, "rb") as f: + return f.read() + return None + + @dataclass(frozen=True) class ExportTask: """Immutable data for a high-resolution export job.""" @@ -248,6 +257,7 @@ def run_contact_sheet( ) os.makedirs(out_dir, exist_ok=True) + sheet_icc = _srgb_icc_bytes() for idx, sheet in enumerate(sheets): suffix = "" if idx == 0 else f"_{idx + 1}" path = os.path.join(out_dir, f"contact_sheet{suffix}.jpg") @@ -260,7 +270,7 @@ def run_contact_sheet( try: with tempfile.NamedTemporaryFile(dir=out_dir, delete=False, suffix=".part") as tmp: tmp_path = tmp.name - sheet.save(tmp, format="JPEG", quality=95, subsampling=0) + sheet.save(tmp, format="JPEG", quality=95, subsampling=0, icc_profile=sheet_icc) os.replace(tmp_path, path) except Exception as write_err: if tmp_path is not None and os.path.exists(tmp_path): diff --git a/negpy/domain/models.py b/negpy/domain/models.py index 787fcc24..6149208e 100644 --- a/negpy/domain/models.py +++ b/negpy/domain/models.py @@ -173,6 +173,12 @@ class ColorSpace(Enum): GREYSCALE = "Greyscale" +# Colour spaces offered for export. ACES and XYZ are rawpy *decode* spaces with no +# bundled ICC profile — an export targeting them can be neither converted nor tagged +# (the encoder falls back to the working space), so they're excluded here. +EXPORT_COLOR_SPACES: list[str] = [cs.value for cs in ColorSpace if cs not in (ColorSpace.ACES, ColorSpace.XYZ)] + + # Colour spaces JPEG XL can tag (mirror _JXL_COLOR in image_processor). Same as # Source is allowed — resolved at export time and rejected by the encoder if it # lands on an unsupported space. diff --git a/negpy/services/rendering/image_processor.py b/negpy/services/rendering/image_processor.py index 51b64f12..c986f671 100644 --- a/negpy/services/rendering/image_processor.py +++ b/negpy/services/rendering/image_processor.py @@ -629,11 +629,20 @@ def _encode_export( Input ICC overrides the source, output ICC the destination; both are always applied so the file matches the preview. """ - is_greyscale = color_space == ColorSpace.GREYSCALE.value fmt = export_settings.export_fmt icc_input = export_settings.icc_input_path icc_output = export_settings.icc_output_path + # A target with no ICC profile (ACES/XYZ, or a stale custom name) can be + # neither converted to nor tagged — the file would silently carry untagged + # working-space pixels. Export the working space itself, correctly tagged, + # instead. An output override supplies its own destination, so it exempts. + if ColorSpaceRegistry.get_icc_path(color_space) is None and not (icc_output and os.path.exists(icc_output)): + logger.warning(f"No ICC profile available for '{color_space}'; exporting as {working_color_space} instead") + color_space = working_color_space + + is_greyscale = color_space == ColorSpace.GREYSCALE.value + if fmt == ExportFormat.TIFF: if is_greyscale: img_int = float_to_uint_luma(np.ascontiguousarray(buffer), bit_depth=16) @@ -716,10 +725,15 @@ def _encode_export( elif fmt == ExportFormat.WEBP: # 8-bit only (WebP has no higher bit depth). Lossy or lossless via a # flag; PIL embeds the ICC profile for any colour space. - img_int = float_to_uint_luma(np.ascontiguousarray(buffer), bit_depth=8) if is_greyscale else float_to_uint8(buffer) - pil_img, icc_bytes = self.apply_color_management( - Image.fromarray(img_int), working_color_space, color_space, icc_output, icc_input - ) + if is_greyscale: + # apply_color_management can't transform an L image with the RGB + # working profile (lcms refuses the transform, leaving the file + # untagged in the working TRC); use the synthetic grey re-encode. + pil_img, icc_bytes = self._greyscale_to_pil_u8(buffer, working_color_space, color_space, icc_output, icc_input) + else: + pil_img, icc_bytes = self.apply_color_management( + Image.fromarray(float_to_uint8(buffer)), working_color_space, color_space, icc_output, icc_input + ) if max(pil_img.size) > 16383: raise ValueError("WebP max dimension is 16383 px; use TIFF/PNG for larger exports.") output_buf = io.BytesIO() @@ -734,15 +748,18 @@ def _encode_export( pil_img.save(output_buf, **save_kwargs) return output_buf.getvalue(), "webp" else: - img_int = float_to_uint_luma(np.ascontiguousarray(buffer), bit_depth=8) if is_greyscale else float_to_uint8(buffer) - - pil_img, icc_bytes = self.apply_color_management( - Image.fromarray(img_int), - working_color_space, - color_space, - icc_output, - icc_input, - ) + if is_greyscale: + # Same constraint as the WebP branch: greyscale can't go through the + # RGB CMS transform, so re-encode at 16-bit and downconvert. + pil_img, icc_bytes = self._greyscale_to_pil_u8(buffer, working_color_space, color_space, icc_output, icc_input) + else: + pil_img, icc_bytes = self.apply_color_management( + Image.fromarray(float_to_uint8(buffer)), + working_color_space, + color_space, + icc_output, + icc_input, + ) output_buf = io.BytesIO() self._save_to_pil_buffer(pil_img, output_buf, export_settings, icc_bytes) return output_buf.getvalue(), "jpg" @@ -924,20 +941,42 @@ def _apply_color_management_u16_greyscale( output_icc_path: Optional[str], input_icc_path: Optional[str] = None, ) -> Tuple[np.ndarray, Optional[bytes]]: - """Re-encode a (H,W) uint16 luma buffer to the target grey profile's gamma. - - The buffer is luma in the working TRC (ProPhoto ROMM 1.8); the grey profile - (GrayGamma2.2) expects a 2.2 gamma. Decode the working TRC to linear and - re-encode to 2.2 so the tagged output matches — an RGB working profile can't - drive a 1-channel ICC transform. + """Re-encode a (H,W) uint16 luma buffer to the tagged grey profile's TRC. + + The buffer is luma in the working TRC. The bundled GrayGamma2.2.icc actually + carries the sRGB TRC despite its name (verified against the profile; see the + _JXL_COLOR note), and JXL tags greyscale as SRGB transfer — so encode with the + sRGB OETF, not a pure 2.2 power, or the pixels won't match their own tag in + the shadows. An RGB working profile can't drive a 1-channel ICC transform, + hence this synthetic re-encode instead of lcms. """ if working_color_space == color_space: return img_u16, self._get_target_icc_bytes(color_space, output_icc_path) - lin = np.asarray(working_oetf_decode(img_u16.astype(np.float32) / 65535.0)) - gray = np.clip(lin, 0.0, 1.0) ** (1.0 / 2.2) + lin = np.clip(np.asarray(working_oetf_decode(img_u16.astype(np.float32) / 65535.0)), 0.0, 1.0) + gray = np.where(lin <= 0.0031308, lin * 12.92, 1.055 * np.power(lin, 1.0 / 2.4) - 0.055) out = np.clip(gray * 65535.0 + 0.5, 0.0, 65535.0).astype(np.uint16) return out, self._get_target_icc_bytes(color_space, output_icc_path) + def _greyscale_to_pil_u8( + self, + buffer: np.ndarray, + working_color_space: str, + color_space: str, + output_icc_path: Optional[str], + input_icc_path: Optional[str] = None, + ) -> Tuple[Image.Image, Optional[bytes]]: + """Greyscale buffer -> colour-managed 8-bit L image for JPEG/WebP. + + Runs the 16-bit grey re-encode first so the 8-bit result carries the same + TRC (and profile bytes) as the greyscale TIFF/PNG of the same edit. + """ + img16 = float_to_uint_luma(np.ascontiguousarray(buffer), bit_depth=16) + img16, icc_bytes = self._apply_color_management_u16_greyscale( + img16, working_color_space, color_space, output_icc_path, input_icc_path + ) + img8 = np.clip(np.round(img16.astype(np.float32) / 257.0), 0.0, 255.0).astype(np.uint8) + return Image.fromarray(img8), icc_bytes + def _apply_color_management_u16( self, img_u16: np.ndarray, diff --git a/tests/test_export_icc.py b/tests/test_export_icc.py new file mode 100644 index 00000000..98456105 --- /dev/null +++ b/tests/test_export_icc.py @@ -0,0 +1,117 @@ +"""Exported files must carry an ICC profile that matches their pixel encoding.""" + +import io + +import numpy as np +import pytest +import tifffile +from PIL import Image, ImageCms + +from negpy.domain.models import EXPORT_COLOR_SPACES, ColorSpace, ExportConfig, ExportFormat +from negpy.infrastructure.display.color_spaces import WORKING_COLOR_SPACE, ColorSpaceRegistry +from negpy.kernel.image.logic import working_oetf_decode +from negpy.services.rendering.image_processor import ImageProcessor + + +@pytest.fixture(scope="module") +def proc(): + return ImageProcessor() + + +def _profile_desc(icc_bytes): + return ImageCms.getProfileDescription(ImageCms.ImageCmsProfile(io.BytesIO(icc_bytes))).strip() + + +def _pil_icc(bits): + im = Image.open(io.BytesIO(bits)) + im.load() + return im.info.get("icc_profile") + + +def _srgb_oetf(lin: float) -> float: + return lin * 12.92 if lin <= 0.0031308 else 1.055 * lin ** (1 / 2.4) - 0.055 + + +def _encode(proc, fmt, cs, buf, **cfg): + settings = ExportConfig(export_fmt=fmt, export_color_space=cs, **cfg) + return proc._encode_export(buf, settings, cs, working_color_space=WORKING_COLOR_SPACE) + + +def test_greyscale_jpeg_is_tagged_and_matches_tiff(proc): + """Regression: greyscale JPEG fed an L image into an RGB CMS transform, which + failed silently and shipped untagged working-TRC pixels — inconsistent with the + greyscale TIFF of the same edit.""" + buf = np.full((8, 8, 3), 0.42, dtype=np.float32) + + jpg_bits, _ = _encode(proc, ExportFormat.JPEG, ColorSpace.GREYSCALE.value, buf.copy()) + icc = _pil_icc(jpg_bits) + assert icc, "greyscale JPEG must embed the grey profile" + assert "Gray" in _profile_desc(icc) + + tif_bits, _ = _encode(proc, ExportFormat.TIFF, ColorSpace.GREYSCALE.value, buf.copy()) + tif_px = tifffile.imread(io.BytesIO(tif_bits)) + jpg_px = np.asarray(Image.open(io.BytesIO(jpg_bits))) + assert jpg_px.ndim == 2 # single-channel L, not RGB + # Same edit, same TRC across formats (JPEG loss allows a small tolerance). + assert abs(int(jpg_px[4, 4]) - int(tif_px[4, 4]) / 257.0) < 2.5 + + +def test_greyscale_webp_is_tagged(proc): + buf = np.full((8, 8, 3), 0.42, dtype=np.float32) + bits, _ = _encode(proc, ExportFormat.WEBP, ColorSpace.GREYSCALE.value, buf.copy(), webp_lossless=True) + icc = _pil_icc(bits) + assert icc, "greyscale WebP must embed the grey profile" + assert "Gray" in _profile_desc(icc) + + +def test_greyscale_encode_uses_srgb_trc_not_pure_2_2(proc): + """The bundled GrayGamma2.2.icc actually holds the sRGB TRC (see _JXL_COLOR in + image_processor); a pure-2.2 encode mistags the shadows. Probe a deep shadow + where the two curves differ by ~3x.""" + v = 0.05 # working-encoded; linear ~0.0014, inside sRGB's linear toe segment + buf = np.full((4, 4, 3), v, dtype=np.float32) + bits, _ = _encode(proc, ExportFormat.TIFF, ColorSpace.GREYSCALE.value, buf) + px = tifffile.imread(io.BytesIO(bits)) + + lin = float(working_oetf_decode(np.float32(v))) + expected_srgb = round(_srgb_oetf(lin) * 65535) + legacy_2_2 = round((lin ** (1 / 2.2)) * 65535) + assert abs(int(px[2, 2]) - expected_srgb) <= 2 + assert abs(int(px[2, 2]) - legacy_2_2) > 100 # proves we're not on the old curve + + +def test_unmapped_space_falls_back_to_tagged_working_space(proc): + """Regression: ACES/XYZ have no ICC profile; export used to skip CMS and ship + untagged working-space pixels. It must instead export the working space with + its own profile embedded.""" + buf = np.random.default_rng(5).random((8, 8, 3)).astype(np.float32) + + aces_bits, _ = _encode(proc, ExportFormat.TIFF, ColorSpace.ACES.value, buf.copy()) + working_bits, _ = _encode(proc, ExportFormat.TIFF, WORKING_COLOR_SPACE, buf.copy()) + + with tifffile.TiffFile(io.BytesIO(aces_bits)) as tf: + icc = tf.pages[0].iccprofile + px = tf.pages[0].asarray() + assert icc, "fallback export must still be tagged" + with open(ColorSpaceRegistry.get_icc_path(WORKING_COLOR_SPACE), "rb") as f: + assert icc == f.read() + with tifffile.TiffFile(io.BytesIO(working_bits)) as tf: + np.testing.assert_array_equal(px, tf.pages[0].asarray()) + + +def test_export_color_spaces_exclude_unmappable(): + """The export UI must not offer spaces the encoder can only fall back on.""" + assert ColorSpace.ACES.value not in EXPORT_COLOR_SPACES + assert ColorSpace.XYZ.value not in EXPORT_COLOR_SPACES + for cs in EXPORT_COLOR_SPACES: + if cs == ColorSpace.SAME_AS_SOURCE.value: + continue # resolved to a concrete space at export time + assert ColorSpaceRegistry.get_icc_path(cs) is not None, f"{cs} offered but unmappable" + + +def test_contact_sheet_srgb_bytes_available(): + from negpy.desktop.workers.export import _srgb_icc_bytes + + icc = _srgb_icc_bytes() + assert icc + assert "sRGB" in _profile_desc(icc)