diff --git a/src/ImageSharp/ColorProfiles/ColorProfileConverterExtensionsIcc.cs b/src/ImageSharp/ColorProfiles/ColorProfileConverterExtensionsIcc.cs index 7e954bdf13..da9a52ae28 100644 --- a/src/ImageSharp/ColorProfiles/ColorProfileConverterExtensionsIcc.cs +++ b/src/ImageSharp/ColorProfiles/ColorProfileConverterExtensionsIcc.cs @@ -39,25 +39,19 @@ internal static class ColorProfileConverterExtensionsIcc TargetWhitePoint = new CieXyz(converter.Options.TargetIccProfile.Header.PcsIlluminant), }); - // output of Matrix TRC calculator is descaled XYZ, needs to be re-scaled to be used as PCS Vector4 sourcePcs = sourceParams.Converter.Calculate(source.ToScaledVector4()); - Vector4 targetPcs; // if both profiles need PCS adjustment, they both share the same unadjusted PCS space // cancelling out the need to make the adjustment - // TODO: handle PCS adjustment for absolute intent? would make this a lot more complicated - // TODO: alternatively throw unsupported error, since most profiles headers contain perceptual (i've encountered a couple of relative intent, but so far no saturation or absolute) - if (sourceParams.AdjustPcsForPerceptual ^ targetParams.AdjustPcsForPerceptual) - { - targetPcs = GetTargetPcsWithPerceptualV2Adjustment(sourcePcs, sourceParams, targetParams, pcsConverter); - } - else - { - targetPcs = GetTargetPcsWithoutAdjustment(sourcePcs, sourceParams, targetParams, pcsConverter); - } + // except if using TRC transforms, which always requires perceptual handling + // TODO: this does not include adjustment for absolute intent, which would double existing complexity, suggest throwing exception and addressing in future update + bool anyProfileNeedsPerceptualAdjustment = sourceParams.HasNoPerceptualHandling || targetParams.HasNoPerceptualHandling; + bool oneProfileHasV2PerceptualAdjustment = sourceParams.HasV2PerceptualHandling ^ targetParams.HasV2PerceptualHandling; + + Vector4 targetPcs = anyProfileNeedsPerceptualAdjustment || oneProfileHasV2PerceptualAdjustment + ? GetTargetPcsWithPerceptualAdjustment(sourcePcs, sourceParams, targetParams, pcsConverter) + : GetTargetPcsWithoutAdjustment(sourcePcs, sourceParams, targetParams, pcsConverter); - // Convert to the target space. - // input to Matrix TRC calculator is descaled XYZ, need to descale PCS before use return TTo.FromScaledVector4(targetParams.Converter.Calculate(targetPcs)); } @@ -191,9 +185,6 @@ internal static class ColorProfileConverterExtensionsIcc sourcePcs = sourceParams.Is16BitLutEntry ? LabV2ToLab(sourcePcs) : sourcePcs; CieLab lab = CieLab.FromScaledVector4(sourcePcs); CieXyz xyz = pcsConverter.Convert(in lab); - - // DemoMaxICC clips negatives as part of IccUtil.cpp : icLabToXYZ > icICubeth - xyz = new CieXyz(Vector3.Clamp(xyz.ToVector3(), Vector3.Zero, new Vector3(float.MaxValue, float.MaxValue, float.MaxValue))); return xyz.ToScaledVector4(); } @@ -243,7 +234,7 @@ internal static class ColorProfileConverterExtensionsIcc /// Effectively this is with an extra step in the middle. /// It adjusts PCS by compensating for the black point used for perceptual intent in v2 profiles. /// The adjustment needs to be performed in XYZ space, potentially an overhead of 2 more conversions. - /// Not required if both spaces need adjustment, since they both have the same understanding of the PCS. + /// Not required if both spaces need V2 correction, since they both have the same understanding of the PCS. /// Not compatible with PCS adjustment for absolute intent. /// /// The source PCS values. @@ -251,7 +242,7 @@ internal static class ColorProfileConverterExtensionsIcc /// The target profile parameters. /// The converter to use for the PCS adjustments. /// Thrown when the source or target PCS is not supported. - private static Vector4 GetTargetPcsWithPerceptualV2Adjustment( + private static Vector4 GetTargetPcsWithPerceptualAdjustment( Vector4 sourcePcs, ConversionParams sourceParams, ConversionParams targetParams, @@ -268,9 +259,6 @@ internal static class ColorProfileConverterExtensionsIcc sourcePcs = sourceParams.Is16BitLutEntry ? LabV2ToLab(sourcePcs) : sourcePcs; CieLab lab = CieLab.FromScaledVector4(sourcePcs); xyz = pcsConverter.Convert(in lab); - - // DemoMaxICC clips negatives as part of IccUtil.cpp : icLabToXYZ > icICubeth - xyz = new CieXyz(Vector3.Max(xyz.ToVector3(), Vector3.Zero)); break; case IccColorSpaceType.CieXyz: xyz = CieXyz.FromScaledVector4(sourcePcs); @@ -279,28 +267,47 @@ internal static class ColorProfileConverterExtensionsIcc throw new ArgumentOutOfRangeException($"Source PCS {sourceParams.PcsType} is not supported"); } + bool oneProfileHasV2PerceptualAdjustment = sourceParams.HasV2PerceptualHandling ^ targetParams.HasV2PerceptualHandling; + // when converting from device to PCS with v2 perceptual intent // the black point needs to be adjusted to v4 after converting the PCS values - if (sourceParams.AdjustPcsForPerceptual) + if (sourceParams.HasNoPerceptualHandling || + (oneProfileHasV2PerceptualAdjustment && sourceParams.HasV2PerceptualHandling)) { - xyz = new CieXyz(AdjustPcsFromV2BlackPoint(xyz.ToVector3())); + Vector3 vector = xyz.ToVector3(); + + // when using LAB PCS, negative values are clipped before PCS adjustment (in DemoIccMAX) + if (sourceParams.PcsType == IccColorSpaceType.CieLab) + { + vector = Vector3.Max(vector, Vector3.Zero); + } + + xyz = new CieXyz(AdjustPcsFromV2BlackPoint(vector)); } // when converting from PCS to device with v2 perceptual intent // the black point needs to be adjusted to v2 before converting the PCS values - if (targetParams.AdjustPcsForPerceptual) + if (targetParams.HasNoPerceptualHandling || + (oneProfileHasV2PerceptualAdjustment && targetParams.HasV2PerceptualHandling)) { - xyz = new CieXyz(AdjustPcsToV2BlackPoint(xyz.ToVector3())); + Vector3 vector = AdjustPcsToV2BlackPoint(xyz.ToVector3()); + + // when using XYZ PCS, negative values are clipped after PCS adjustment (in DemoIccMAX) + if (targetParams.PcsType == IccColorSpaceType.CieXyz) + { + vector = Vector3.Max(vector, Vector3.Zero); + } + + xyz = new CieXyz(vector); } - Vector4 targetPcs; switch (targetParams.PcsType) { // 16-bit Lab encodings changed from v2 to v4, but 16-bit LUTs always use the legacy encoding regardless of version // so convert Lab back to legacy encoding before using in a 16-bit LUT case IccColorSpaceType.CieLab: CieLab lab = pcsConverter.Convert(in xyz); - targetPcs = lab.ToScaledVector4(); + Vector4 targetPcs = lab.ToScaledVector4(); return targetParams.Is16BitLutEntry ? LabToLabV2(targetPcs) : targetPcs; case IccColorSpaceType.CieXyz: return xyz.ToScaledVector4(); @@ -396,7 +403,9 @@ internal static class ColorProfileConverterExtensionsIcc internal IccVersion Version => this.Header.Version; - internal bool AdjustPcsForPerceptual => this.Intent == IccRenderingIntent.Perceptual && this.Version.Major == 2; + internal bool HasV2PerceptualHandling => this.Intent == IccRenderingIntent.Perceptual && this.Version.Major == 2; + + internal bool HasNoPerceptualHandling => this.Intent == IccRenderingIntent.Perceptual && this.Converter.IsTrc; internal bool Is16BitLutEntry => this.Converter.Is16BitLutEntry; } diff --git a/src/ImageSharp/ColorProfiles/Icc/Calculators/ColorTrcCalculator.cs b/src/ImageSharp/ColorProfiles/Icc/Calculators/ColorTrcCalculator.cs index ecd9a6f50b..cc0b727ba3 100644 --- a/src/ImageSharp/ColorProfiles/Icc/Calculators/ColorTrcCalculator.cs +++ b/src/ImageSharp/ColorProfiles/Icc/Calculators/ColorTrcCalculator.cs @@ -10,7 +10,7 @@ namespace SixLabors.ImageSharp.ColorProfiles.Icc.Calculators; internal class ColorTrcCalculator : IVector4Calculator { private readonly TrcCalculator curveCalculator; - private Matrix4x4 matrix; + private readonly Matrix4x4 matrix; private readonly bool toPcs; public ColorTrcCalculator( @@ -41,21 +41,25 @@ internal class ColorTrcCalculator : IVector4Calculator { if (this.toPcs) { - // when data to PCS, output from calculator is descaled XYZ - // but expected return value is scaled XYZ - // see DemoMaxICC IccCmm.cpp : CIccXformMatrixTRC::Apply() + // input is always linear RGB value = this.curveCalculator.Calculate(value); CieXyz xyz = new(Vector4.Transform(value, this.matrix).AsVector3()); + + // when data to PCS, output from calculator is descaled XYZ + // but downstream process requires scaled XYZ + // (see DemoMaxICC IccCmm.cpp : CIccXformMatrixTRC::Apply) return xyz.ToScaledVector4(); } else { - // when PCS to data, input to calculator is scaled XYZ - // but need descaled XYZ for matrix multiplication - // see DemoMaxICC IccCmm.cpp : CIccXformMatrixTRC::Apply() - Vector4 xyz = new(CieXyz.FromScaledVector4(value).ToVector3(), 1); - value = Vector4.Transform(xyz, this.matrix); - return this.curveCalculator.Calculate(value); + // input is always XYZ + Vector4 xyz = Vector4.Transform(value, this.matrix); + + // when data to PCS, upstream process provides scaled XYZ + // but input to calculator is descaled XYZ + // (see DemoMaxICC IccCmm.cpp : CIccXformMatrixTRC::Apply) + xyz = new(CieXyz.FromScaledVector4(xyz).ToVector3(), 1); + return this.curveCalculator.Calculate(xyz); } } } diff --git a/src/ImageSharp/ColorProfiles/Icc/IccConverterbase.Conversions.cs b/src/ImageSharp/ColorProfiles/Icc/IccConverterbase.Conversions.cs index 530459f93c..b06d3e7b0c 100644 --- a/src/ImageSharp/ColorProfiles/Icc/IccConverterbase.Conversions.cs +++ b/src/ImageSharp/ColorProfiles/Icc/IccConverterbase.Conversions.cs @@ -13,7 +13,9 @@ internal abstract partial class IccConverterBase { private IVector4Calculator calculator; - public bool Is16BitLutEntry => this.calculator is LutEntryCalculator { Is16Bit: true }; + internal bool Is16BitLutEntry => this.calculator is LutEntryCalculator { Is16Bit: true }; + + internal bool IsTrc => this.calculator is ColorTrcCalculator or GrayTrcCalculator; /// /// Checks the profile for available conversion methods and gathers all the information's necessary for it. diff --git a/tests/ImageSharp.Tests/ColorProfiles/Icc/ColorProfileConverterTests.Icc.cs b/tests/ImageSharp.Tests/ColorProfiles/Icc/ColorProfileConverterTests.Icc.cs index e7be6b793c..b8564a3489 100644 --- a/tests/ImageSharp.Tests/ColorProfiles/Icc/ColorProfileConverterTests.Icc.cs +++ b/tests/ImageSharp.Tests/ColorProfiles/Icc/ColorProfileConverterTests.Icc.cs @@ -25,10 +25,11 @@ public class ColorProfileConverterTests(ITestOutputHelper testOutputHelper) [InlineData(TestIccProfiles.StandardRgbV4, TestIccProfiles.Fogra39)] // RGB -> LAB -> XYZ -> RGB (different LUT elements, B-Matrix-M-CLUT-A vs B-Matrix-M) [InlineData(TestIccProfiles.StandardRgbV4, TestIccProfiles.RommRgb)] // RGB -> XYZ -> LAB -> RGB (different LUT elements, B-Matrix-M vs B-Matrix-M-CLUT-A) [InlineData(TestIccProfiles.RommRgb, TestIccProfiles.StandardRgbV4)] // CMYK -> LAB -> CMYK (different bit depth v2 LUTs, 16-bit vs 8-bit) - [InlineData(TestIccProfiles.Fogra39, TestIccProfiles.StandardRgbV2)] // CMYK -> LAB -> XYZ -> RGB (different LUT tags, A2B vs TRC) + [InlineData(TestIccProfiles.Fogra39, TestIccProfiles.StandardRgbV2, 0.0005)] // CMYK -> LAB -> XYZ -> RGB (different LUT tags, A2B vs TRC) --- tolerance slightly higher due to difference in inverse curve implementation [InlineData(TestIccProfiles.StandardRgbV2, TestIccProfiles.Fogra39)] // RGB -> XYZ -> LAB -> CMYK (different LUT tags, TRC vs A2B) - public void CanConvertIccProfiles(string sourceProfile, string targetProfile) + public void CanConvertIccProfiles(string sourceProfile, string targetProfile, double tolerance = 0.00005) { + // for 3-channel spaces, 4th item is ignored List inputs = [ [0, 0, 0, 0], @@ -38,6 +39,8 @@ public class ColorProfileConverterTests(ITestOutputHelper testOutputHelper) [0, 0, 0, 1], [1, 1, 1, 1], [0.5f, 0.5f, 0.5f, 0.5f], + [0.199678659f, 0.67982769f, 0.805381715f, 0.982666492f], // requires clipping before source is PCS adjusted for Fogra39 -> sRGBv2 + [0.776568174f, 0.961630166f, 0.31032759f, 0.895294666f], // requires clipping after target is PCS adjusted for Fogra39 -> sRGBv2 [GetNormalizedRandomValue(), GetNormalizedRandomValue(), GetNormalizedRandomValue(), GetNormalizedRandomValue()] ]; @@ -47,7 +50,6 @@ public class ColorProfileConverterTests(ITestOutputHelper testOutputHelper) Vector4 actualTargetValues = GetActualTargetValues(input, sourceProfile, targetProfile); testOutputHelper.WriteLine($"Input {string.Join(", ", input)} ยท Expected output {string.Join(", ", expectedTargetValues)}"); - const double tolerance = 0.00005; for (int i = 0; i < expectedTargetValues.Length; i++) { Assert.Equal(expectedTargetValues[i], actualTargetValues[i], tolerance); @@ -67,12 +69,12 @@ public class ColorProfileConverterTests(ITestOutputHelper testOutputHelper) /* This is a hack to trick Unicolour to work in the same way as ImageSharp. * ImageSharp bypasses PCS adjustment for v2 perceptual intent if source and target both need it - * as they both share the same understanding of what the PCS is (see ColorProfileConverterExtensionsIcc.GetTargetPcsWithPerceptualV2Adjustment) + * as they both share the same understanding of what the PCS is (see ColorProfileConverterExtensionsIcc.GetTargetPcsWithPerceptualAdjustment) * Unicolour does not support a direct profile-to-profile conversion so will always perform PCS adjustment for v2 perceptual intent. * However, PCS adjustment clips negative XYZ values, causing those particular values in Unicolour and ImageSharp to diverge. * It's unclear to me if there's a fundamental correct answer here. * - * There are 2 obvious ways to keep Unicolour and ImageSharp values aligned: + * There are two obvious ways to keep Unicolour and ImageSharp values aligned: * 1. Make ImageSharp always perform PCS adjustment, clipping negative XYZ values during the process - but creates a lot more calculations * 2. Make Unicolour stop performing PCS adjustment, allowing negative XYZ values during conversion * diff --git a/tests/ImageSharp.Tests/ColorProfiles/Icc/TestIccProfiles.cs b/tests/ImageSharp.Tests/ColorProfiles/Icc/TestIccProfiles.cs index c0d6f54e38..3fc36a91b0 100644 --- a/tests/ImageSharp.Tests/ColorProfiles/Icc/TestIccProfiles.cs +++ b/tests/ImageSharp.Tests/ColorProfiles/Icc/TestIccProfiles.cs @@ -57,7 +57,7 @@ internal static class TestIccProfiles public static Wacton.Unicolour.Configuration GetUnicolourConfiguration(string file) => UnicolourConfigurationCache.GetOrAdd( file, - f => new Wacton.Unicolour.Configuration(iccConfiguration: new(GetFullPath(f), Intent.Unspecified, f))); + f => new Wacton.Unicolour.Configuration(iccConfig: new(GetFullPath(f), Intent.Unspecified, f))); public static bool HasUnicolourConfiguration(string file) => UnicolourConfigurationCache.ContainsKey(file); diff --git a/tests/ImageSharp.Tests/ImageSharp.Tests.csproj b/tests/ImageSharp.Tests/ImageSharp.Tests.csproj index 2258b15022..880bf84aa8 100644 --- a/tests/ImageSharp.Tests/ImageSharp.Tests.csproj +++ b/tests/ImageSharp.Tests/ImageSharp.Tests.csproj @@ -50,7 +50,7 @@ - +