-
Notifications
You must be signed in to change notification settings - Fork 0
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 #8
base: main
Are you sure you want to change the base?
Changes from all commits
16179b3
eeb40f5
8b96d37
aa2f1e9
3debb26
0812085
1094dc6
759f053
4cde4ef
a973713
33339d0
cdb495c
5f7acc1
7e2bc20
dc166a7
dcc8147
73c8d8f
d229fed
0a08da0
daf366b
54856ff
eee14c6
f60d4b8
8de137e
fb8003c
9f0f9cb
ece11eb
bd7257b
9a21485
ba76964
98d1758
66554cb
e90f165
8c580a7
a81dac9
902ed99
e3aa452
0dd68fe
6e3dc81
b7833a4
c3984aa
0fff06d
f1c05ee
3be31c3
6c2ee90
20e9b7f
d6fbc01
67ed4ce
b036cc3
52f88c8
ed47678
10bea86
5b131ad
ed8091b
6225db3
a65c599
60f3d9d
c940b86
a567613
3e06687
c1ebbfe
d89d8c5
63c89ca
3389d7a
7b0ff3b
29ed2b4
5f975e5
79f5dfa
b88b2a9
bca4cad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
// Copyright (c) Six Labors. | ||
// Licensed under the Six Labors Split License. | ||
|
||
using System.Numerics; | ||
using System.Runtime.CompilerServices; | ||
using SixLabors.ImageSharp.Metadata.Profiles.Icc; | ||
|
||
namespace SixLabors.ImageSharp.ColorProfiles.Icc.Calculators; | ||
|
||
internal class ColorTrcCalculator : IVector4Calculator | ||
{ | ||
private readonly TrcCalculator curveCalculator; | ||
private Matrix4x4 matrix; | ||
private readonly bool toPcs; | ||
|
||
public ColorTrcCalculator( | ||
IccXyzTagDataEntry redMatrixColumn, | ||
IccXyzTagDataEntry greenMatrixColumn, | ||
IccXyzTagDataEntry blueMatrixColumn, | ||
IccTagDataEntry redTrc, | ||
IccTagDataEntry greenTrc, | ||
IccTagDataEntry blueTrc, | ||
bool toPcs) | ||
{ | ||
this.toPcs = toPcs; | ||
this.curveCalculator = new TrcCalculator(new IccTagDataEntry[] { redTrc, greenTrc, blueTrc }, !toPcs); | ||
|
||
Vector3 mr = redMatrixColumn.Data[0]; | ||
Vector3 mg = greenMatrixColumn.Data[0]; | ||
Vector3 mb = blueMatrixColumn.Data[0]; | ||
Comment on lines
+28
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: Accessing Data[0] without checking array bounds could throw IndexOutOfRangeException if Data is empty. |
||
this.matrix = new Matrix4x4(mr.X, mr.Y, mr.Z, 0, mg.X, mg.Y, mg.Z, 0, mb.X, mb.Y, mb.Z, 0, 0, 0, 0, 1); | ||
|
||
if (!toPcs) | ||
{ | ||
Matrix4x4.Invert(this.matrix, out this.matrix); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: Matrix4x4.Invert() could fail for a singular matrix. Should check the return value and handle invalid matrices. |
||
} | ||
} | ||
|
||
[MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
public Vector4 Calculate(Vector4 value) | ||
{ | ||
if (this.toPcs) | ||
{ | ||
value = this.curveCalculator.Calculate(value); | ||
return Vector4.Transform(value, this.matrix); | ||
} | ||
|
||
value = Vector4.Transform(value, this.matrix); | ||
return this.curveCalculator.Calculate(value); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
// Copyright (c) Six Labors. | ||
// Licensed under the Six Labors Split License. | ||
|
||
namespace SixLabors.ImageSharp.ColorSpaces.Conversion.Icc; | ||
|
||
internal partial class CurveCalculator | ||
{ | ||
private enum CalculationType | ||
{ | ||
Identity, | ||
Gamma, | ||
Lut, | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
// Copyright (c) Six Labors. | ||
// Licensed under the Six Labors Split License. | ||
#nullable disable | ||
|
||
using SixLabors.ImageSharp.ColorProfiles.Icc.Calculators; | ||
using SixLabors.ImageSharp.Metadata.Profiles.Icc; | ||
|
||
namespace SixLabors.ImageSharp.ColorSpaces.Conversion.Icc; | ||
|
||
internal partial class CurveCalculator : ISingleCalculator | ||
{ | ||
private readonly LutCalculator lutCalculator; | ||
private readonly float gamma; | ||
private readonly CalculationType type; | ||
|
||
public CurveCalculator(IccCurveTagDataEntry entry, bool inverted) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: entry parameter should be validated with Guard.NotNull |
||
{ | ||
if (entry.IsIdentityResponse) | ||
{ | ||
this.type = CalculationType.Identity; | ||
} | ||
else if (entry.IsGamma) | ||
{ | ||
this.gamma = entry.Gamma; | ||
if (inverted) | ||
{ | ||
this.gamma = 1f / this.gamma; | ||
} | ||
|
||
this.type = CalculationType.Gamma; | ||
} | ||
else | ||
{ | ||
this.lutCalculator = new LutCalculator(entry.CurveData, inverted); | ||
this.type = CalculationType.Lut; | ||
} | ||
} | ||
|
||
public float Calculate(float value) | ||
=> this.type switch | ||
{ | ||
CalculationType.Identity => value, | ||
CalculationType.Gamma => MathF.Pow(value, this.gamma), // TODO: This could be optimized using a LUT. See SrgbCompanding | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: gamma calculation could be optimized using a pre-computed LUT for common gamma values |
||
CalculationType.Lut => this.lutCalculator.Calculate(value), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: check that lutCalculator is not null before using |
||
_ => throw new InvalidOperationException("Invalid calculation type"), | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
// Copyright (c) Six Labors. | ||
// Licensed under the Six Labors Split License. | ||
|
||
using System.Numerics; | ||
using System.Runtime.CompilerServices; | ||
using SixLabors.ImageSharp.Metadata.Profiles.Icc; | ||
|
||
namespace SixLabors.ImageSharp.ColorProfiles.Icc.Calculators; | ||
|
||
internal class GrayTrcCalculator : IVector4Calculator | ||
{ | ||
private readonly TrcCalculator calculator; | ||
|
||
public GrayTrcCalculator(IccTagDataEntry grayTrc, bool toPcs) | ||
=> this.calculator = new TrcCalculator(new IccTagDataEntry[] { grayTrc }, !toPcs); | ||
Comment on lines
+14
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: Consider adding Guard.NotNull check for grayTrc parameter to prevent null reference exceptions |
||
|
||
[MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
public Vector4 Calculate(Vector4 value) => this.calculator.Calculate(value); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
// Copyright (c) Six Labors. | ||
// Licensed under the Six Labors Split License. | ||
|
||
namespace SixLabors.ImageSharp.ColorProfiles.Icc.Calculators; | ||
|
||
/// <summary> | ||
/// Represents an ICC calculator with a single floating point value and result | ||
/// </summary> | ||
internal interface ISingleCalculator | ||
{ | ||
/// <summary> | ||
/// Calculates a result from the given value | ||
/// </summary> | ||
/// <param name="value">The input value</param> | ||
/// <returns>The calculated result</returns> | ||
float Calculate(float value); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
// Copyright (c) Six Labors. | ||
// Licensed under the Six Labors Split License. | ||
|
||
using System.Numerics; | ||
|
||
namespace SixLabors.ImageSharp.ColorProfiles.Icc.Calculators; | ||
|
||
/// <summary> | ||
/// Represents an ICC calculator with <see cref="Vector4"/> values and results | ||
/// </summary> | ||
internal interface IVector4Calculator | ||
{ | ||
/// <summary> | ||
/// Calculates a result from the given values | ||
/// </summary> | ||
/// <param name="value">The input values</param> | ||
/// <returns>The calculated result</returns> | ||
Vector4 Calculate(Vector4 value); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
// Copyright (c) Six Labors. | ||
// Licensed under the Six Labors Split License. | ||
|
||
namespace SixLabors.ImageSharp.ColorSpaces.Conversion.Icc; | ||
|
||
internal partial class LutABCalculator | ||
{ | ||
private enum CalculationType | ||
{ | ||
AtoB = 1 << 3, | ||
BtoA = 1 << 4, | ||
|
||
SingleCurve = 1, | ||
CurveMatrix = 2, | ||
CurveClut = 3, | ||
Full = 4, | ||
Comment on lines
+13
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: sequential values 1-4 suggest these are not meant to be flags but rather enum values - this could cause issues when combining with AtoB/BtoA flags |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,135 @@ | ||
// Copyright (c) Six Labors. | ||
// Licensed under the Six Labors Split License. | ||
#nullable disable | ||
|
||
using System.Numerics; | ||
using SixLabors.ImageSharp.ColorProfiles.Icc.Calculators; | ||
using SixLabors.ImageSharp.Metadata.Profiles.Icc; | ||
|
||
namespace SixLabors.ImageSharp.ColorSpaces.Conversion.Icc; | ||
|
||
internal partial class LutABCalculator : IVector4Calculator | ||
{ | ||
private CalculationType type; | ||
private TrcCalculator curveACalculator; | ||
private TrcCalculator curveBCalculator; | ||
private TrcCalculator curveMCalculator; | ||
private MatrixCalculator matrixCalculator; | ||
private ClutCalculator clutCalculator; | ||
|
||
public LutABCalculator(IccLutAToBTagDataEntry entry) | ||
{ | ||
Guard.NotNull(entry, nameof(entry)); | ||
this.Init(entry.CurveA, entry.CurveB, entry.CurveM, entry.Matrix3x1, entry.Matrix3x3, entry.ClutValues); | ||
this.type |= CalculationType.AtoB; | ||
} | ||
|
||
public LutABCalculator(IccLutBToATagDataEntry entry) | ||
{ | ||
Guard.NotNull(entry, nameof(entry)); | ||
this.Init(entry.CurveA, entry.CurveB, entry.CurveM, entry.Matrix3x1, entry.Matrix3x3, entry.ClutValues); | ||
this.type |= CalculationType.BtoA; | ||
} | ||
|
||
public Vector4 Calculate(Vector4 value) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider adding XML documentation for the Calculate method to describe expected input ranges and behavior |
||
{ | ||
switch (this.type) | ||
{ | ||
case CalculationType.Full | CalculationType.AtoB: | ||
value = this.curveACalculator.Calculate(value); | ||
value = this.clutCalculator.Calculate(value); | ||
value = this.curveMCalculator.Calculate(value); | ||
value = this.matrixCalculator.Calculate(value); | ||
return this.curveBCalculator.Calculate(value); | ||
|
||
case CalculationType.Full | CalculationType.BtoA: | ||
value = this.curveBCalculator.Calculate(value); | ||
value = this.matrixCalculator.Calculate(value); | ||
value = this.curveMCalculator.Calculate(value); | ||
value = this.clutCalculator.Calculate(value); | ||
return this.curveACalculator.Calculate(value); | ||
|
||
case CalculationType.CurveClut | CalculationType.AtoB: | ||
value = this.curveACalculator.Calculate(value); | ||
value = this.clutCalculator.Calculate(value); | ||
return this.curveBCalculator.Calculate(value); | ||
|
||
case CalculationType.CurveClut | CalculationType.BtoA: | ||
value = this.curveBCalculator.Calculate(value); | ||
value = this.clutCalculator.Calculate(value); | ||
return this.curveACalculator.Calculate(value); | ||
|
||
case CalculationType.CurveMatrix | CalculationType.AtoB: | ||
value = this.curveMCalculator.Calculate(value); | ||
value = this.matrixCalculator.Calculate(value); | ||
return this.curveBCalculator.Calculate(value); | ||
|
||
case CalculationType.CurveMatrix | CalculationType.BtoA: | ||
value = this.curveBCalculator.Calculate(value); | ||
value = this.matrixCalculator.Calculate(value); | ||
return this.curveMCalculator.Calculate(value); | ||
|
||
case CalculationType.SingleCurve | CalculationType.AtoB: | ||
case CalculationType.SingleCurve | CalculationType.BtoA: | ||
return this.curveBCalculator.Calculate(value); | ||
|
||
default: | ||
throw new InvalidOperationException("Invalid calculation type"); | ||
} | ||
} | ||
|
||
private void Init(IccTagDataEntry[] curveA, IccTagDataEntry[] curveB, IccTagDataEntry[] curveM, Vector3? matrix3x1, Matrix4x4? matrix3x3, IccClut clut) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: The Init method could potentially be called multiple times. Consider making it throw if already initialized |
||
{ | ||
bool hasACurve = curveA != null; | ||
bool hasBCurve = curveB != null; | ||
bool hasMCurve = curveM != null; | ||
bool hasMatrix = matrix3x1 != null && matrix3x3 != null; | ||
bool hasClut = clut != null; | ||
|
||
if (hasBCurve && hasMatrix && hasMCurve && hasClut && hasACurve) | ||
{ | ||
this.type = CalculationType.Full; | ||
} | ||
Comment on lines
+89
to
+92
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: The order of checks may allow invalid combinations to pass. Consider validating that all required components are present for each type before setting |
||
else if (hasBCurve && hasClut && hasACurve) | ||
{ | ||
this.type = CalculationType.CurveClut; | ||
} | ||
else if (hasBCurve && hasMatrix && hasMCurve) | ||
{ | ||
this.type = CalculationType.CurveMatrix; | ||
} | ||
else if (hasBCurve) | ||
{ | ||
this.type = CalculationType.SingleCurve; | ||
} | ||
else | ||
{ | ||
throw new InvalidIccProfileException("AToB or BToA tag has an invalid configuration"); | ||
} | ||
|
||
if (hasACurve) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: Calculators are not checked for null before use in Calculate method, which could cause NullReferenceException |
||
{ | ||
this.curveACalculator = new TrcCalculator(curveA, false); | ||
} | ||
|
||
if (hasBCurve) | ||
{ | ||
this.curveBCalculator = new TrcCalculator(curveB, false); | ||
} | ||
|
||
if (hasMCurve) | ||
{ | ||
this.curveMCalculator = new TrcCalculator(curveM, false); | ||
} | ||
|
||
if (hasMatrix) | ||
{ | ||
this.matrixCalculator = new MatrixCalculator(matrix3x3.Value, matrix3x1.Value); | ||
} | ||
|
||
if (hasClut) | ||
{ | ||
this.clutCalculator = new ClutCalculator(clut); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Missing null checks for all constructor parameters. Could throw NullReferenceException if any matrix columns or TRC entries are null.