From c91d9154f663d2fbc9816462cbb6d1a68588331f Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 12 Sep 2018 08:30:42 +0100 Subject: [PATCH] Simplify metadata API --- src/ImageSharp/Common/Helpers/ImageMaths.cs | 2 +- src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs | 3 +-- src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs | 2 +- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 15 ++++------- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 6 ++--- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 6 ++--- src/ImageSharp/Formats/Png/PngEncoderCore.cs | 2 +- src/ImageSharp/MetaData/ImageFrameMetaData.cs | 25 ++----------------- src/ImageSharp/MetaData/ImageMetaData.cs | 21 ++-------------- .../Formats/Bmp/BmpEncoderTests.cs | 2 +- .../Formats/Gif/GifEncoderTests.cs | 10 ++++---- .../Formats/Png/PngEncoderTests.cs | 4 +-- .../MetaData/ImageFrameMetaDataTests.cs | 14 ++++------- 13 files changed, 31 insertions(+), 81 deletions(-) diff --git a/src/ImageSharp/Common/Helpers/ImageMaths.cs b/src/ImageSharp/Common/Helpers/ImageMaths.cs index cacaca0bb..c15e0a732 100644 --- a/src/ImageSharp/Common/Helpers/ImageMaths.cs +++ b/src/ImageSharp/Common/Helpers/ImageMaths.cs @@ -44,7 +44,7 @@ namespace SixLabors.ImageSharp /// The bit depth. /// The [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static int GetColorCountForBitDepth(int bitDepth) => (int)Math.Pow(2, bitDepth); + public static int GetColorCountForBitDepth(int bitDepth) => 1 << bitDepth; /// /// Implementation of 1D Gaussian G(x) function diff --git a/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs b/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs index a574d5178..71852acdd 100644 --- a/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs +++ b/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs @@ -537,8 +537,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp this.metaData = meta; short bitsPerPixel = this.infoHeader.BitsPerPixel; - var bmpMetaData = new BmpMetaData(); - this.metaData.AddOrUpdateFormatMetaData(BmpFormat.Instance, bmpMetaData); + var bmpMetaData = this.metaData.GetFormatMetaData(BmpFormat.Instance); // We can only encode at these bit rates so far. if (bitsPerPixel.Equals((short)BmpBitsPerPixel.Pixel24) diff --git a/src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs b/src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs index 7a09a47f7..4ffaf3950 100644 --- a/src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs +++ b/src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs @@ -48,7 +48,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp Guard.NotNull(image, nameof(image)); Guard.NotNull(stream, nameof(stream)); - BmpMetaData bmpMetaData = image.MetaData.GetOrAddFormatMetaData(BmpFormat.Instance); + BmpMetaData bmpMetaData = image.MetaData.GetFormatMetaData(BmpFormat.Instance); this.bitsPerPixel = this.bitsPerPixel ?? bmpMetaData.BitsPerPixel; short bpp = (short)this.bitsPerPixel; diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index f50381264..207f126f9 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -164,7 +164,6 @@ namespace SixLabors.ImageSharp.Formats.Gif this.globalColorTable?.Dispose(); } - image?.MetaData.AddOrUpdateFormatMetaData(GifFormat.Instance, this.gifMetaData); return image; } @@ -224,7 +223,6 @@ namespace SixLabors.ImageSharp.Formats.Gif this.globalColorTable?.Dispose(); } - this.metaData.AddOrUpdateFormatMetaData(GifFormat.Instance, this.gifMetaData); return new ImageInfo( new PixelTypeInfo(this.logicalScreenDescriptor.BitsPerPixel), this.logicalScreenDescriptor.Width, @@ -542,7 +540,7 @@ namespace SixLabors.ImageSharp.Formats.Gif [MethodImpl(MethodImplOptions.AggressiveInlining)] private void SetFrameMetaData(ImageFrameMetaData meta) { - var gifMeta = new GifFrameMetaData(); + GifFrameMetaData gifMeta = meta.GetFormatMetaData(GifFormat.Instance); if (this.graphicsControlExtension.DelayTime > 0) { gifMeta.FrameDelay = this.graphicsControlExtension.DelayTime; @@ -561,7 +559,6 @@ namespace SixLabors.ImageSharp.Formats.Gif } gifMeta.DisposalMethod = this.graphicsControlExtension.DisposalMethod; - meta.AddOrUpdateFormatMetaData(GifFormat.Instance, gifMeta); } /// @@ -605,12 +602,10 @@ namespace SixLabors.ImageSharp.Formats.Gif } this.metaData = meta; - this.gifMetaData = new GifMetaData - { - ColorTableMode = this.logicalScreenDescriptor.GlobalColorTableFlag - ? GifColorTableMode.Global - : GifColorTableMode.Local - }; + this.gifMetaData = meta.GetFormatMetaData(GifFormat.Instance); + this.gifMetaData.ColorTableMode = this.logicalScreenDescriptor.GlobalColorTableFlag + ? GifColorTableMode.Global + : GifColorTableMode.Local; if (this.logicalScreenDescriptor.GlobalColorTableFlag) { diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index f2e0eab5c..a516b5fef 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -86,7 +86,7 @@ namespace SixLabors.ImageSharp.Formats.Gif Guard.NotNull(image, nameof(image)); Guard.NotNull(stream, nameof(stream)); - this.gifMetaData = image.MetaData.GetOrAddFormatMetaData(GifFormat.Instance); + this.gifMetaData = image.MetaData.GetFormatMetaData(GifFormat.Instance); this.colorTableMode = this.colorTableMode ?? this.gifMetaData.ColorTableMode; bool useGlobalTable = this.colorTableMode.Equals(GifColorTableMode.Global); @@ -143,7 +143,7 @@ namespace SixLabors.ImageSharp.Formats.Gif for (int i = 0; i < image.Frames.Count; i++) { ImageFrame frame = image.Frames[i]; - GifFrameMetaData frameMetaData = frame.MetaData.GetOrAddFormatMetaData(GifFormat.Instance); + GifFrameMetaData frameMetaData = frame.MetaData.GetFormatMetaData(GifFormat.Instance); this.WriteGraphicalControlExtension(frameMetaData, transparencyIndex, stream); this.WriteImageDescriptor(frame, false, stream); @@ -169,7 +169,7 @@ namespace SixLabors.ImageSharp.Formats.Gif GifFrameMetaData previousMeta = null; foreach (ImageFrame frame in image.Frames) { - GifFrameMetaData meta = frame.MetaData.GetOrAddFormatMetaData(GifFormat.Instance); + GifFrameMetaData meta = frame.MetaData.GetFormatMetaData(GifFormat.Instance); if (quantized is null) { // Allow each frame to be encoded at whatever color depth the frame designates if set. diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 3daee991c..7837c2da5 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -217,8 +217,7 @@ namespace SixLabors.ImageSharp.Formats.Png where TPixel : struct, IPixel { var metaData = new ImageMetaData(); - var pngMetaData = new PngMetaData(); - metaData.AddOrUpdateFormatMetaData(PngFormat.Instance, pngMetaData); + var pngMetaData = metaData.GetFormatMetaData(PngFormat.Instance); this.currentStream = stream; this.currentStream.Skip(8); Image image = null; @@ -308,8 +307,7 @@ namespace SixLabors.ImageSharp.Formats.Png public IImageInfo Identify(Stream stream) { var metaData = new ImageMetaData(); - var pngMetaData = new PngMetaData(); - metaData.AddOrUpdateFormatMetaData(PngFormat.Instance, pngMetaData); + var pngMetaData = metaData.GetFormatMetaData(PngFormat.Instance); this.currentStream = stream; this.currentStream.Skip(8); try diff --git a/src/ImageSharp/Formats/Png/PngEncoderCore.cs b/src/ImageSharp/Formats/Png/PngEncoderCore.cs index 20fc8b8e3..906e6d000 100644 --- a/src/ImageSharp/Formats/Png/PngEncoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngEncoderCore.cs @@ -181,7 +181,7 @@ namespace SixLabors.ImageSharp.Formats.Png this.height = image.Height; // Always take the encoder options over the metadata values. - PngMetaData pngMetaData = image.MetaData.GetOrAddFormatMetaData(PngFormat.Instance); + PngMetaData pngMetaData = image.MetaData.GetFormatMetaData(PngFormat.Instance); this.gamma = this.gamma ?? pngMetaData.Gamma; this.writeGamma = this.gamma > 0; this.pngColorType = this.pngColorType ?? pngMetaData.ColorType; diff --git a/src/ImageSharp/MetaData/ImageFrameMetaData.cs b/src/ImageSharp/MetaData/ImageFrameMetaData.cs index 908a0e624..4b819e201 100644 --- a/src/ImageSharp/MetaData/ImageFrameMetaData.cs +++ b/src/ImageSharp/MetaData/ImageFrameMetaData.cs @@ -44,27 +44,6 @@ namespace SixLabors.ImageSharp.MetaData /// The cloned instance. public ImageFrameMetaData Clone() => new ImageFrameMetaData(this); - /// - /// Adds or updates the specified key and value to the . - /// - /// The type of format metadata. - /// The type of format frame metadata. - /// The key of the metadata to add. - /// The value of the element to add. - /// key is null. - /// value is null. - /// An element with the same key already exists in the . - public void AddOrUpdateFormatMetaData( - IImageFormat key, - TFormatFrameMetaData value) - where TFormatMetaData : class - where TFormatFrameMetaData : class - { - // Don't think this needs to be threadsafe. - Guard.NotNull(value, nameof(value)); - this.formatMetaData[key] = value; - } - /// /// Gets the metadata value associated with the specified key. /// @@ -74,7 +53,7 @@ namespace SixLabors.ImageSharp.MetaData /// /// The . /// - public TFormatFrameMetaData GetOrAddFormatMetaData(IImageFormat key) + public TFormatFrameMetaData GetFormatMetaData(IImageFormat key) where TFormatMetaData : class where TFormatFrameMetaData : class { @@ -84,7 +63,7 @@ namespace SixLabors.ImageSharp.MetaData } TFormatFrameMetaData newMeta = key.CreateDefaultFormatFrameMetaData(); - this.AddOrUpdateFormatMetaData(key, newMeta); + this.formatMetaData[key] = newMeta; return newMeta; } } diff --git a/src/ImageSharp/MetaData/ImageMetaData.cs b/src/ImageSharp/MetaData/ImageMetaData.cs index 74c00cf8a..7e74157e7 100644 --- a/src/ImageSharp/MetaData/ImageMetaData.cs +++ b/src/ImageSharp/MetaData/ImageMetaData.cs @@ -131,23 +131,6 @@ namespace SixLabors.ImageSharp.MetaData /// public IList Properties { get; } = new List(); - /// - /// Adds or updates the specified key and value to the . - /// - /// The type of format metadata. - /// The key of the metadata to add. - /// The value of the element to add. - /// key is null. - /// value is null. - /// An element with the same key already exists in the . - public void AddOrUpdateFormatMetaData(IImageFormat key, TFormatMetaData value) - where TFormatMetaData : class - { - // Don't think this needs to be threadsafe. - Guard.NotNull(value, nameof(value)); - this.formatMetaData[key] = value; - } - /// /// Gets the metadata value associated with the specified key. /// @@ -156,7 +139,7 @@ namespace SixLabors.ImageSharp.MetaData /// /// The . /// - public TFormatMetaData GetOrAddFormatMetaData(IImageFormat key) + public TFormatMetaData GetFormatMetaData(IImageFormat key) where TFormatMetaData : class { if (this.formatMetaData.TryGetValue(key, out object meta)) @@ -165,7 +148,7 @@ namespace SixLabors.ImageSharp.MetaData } TFormatMetaData newMeta = key.CreateDefaultFormatMetaData(); - this.AddOrUpdateFormatMetaData(key, newMeta); + this.formatMetaData[key] = newMeta; return newMeta; } diff --git a/tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs index 311b28f2d..b9f855cf1 100644 --- a/tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs @@ -80,7 +80,7 @@ namespace SixLabors.ImageSharp.Tests memStream.Position = 0; using (var output = Image.Load(memStream)) { - BmpMetaData meta = output.MetaData.GetOrAddFormatMetaData(BmpFormat.Instance); + BmpMetaData meta = output.MetaData.GetFormatMetaData(BmpFormat.Instance); Assert.Equal(bmpBitsPerPixel, meta.BitsPerPixel); } diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs index 0c32689f0..4a17f867f 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs @@ -189,8 +189,8 @@ namespace SixLabors.ImageSharp.Tests.Formats.Gif inStream.Position = 0; var image = Image.Load(inStream); - GifMetaData metaData = image.MetaData.GetOrAddFormatMetaData(GifFormat.Instance); - GifFrameMetaData frameMetaData = image.Frames.RootFrame.MetaData.GetOrAddFormatMetaData(GifFormat.Instance); + GifMetaData metaData = image.MetaData.GetFormatMetaData(GifFormat.Instance); + GifFrameMetaData frameMetaData = image.Frames.RootFrame.MetaData.GetFormatMetaData(GifFormat.Instance); GifColorTableMode colorMode = metaData.ColorTableMode; var encoder = new GifEncoder() { @@ -204,7 +204,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Gif outStream.Position = 0; var clone = Image.Load(outStream); - GifMetaData cloneMetaData = clone.MetaData.GetOrAddFormatMetaData(GifFormat.Instance); + GifMetaData cloneMetaData = clone.MetaData.GetFormatMetaData(GifFormat.Instance); Assert.Equal(metaData.ColorTableMode, cloneMetaData.ColorTableMode); // Gifiddle and Cyotek GifInfo say this image has 64 colors. @@ -212,8 +212,8 @@ namespace SixLabors.ImageSharp.Tests.Formats.Gif for (int i = 0; i < image.Frames.Count; i++) { - GifFrameMetaData ifm = image.Frames[i].MetaData.GetOrAddFormatMetaData(GifFormat.Instance); - GifFrameMetaData cifm = clone.Frames[i].MetaData.GetOrAddFormatMetaData(GifFormat.Instance); + GifFrameMetaData ifm = image.Frames[i].MetaData.GetFormatMetaData(GifFormat.Instance); + GifFrameMetaData cifm = clone.Frames[i].MetaData.GetFormatMetaData(GifFormat.Instance); Assert.Equal(ifm.ColorTableLength, cifm.ColorTableLength); Assert.Equal(ifm.FrameDelay, cifm.FrameDelay); diff --git a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs index c9435a37d..0508ac8c2 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs @@ -228,7 +228,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png : image; float paletteToleranceHack = 80f / paletteSize; - paletteToleranceHack = paletteToleranceHack * paletteToleranceHack; + paletteToleranceHack *= paletteToleranceHack; ImageComparer comparer = pngColorType == PngColorType.Palette ? ImageComparer.Tolerant(ToleranceThresholdForPaletteEncoder * paletteToleranceHack) : ImageComparer.Exact; @@ -314,7 +314,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png memStream.Position = 0; using (var output = Image.Load(memStream)) { - PngMetaData meta = output.MetaData.GetOrAddFormatMetaData(PngFormat.Instance); + PngMetaData meta = output.MetaData.GetFormatMetaData(PngFormat.Instance); Assert.Equal(pngBitDepth, meta.BitDepth); } diff --git a/tests/ImageSharp.Tests/MetaData/ImageFrameMetaDataTests.cs b/tests/ImageSharp.Tests/MetaData/ImageFrameMetaDataTests.cs index 54441f0cb..0a0ca1efa 100644 --- a/tests/ImageSharp.Tests/MetaData/ImageFrameMetaDataTests.cs +++ b/tests/ImageSharp.Tests/MetaData/ImageFrameMetaDataTests.cs @@ -19,18 +19,14 @@ namespace SixLabors.ImageSharp.Tests const int colorTableLength = 128; const GifDisposalMethod disposalMethod = GifDisposalMethod.RestoreToBackground; - var gifFrameMetaData = new GifFrameMetaData - { - FrameDelay = frameDelay, - ColorTableLength = colorTableLength, - DisposalMethod = disposalMethod - }; - var metaData = new ImageFrameMetaData(); - metaData.AddOrUpdateFormatMetaData(GifFormat.Instance, gifFrameMetaData); + GifFrameMetaData gifFrameMetaData = metaData.GetFormatMetaData(GifFormat.Instance); + gifFrameMetaData.FrameDelay = frameDelay; + gifFrameMetaData.ColorTableLength = colorTableLength; + gifFrameMetaData.DisposalMethod = disposalMethod; var clone = new ImageFrameMetaData(metaData); - GifFrameMetaData cloneGifFrameMetaData = clone.GetOrAddFormatMetaData(GifFormat.Instance); + GifFrameMetaData cloneGifFrameMetaData = clone.GetFormatMetaData(GifFormat.Instance); Assert.Equal(frameDelay, cloneGifFrameMetaData.FrameDelay); Assert.Equal(colorTableLength, cloneGifFrameMetaData.ColorTableLength);