Fix perceptual handling for TRC transforms

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<CieLab, CieXyz>(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 <see cref="GetTargetPcsWithoutAdjustment"/> 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.
     /// </summary>
     /// <param name="sourcePcs">The source PCS values.</param>
@@ -251,7 +242,7 @@ internal static class ColorProfileConverterExtensionsIcc
     /// <param name="targetParams">The target profile parameters.</param>
     /// <param name="pcsConverter">The converter to use for the PCS adjustments.</param>
     /// <exception cref="ArgumentOutOfRangeException">Thrown when the source or target PCS is not supported.</exception>
-    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<CieLab, CieXyz>(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<CieXyz, CieLab>(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;
     }

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);
         }
     }
 }

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;
 
     /// <summary>
     /// Checks the profile for available conversion methods and gathers all the information's necessary for it.

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<float[]> 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
          *

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);

tests/ImageSharp.Tests/ImageSharp.Tests.csproj

@@ -50,7 +50,7 @@
     <PackageReference Include="runtime.osx.10.10-x64.CoreCompat.System.Drawing" Condition="'$(IsOSX)'=='true'" />
     <PackageReference Include="SharpZipLib" />
     <PackageReference Include="System.Drawing.Common" />
-    <PackageReference Include="Wacton.Unicolour" Version="4.8.0" />
+    <PackageReference Include="Wacton.Unicolour" Version="5.0.0" />
   </ItemGroup>
 
   <ItemGroup>