Color conversion with ICC profiles

[JimBobSquarePants]

Prerequisites

• I have written a descriptive pull-request title
• I have verified that there are no overlapping pull-requests open
• I have verified that I am following matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
• I have provided test coverage for my change (where applicable)

Description

Note: This is a replacement for the original PR #273 from @JBildstein that was automatically closed by our Git LFS history rewrite. Individual commits have unfortunately been lost in the process. Help is very much needed to complete the work.

As the title says, this adds methods for converting colors with an ICC profile.

Architecturally, the idea is that the profile is checked once for available and appropriate conversion methods and a then a delegate is stored that only takes the color values to convert and returns the calculated values. The possible performance penalty for using a delegate is far smaller than searching through the profile for every conversion. I'm open for other suggestions though.

There are classes to convert from the profile connection space (=PCS, can be XYZ or Lab) to the data space (RGB, CMYK, etc.) and vice versa. There are also classes to convert from PCS to PCS and Data to Data but they are only used for special profiles and are not important for us now but I still added them for completeness sake.

A challenge here is writing tests for this because of the complexity of the calculations and the big amount of different possible conversion paths. This is a rough list of the paths that exist:

• "A to B" and "B to A" tags
  • IccLut8TagDataEntry
    • Input IccLut[], Clut, Output IccLut[]
    • Matrix(3x3), Input IccLut[], IccClut, Output IccLut[]
  • IccLut16TagDataEntry
    • Input IccLut[], IccClut, Output IccLut[]
    • Matrix(3x3), Input IccLut[], IccClut, Output IccLut[]
  • IccLutAToBTagDataEntry/IccLutBToATagDataEntry (Curve types can either be IccCurveTagDataEntry or IccParametricCurveTagDataEntry (which has several curve subtypes))
    • CurveA[], Clut, CurveM[], Matrix(3x1), Matrix(3x3), CurveB[]
    • CurveA[], Clut, CurveB[]
    • CurveM[], Matrix(3x1), Matrix(3x3), CurveB[]
    • CurveB[]
• "D to B" tags
  • IccMultiProcessElementsTagDataEntry that contains an array of any of those types in any order:
    • IccCurveSetProcessElement
      • IccOneDimensionalCurve[] where each curve can have several curve subtypes
    • IccMatrixProcessElement
      • Matrix(Nr. of input Channels by Nr. of output Channels), Matrix(Nr. of output channels by 1)
    • IccClutProcessElement
      • IccClut
• Color Trc
  • Matrix(3x3), one curve for R, G and B each (Curve types can either be IccCurveTagDataEntry or IccParametricCurveTagDataEntry (which has several curve subtypes))
• Gray Trc
  • Curve (Curve type can either be IccCurveTagDataEntry or IccParametricCurveTagDataEntry (which has several curve subtypes))

The three main approaches in that list are

• A to B/B to A: using a combination of lookup tables, matrices and curves
• D to B: using a chain of multi process elements (curves, matrices or lookup)
• Trc: using curves (and matrices for color but not for gray)

The most used approaches are Color Trc for RGB profiles and LutAToB/LutBToA for CMYK profiles.

Todo list:

• Integrate with the rest of the project
• Write tests that cover all conversion paths
• Review architecture
• Improve speed and accuracy of the calculations

Help and suggestions are very welcome.

[brianpopow]

I wonder why the test MatrixCalculator_WithMatrix_ReturnsResult only fails with netcoreapp2.1 and not with the other frameworks.

[JimBobSquarePants]

@brianpopow It'll be an accuracy issue most likely. (I hope it's not a JIT issue). It should be possible to inspect the result and see.

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (main@ca20c92). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 5834c39 differs from pull request most recent head f60d4b8. Consider uploading reports for the commit f60d4b8 to get more accurate results

@@          Coverage Diff           @@
##             main   #1567   +/-   ##
======================================
  Coverage        ?     87%           
======================================
  Files           ?    1023           
  Lines           ?   55212           
  Branches        ?    7052           
======================================
  Hits            ?   48227           
  Misses          ?    5768           
  Partials        ?    1217           

Flag	Coverage Δ
unittests	87% <0%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[brianpopow]

@brianpopow It'll be an accuracy issue most likely. (I hope it's not a JIT issue). It should be possible to inspect the result and see.

The issue only happens with a Release build. I think i found the reason, but it seems very weird. Vector3.Zero does not have the expected value (0, 0, 0).

This can be seen with the testoutput:

[xUnit.net 00:00:07.19]     MatrixCalculator_WithMatrix_ReturnsResult(matrix2D: { {M11:1 M12:0 M13:0 M14:0} {M21:0 M22:1 M23:0 M24:0} {M31:0 M32:0 M33:1 M34:0} {M41:0 M42:0 M43:0 M44:1} }, matrix1D: <-0,0007887525. 4,590794E-41. 1>, input: <0,5. 0,5. 0,5. 0>, expected: <0,5. 0,5. 0,5. 0>) [FAIL]

matrix1D is supposed to be Vector3.Zero

[JimBobSquarePants]

Vector3.Zero does not have the expected value (0, 0, 0).

@brianpopow Woah! That's bonkers!

[brianpopow]

Vector3.Zero does not have the expected value (0, 0, 0).

@brianpopow Woah! That's bonkers!

I have reported this issue: dotnet/runtime#55623

They confirmed the issue, but they say its unlikely to be fixed because netcore2.1 is out of support in august.
So long story short: be careful with default values or Vector.Zero in testdata.

[brianpopow]

@JimBobSquarePants It would be really nice, if we could bring this PR forward. This would be a good addition to ImageSharp. I thought, I may ask you, if you know what the PR needs (besides tests) to be finished?

What would be the right way to apply an embedded color profile? My first attempt was:

var converter = new IccPcsToDataConverter(profile);
for (int y = 0; y < image.Height; y++)
{
    for (int x = 0; x < image.Width; x++)
    {
        var inputVec = image[x, y].ToVector4();
        Vector4 converted = converter.Calculate(inputVec);
        image[x, y] = new RgbaVector(converted.X, converted.Y, converted.Z);
    }
}

Here is an example image with adobe rgb color profile:

This does not seems to work, the colors seem to be wrong. Here are more example images

[JimBobSquarePants]

@brianpopow Honestly.....

I don't know. I was hoping the OP would come back to finish things off. I've just kept things updated over the years and hadn't gotten involved at all in the implementation as yet.

Judging from the old comments in the previous PR I believe the code is based somewhat based on the following

https://github.com/InternationalColorConsortium/DemoIccMAX/tree/master/IccProfLib

As for accuracy. That result looks like it's just spitting out the sRGB values again.

I do agree that it would be an awesome addition to the library and would save a ton of headaches. I hoped we'd get it in V3 but that's a mighty big ask.

[brianpopow]

I think we definitely need a reference implementation to compare the results against. I tried BABL which gnome is using, but i could not get it to work on windows. I will take a look at DemoIccMAX

[JimBobSquarePants]

@brianpopow @saucecontrol I've been trying to debug the port of the Clut 4d interpolator but I'm drawing a blank because I have no idea how to run DemoIccMAX IccProfLib to do a comparison. Any hints?

My money is on the port being sound but I'm doing something stupid porting the pcs to CIELab to convert via the color converter in IccProfileConverter

[brianpopow]

@brianpopow @saucecontrol I've been trying to debug the port of the Clut 4d interpolator but I'm drawing a blank because I have no idea how to run DemoIccMAX IccProfLib to do a comparison. Any hints?

@JimBobSquarePants I struggle myself to understand how DemoIccMAX is supposed to work.

I would suggest trying to understand how Little-CMS does it. There are alot of tests included in Little-CMS, maybe we can try to replicate one of them (maybe Check4Dinterp) and see what is the difference to our implementation.

You can generate visual studio solution files with the meson build system of LittleCms like this:

meson --backend=vs2022 --buildtype=debug buildvs2022

edit: There is no need to generate the visual studio project files, there are already ones in the repo: https://github.com/mm2/Little-CMS/tree/master/Projects/VC2022

[JimBobSquarePants]

Thanks @brianpopow I can't find anything useful there unfortunately.

[JimBobSquarePants]

I added some code and notes to the converter to help debugging. What I'm seeing is the results of the lut transform result in very minor differences to the output value for each pixel 1e05. I'm wondering if somehow, we have a scaling problem.

[cguedel]

What is the state of this? Lack of ICC support is a major blocker for us.

[JimBobSquarePants]

The state is as discussed in the various comments above, without contribution this PR will not be completed.

We support ICC profile preservation but not transformation during decode.

[Socolin]

Hello,

I would be interested in looking into this, I spent some time trying to understand ICC Profiles already. I started looking into this PR but I'm having hard time figuring out the exact state of the PR and what is missing.

From what my quick overview most (all ?) of the pieces are already there, and we just need to test this / fix bugs found during testing ?
Maybe it's worth go through each point of the specification (https://www.color.org/specification/ICC.1-2022-05.pdf) and make sure every point have been implemented ?

For the tests, would it be valid to take random image (or the same for all the test) embed various profile we can find, convert them with a reference implementation (Little CMS or library like Skia or Gid+ ?) and the validate that the implementation here is giving the exact same results ?

[JimBobSquarePants]

Hello,

I would be interested in looking into this, I spent some time trying to understand ICC Profiles already. I started looking into this PR but I'm having hard time figuring out the exact state of the PR and what is missing.

From what my quick overview most (all ?) of the pieces are already there, and we just need to test this / fix bugs found during testing ? Maybe it's worth go through each point of the specification (https://www.color.org/specification/ICC.1-2022-05.pdf) and make sure every point have been implemented ?

For the tests, would it be valid to take random image (or the same for all the test) embed various profile we can find, convert them with a reference implementation (Little CMS or library like Skia or Gid+ ?) and the validate that the implementation here is giving the exact same results ?

Thanks @Socolin any assistance would be useful.

I also believe that most of the pieces are also there and with some rough initial testing accuracy for the working parts. However we have an issue with Multi LUT profiles. There's some sort of scaling issue there that I haven't been able to debug as I am not able to get any reference implementations running.

I have wondered also, once we get things working whether we can use instances of IColorProfile created from the data gathered from the profiles and let the converter there correctly apply the transform

[JimBobSquarePants]

I've updated this with the latest codebase from main with the new color converter for switching between PCS. We still have the same issue with the multi-lut conversion (I haven't had another crack yet) so if anyone wants to look at this look at the failing test in IccProfileConverterTests

[Socolin]

I still plan to investigate this, I was busy last few weeks, I should find some time for this again in few weeks

[waacton]

I'm slowly piecing together ICC colour conversion for my own C# library, specifically targeting output "prtr" class profiles + LAB PCS + 16-bit and 8-bit LUT - so basically only version 2 CMYK currently.

Gotta start small, the ICC spec is too overwhelming otherwise! Anyway, I'm 99% sure I've got that use case implemented with input curves > n-dimensional CLUT interpolation > output curves (for example, getting expected values from 7D CMYKOGV). If your issues lie in this area, I'd be happy to try and help out once I've wrapped up my changes?

I also plan to support "A to B" type LUT (i.e. version 4 CMYK profiles) - if that's the problem area, it'll be a bit longer before I've tackled that.

Apologies if I'm way off, it's quite a dense thread to follow 😅

[JimBobSquarePants]

I'm slowly piecing together ICC colour conversion for my own C# library, specifically targeting output "prtr" class profiles + LAB PCS + 16-bit and 8-bit LUT - so basically only version 2 CMYK currently.

Gotta start small, the ICC spec is too overwhelming otherwise! Anyway, I'm 99% sure I've got that use case implemented with input curves > n-dimensional CLUT interpolation > output curves (for example, getting expected values from 7D CMYKOGV). If your issues lie in this area, I'd be happy to try and help out once I've wrapped up my changes?

I also plan to support "A to B" type LUT (i.e. version 4 CMYK profiles) - if that's the problem area, it'll be a bit longer before I've tackled that.

Apologies if I'm way off, it's quite a dense thread to follow 😅

@waacton It's a very hard thread to follow!!

It reads to me like you've made progress in the specific areas that we don't currently have working. I'd be overjoyed if you were able to contribute code to get this working, obviously anything here is also up-for-grabs in your separate implementation.

BTW Unicolour is a very intriguing library, I plan to have a good dig around there! I've recently reimplemented the color space conversion APIs in ImageSharp for V4 #2739 and am always keen to learn.

[waacton]

Wonderful, I'll try to find time soon to see if/how our implementations differ. Even if there's something I can do to get it working, I doubt any code I write will meet your performance criteria 😅 so I expect someone might need step in for optimisation.

And fingers crossed Unicolour is useful for someone other than me! If it doesn't give the right answers after the absolute battering it's taken from the unit tests, then... 🫠 but feel free to ask questions or highlight anything suspicious or questionable, I'm still learning as I go.

[waacton]

Just a quick update: some initial testing suggests that the core LUT functionality is working, at least for 4D CMYK device to LAB PCS, with respect to DemoIccMAX.

For example, with some careful handling of numeric types, this Fogra39 profile with hardcoded relative intent (an easier intent to test, no need for PCS adjustment) gives exactly the same LAB values.

var bytes = File.ReadAllBytes("./Coated_Fogra39L_VIGC_300.icc");
var converter = new IccDataToPcsConverter(new IccProfile(bytes));
var pcs = converter.Calculate(new Vector4(0.8f, 0.6f, 0.4f, 0.2f));
var iccLab = Vector4.Multiply(pcs, 65535f / 65280f); // Lab2 to Lab4

var l = iccLab[0] * 100;                    // 37.8027306
var a = (float)(iccLab[1] * 255.0 - 128.0); // -3.48995304
var b = (float)(iccLab[2] * 255.0 - 128.0); // -15.3412971

For what it's worth, my library uses doubles and different CLUT implementation and gets the result (37.802734372871825, -3.489946905189541, -15.341296133649976). The iccLab values, before the scaling, begin to differ at around 7 decimal places, which I guess is expected.

Given that the CLUT calculator is a direct port of the DemoIccMAX code, it's no real surprise that everything looks to work sensibly, but there are 5 different interpolation functions there; I only tested one usage of the 4D interpolation, plenty of room for bugs elsewhere!

Finally, the failing tests in IccProfileConverterTests

• CMYK isn't roundtrippable, so I'm not sure that CanRoundTripProfile makes sense
• CanConvertToSRGB looks suspiciously like the pixel values coming from issue-129.jpg are in an RGBA format
  • First pixel = <0.43921572, 1, 0.62352943, 1> = what I'd call a "light vibrant mint" #70FF9F
  • Last pixel = <0.30588236, 0.85490197, 1, 1> = what I'd call a "light vibrant cyan" #4EDAFF
  • ...and this matches the test input image pretty well
  • Passing these values in to a CMYK to PCS converter is just going to become basically black thanks to the K = 1
  • ... which also matches the test output image

Sadly I don't know anything about how images actually encode CMYK data, so I'm not sure there's anything I can contribute in code just yet 😥. Happy to help get to the bottom of any other specific ICC conversion concerns!

[JimBobSquarePants]

Just a quick update: some initial testing suggests that the core LUT functionality is working, at least for 4D CMYK device to LAB PCS, with respect to DemoIccMAX.

For example, with some careful handling of numeric types, this Fogra39 profile with hardcoded relative intent (an easier intent to test, no need for PCS adjustment) gives exactly the same LAB values.

var bytes = File.ReadAllBytes("./Coated_Fogra39L_VIGC_300.icc");
var converter = new IccDataToPcsConverter(new IccProfile(bytes));
var pcs = converter.Calculate(new Vector4(0.8f, 0.6f, 0.4f, 0.2f));
var iccLab = Vector4.Multiply(pcs, 65535f / 65280f); // Lab2 to Lab4

var l = iccLab[0] * 100;                    // 37.8027306
var a = (float)(iccLab[1] * 255.0 - 128.0); // -3.48995304
var b = (float)(iccLab[2] * 255.0 - 128.0); // -15.3412971

For what it's worth, my library uses doubles and different CLUT implementation and gets the result (37.802734372871825, -3.489946905189541, -15.341296133649976). The iccLab values, before the scaling, begin to differ at around 7 decimal places, which I guess is expected.

Given that the CLUT calculator is a direct port of the DemoIccMAX code, it's no real surprise that everything looks to work sensibly, but there are 5 different interpolation functions there; I only tested one usage of the 4D interpolation, plenty of room for bugs elsewhere!

Finally, the failing tests in IccProfileConverterTests

• CMYK isn't roundtrippable, so I'm not sure that CanRoundTripProfile makes sense

• CanConvertToSRGB looks suspiciously like the pixel values coming from issue-129.jpg are in an RGBA format

  • First pixel = <0.43921572, 1, 0.62352943, 1> = what I'd call a "light vibrant mint" #70FF9F
  • Last pixel = <0.30588236, 0.85490197, 1, 1> = what I'd call a "light vibrant cyan" #4EDAFF
  • ...and this matches the test input image pretty well
  • Passing these values in to a CMYK to PCS converter is just going to become basically black thanks to the K = 1
  • ... which also matches the test output image

Sadly I don't know anything about how images actually encode CMYK data, so I'm not sure there's anything I can contribute in code just yet 😥. Happy to help get to the bottom of any other specific ICC conversion concerns!

Apologies for the slow response and thank you for your investigation. Yes...That makes perfect sense.... 😞

Our code is assuming that the converters are operating in a manner that can handle the input Vector using an RGBA layout with values ranging from 0-1. Of course, this isn't the case and is a huge bug.

When converting from a CMYK profile we're going to need to convert from TPixel to Vector4 to CMYK, then adjust the values, then convert back to the TPixel layout. Looks like we should be checking the IccColorSpaceType property there and working out the conversion.

This is going to get messy given that our color-space converter expects a generic type.

[JimBobSquarePants]

The fix for this (dear reader) will be to update ColorProfileConverter to utilize ICCProfile when provided in the same manner as UniColour and to use the converter whenever a profile is present and we wish to normalize the input on decode. It's not possible to use a single pipeline to handle this.

If anyone has time to make a start on the first part I can run with the second.

[waacton]

... update ColorProfileConverter to utilize ICCProfile when provided

This sounds like something I'm conceptually familiar with so I'll aim to find some time over the holidays (though if anyone else is keen, please go ahead)

[JimBobSquarePants]

@waacton if you can please do. 👍

[waacton]

@JimBobSquarePants I've been exploring the ColorProfileConverter code a little bit to get the ball rolling and I think it's going to spawn a lot of questions. Is there anything like an internal wiki that I can dump thoughts into and record the latest understanding, to avoid spamming this PR thread?

The kind of things lurking in my mind are:

1. Implementation details - ICC profile conversions don't fit neatly into the existing architecture (which I think you alluded to with "It's not possible to use a single pipeline to handle this.")
2. Validation - I've had a hard time finding a definitive answer for ICC conversions, and ended up jury-rigging something from the DemoIccMAX reference implementation to generate test data. The repo to generate test data is currently private but will become public once it's less painful to use. (Note that these seem to be different than System.Windows.Media.Color.FromValues so I've had to assume the ICC reference implementation is the gold standard here)
3. Code reuse - it's taken a lot of fiddling in Unicolour to handle edge cases (e.g. adjustments need to be made after the ICC conversion, depending on rendering intent, profile version, and the bit-depth of the LUT). I suspect a direct dependency isn't suitable, except for perhaps a first-pass proof-of-concept, but maybe an almost-verbatim copy of utility classes is? And a conversion comparison with Unicolour in unit tests to spot diverging implementation?
4. Profile restrictions - there are an unholy number of ICC conversion permutations; my Unicolour implementation only supports a specific subset so that I can focus on getting them right (with the intent of expanding the subset over time). As a result, I'm not familiar with edge cases and adjustments for the scenarios I'm not yet supporting, so there's a higher risk of those conversions being subtly and going undetected. (However, Unicolour technically supports CMYK ⇔ XYZ but I can't find a profile that does this, so I can't generate test data to validate it - just one of many permutations that might not actually be correct 😓)