From ae9ca4c5bf56f1dec79c9fe96461fbfd5d5434b2 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 6 Mar 2026 22:30:35 +1000 Subject: [PATCH 1/2] Fix #3064 R --- .../LutABCalculator.CalculationType.cs | 17 ++- .../Icc/Calculators/LutABCalculator.cs | 141 ++++++++++-------- .../Icc/IccConverterbase.Conversions.cs | 2 +- .../DataReader/IccDataReader.TagDataEntry.cs | 110 +++++--------- .../Metadata/Profiles/ICC/IccProfile.cs | 6 +- .../TagDataEntries/IccLutAToBTagDataEntry.cs | 121 ++++++++------- .../TagDataEntries/IccLutBToATagDataEntry.cs | 111 ++++++++------ .../Formats/Jpg/JpegDecoderTests.cs | 15 ++ tests/ImageSharp.Tests/TestImages.cs | 1 + ...B_ICC_Jpeg_Issue3064_Rgba32_issue-3064.png | 3 + .../Input/Jpg/icc-profiles/issue-3064.jpg | 3 + 11 files changed, 288 insertions(+), 242 deletions(-) create mode 100644 tests/Images/External/ReferenceOutput/JpegDecoderTests/Decode_RGB_ICC_Jpeg_Issue3064_Rgba32_issue-3064.png create mode 100644 tests/Images/Input/Jpg/icc-profiles/issue-3064.jpg diff --git a/src/ImageSharp/ColorProfiles/Icc/Calculators/LutABCalculator.CalculationType.cs b/src/ImageSharp/ColorProfiles/Icc/Calculators/LutABCalculator.CalculationType.cs index 253239cb79..60a7e50b91 100644 --- a/src/ImageSharp/ColorProfiles/Icc/Calculators/LutABCalculator.CalculationType.cs +++ b/src/ImageSharp/ColorProfiles/Icc/Calculators/LutABCalculator.CalculationType.cs @@ -5,14 +5,19 @@ namespace SixLabors.ImageSharp.ColorProfiles.Conversion.Icc; internal partial class LutABCalculator { + /// + /// Identifies the transform direction for the configured LUT calculator. + /// private enum CalculationType { - AtoB = 1 << 3, - BtoA = 1 << 4, + /// + /// Converts from device space to PCS using ICC mAB stage order. + /// + AtoB, - SingleCurve = 1, - CurveMatrix = 2, - CurveClut = 3, - Full = 4, + /// + /// Converts from PCS to device space using ICC mBA stage order. + /// + BtoA, } } diff --git a/src/ImageSharp/ColorProfiles/Icc/Calculators/LutABCalculator.cs b/src/ImageSharp/ColorProfiles/Icc/Calculators/LutABCalculator.cs index 172d806394..83e51206a0 100644 --- a/src/ImageSharp/ColorProfiles/Icc/Calculators/LutABCalculator.cs +++ b/src/ImageSharp/ColorProfiles/Icc/Calculators/LutABCalculator.cs @@ -17,67 +17,106 @@ internal partial class LutABCalculator : IVector4Calculator private MatrixCalculator matrixCalculator; private ClutCalculator clutCalculator; + /// + /// Initializes a new instance of the class for an ICC mAB transform. + /// + /// The parsed A-to-B LUT entry. 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; + this.type = CalculationType.AtoB; } + /// + /// Initializes a new instance of the class for an ICC mBA transform. + /// + /// The parsed B-to-A LUT entry. 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; + this.type = CalculationType.BtoA; } + /// + /// Calculates the transformed value by applying the configured ICC LUT stages in specification order. + /// + /// The input value. + /// The transformed value. public Vector4 Calculate(Vector4 value) { 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); + case CalculationType.AtoB: + // ICC mAB order: A, CLUT, M, Matrix, B. + if (this.curveACalculator != null) + { + value = this.curveACalculator.Calculate(value); + } + + if (this.clutCalculator != null) + { + value = this.clutCalculator.Calculate(value); + } + + if (this.curveMCalculator != null) + { + value = this.curveMCalculator.Calculate(value); + } + + if (this.matrixCalculator != null) + { + value = this.matrixCalculator.Calculate(value); + } + + if (this.curveBCalculator != null) + { + value = this.curveBCalculator.Calculate(value); + } + + return value; + + case CalculationType.BtoA: + // ICC mBA order: B, Matrix, M, CLUT, A. + if (this.curveBCalculator != null) + { + value = this.curveBCalculator.Calculate(value); + } + + if (this.matrixCalculator != null) + { + value = this.matrixCalculator.Calculate(value); + } + + if (this.curveMCalculator != null) + { + value = this.curveMCalculator.Calculate(value); + } + + if (this.clutCalculator != null) + { + value = this.clutCalculator.Calculate(value); + } + + if (this.curveACalculator != null) + { + value = this.curveACalculator.Calculate(value); + } + + return value; default: throw new InvalidOperationException("Invalid calculation type"); } } + /// + /// Creates calculators for the processing stages present in the LUT entry. + /// + /// + /// The tag entry classes already validate channel continuity, so this method only materializes the available stages. + /// private void Init(IccTagDataEntry[] curveA, IccTagDataEntry[] curveB, IccTagDataEntry[] curveM, Vector3? matrix3x1, Matrix4x4? matrix3x3, IccClut clut) { bool hasACurve = curveA != null; @@ -86,26 +125,10 @@ private void Init(IccTagDataEntry[] curveA, IccTagDataEntry[] curveB, IccTagData bool hasMatrix = matrix3x1 != null && matrix3x3 != null; bool hasClut = clut != null; - if (hasBCurve && hasMatrix && hasMCurve && hasClut && hasACurve) - { - this.type = CalculationType.Full; - } - 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"); - } + Guard.IsTrue( + hasACurve || hasBCurve || hasMCurve || hasMatrix || hasClut, + nameof(curveB), + "AToB or BToA tag must contain at least one processing element"); if (hasACurve) { diff --git a/src/ImageSharp/ColorProfiles/Icc/IccConverterbase.Conversions.cs b/src/ImageSharp/ColorProfiles/Icc/IccConverterbase.Conversions.cs index 20df08e378..5875b74f13 100644 --- a/src/ImageSharp/ColorProfiles/Icc/IccConverterbase.Conversions.cs +++ b/src/ImageSharp/ColorProfiles/Icc/IccConverterbase.Conversions.cs @@ -60,7 +60,7 @@ private static IVector4Calculator InitA(IccProfile profile, IccProfileTag tag) IccLut16TagDataEntry lut16 => new LutEntryCalculator(lut16), IccLutAToBTagDataEntry lutAtoB => new LutABCalculator(lutAtoB), IccLutBToATagDataEntry lutBtoA => new LutABCalculator(lutBtoA), - _ => throw new InvalidIccProfileException("Invalid entry."), + _ => throw new InvalidIccProfileException($"Invalid entry {tag}."), }; private static IVector4Calculator InitD(IccProfile profile, IccProfileTag tag) diff --git a/src/ImageSharp/Metadata/Profiles/ICC/DataReader/IccDataReader.TagDataEntry.cs b/src/ImageSharp/Metadata/Profiles/ICC/DataReader/IccDataReader.TagDataEntry.cs index 9e89d24ff4..2f1b15b6b7 100644 --- a/src/ImageSharp/Metadata/Profiles/ICC/DataReader/IccDataReader.TagDataEntry.cs +++ b/src/ImageSharp/Metadata/Profiles/ICC/DataReader/IccDataReader.TagDataEntry.cs @@ -20,82 +20,46 @@ internal sealed partial class IccDataReader public IccTagDataEntry ReadTagDataEntry(IccTagTableEntry info) { this.currentIndex = (int)info.Offset; - switch (this.ReadTagDataEntryHeader()) - { - case IccTypeSignature.Chromaticity: - return this.ReadChromaticityTagDataEntry(); - case IccTypeSignature.ColorantOrder: - return this.ReadColorantOrderTagDataEntry(); - case IccTypeSignature.ColorantTable: - return this.ReadColorantTableTagDataEntry(); - case IccTypeSignature.Curve: - return this.ReadCurveTagDataEntry(); - case IccTypeSignature.Data: - return this.ReadDataTagDataEntry(info.DataSize); - case IccTypeSignature.DateTime: - return this.ReadDateTimeTagDataEntry(); - case IccTypeSignature.Lut16: - return this.ReadLut16TagDataEntry(); - case IccTypeSignature.Lut8: - return this.ReadLut8TagDataEntry(); - case IccTypeSignature.LutAToB: - return this.ReadLutAtoBTagDataEntry(); - case IccTypeSignature.LutBToA: - return this.ReadLutBtoATagDataEntry(); - case IccTypeSignature.Measurement: - return this.ReadMeasurementTagDataEntry(); - case IccTypeSignature.MultiLocalizedUnicode: - return this.ReadMultiLocalizedUnicodeTagDataEntry(); - case IccTypeSignature.MultiProcessElements: - return this.ReadMultiProcessElementsTagDataEntry(); - case IccTypeSignature.NamedColor2: - return this.ReadNamedColor2TagDataEntry(); - case IccTypeSignature.ParametricCurve: - return this.ReadParametricCurveTagDataEntry(); - case IccTypeSignature.ProfileSequenceDesc: - return this.ReadProfileSequenceDescTagDataEntry(); - case IccTypeSignature.ProfileSequenceIdentifier: - return this.ReadProfileSequenceIdentifierTagDataEntry(); - case IccTypeSignature.ResponseCurveSet16: - return this.ReadResponseCurveSet16TagDataEntry(); - case IccTypeSignature.S15Fixed16Array: - return this.ReadFix16ArrayTagDataEntry(info.DataSize); - case IccTypeSignature.Signature: - return this.ReadSignatureTagDataEntry(); - case IccTypeSignature.Text: - return this.ReadTextTagDataEntry(info.DataSize); - case IccTypeSignature.U16Fixed16Array: - return this.ReadUFix16ArrayTagDataEntry(info.DataSize); - case IccTypeSignature.UInt16Array: - return this.ReadUInt16ArrayTagDataEntry(info.DataSize); - case IccTypeSignature.UInt32Array: - return this.ReadUInt32ArrayTagDataEntry(info.DataSize); - case IccTypeSignature.UInt64Array: - return this.ReadUInt64ArrayTagDataEntry(info.DataSize); - case IccTypeSignature.UInt8Array: - return this.ReadUInt8ArrayTagDataEntry(info.DataSize); - case IccTypeSignature.ViewingConditions: - return this.ReadViewingConditionsTagDataEntry(); - case IccTypeSignature.Xyz: - return this.ReadXyzTagDataEntry(info.DataSize); + return this.ReadTagDataEntryHeader() switch + { + IccTypeSignature.Chromaticity => this.ReadChromaticityTagDataEntry(), + IccTypeSignature.ColorantOrder => this.ReadColorantOrderTagDataEntry(), + IccTypeSignature.ColorantTable => this.ReadColorantTableTagDataEntry(), + IccTypeSignature.Curve => this.ReadCurveTagDataEntry(), + IccTypeSignature.Data => this.ReadDataTagDataEntry(info.DataSize), + IccTypeSignature.DateTime => this.ReadDateTimeTagDataEntry(), + IccTypeSignature.Lut16 => this.ReadLut16TagDataEntry(), + IccTypeSignature.Lut8 => this.ReadLut8TagDataEntry(), + IccTypeSignature.LutAToB => this.ReadLutAtoBTagDataEntry(), + IccTypeSignature.LutBToA => this.ReadLutBtoATagDataEntry(), + IccTypeSignature.Measurement => this.ReadMeasurementTagDataEntry(), + IccTypeSignature.MultiLocalizedUnicode => this.ReadMultiLocalizedUnicodeTagDataEntry(), + IccTypeSignature.MultiProcessElements => this.ReadMultiProcessElementsTagDataEntry(), + IccTypeSignature.NamedColor2 => this.ReadNamedColor2TagDataEntry(), + IccTypeSignature.ParametricCurve => this.ReadParametricCurveTagDataEntry(), + IccTypeSignature.ProfileSequenceDesc => this.ReadProfileSequenceDescTagDataEntry(), + IccTypeSignature.ProfileSequenceIdentifier => this.ReadProfileSequenceIdentifierTagDataEntry(), + IccTypeSignature.ResponseCurveSet16 => this.ReadResponseCurveSet16TagDataEntry(), + IccTypeSignature.S15Fixed16Array => this.ReadFix16ArrayTagDataEntry(info.DataSize), + IccTypeSignature.Signature => this.ReadSignatureTagDataEntry(), + IccTypeSignature.Text => this.ReadTextTagDataEntry(info.DataSize), + IccTypeSignature.U16Fixed16Array => this.ReadUFix16ArrayTagDataEntry(info.DataSize), + IccTypeSignature.UInt16Array => this.ReadUInt16ArrayTagDataEntry(info.DataSize), + IccTypeSignature.UInt32Array => this.ReadUInt32ArrayTagDataEntry(info.DataSize), + IccTypeSignature.UInt64Array => this.ReadUInt64ArrayTagDataEntry(info.DataSize), + IccTypeSignature.UInt8Array => this.ReadUInt8ArrayTagDataEntry(info.DataSize), + IccTypeSignature.ViewingConditions => this.ReadViewingConditionsTagDataEntry(), + IccTypeSignature.Xyz => this.ReadXyzTagDataEntry(info.DataSize), // V2 Types: - case IccTypeSignature.TextDescription: - return this.ReadTextDescriptionTagDataEntry(); - case IccTypeSignature.CrdInfo: - return this.ReadCrdInfoTagDataEntry(); - case IccTypeSignature.Screening: - return this.ReadScreeningTagDataEntry(); - case IccTypeSignature.UcrBg: - return this.ReadUcrBgTagDataEntry(info.DataSize); + IccTypeSignature.TextDescription => this.ReadTextDescriptionTagDataEntry(), + IccTypeSignature.CrdInfo => this.ReadCrdInfoTagDataEntry(), + IccTypeSignature.Screening => this.ReadScreeningTagDataEntry(), + IccTypeSignature.UcrBg => this.ReadUcrBgTagDataEntry(info.DataSize), // Unsupported or unknown - case IccTypeSignature.DeviceSettings: - case IccTypeSignature.NamedColor: - case IccTypeSignature.Unknown: - default: - return this.ReadUnknownTagDataEntry(info.DataSize); - } + _ => this.ReadUnknownTagDataEntry(info.DataSize), + }; } /// @@ -477,7 +441,7 @@ public IccMultiLocalizedUnicodeTagDataEntry ReadMultiLocalizedUnicodeTagDataEntr return new IccMultiLocalizedUnicodeTagDataEntry(text); - CultureInfo ReadCulture(string language, string country) + static CultureInfo ReadCulture(string language, string country) { if (string.IsNullOrWhiteSpace(language)) { diff --git a/src/ImageSharp/Metadata/Profiles/ICC/IccProfile.cs b/src/ImageSharp/Metadata/Profiles/ICC/IccProfile.cs index 05be3eb5dd..eaba0a045c 100644 --- a/src/ImageSharp/Metadata/Profiles/ICC/IccProfile.cs +++ b/src/ImageSharp/Metadata/Profiles/ICC/IccProfile.cs @@ -110,8 +110,8 @@ public static IccProfileId CalculateHash(byte[] data) // need to copy some values because they need to be zero for the hashing Span temp = stackalloc byte[24]; data.AsSpan(profileFlagPos, 4).CopyTo(temp); - data.AsSpan(renderingIntentPos, 4).CopyTo(temp.Slice(4)); - data.AsSpan(profileIdPos, 16).CopyTo(temp.Slice(8)); + data.AsSpan(renderingIntentPos, 4).CopyTo(temp[4..]); + data.AsSpan(profileIdPos, 16).CopyTo(temp[8..]); try { @@ -131,7 +131,7 @@ public static IccProfileId CalculateHash(byte[] data) } finally { - temp.Slice(0, 4).CopyTo(data.AsSpan(profileFlagPos)); + temp[..4].CopyTo(data.AsSpan(profileFlagPos)); temp.Slice(4, 4).CopyTo(data.AsSpan(renderingIntentPos)); temp.Slice(8, 16).CopyTo(data.AsSpan(profileIdPos)); } diff --git a/src/ImageSharp/Metadata/Profiles/ICC/TagDataEntries/IccLutAToBTagDataEntry.cs b/src/ImageSharp/Metadata/Profiles/ICC/TagDataEntries/IccLutAToBTagDataEntry.cs index 9bf3232633..2219c4ca06 100644 --- a/src/ImageSharp/Metadata/Profiles/ICC/TagDataEntries/IccLutAToBTagDataEntry.cs +++ b/src/ImageSharp/Metadata/Profiles/ICC/TagDataEntries/IccLutAToBTagDataEntry.cs @@ -64,44 +64,7 @@ public IccLutAToBTagDataEntry( this.CurveM = curveM; this.ClutValues = clutValues; - if (this.IsAClutMMatrixB()) - { - Guard.IsTrue(this.CurveB.Length == 3, nameof(this.CurveB), $"{nameof(this.CurveB)} must have a length of three"); - Guard.IsTrue(this.CurveM.Length == 3, nameof(this.CurveM), $"{nameof(this.CurveM)} must have a length of three"); - Guard.MustBeBetweenOrEqualTo(this.CurveA.Length, 1, 15, nameof(this.CurveA)); - - this.InputChannelCount = curveA.Length; - this.OutputChannelCount = 3; - - Guard.IsTrue(this.InputChannelCount == clutValues.InputChannelCount, nameof(clutValues), "Input channel count does not match the CLUT size"); - Guard.IsTrue(this.OutputChannelCount == clutValues.OutputChannelCount, nameof(clutValues), "Output channel count does not match the CLUT size"); - } - else if (this.IsMMatrixB()) - { - Guard.IsTrue(this.CurveB.Length == 3, nameof(this.CurveB), $"{nameof(this.CurveB)} must have a length of three"); - Guard.IsTrue(this.CurveM.Length == 3, nameof(this.CurveM), $"{nameof(this.CurveM)} must have a length of three"); - - this.InputChannelCount = this.OutputChannelCount = 3; - } - else if (this.IsAClutB()) - { - Guard.MustBeBetweenOrEqualTo(this.CurveA.Length, 1, 15, nameof(this.CurveA)); - Guard.MustBeBetweenOrEqualTo(this.CurveB.Length, 1, 15, nameof(this.CurveB)); - - this.InputChannelCount = curveA.Length; - this.OutputChannelCount = curveB.Length; - - Guard.IsTrue(this.InputChannelCount == clutValues.InputChannelCount, nameof(clutValues), "Input channel count does not match the CLUT size"); - Guard.IsTrue(this.OutputChannelCount == clutValues.OutputChannelCount, nameof(clutValues), "Output channel count does not match the CLUT size"); - } - else if (this.IsB()) - { - this.InputChannelCount = this.OutputChannelCount = this.CurveB.Length; - } - else - { - throw new ArgumentException("Invalid combination of values given"); - } + (this.InputChannelCount, this.OutputChannelCount) = this.GetChannelCounts(); } /// @@ -192,6 +155,9 @@ public override int GetHashCode() return hashCode.ToHashCode(); } + /// + /// Compares two curve arrays, treating consistently. + /// private static bool EqualsCurve(IccTagDataEntry[] thisCurves, IccTagDataEntry[] entryCurves) { bool thisNull = thisCurves is null; @@ -210,27 +176,63 @@ private static bool EqualsCurve(IccTagDataEntry[] thisCurves, IccTagDataEntry[] return thisCurves.SequenceEqual(entryCurves); } - private bool IsAClutMMatrixB() - => this.CurveB != null - && this.Matrix3x3 != null - && this.Matrix3x1 != null - && this.CurveM != null - && this.ClutValues != null - && this.CurveA != null; + /// + /// Validates the configured processing stages and derives the external channel counts. + /// + /// + /// Stages are evaluated in ICC mAB order: A, CLUT, M, Matrix, B. + /// Sparse pipelines are valid as long as adjacent stages agree on channel counts. + /// + private (int InputChannelCount, int OutputChannelCount) GetChannelCounts() + { + // There are at most five possible mAB stages: A, CLUT, M, Matrix, and B. + List<(int Input, int Output, string Name)> stages = new(5); - private bool IsMMatrixB() - => this.CurveB != null - && this.Matrix3x3 != null - && this.Matrix3x1 != null - && this.CurveM != null; + if (this.CurveA != null) + { + Guard.MustBeBetweenOrEqualTo(this.CurveA.Length, 1, 15, nameof(this.CurveA)); + stages.Add((this.CurveA.Length, this.CurveA.Length, nameof(this.CurveA))); + } - private bool IsAClutB() - => this.CurveB != null - && this.ClutValues != null - && this.CurveA != null; + if (this.ClutValues != null) + { + stages.Add((this.ClutValues.InputChannelCount, this.ClutValues.OutputChannelCount, nameof(this.ClutValues))); + } - private bool IsB() => this.CurveB != null; + if (this.CurveM != null) + { + Guard.MustBeBetweenOrEqualTo(this.CurveM.Length, 1, 15, nameof(this.CurveM)); + stages.Add((this.CurveM.Length, this.CurveM.Length, nameof(this.CurveM))); + } + + if (this.Matrix3x3 != null || this.Matrix3x1 != null) + { + Guard.IsTrue(this.Matrix3x3 != null && this.Matrix3x1 != null, nameof(this.Matrix3x3), "Matrix must include both the 3x3 and 3x1 components"); + stages.Add((3, 3, nameof(this.Matrix3x3))); + } + if (this.CurveB != null) + { + Guard.MustBeBetweenOrEqualTo(this.CurveB.Length, 1, 15, nameof(this.CurveB)); + stages.Add((this.CurveB.Length, this.CurveB.Length, nameof(this.CurveB))); + } + + Guard.IsTrue(stages.Count > 0, nameof(this.CurveB), "AToB tag must contain at least one processing element"); + + for (int i = 1; i < stages.Count; i++) + { + Guard.IsTrue( + stages[i - 1].Output == stages[i].Input, + stages[i].Name, + $"Output channel count of {stages[i - 1].Name} does not match input channel count of {stages[i].Name}"); + } + + return (stages[0].Input, stages[^1].Output); + } + + /// + /// Verifies that every supplied curve entry is a supported one-dimensional curve type. + /// private void VerifyCurve(IccTagDataEntry[] curves, string name) { if (curves != null) @@ -240,6 +242,9 @@ private void VerifyCurve(IccTagDataEntry[] curves, string name) } } + /// + /// Verifies the dimensions of the optional matrix components. + /// private static void VerifyMatrix(float[,] matrix3x3, float[] matrix3x1) { if (matrix3x1 != null) @@ -254,6 +259,9 @@ private static void VerifyMatrix(float[,] matrix3x3, float[] matrix3x1) } } + /// + /// Creates the one-dimensional matrix vector when present. + /// private static Vector3? CreateMatrix3x1(float[] matrix) { if (matrix is null) @@ -264,6 +272,9 @@ private static void VerifyMatrix(float[,] matrix3x3, float[] matrix3x1) return new Vector3(matrix[0], matrix[1], matrix[2]); } + /// + /// Creates the three-by-three matrix when present. + /// private static Matrix4x4? CreateMatrix3x3(float[,] matrix) { if (matrix is null) diff --git a/src/ImageSharp/Metadata/Profiles/ICC/TagDataEntries/IccLutBToATagDataEntry.cs b/src/ImageSharp/Metadata/Profiles/ICC/TagDataEntries/IccLutBToATagDataEntry.cs index 033b809894..2df1df1378 100644 --- a/src/ImageSharp/Metadata/Profiles/ICC/TagDataEntries/IccLutBToATagDataEntry.cs +++ b/src/ImageSharp/Metadata/Profiles/ICC/TagDataEntries/IccLutBToATagDataEntry.cs @@ -64,44 +64,7 @@ public IccLutBToATagDataEntry( this.CurveM = curveM; this.ClutValues = clutValues; - if (this.IsBMatrixMClutA()) - { - Guard.IsTrue(this.CurveB.Length == 3, nameof(this.CurveB), $"{nameof(this.CurveB)} must have a length of three"); - Guard.IsTrue(this.CurveM.Length == 3, nameof(this.CurveM), $"{nameof(this.CurveM)} must have a length of three"); - Guard.MustBeBetweenOrEqualTo(this.CurveA.Length, 1, 15, nameof(this.CurveA)); - - this.InputChannelCount = 3; - this.OutputChannelCount = curveA.Length; - - Guard.IsTrue(this.InputChannelCount == clutValues.InputChannelCount, nameof(clutValues), "Input channel count does not match the CLUT size"); - Guard.IsTrue(this.OutputChannelCount == clutValues.OutputChannelCount, nameof(clutValues), "Output channel count does not match the CLUT size"); - } - else if (this.IsBMatrixM()) - { - Guard.IsTrue(this.CurveB.Length == 3, nameof(this.CurveB), $"{nameof(this.CurveB)} must have a length of three"); - Guard.IsTrue(this.CurveM.Length == 3, nameof(this.CurveM), $"{nameof(this.CurveM)} must have a length of three"); - - this.InputChannelCount = this.OutputChannelCount = 3; - } - else if (this.IsBClutA()) - { - Guard.MustBeBetweenOrEqualTo(this.CurveA.Length, 1, 15, nameof(this.CurveA)); - Guard.MustBeBetweenOrEqualTo(this.CurveB.Length, 1, 15, nameof(this.CurveB)); - - this.InputChannelCount = curveB.Length; - this.OutputChannelCount = curveA.Length; - - Guard.IsTrue(this.InputChannelCount == clutValues.InputChannelCount, nameof(clutValues), "Input channel count does not match the CLUT size"); - Guard.IsTrue(this.OutputChannelCount == clutValues.OutputChannelCount, nameof(clutValues), "Output channel count does not match the CLUT size"); - } - else if (this.IsB()) - { - this.InputChannelCount = this.OutputChannelCount = this.CurveB.Length; - } - else - { - throw new ArgumentException("Invalid combination of values given"); - } + (this.InputChannelCount, this.OutputChannelCount) = this.GetChannelCounts(); } /// @@ -191,6 +154,9 @@ public override int GetHashCode() return hashCode.ToHashCode(); } + /// + /// Compares two curve arrays, treating consistently. + /// private static bool EqualsCurve(IccTagDataEntry[] thisCurves, IccTagDataEntry[] entryCurves) { bool thisNull = thisCurves is null; @@ -209,17 +175,63 @@ private static bool EqualsCurve(IccTagDataEntry[] thisCurves, IccTagDataEntry[] return thisCurves.SequenceEqual(entryCurves); } - private bool IsBMatrixMClutA() - => this.CurveB != null && this.Matrix3x3 != null && this.Matrix3x1 != null && this.CurveM != null && this.ClutValues != null && this.CurveA != null; + /// + /// Validates the configured processing stages and derives the external channel counts. + /// + /// + /// Stages are evaluated in ICC mBA order: B, Matrix, M, CLUT, A. + /// Sparse pipelines are valid as long as adjacent stages agree on channel counts. + /// + private (int InputChannelCount, int OutputChannelCount) GetChannelCounts() + { + // There are at most five possible mBA stages: B, Matrix, M, CLUT, and A. + List<(int Input, int Output, string Name)> stages = new(5); - private bool IsBMatrixM() - => this.CurveB != null && this.Matrix3x3 != null && this.Matrix3x1 != null && this.CurveM != null; + if (this.CurveB != null) + { + Guard.MustBeBetweenOrEqualTo(this.CurveB.Length, 1, 15, nameof(this.CurveB)); + stages.Add((this.CurveB.Length, this.CurveB.Length, nameof(this.CurveB))); + } - private bool IsBClutA() - => this.CurveB != null && this.ClutValues != null && this.CurveA != null; + if (this.Matrix3x3 != null || this.Matrix3x1 != null) + { + Guard.IsTrue(this.Matrix3x3 != null && this.Matrix3x1 != null, nameof(this.Matrix3x3), "Matrix must include both the 3x3 and 3x1 components"); + stages.Add((3, 3, nameof(this.Matrix3x3))); + } - private bool IsB() => this.CurveB != null; + if (this.CurveM != null) + { + Guard.MustBeBetweenOrEqualTo(this.CurveM.Length, 1, 15, nameof(this.CurveM)); + stages.Add((this.CurveM.Length, this.CurveM.Length, nameof(this.CurveM))); + } + + if (this.ClutValues != null) + { + stages.Add((this.ClutValues.InputChannelCount, this.ClutValues.OutputChannelCount, nameof(this.ClutValues))); + } + if (this.CurveA != null) + { + Guard.MustBeBetweenOrEqualTo(this.CurveA.Length, 1, 15, nameof(this.CurveA)); + stages.Add((this.CurveA.Length, this.CurveA.Length, nameof(this.CurveA))); + } + + Guard.IsTrue(stages.Count > 0, nameof(this.CurveB), "BToA tag must contain at least one processing element"); + + for (int i = 1; i < stages.Count; i++) + { + Guard.IsTrue( + stages[i - 1].Output == stages[i].Input, + stages[i].Name, + $"Output channel count of {stages[i - 1].Name} does not match input channel count of {stages[i].Name}"); + } + + return (stages[0].Input, stages[^1].Output); + } + + /// + /// Verifies that every supplied curve entry is a supported one-dimensional curve type. + /// private void VerifyCurve(IccTagDataEntry[] curves, string name) { if (curves != null) @@ -229,6 +241,9 @@ private void VerifyCurve(IccTagDataEntry[] curves, string name) } } + /// + /// Verifies the dimensions of the optional matrix components. + /// private static void VerifyMatrix(float[,] matrix3x3, float[] matrix3x1) { if (matrix3x1 != null) @@ -243,6 +258,9 @@ private static void VerifyMatrix(float[,] matrix3x3, float[] matrix3x1) } } + /// + /// Creates the one-dimensional matrix vector when present. + /// private static Vector3? CreateMatrix3x1(float[] matrix) { if (matrix is null) @@ -253,6 +271,9 @@ private static void VerifyMatrix(float[,] matrix3x3, float[] matrix3x1) return new Vector3(matrix[0], matrix[1], matrix[2]); } + /// + /// Creates the three-by-three matrix when present. + /// private static Matrix4x4? CreateMatrix3x3(float[,] matrix) { if (matrix is null) diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index 3fd55eb915..d047ed2357 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -418,6 +418,21 @@ public void Decode_RGB_ICC_Jpeg(TestImageProvider provider) image.CompareToReferenceOutput(provider); } + [Theory] + [WithFile(TestImages.Jpeg.ICC.Issue3064, PixelTypes.Rgba32)] + public void Decode_RGB_ICC_Jpeg_Issue3064(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + JpegDecoderOptions options = new() + { + GeneralOptions = new DecoderOptions { ColorProfileHandling = ColorProfileHandling.Convert } + }; + + using Image image = provider.GetImage(JpegDecoder.Instance, options); + image.DebugSave(provider); + image.CompareToReferenceOutput(provider); + } + // https://github.com/SixLabors/ImageSharp/issues/2948 [Theory] [WithFile(TestImages.Jpeg.Issues.Issue2948, PixelTypes.Rgb24)] diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 764954cae0..fab1b2891c 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -232,6 +232,7 @@ public static class ICC public const string SRgbGray = "Jpg/icc-profiles/sRGB_Gray.jpg"; public const string Perceptual = "Jpg/icc-profiles/Perceptual.jpg"; public const string PerceptualcLUTOnly = "Jpg/icc-profiles/Perceptual-cLUT-only.jpg"; + public const string Issue3064 = "Jpg/icc-profiles/issue-3064.jpg"; } public static class Progressive diff --git a/tests/Images/External/ReferenceOutput/JpegDecoderTests/Decode_RGB_ICC_Jpeg_Issue3064_Rgba32_issue-3064.png b/tests/Images/External/ReferenceOutput/JpegDecoderTests/Decode_RGB_ICC_Jpeg_Issue3064_Rgba32_issue-3064.png new file mode 100644 index 0000000000..12692cb9ea --- /dev/null +++ b/tests/Images/External/ReferenceOutput/JpegDecoderTests/Decode_RGB_ICC_Jpeg_Issue3064_Rgba32_issue-3064.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:73497e1eaddaa86cc915fe59a177e9ab6423d725ab4e6af21c8414c9edc6ceaf +size 347 diff --git a/tests/Images/Input/Jpg/icc-profiles/issue-3064.jpg b/tests/Images/Input/Jpg/icc-profiles/issue-3064.jpg new file mode 100644 index 0000000000..477372c033 --- /dev/null +++ b/tests/Images/Input/Jpg/icc-profiles/issue-3064.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:e02fff450519423fd5746e610d65bd7296553252567e93de9c051250139e8adc +size 27537 From 4fc4d660fadccb4737528319e625a82f0cc23ea4 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 6 Mar 2026 23:02:20 +1000 Subject: [PATCH 2/2] Respond to feedback --- .../ColorProfiles/Icc/Calculators/LutABCalculator.cs | 2 +- .../Profiles/ICC/TagDataEntries/IccLutAToBTagDataEntry.cs | 4 ++-- .../Profiles/ICC/TagDataEntries/IccLutBToATagDataEntry.cs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ImageSharp/ColorProfiles/Icc/Calculators/LutABCalculator.cs b/src/ImageSharp/ColorProfiles/Icc/Calculators/LutABCalculator.cs index 83e51206a0..10ac6e596f 100644 --- a/src/ImageSharp/ColorProfiles/Icc/Calculators/LutABCalculator.cs +++ b/src/ImageSharp/ColorProfiles/Icc/Calculators/LutABCalculator.cs @@ -127,7 +127,7 @@ private void Init(IccTagDataEntry[] curveA, IccTagDataEntry[] curveB, IccTagData Guard.IsTrue( hasACurve || hasBCurve || hasMCurve || hasMatrix || hasClut, - nameof(curveB), + "entry", "AToB or BToA tag must contain at least one processing element"); if (hasACurve) diff --git a/src/ImageSharp/Metadata/Profiles/ICC/TagDataEntries/IccLutAToBTagDataEntry.cs b/src/ImageSharp/Metadata/Profiles/ICC/TagDataEntries/IccLutAToBTagDataEntry.cs index 2219c4ca06..77bc45bd4f 100644 --- a/src/ImageSharp/Metadata/Profiles/ICC/TagDataEntries/IccLutAToBTagDataEntry.cs +++ b/src/ImageSharp/Metadata/Profiles/ICC/TagDataEntries/IccLutAToBTagDataEntry.cs @@ -128,7 +128,7 @@ public bool Equals(IccLutAToBTagDataEntry other) && this.OutputChannelCount == other.OutputChannelCount && this.Matrix3x3.Equals(other.Matrix3x3) && this.Matrix3x1.Equals(other.Matrix3x1) - && this.ClutValues.Equals(other.ClutValues) + && Equals(this.ClutValues, other.ClutValues) && EqualsCurve(this.CurveB, other.CurveB) && EqualsCurve(this.CurveM, other.CurveM) && EqualsCurve(this.CurveA, other.CurveA); @@ -168,7 +168,7 @@ private static bool EqualsCurve(IccTagDataEntry[] thisCurves, IccTagDataEntry[] return true; } - if (entryNull) + if (thisNull || entryNull) { return false; } diff --git a/src/ImageSharp/Metadata/Profiles/ICC/TagDataEntries/IccLutBToATagDataEntry.cs b/src/ImageSharp/Metadata/Profiles/ICC/TagDataEntries/IccLutBToATagDataEntry.cs index 2df1df1378..37e7b408d2 100644 --- a/src/ImageSharp/Metadata/Profiles/ICC/TagDataEntries/IccLutBToATagDataEntry.cs +++ b/src/ImageSharp/Metadata/Profiles/ICC/TagDataEntries/IccLutBToATagDataEntry.cs @@ -128,7 +128,7 @@ public bool Equals(IccLutBToATagDataEntry other) && this.OutputChannelCount == other.OutputChannelCount && this.Matrix3x3.Equals(other.Matrix3x3) && this.Matrix3x1.Equals(other.Matrix3x1) - && this.ClutValues.Equals(other.ClutValues) + && Equals(this.ClutValues, other.ClutValues) && EqualsCurve(this.CurveB, other.CurveB) && EqualsCurve(this.CurveM, other.CurveM) && EqualsCurve(this.CurveA, other.CurveA); @@ -167,7 +167,7 @@ private static bool EqualsCurve(IccTagDataEntry[] thisCurves, IccTagDataEntry[] return true; } - if (entryNull) + if (thisNull || entryNull) { return false; }