From 54856ff945233315f486c61d17d7b9e83e5f4a22 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 4 Dec 2022 18:24:58 +1000 Subject: [PATCH] Fix reader and out of range exception --- .../Icc/Calculators/LutABCalculator.cs | 1 - .../Icc/Calculators/LutCalculator.cs | 21 +++++++++++-------- .../Icc/IccConverterbase.Conversions.cs | 8 +++---- .../Metadata/Profiles/ICC/IccProfile.cs | 1 - .../Metadata/Profiles/ICC/IccReader.cs | 21 ++++++------------- .../Profiles/ICC/Various/IccTagTableEntry.cs | 10 +++------ .../Icc/IccProfileConverterTests.cs | 10 ++++----- 7 files changed, 29 insertions(+), 43 deletions(-) diff --git a/src/ImageSharp/ColorSpaces/Conversion/Implementation/Icc/Calculators/LutABCalculator.cs b/src/ImageSharp/ColorSpaces/Conversion/Implementation/Icc/Calculators/LutABCalculator.cs index 5ea734d713..817566c850 100644 --- a/src/ImageSharp/ColorSpaces/Conversion/Implementation/Icc/Calculators/LutABCalculator.cs +++ b/src/ImageSharp/ColorSpaces/Conversion/Implementation/Icc/Calculators/LutABCalculator.cs @@ -1,7 +1,6 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -using System; using System.Numerics; using SixLabors.ImageSharp.Metadata.Profiles.Icc; diff --git a/src/ImageSharp/ColorSpaces/Conversion/Implementation/Icc/Calculators/LutCalculator.cs b/src/ImageSharp/ColorSpaces/Conversion/Implementation/Icc/Calculators/LutCalculator.cs index 18fd622e6c..f15cc16588 100644 --- a/src/ImageSharp/ColorSpaces/Conversion/Implementation/Icc/Calculators/LutCalculator.cs +++ b/src/ImageSharp/ColorSpaces/Conversion/Implementation/Icc/Calculators/LutCalculator.cs @@ -1,15 +1,14 @@ -// Copyright (c) Six Labors. +// Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -using System; using System.Runtime.CompilerServices; namespace SixLabors.ImageSharp.ColorSpaces.Conversion.Icc; internal class LutCalculator : ISingleCalculator { - private float[] lut; - private bool inverse; + private readonly float[] lut; + private readonly bool inverse; public LutCalculator(float[] lut, bool inverse) { @@ -25,10 +24,8 @@ internal class LutCalculator : ISingleCalculator { return this.LookupInverse(value); } - else - { - return this.Lookup(value); - } + + return this.Lookup(value); } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -37,7 +34,13 @@ internal class LutCalculator : ISingleCalculator float factor = value * (this.lut.Length - 1); int index = (int)factor; float low = this.lut[index]; - float high = this.lut[index + 1]; + + float high = 1F; + if (index < this.lut.Length - 1) + { + high = this.lut[index + 1]; + } + return low + ((high - low) * (factor - index)); } diff --git a/src/ImageSharp/ColorSpaces/Conversion/Implementation/Icc/IccConverterbase.Conversions.cs b/src/ImageSharp/ColorSpaces/Conversion/Implementation/Icc/IccConverterbase.Conversions.cs index 56941c19e3..f40361696c 100644 --- a/src/ImageSharp/ColorSpaces/Conversion/Implementation/Icc/IccConverterbase.Conversions.cs +++ b/src/ImageSharp/ColorSpaces/Conversion/Implementation/Icc/IccConverterbase.Conversions.cs @@ -22,8 +22,7 @@ internal abstract partial class IccConverterBase /// The wanted rendering intent. Can be ignored if not available protected void Init(IccProfile profile, bool toPcs, IccRenderingIntent renderingIntent) { - ConversionMethod method = GetConversionMethod(profile, renderingIntent); - switch (method) + switch (GetConversionMethod(profile, renderingIntent)) { case ConversionMethod.D0: this.calculator = toPcs ? @@ -83,8 +82,7 @@ internal abstract partial class IccConverterBase private static IVector4Calculator InitA(IccProfile profile, IccProfileTag tag) { - IccTagDataEntry entry = GetTag(profile, tag); - switch (entry) + switch (GetTag(profile, tag)) { case IccLut8TagDataEntry lut8: return new LutEntryCalculator(lut8); @@ -96,6 +94,8 @@ internal abstract partial class IccConverterBase return new LutABCalculator(lutBtoA); default: + + // TODO: This is where we likely return a matrix calculator. throw new InvalidIccProfileException("Invalid entry."); } } diff --git a/src/ImageSharp/Metadata/Profiles/ICC/IccProfile.cs b/src/ImageSharp/Metadata/Profiles/ICC/IccProfile.cs index 00c545c88a..aa0ebe4880 100644 --- a/src/ImageSharp/Metadata/Profiles/ICC/IccProfile.cs +++ b/src/ImageSharp/Metadata/Profiles/ICC/IccProfile.cs @@ -206,7 +206,6 @@ public sealed class IccProfile : IDeepCloneable return; } - IccReader reader = new(); this.entries = IccReader.ReadTagData(this.data); } } diff --git a/src/ImageSharp/Metadata/Profiles/ICC/IccReader.cs b/src/ImageSharp/Metadata/Profiles/ICC/IccReader.cs index 0fe01fbdb2..074712d302 100644 --- a/src/ImageSharp/Metadata/Profiles/ICC/IccReader.cs +++ b/src/ImageSharp/Metadata/Profiles/ICC/IccReader.cs @@ -83,28 +83,19 @@ internal sealed class IccReader { IccTagTableEntry[] tagTable = ReadTagTable(reader); List entries = new(tagTable.Length); - Dictionary store = new(); foreach (IccTagTableEntry tag in tagTable) { IccTagDataEntry entry; - if (store.ContainsKey(tag.Offset)) + + try { - entry = store[tag.Offset]; + entry = reader.ReadTagDataEntry(tag); } - else + catch { - try - { - entry = reader.ReadTagDataEntry(tag); - } - catch - { - // Ignore tags that could not be read - continue; - } - - store.Add(tag.Offset, entry); + // Ignore tags that could not be read + continue; } entry.TagSignature = tag.Signature; diff --git a/src/ImageSharp/Metadata/Profiles/ICC/Various/IccTagTableEntry.cs b/src/ImageSharp/Metadata/Profiles/ICC/Various/IccTagTableEntry.cs index 539fe723e2..2e0a5eaa96 100644 --- a/src/ImageSharp/Metadata/Profiles/ICC/Various/IccTagTableEntry.cs +++ b/src/ImageSharp/Metadata/Profiles/ICC/Various/IccTagTableEntry.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors. +// Copyright (c) Six Labors. // Licensed under the Six Labors Split License. namespace SixLabors.ImageSharp.Metadata.Profiles.Icc; @@ -49,9 +49,7 @@ internal readonly struct IccTagTableEntry : IEquatable /// True if the parameter is equal to the parameter; otherwise, false. /// public static bool operator ==(IccTagTableEntry left, IccTagTableEntry right) - { - return left.Equals(right); - } + => left.Equals(right); /// /// Compares two objects for equality. @@ -62,9 +60,7 @@ internal readonly struct IccTagTableEntry : IEquatable /// True if the parameter is not equal to the parameter; otherwise, false. /// public static bool operator !=(IccTagTableEntry left, IccTagTableEntry right) - { - return !left.Equals(right); - } + => !left.Equals(right); /// public override bool Equals(object obj) => obj is IccTagTableEntry other && this.Equals(other); diff --git a/tests/ImageSharp.Tests/Colorspaces/Icc/IccProfileConverterTests.cs b/tests/ImageSharp.Tests/Colorspaces/Icc/IccProfileConverterTests.cs index 272219a5cf..67aa347b2f 100644 --- a/tests/ImageSharp.Tests/Colorspaces/Icc/IccProfileConverterTests.cs +++ b/tests/ImageSharp.Tests/Colorspaces/Icc/IccProfileConverterTests.cs @@ -9,16 +9,15 @@ using SixLabors.ImageSharp.PixelFormats; namespace SixLabors.ImageSharp.Tests.Colorspaces.Icc; public class IccProfileConverterTests { - private static PngEncoder Encoder = new PngEncoder(); + private static readonly PngEncoder Encoder = new(); [Theory] [WithFile(TestImages.Jpeg.ICC.AdobeRgb, PixelTypes.Rgb24)] [WithFile(TestImages.Jpeg.ICC.AppleRGB, PixelTypes.Rgb24)] [WithFile(TestImages.Jpeg.ICC.ColorMatch, PixelTypes.Rgb24)] [WithFile(TestImages.Jpeg.ICC.WideRGB, PixelTypes.Rgb24)] - - // [WithFile(TestImages.Jpeg.ICC.SRgb, PixelTypes.Rgb24)] ConverterBase says this is invalid. - // [WithFile(TestImages.Jpeg.ICC.ProPhoto, PixelTypes.Rgb24)] ConverterBase says this is invalid. + [WithFile(TestImages.Jpeg.ICC.SRgb, PixelTypes.Rgb24)] + [WithFile(TestImages.Jpeg.ICC.ProPhoto, PixelTypes.Rgb24)] public void CanRoundTripProfile(TestImageProvider provider) where TPixel : unmanaged, IPixel { @@ -37,10 +36,9 @@ public class IccProfileConverterTests Assert.Equal(expected, actual); } - // TODO: This fails as the base calculator says sRGB is invalid. [Theory] [WithFile(TestImages.Jpeg.ICC.AdobeRgb, PixelTypes.Rgb24)] - public void CanConvertTosRGB(TestImageProvider provider) + public void CanConvertToWide(TestImageProvider provider) where TPixel : unmanaged, IPixel { using Image image = provider.GetImage();