WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -60,8 +60,8 @@ internal static TTo ConvertUsingIccProfile<TFrom, TTo>(this ColorProfileConverte
ColorProfileConverter pcsConverter = new(new ColorConversionOptions
{
MemoryAllocator = converter.Options.MemoryAllocator,
SourceWhitePoint = new CieXyz(converter.Options.SourceIccProfile.Header.PcsIlluminant),
TargetWhitePoint = new CieXyz(converter.Options.TargetIccProfile.Header.PcsIlluminant),
SourceWhitePoint = KnownIlluminants.D50Icc,
Copy link
Member

Choose a reason for hiding this comment

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

Should we not be testing for a version number < 5 and assigning the value based on the version?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Potentially, though whether the ICC code in ImageSharp can support v5+ correctly at all is another question. I wouldn't be surprised if v5-level features were only added to the reference implementation after it was ported here (and now that the reference has been refactored, could be harder to match up). Adapting between PCS illuminants seems a sensible step but it might not meet v5 spec, given how wordy section 10.2.22 spectralViewingConditionsType is for the svcn tag.

And their description on https://www.color.org/icclabs.xalter tries to emphasise how different v5 is:

this new system is not a simple "dot" release to the V4 spec and that a V5 profile is very different than a V2 or V4

in many industries, v4 (and even v2) meets existing color management needs and in these industries there will be no drive to move to adopt the v5 specification

I've still not got around to implementing full support for v4 profiles in Unicolour so I've largely buried my head in the sand for v5 🙈. I'm hedging my bets that because v4 took painstaking effort to meet, v5 surely can't be a smoother experience.

In summary I think it's a choice between:

  • Explicitly no support for v5 until (or if) it becomes more widely adopted
  • Best attempt to support v5 while acknowledging it might deviate from spec

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for confirming. Yeah, definitely best to deal with V4 for now and V5 if and when it's ever needed.

TargetWhitePoint = KnownIlluminants.D50Icc
});

// Normalize the source, then convert to the PCS space.
Expand Down Expand Up @@ -104,8 +104,8 @@ internal static void ConvertUsingIccProfile<TFrom, TTo>(this ColorProfileConvert
ColorProfileConverter pcsConverter = new(new ColorConversionOptions
{
MemoryAllocator = converter.Options.MemoryAllocator,
SourceWhitePoint = new CieXyz(converter.Options.SourceIccProfile.Header.PcsIlluminant),
TargetWhitePoint = new CieXyz(converter.Options.TargetIccProfile.Header.PcsIlluminant),
SourceWhitePoint = KnownIlluminants.D50Icc,
TargetWhitePoint = KnownIlluminants.D50Icc
});

using IMemoryOwner<Vector4> pcsBuffer = converter.Options.MemoryAllocator.Allocate<Vector4>(source.Length);
Expand Down
8 changes: 7 additions & 1 deletion src/ImageSharp/ColorProfiles/KnownIlluminants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ namespace SixLabors.ImageSharp.ColorProfiles;
/// </summary>
/// <remarks>
/// Coefficients taken from: http://www.brucelindbloom.com/index.html?Eqn_ChromAdapt.html
/// and https://color.org/specification/ICC.1-2022-05.pdf
/// <br />
/// Descriptions taken from: http://en.wikipedia.org/wiki/Standard_illuminant
/// </remarks>
Expand All @@ -30,10 +31,15 @@ public static class KnownIlluminants
public static CieXyz C { get; } = new(0.98074F, 1F, 1.18232F);

/// <summary>
/// Gets the Horizon Light. ICC profile PCS illuminant.
/// Gets the Horizon Light.
/// </summary>
public static CieXyz D50 { get; } = new(0.96422F, 1F, 0.82521F);

/// <summary>
/// Gets the D50 illuminant used in the ICC profile specification.
/// </summary>
public static CieXyz D50Icc { get; } = new(0.9642F, 1F, 0.8249F);

/// <summary>
/// Gets the Mid-morning / Mid-afternoon Daylight illuminant.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public class ColorProfileConverterTests(ITestOutputHelper testOutputHelper)
[InlineData(TestIccProfiles.RommRgb, TestIccProfiles.StandardRgbV4)] // CMYK -> LAB -> CMYK (different bit depth v2 LUTs, 16-bit vs 8-bit)
[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, double tolerance = 0.00005)
public void CanConvertIccProfiles(string sourceProfile, string targetProfile, double tolerance = 0.000005)
{
List<Vector4> actual = Inputs.ConvertAll(input => GetActualTargetValues(input, sourceProfile, targetProfile));
AssertConversion(sourceProfile, targetProfile, actual, tolerance, testOutputHelper);
Expand All @@ -63,7 +63,7 @@ public void CanConvertIccProfiles(string sourceProfile, string targetProfile, do
[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)
[InlineData(TestIccProfiles.Issue129, TestIccProfiles.StandardRgbV4)] // CMYK -> LAB -> -> XYZ -> RGB
public void CanBulkConvertIccProfiles(string sourceProfile, string targetProfile, double tolerance = 0.00005)
public void CanBulkConvertIccProfiles(string sourceProfile, string targetProfile, double tolerance = 0.000005)
{
List<Vector4> actual = GetBulkActualTargetValues(Inputs, sourceProfile, targetProfile);
AssertConversion(sourceProfile, targetProfile, actual, tolerance, testOutputHelper);
Expand Down
Loading