Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Color conversion with ICC profiles #1567

Draft
wants to merge 96 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
96 commits
Select commit Hold shift + click to select a range
16179b3
Copy original PR changes.
JimBobSquarePants Feb 27, 2021
eeb40f5
Merge branch 'master' into icc-color-conversion
JimBobSquarePants Apr 23, 2021
8b96d37
Merge branch 'master' into icc-color-conversion
JimBobSquarePants May 22, 2021
aa2f1e9
Merge branch 'master' into icc-color-conversion
JimBobSquarePants Jul 7, 2021
3debb26
Merge branch 'master' into icc-color-conversion
brianpopow Jul 12, 2021
0812085
Fix warnings
brianpopow Jul 12, 2021
1094dc6
Add color conversion trait to be able to filter for those tests
brianpopow Jul 12, 2021
759f053
Fix failing MatrixCalculator test
brianpopow Jul 13, 2021
4cde4ef
Merge branch 'master' into icc-color-conversion
JimBobSquarePants Nov 9, 2021
a973713
Merge branch 'master' into icc-color-conversion
JimBobSquarePants Nov 11, 2021
33339d0
Merge branch 'master' into icc-color-conversion
JimBobSquarePants Jan 18, 2022
cdb495c
Merge branch 'master' into icc-color-conversion
JimBobSquarePants Jan 30, 2022
5f7acc1
Merge remote-tracking branch 'origin/main' into icc-color-conversion
brianpopow May 15, 2022
7e2bc20
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Nov 23, 2022
dc166a7
Fix build
JimBobSquarePants Nov 23, 2022
dcc8147
Cleanup icc tests
brianpopow Nov 24, 2022
73c8d8f
Fix Copyright notice
brianpopow Nov 24, 2022
d229fed
Fix icc namespaces
brianpopow Nov 24, 2022
0a08da0
Use file scoped namespaces
brianpopow Nov 27, 2022
daf366b
Cleanup and add conversion tests
JimBobSquarePants Nov 30, 2022
54856ff
Fix reader and out of range exception
JimBobSquarePants Dec 4, 2022
eee14c6
Remove invalid test.
JimBobSquarePants Dec 4, 2022
f60d4b8
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Dec 14, 2022
8de137e
Cleanup code style
JimBobSquarePants Dec 15, 2022
fb8003c
Remove double clamping
JimBobSquarePants Dec 15, 2022
9f0f9cb
Optimize matrix read/write
JimBobSquarePants Dec 15, 2022
ece11eb
Create ColorProfileHandling.cs
JimBobSquarePants Dec 17, 2022
bd7257b
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Dec 17, 2022
9a21485
Nullable disable
JimBobSquarePants Dec 17, 2022
ba76964
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Dec 18, 2022
98d1758
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Jan 2, 2023
66554cb
Add ability to convert ICC profile on decode
JimBobSquarePants Jan 4, 2023
e90f165
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Jan 4, 2023
8c580a7
Handle nullability in decoder base
JimBobSquarePants Jan 5, 2023
a81dac9
Add reference files.
JimBobSquarePants Jan 9, 2023
902ed99
Add tolerance for Mac
JimBobSquarePants Jan 9, 2023
e3aa452
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Jan 10, 2023
0dd68fe
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Jan 15, 2023
6e3dc81
Update decoder bases following merge
JimBobSquarePants Jan 15, 2023
b7833a4
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Jan 16, 2023
c3984aa
Add failing tests
JimBobSquarePants Jan 16, 2023
0fff06d
Port 1d, 2d, 3d, 4d and nd interpolation from reference implementation
brianpopow Jan 22, 2023
f1c05ee
Remove not used IccClut constructors
brianpopow Jan 22, 2023
3be31c3
Preserve alpha component
JimBobSquarePants Jan 22, 2023
6c2ee90
Fix CieLab docs
JimBobSquarePants Jan 22, 2023
20e9b7f
Fix out of bounds error
brianpopow Jan 22, 2023
d6fbc01
Change clut values from jagged array to flat array
brianpopow Jan 22, 2023
67ed4ce
Fix warnings
brianpopow Jan 22, 2023
b036cc3
Fix mistake reading the clut values
brianpopow Jan 22, 2023
52f88c8
Fix oob in n-dimension calculator.
JimBobSquarePants Jan 23, 2023
ed47678
Add Lab<=>Xyz conversion
JimBobSquarePants Jan 23, 2023
10bea86
Add ICC reader tests
brianpopow Jan 23, 2023
5b131ad
Add reference output for issue-129
brianpopow Feb 3, 2023
ed8091b
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Feb 3, 2023
6225db3
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Feb 9, 2023
a65c599
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Mar 30, 2023
60f3d9d
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Sep 2, 2023
c940b86
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Sep 25, 2023
a567613
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Nov 1, 2023
3e06687
Update IccReader.cs
JimBobSquarePants Nov 1, 2023
c1ebbfe
Fix build
JimBobSquarePants Nov 1, 2023
d89d8c5
Update IccReader.cs
JimBobSquarePants Nov 1, 2023
63c89ca
Use scaled Vector4 conversion and optimize
JimBobSquarePants Nov 1, 2023
3389d7a
Add some debugging helpers to the converter
JimBobSquarePants Nov 9, 2023
7b0ff3b
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Jun 11, 2024
29ed2b4
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Jul 9, 2024
5f975e5
Update to latest main build
JimBobSquarePants Jul 9, 2024
79f5dfa
Update IccProfileConverterTests.cs
JimBobSquarePants Jul 9, 2024
b88b2a9
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Jul 10, 2024
bca4cad
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Aug 12, 2024
6e2c29c
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Dec 2, 2024
5b374da
Fix IccClut test setup and calculator guards
JimBobSquarePants Dec 3, 2024
441f07e
Update ColorProfileConverter to handle ICCProfiles
JimBobSquarePants Dec 3, 2024
6654218
Suppress warning
JimBobSquarePants Dec 3, 2024
21fec4e
Demonstrate ICC conversion comparison to Unicolour
waacton Dec 6, 2024
fd2e8a9
Migrate tests
JimBobSquarePants Dec 10, 2024
19ff69d
Add ICC files to LFS
JimBobSquarePants Dec 10, 2024
3cd7d67
Update TestIccProfiles.cs
JimBobSquarePants Dec 10, 2024
0327dca
Adjust PCS values for v2 profiles using perceptual intent
waacton Dec 10, 2024
9de3935
Remove TODO
waacton Dec 10, 2024
8e92f20
Fix XYZ PCS conversions
waacton Dec 10, 2024
312b55e
Cleanup
JimBobSquarePants Dec 11, 2024
44aae40
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Dec 11, 2024
df3d230
Extract conversion for v2 perceptual intent
waacton Dec 11, 2024
d20fddb
Precalculate v2 perceptual PCS adjustment
waacton Dec 12, 2024
d60ac76
Bypass PCS adjustment when not needed
waacton Dec 14, 2024
cfa2760
Add failing tests for CMYK to RGB using Matrix TRC
waacton Dec 14, 2024
369bf5f
Fix CMYK to RGB using TRCs
waacton Dec 14, 2024
7d4a742
Add RGB to CMYK tests and fix TRC calculator
waacton Dec 15, 2024
f4e9509
Handle tests in cases where PCS adjustment is bypassed
waacton Dec 16, 2024
9ceed23
Fix expected values of CLUT unit tests
waacton Dec 17, 2024
f21c0c2
Fix LUT entry calculator for XYZ PCS with non-identity matrix
waacton Dec 17, 2024
f147aad
Minor cleanup
JimBobSquarePants Dec 18, 2024
7492109
Merge branch 'main' into icc-color-conversion
JimBobSquarePants Dec 18, 2024
630df8a
Add TRC conversion comparison with Unicolour
waacton Jan 6, 2025
54143df
Remove unnecessary tests
waacton Jan 6, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,9 @@ private static Vector4 GetTargetPcsWithoutAdjustment(
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)));
waacton marked this conversation as resolved.
Show resolved Hide resolved
return xyz.ToScaledVector4();
}

Expand Down Expand Up @@ -258,6 +261,9 @@ private static Vector4 GetTargetPcsWithPerceptualV2Adjustment(
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.Clamp(xyz.ToVector3(), Vector3.Zero, new Vector3(float.MaxValue, float.MaxValue, float.MaxValue)));
break;
case IccColorSpaceType.CieXyz:
xyz = CieXyz.FromScaledVector4(sourcePcs);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,16 @@ public ColorTrcCalculator(
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public Vector4 Calculate(Vector4 value)
{
// uses the descaled XYZ as DemoMaxICC IccCmm.cpp : CIccXformMatrixTRC::Apply()
Vector4 xyz = new(CieXyz.FromScaledVector4(value).ToVector3(), 1);
Copy link
Collaborator

@waacton waacton Dec 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amusingly the matrix is applied to the actual XYZ not the scaled XYZ. Is there a better way to descale this? (Apologies for my rookie vector wrangling)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmmm… this means we are scaling then descaling each pixel. Do all other calculators use scaled values? If so can we avoid the initial scaling or update this to match?

Copy link
Collaborator

@waacton waacton Dec 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only calculator I've encountered so far that requires the non-scaled values. Won't be sure until we find profiles with the other types of curves but it's likely to be just this one, for the matrix multiplication to apply correctly.

We might be able to do something similar to the Is16BitLutEntry check and avoid scaling XYZ if we know we're using ColorTrcCalculator, or add a IVector4Calculator.NeedsScaled property for calculators themselves to define? It would add more complexity to the target PCS calculation but save on a couple of potential scaling roundtrips.

Also, I'm aware that in general I'm flopping between e.g. Vector and CieXyz quite a bit, is that problematic?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should try to avoid switching between the types where possible as that causes per-pixel overhead. Perhaps we should be trying to provide this information up-front? It's tricky...

Copy link
Member Author

@JimBobSquarePants JimBobSquarePants Dec 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little confused by this comment?

// when data to PCS, output from calculator is descaled XYZ
// but expected return value is scaled XYZ
// see DemoMaxICC IccCmm.cpp : CIccXformMatrixTRC::Apply()

How does the calculator know how to descale the output from the scaled input?

Also doesn't the Gray TRC caclulator require the same?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I follow your question but I'm still getting my head around it myself. Does it help if I say...

  • Matrix TRC can only be used with XYZ PCS
  • Simple curve lookups use the scaled XYZ
  • In this case, because the output of the curve needs to be multiplied with a matrix, we need to use the actual descaled XYZ it represents
  • Downstream operations tend to assume the PCS will be in scaled form

I've experimented with a different approach that might make things clearer, moving the responsibility of knowing about this from ColorTrcCalculator to ColorProfileConverter. The downside is you can't "just use" the calculator, but that's true of ICC transforms generally anyway once you start looking closely.

Imagine ColorTrcCalculator doesn't have these changes, and instead ColorProfileConverter has these extra steps:

// 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());
sourcePcs = sourceParams.IsMatrixTrc ? new CieXyz(sourcePcs.AsVector3()).ToScaledVector4() : sourcePcs;
Vector4 targetPcs = ... // do conversion from source PCS to target PCS, potentially with perceptual adjustments

// input to Matrix TRC calculator is descaled XYZ, need to descale PCS before use
targetPcs = targetParams.IsMatrixTrc ? new Vector4(CieXyz.FromScaledVector4(targetPcs).ToVector3(), 1) : targetPcs;
return TTo.FromScaledVector4(targetParams.Converter.Calculate(targetPcs));

I can't tell if it feels better or worse. Depends what mood I'm in when I look!

As for Gray TRC, I can't see any evidence that it does the same thing, just multiplies the scaled monochrome by scaled D50.


💡 If that matrix multiplication in LutEntryCalculator does indeed need fixing, then presumably the matrix multiplication in ColorTrcCalculator does too.


if (this.toPcs)
{
value = this.curveCalculator.Calculate(value);
value = this.curveCalculator.Calculate(xyz);
return Vector4.Transform(value, this.matrix);
}

value = Vector4.Transform(value, this.matrix);
value = Vector4.Transform(xyz, this.matrix);
return this.curveCalculator.Calculate(value);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,12 @@ public void CanConvertCmykIccProfilesToRgbUsingMatrixTrc(float c, float m, float

Rgb actualRgb = converter.Convert<Cmyk, Rgb>(new Cmyk(new Vector4(input)));

Assert.Equal(expectedR, actualRgb.R);
Assert.Equal(expectedG, actualRgb.G);
Assert.Equal(expectedB, actualRgb.B);
// TODO: investigate lower tolerance than CanConvertCmykIccProfiles()
// currently assuming it's a rounding error in the process of gathering test data manually
const double tolerance = 0.0005;
Assert.Equal(expectedR, actualRgb.R, tolerance);
Assert.Equal(expectedG, actualRgb.G, tolerance);
Assert.Equal(expectedB, actualRgb.B, tolerance);
}

private static double[] GetExpectedTargetValues(string sourceProfile, string targetProfile, float[] input)
Expand Down
Loading