From dbc7da90555ee74bf0fdadbe463ccadb3d79c077 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 21 Sep 2026 15:24:37 +1000 Subject: [PATCH] Fix ICC rendering intent consistency and clamp sampled curves --- .../ColorProfileConverterExtensionsIcc.cs | 30 +++++++--- .../Icc/Calculators/LutCalculator.cs | 4 +- ...ons.cs => IccConverterBase.Conversions.cs} | 0 ...ccConverterbase.cs => IccConverterBase.cs} | 5 +- .../Icc/IccDataToDataConverter.cs | 6 +- .../Icc/IccDataToPcsConverter.cs | 5 +- .../Icc/IccPcsToDataConverter.cs | 5 +- .../ColorProfiles/Icc/IccPcsToPcsConverter.cs | 5 +- .../Icc/Calculators/LutCalculatorTests.cs | 25 ++++++++ .../Icc/ColorProfileConverterTests.Icc.cs | 57 +++++++++++++++++++ .../Formats/Tiff/TiffDecoderTests.cs | 22 +++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + ...Decode_YCCK_ICC_Jpeg_Rgba32_issue_2723.png | 4 +- ...ode_CmykIcc_Issue3198_Rgba32_Issue3198.png | 3 + tests/Images/Input/Tiff/Issues/Issue3198.tiff | 3 + 15 files changed, 157 insertions(+), 18 deletions(-) rename src/ImageSharp/ColorProfiles/Icc/{IccConverterbase.Conversions.cs => IccConverterBase.Conversions.cs} (100%) rename src/ImageSharp/ColorProfiles/Icc/{IccConverterbase.cs => IccConverterBase.cs} (86%) create mode 100644 tests/Images/External/ReferenceOutput/TiffDecoderTests/Decode_CmykIcc_Issue3198_Rgba32_Issue3198.png create mode 100644 tests/Images/Input/Tiff/Issues/Issue3198.tiff diff --git a/src/ImageSharp/ColorProfiles/ColorProfileConverterExtensionsIcc.cs b/src/ImageSharp/ColorProfiles/ColorProfileConverterExtensionsIcc.cs index 7f08a7a9b3..10a9d29bb4 100644 --- a/src/ImageSharp/ColorProfiles/ColorProfileConverterExtensionsIcc.cs +++ b/src/ImageSharp/ColorProfiles/ColorProfileConverterExtensionsIcc.cs @@ -74,8 +74,11 @@ internal static TTo ConvertUsingIccProfile(this ColorProfileConverte throw new InvalidOperationException("Target ICC profile is missing."); } - ConversionParams sourceParams = new(converter.Options.SourceIccProfile, toPcs: true); - ConversionParams targetParams = new(converter.Options.TargetIccProfile, toPcs: false); + // The embedded source profile supplies the connection's intent. Use it for both transform + // selection and PCS adjustment; the destination header may recommend a different intent. + IccRenderingIntent renderingIntent = converter.Options.SourceIccProfile.Header.RenderingIntent; + ConversionParams sourceParams = new(converter.Options.SourceIccProfile, toPcs: true, renderingIntent); + ConversionParams targetParams = new(converter.Options.TargetIccProfile, toPcs: false, renderingIntent); ColorProfileConverter pcsConverter = new(new ColorConversionOptions { @@ -141,8 +144,11 @@ internal static void ConvertUsingIccProfile(this ColorProfileConvert Guard.MustBeGreaterThanOrEqualTo(source.Length, destination.Length, nameof(destination)); - ConversionParams sourceParams = new(converter.Options.SourceIccProfile, toPcs: true); - ConversionParams targetParams = new(converter.Options.TargetIccProfile, toPcs: false); + // Resolve the connection's intent once for the entire span, before selecting either transform + // or deciding whether perceptual PCS adjustment is needed. + IccRenderingIntent renderingIntent = converter.Options.SourceIccProfile.Header.RenderingIntent; + ConversionParams sourceParams = new(converter.Options.SourceIccProfile, toPcs: true, renderingIntent); + ConversionParams targetParams = new(converter.Options.TargetIccProfile, toPcs: false, renderingIntent); ColorProfileConverter pcsConverter = new(new ColorConversionOptions { @@ -691,17 +697,27 @@ private class ConversionParams { private readonly IccProfile profile; - internal ConversionParams(IccProfile profile, bool toPcs) + /// + /// Initializes a new instance of the class. + /// + /// The profile used by this transform. + /// Whether this transform converts device values to the PCS. + /// The rendering intent shared by both profiles. + internal ConversionParams(IccProfile profile, bool toPcs, IccRenderingIntent renderingIntent) { this.profile = profile; - this.Converter = toPcs ? new IccDataToPcsConverter(profile) : new IccPcsToDataConverter(profile); + this.Intent = renderingIntent; + this.Converter = toPcs ? new IccDataToPcsConverter(profile, renderingIntent) : new IccPcsToDataConverter(profile, renderingIntent); } internal IccConverterBase Converter { get; } internal IccProfileHeader Header => this.profile.Header; - internal IccRenderingIntent Intent => this.Header.RenderingIntent; + /// + /// Gets the rendering intent selected for this profile connection. + /// + internal IccRenderingIntent Intent { get; } internal IccColorSpaceType PcsType => this.Header.ProfileConnectionSpace; diff --git a/src/ImageSharp/ColorProfiles/Icc/Calculators/LutCalculator.cs b/src/ImageSharp/ColorProfiles/Icc/Calculators/LutCalculator.cs index 83704ae214..74e342abc7 100644 --- a/src/ImageSharp/ColorProfiles/Icc/Calculators/LutCalculator.cs +++ b/src/ImageSharp/ColorProfiles/Icc/Calculators/LutCalculator.cs @@ -31,7 +31,9 @@ public float Calculate(float value) [MethodImpl(MethodImplOptions.AggressiveInlining)] private float Lookup(float value) { - value = Math.Max(value, 0); + // Sampled ICC curves cover [0, 1]. Saturate before scaling so out-of-domain values + // select the endpoint instead of extrapolating or producing an invalid table index. + value = Numerics.Clamp(value, 0, 1); float factor = value * (this.lut.Length - 1); int index = (int)factor; diff --git a/src/ImageSharp/ColorProfiles/Icc/IccConverterbase.Conversions.cs b/src/ImageSharp/ColorProfiles/Icc/IccConverterBase.Conversions.cs similarity index 100% rename from src/ImageSharp/ColorProfiles/Icc/IccConverterbase.Conversions.cs rename to src/ImageSharp/ColorProfiles/Icc/IccConverterBase.Conversions.cs diff --git a/src/ImageSharp/ColorProfiles/Icc/IccConverterbase.cs b/src/ImageSharp/ColorProfiles/Icc/IccConverterBase.cs similarity index 86% rename from src/ImageSharp/ColorProfiles/Icc/IccConverterbase.cs rename to src/ImageSharp/ColorProfiles/Icc/IccConverterBase.cs index d9976dc2ac..38a268a80b 100644 --- a/src/ImageSharp/ColorProfiles/Icc/IccConverterbase.cs +++ b/src/ImageSharp/ColorProfiles/Icc/IccConverterBase.cs @@ -18,10 +18,11 @@ internal abstract partial class IccConverterBase /// /// The ICC profile to use for the conversions /// True if the conversion is to the profile connection space (PCS); False if the conversion is to the data space - protected IccConverterBase(IccProfile profile, bool toPcs) + /// The rendering intent selected for the profile connection. + protected IccConverterBase(IccProfile profile, bool toPcs, IccRenderingIntent renderingIntent) { Guard.NotNull(profile, nameof(profile)); - this.Init(profile, toPcs, profile.Header.RenderingIntent); + this.Init(profile, toPcs, renderingIntent); } /// diff --git a/src/ImageSharp/ColorProfiles/Icc/IccDataToDataConverter.cs b/src/ImageSharp/ColorProfiles/Icc/IccDataToDataConverter.cs index cb4d89bb53..ffef85271e 100644 --- a/src/ImageSharp/ColorProfiles/Icc/IccDataToDataConverter.cs +++ b/src/ImageSharp/ColorProfiles/Icc/IccDataToDataConverter.cs @@ -16,7 +16,11 @@ internal class IccDataToDataConverter : IccConverterBase /// /// The ICC profile to use for the conversions public IccDataToDataConverter(IccProfile profile) - : base(profile, true) // toPCS is true because in this case the PCS space is also a data space + + // toPCS is true because the PCS space is also a data space for a DeviceLink profile. + // The shared base constructor requires an intent. Pass the profile's header value; + // DeviceLink transform selection uses CheckMethod2 and ignores rendering intent. + : base(profile, true, profile.Header.RenderingIntent) { } } diff --git a/src/ImageSharp/ColorProfiles/Icc/IccDataToPcsConverter.cs b/src/ImageSharp/ColorProfiles/Icc/IccDataToPcsConverter.cs index 6e95d3cb32..8160bee67c 100644 --- a/src/ImageSharp/ColorProfiles/Icc/IccDataToPcsConverter.cs +++ b/src/ImageSharp/ColorProfiles/Icc/IccDataToPcsConverter.cs @@ -15,8 +15,9 @@ internal class IccDataToPcsConverter : IccConverterBase /// Initializes a new instance of the class. /// /// The ICC profile to use for the conversions - public IccDataToPcsConverter(IccProfile profile) - : base(profile, true) + /// The rendering intent selected for the profile connection. + public IccDataToPcsConverter(IccProfile profile, IccRenderingIntent renderingIntent) + : base(profile, true, renderingIntent) { } } diff --git a/src/ImageSharp/ColorProfiles/Icc/IccPcsToDataConverter.cs b/src/ImageSharp/ColorProfiles/Icc/IccPcsToDataConverter.cs index d29517fca2..1ab9c4000d 100644 --- a/src/ImageSharp/ColorProfiles/Icc/IccPcsToDataConverter.cs +++ b/src/ImageSharp/ColorProfiles/Icc/IccPcsToDataConverter.cs @@ -15,8 +15,9 @@ internal class IccPcsToDataConverter : IccConverterBase /// Initializes a new instance of the class. /// /// The ICC profile to use for the conversions - public IccPcsToDataConverter(IccProfile profile) - : base(profile, false) + /// The rendering intent selected for the profile connection. + public IccPcsToDataConverter(IccProfile profile, IccRenderingIntent renderingIntent) + : base(profile, false, renderingIntent) { } } diff --git a/src/ImageSharp/ColorProfiles/Icc/IccPcsToPcsConverter.cs b/src/ImageSharp/ColorProfiles/Icc/IccPcsToPcsConverter.cs index 30b44ca75c..e25a2cb906 100644 --- a/src/ImageSharp/ColorProfiles/Icc/IccPcsToPcsConverter.cs +++ b/src/ImageSharp/ColorProfiles/Icc/IccPcsToPcsConverter.cs @@ -16,7 +16,10 @@ internal class IccPcsToPcsConverter : IccConverterBase /// /// The ICC profile to use for the conversions public IccPcsToPcsConverter(IccProfile profile) - : base(profile, true) + + // The shared base constructor requires an intent. Pass the profile's header value; + // Abstract transform selection uses CheckMethod2 and ignores rendering intent. + : base(profile, true, profile.Header.RenderingIntent) { } } diff --git a/tests/ImageSharp.Tests/ColorProfiles/Icc/Calculators/LutCalculatorTests.cs b/tests/ImageSharp.Tests/ColorProfiles/Icc/Calculators/LutCalculatorTests.cs index 6cc77247a9..8465e22b62 100644 --- a/tests/ImageSharp.Tests/ColorProfiles/Icc/Calculators/LutCalculatorTests.cs +++ b/tests/ImageSharp.Tests/ColorProfiles/Icc/Calculators/LutCalculatorTests.cs @@ -12,6 +12,31 @@ namespace SixLabors.ImageSharp.Tests.ColorProfiles.Icc.Calculators; [Trait("Color", "Conversion")] public class LutCalculatorTests { + /// + /// Verifies that forward lookup saturates at the table endpoints outside its normalized domain. + /// + /// The normalized lookup input. + /// The expected table value. + [Theory] + [InlineData(float.NaN, 0.25F)] + [InlineData(float.NegativeInfinity, 0.25F)] + [InlineData(float.MinValue, 0.25F)] + [InlineData(-0.0001F, 0.25F)] + [InlineData(0F, 0.25F)] + [InlineData(0.5F, 0.5F)] + [InlineData(1F, 0.75F)] + [InlineData(1.0001F, 0.75F)] + [InlineData(2F, 0.75F)] + [InlineData(float.MaxValue, 0.75F)] + [InlineData(float.PositiveInfinity, 0.75F)] + public void ForwardLookup_ClampsToTableDomain(float input, float expected) + { + // Nonzero and nonunit endpoints distinguish table saturation from clamping the output to 0 or 1. + LutCalculator calculator = new([0.25F, 0.5F, 0.75F], inverse: false); + + Assert.Equal(expected, calculator.Calculate(input)); + } + [Theory] [MemberData(nameof(IccConversionDataLut.LutConversionTestData), MemberType = typeof(IccConversionDataLut))] internal void LutCalculator_WithLut_ReturnsResult(float[] lut, bool inverted, float input, float expected) diff --git a/tests/ImageSharp.Tests/ColorProfiles/Icc/ColorProfileConverterTests.Icc.cs b/tests/ImageSharp.Tests/ColorProfiles/Icc/ColorProfileConverterTests.Icc.cs index cb349af96a..dd546b76d7 100644 --- a/tests/ImageSharp.Tests/ColorProfiles/Icc/ColorProfileConverterTests.Icc.cs +++ b/tests/ImageSharp.Tests/ColorProfiles/Icc/ColorProfileConverterTests.Icc.cs @@ -69,6 +69,63 @@ public void CanBulkConvertIccProfiles(string sourceProfile, string targetProfile AssertConversion(sourceProfile, targetProfile, actual, tolerance, testOutputHelper); } + /// + /// Verifies that both transforms use the source rendering intent when the profile headers disagree. + /// + /// The destination profile, using either curves or a LUT. + /// The normalized tolerance for differences in reference interpolation. + [Theory] + [InlineData(TestIccProfiles.StandardRgbV2, 0.0005)] + [InlineData(TestIccProfiles.StandardRgbV4, 1D / ushort.MaxValue)] // One 16-bit LUT code value for float versus double interpolation. + public void Convert_UsesSourceRenderingIntentForBothProfiles(string targetFile, double tolerance) + { + IccProfile sourceProfile = TestIccProfiles.GetProfile(TestIccProfiles.Fogra39).DeepClone(); + sourceProfile.Header.RenderingIntent = IccRenderingIntent.MediaRelativeColorimetric; + IccProfile targetProfile = TestIccProfiles.GetProfile(targetFile); + ColorProfileConverter converter = new(new ColorConversionOptions + { + SourceIccProfile = sourceProfile, + TargetIccProfile = targetProfile + }); + + // Explicitly select the same intent in the independent reference converter. Its unspecified + // intent uses each profile's header, which would reproduce the conflicting-intent defect. + Wacton.Unicolour.Configuration sourceConfig = new(iccConfig: new IccConfiguration( + Path.Combine("TestDataIcc", "Profiles", TestIccProfiles.Fogra39), Intent.RelativeColorimetric)); + + Wacton.Unicolour.Configuration targetConfig = new(iccConfig: new IccConfiguration( + Path.Combine("TestDataIcc", "Profiles", targetFile), Intent.RelativeColorimetric)); + + Cmyk[] inputs = + [ + new(0, 0, 0, 0), new(0, 0, 0, 1), + new(0, 1, 1, 0), new(1, 0, 1, 0), new(1, 1, 0, 0), new(0, 0, 1, 0), + new(0.25F, 0.5F, 0.75F, 0.125F) + ]; + + Rgb[] bulk = new Rgb[inputs.Length]; + converter.Convert(inputs, bulk); + + for (int i = 0; i < inputs.Length; i++) + { + Cmyk input = inputs[i]; + Unicolour reference = new(sourceConfig, new Channels(input.C, input.M, input.Y, input.K)); + Unicolour expected = reference.ConvertToConfiguration(targetConfig); + Assert.Null(expected.Icc.Error); + Vector4 scalar = converter.Convert(input).ToScaledVector4(); + Vector4 span = bulk[i].ToScaledVector4(); + + for (int channel = 0; channel < 3; channel++) + { + Assert.Equal(expected.Icc.Values[channel], scalar[channel], tolerance); + Assert.Equal(expected.Icc.Values[channel], span[channel], tolerance); + } + } + + // Conversion must not rewrite a profile that may be shared with other callers. + Assert.Equal(IccRenderingIntent.Perceptual, targetProfile.Header.RenderingIntent); + } + private static void AssertConversion(string sourceProfile, string targetProfile, List actual, double tolerance, ITestOutputHelper testOutputHelper) { List expected = Inputs.ConvertAll(input => GetExpectedTargetValues(sourceProfile, targetProfile, input, testOutputHelper)); diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs index e5ecb50f5c..6868395542 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -414,6 +414,28 @@ public void Decode_WhenColorProfileHandlingIsConvert_ApplyIccProfile(Tes Assert.Null(image.Metadata.IccProfile); } + /// + /// Verifies conversion of the reporter's CMYK profile using its relative-colorimetric intent. + /// + /// The pixel type. + /// The image provider. + [Theory] + [WithFile(Issue3198, PixelTypes.Rgba32)] + public void Decode_CmykIcc_Issue3198(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + DecoderOptions options = new() { ColorProfileHandling = ColorProfileHandling.Convert }; + using Image image = provider.GetImage(TiffDecoder.Instance, options); + image.DebugSave(provider); + + // LittleCMS 2.19 generated the reference from the embedded profile to CompactSrgbV4Profile, + // using relative colorimetric intent without black-point compensation. + // Measured differences are at most one 8-bit channel value, totaling 0.000083% of the image. + image.CompareToReferenceOutput(ImageComparer.TolerantPercentage(0.0001F), provider); + Assert.Null(image.Metadata.IccProfile); + Assert.Null(image.Frames.RootFrame.Metadata.IccProfile); + } + [Theory] [WithFile(Icc.PerceptualRgb8, PixelTypes.Rgba32)] [WithFile(Icc.PerceptualRgb16, PixelTypes.Rgba32)] diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index ae36d245de..adbade2bc5 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -1213,6 +1213,7 @@ public static class Tiff public const string Issue2983 = "Tiff/Issues/Issue2983.tiff"; public const string Issue3182ColorMap8Bit = "Tiff/Issues/Issue3182ColorMap8Bit.tiff"; public const string Issue3182ColorMap16Bit = "Tiff/Issues/Issue3182ColorMap16Bit.tiff"; + public const string Issue3198 = "Tiff/Issues/Issue3198.tiff"; public static readonly string[] Multiframes = [MultiframeDeflateWithPreview, MultiframeLzwPredictor /*, MultiFrameDifferentSize, MultiframeDifferentSizeTiled, MultiFrameDifferentVariants,*/ ]; diff --git a/tests/Images/External/ReferenceOutput/JpegDecoderTests/Decode_YCCK_ICC_Jpeg_Rgba32_issue_2723.png b/tests/Images/External/ReferenceOutput/JpegDecoderTests/Decode_YCCK_ICC_Jpeg_Rgba32_issue_2723.png index a73e5f31ef..052c20b329 100644 --- a/tests/Images/External/ReferenceOutput/JpegDecoderTests/Decode_YCCK_ICC_Jpeg_Rgba32_issue_2723.png +++ b/tests/Images/External/ReferenceOutput/JpegDecoderTests/Decode_YCCK_ICC_Jpeg_Rgba32_issue_2723.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:de73fa7a2cdd79763e40c51381f9a0cf5c7d43db2dcd7a86dad0a5b68377665f -size 421249 +oid sha256:9226fd9276859f7bbb358aa3bb5b5fed967571e93ac533444cf072f15ed92ca9 +size 419466 diff --git a/tests/Images/External/ReferenceOutput/TiffDecoderTests/Decode_CmykIcc_Issue3198_Rgba32_Issue3198.png b/tests/Images/External/ReferenceOutput/TiffDecoderTests/Decode_CmykIcc_Issue3198_Rgba32_Issue3198.png new file mode 100644 index 0000000000..474d4cb44b --- /dev/null +++ b/tests/Images/External/ReferenceOutput/TiffDecoderTests/Decode_CmykIcc_Issue3198_Rgba32_Issue3198.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:a51fefb8a9bb69287f87eb9f1879b9ecec49c0618c064c5e8925527f450ccb53 +size 45143 diff --git a/tests/Images/Input/Tiff/Issues/Issue3198.tiff b/tests/Images/Input/Tiff/Issues/Issue3198.tiff new file mode 100644 index 0000000000..faae028e39 --- /dev/null +++ b/tests/Images/Input/Tiff/Issues/Issue3198.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:31502f43e6bb7251b15df0edd9c7f34561dda00dbb891acd625fadfa76349cfc +size 756213