diff --git a/src/ImageSharp/Image/ImageFrameCollection.cs b/src/ImageSharp/Image/ImageFrameCollection.cs index 466e2ec392..b8b46c88b2 100644 --- a/src/ImageSharp/Image/ImageFrameCollection.cs +++ b/src/ImageSharp/Image/ImageFrameCollection.cs @@ -13,11 +13,25 @@ namespace SixLabors.ImageSharp /// Encapsulates an imaged collection of frames. /// /// The type of the pixel. - public sealed class ImageFrameCollection : IEnumerable> + public sealed class ImageFrameCollection : IEnumerable>, IDisposable where TPixel : struct, IPixel { private readonly IList> frames = new List>(); + internal ImageFrameCollection(int width, int height) + { + this.Add(new ImageFrame(width, height)); + } + + internal ImageFrameCollection(IEnumerable> frames) + { + Guard.NotNullOrEmpty(frames, nameof(frames)); + foreach (ImageFrame f in frames) + { + this.Add(f); + } + } + /// /// Gets the count. /// @@ -42,7 +56,7 @@ namespace SixLabors.ImageSharp set { - this.ValidateFrameSize(value); + this.ValidateFrame(value); this.frames[index] = value; } } @@ -61,7 +75,7 @@ namespace SixLabors.ImageSharp /// The to insert into the . public void Insert(int index, ImageFrame frame) { - this.ValidateFrameSize(frame); + this.ValidateFrame(frame); this.frames.Insert(index, frame); } @@ -72,12 +86,12 @@ namespace SixLabors.ImageSharp /// Cannot remove last frame. public void RemoveAt(int index) { - if (index > 0 || this.frames.Count > 1) + if (index == 0 && this.Count == 1) { - this.frames.RemoveAt(index); + throw new InvalidOperationException("Cannot remove last frame."); } - throw new InvalidOperationException("Cannot remove last frame."); + this.frames.RemoveAt(index); } /// @@ -87,7 +101,7 @@ namespace SixLabors.ImageSharp /// Frame must have the same dimensions as the image - frame public void Add(ImageFrame frame) { - this.ValidateFrameSize(frame); + this.ValidateFrame(frame); this.frames.Add(frame); } @@ -113,7 +127,7 @@ namespace SixLabors.ImageSharp { if (this.Count == 1 && this.frames.Contains(frame)) { - throw new InvalidOperationException("Cannot remove last frame"); + throw new InvalidOperationException("Cannot remove last frame."); } return this.frames.Remove(frame); @@ -125,7 +139,7 @@ namespace SixLabors.ImageSharp /// IEnumerator IEnumerable.GetEnumerator() => ((IEnumerable)this.frames).GetEnumerator(); - private void ValidateFrameSize(ImageFrame frame) + private void ValidateFrame(ImageFrame frame) { Guard.NotNull(frame, nameof(frame)); @@ -133,9 +147,20 @@ namespace SixLabors.ImageSharp { if (this.RootFrame.Width != frame.Width || this.RootFrame.Height != frame.Height) { - throw new ArgumentException("Frame must have the same dimensions as the image", nameof(frame)); + throw new ArgumentException("Frame must have the same dimensions as the image.", nameof(frame)); } } } + + /// + public void Dispose() + { + foreach (ImageFrame f in this.frames) + { + f.Dispose(); + } + + this.frames.Clear(); + } } } \ No newline at end of file diff --git a/src/ImageSharp/Image/ImageFrame{TPixel}.cs b/src/ImageSharp/Image/ImageFrame{TPixel}.cs index 421750d226..fc6da8b89b 100644 --- a/src/ImageSharp/Image/ImageFrame{TPixel}.cs +++ b/src/ImageSharp/Image/ImageFrame{TPixel}.cs @@ -25,7 +25,7 @@ namespace SixLabors.ImageSharp /// private Buffer2D pixelBuffer; - private bool isDisposed = false; + private bool isDisosed = false; /// /// Initializes a new instance of the class. @@ -180,7 +180,7 @@ namespace SixLabors.ImageSharp /// public void Dispose() { - if (this.isDisposed) + if (this.isDisosed) { return; } @@ -189,7 +189,7 @@ namespace SixLabors.ImageSharp this.pixelBuffer = null; // Note disposing is done. - this.isDisposed = true; + this.isDisosed = true; } /// diff --git a/src/ImageSharp/Image/Image{TPixel}.cs b/src/ImageSharp/Image/Image{TPixel}.cs index f228d766d1..36335b71b6 100644 --- a/src/ImageSharp/Image/Image{TPixel}.cs +++ b/src/ImageSharp/Image/Image{TPixel}.cs @@ -58,8 +58,10 @@ namespace SixLabors.ImageSharp /// The height of the image in pixels. /// The images metadata. internal Image(Configuration configuration, int width, int height, ImageMetaData metadata) - : this(configuration, width, height, metadata, null) { + this.configuration = configuration ?? Configuration.Default; + this.MetaData = metadata ?? new ImageMetaData(); + this.Frames = new ImageFrameCollection(width, height); } /// @@ -67,29 +69,14 @@ namespace SixLabors.ImageSharp /// with the height and the width of the image. /// /// The configuration providing initialization code which allows extending the library. - /// The width of the image in pixels. - /// The height of the image in pixels. /// The images metadata. /// The frames that will be owned by this image instance. - internal Image(Configuration configuration, int width, int height, ImageMetaData metadata, IEnumerable> frames) + internal Image(Configuration configuration, ImageMetaData metadata, IEnumerable> frames) { this.configuration = configuration ?? Configuration.Default; this.MetaData = metadata ?? new ImageMetaData(); - this.Frames = new ImageFrameCollection(); - - if (frames != null) - { - foreach (ImageFrame f in frames) - { - this.Frames.Add(f); - } - } - - if (this.Frames.Count == 0) - { - this.Frames.Add(new ImageFrame(width, height)); - } + this.Frames = new ImageFrameCollection(frames); } /// @@ -157,7 +144,7 @@ namespace SixLabors.ImageSharp { IEnumerable> frames = this.Frames.Select(x => x.Clone()).ToArray(); - return new Image(this.configuration, this.Width, this.Height, this.MetaData.Clone(), frames); + return new Image(this.configuration, this.MetaData.Clone(), frames); } /// @@ -175,7 +162,7 @@ namespace SixLabors.ImageSharp where TPixel2 : struct, IPixel { IEnumerable> frames = this.Frames.Select(x => x.CloneAs()).ToArray(); - var target = new Image(this.configuration, this.Width, this.Height, this.MetaData, frames); + var target = new Image(this.configuration, this.MetaData, frames); return target; } @@ -185,10 +172,7 @@ namespace SixLabors.ImageSharp /// public void Dispose() { - for (int i = 0; i < this.Frames.Count; i++) - { - this.Frames[i].Dispose(); - } + this.Frames.Dispose(); } /// diff --git a/src/ImageSharp/Processing/Processors/Transforms/ResizeProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/ResizeProcessor.cs index 05c70b75e8..a4fdb1a1b4 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/ResizeProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/ResizeProcessor.cs @@ -60,7 +60,7 @@ namespace SixLabors.ImageSharp.Processing.Processors // For resize we know we are going to populate every pixel with fresh data and we want a different target size so // let's manually clone an empty set of images at the correct target and then have the base class processs them in turn. IEnumerable> frames = source.Frames.Select(x => new ImageFrame(this.Width, this.Height, x.MetaData.Clone())); // this will create places holders - var image = new Image(config, this.Width, this.Height, source.MetaData.Clone(), frames); // base the place holder images in to prevet a extra frame being added + var image = new Image(config, source.MetaData.Clone(), frames); // base the place holder images in to prevet a extra frame being added return image; } diff --git a/tests/ImageSharp.Tests/Image/ImageFramesCollectionTests.cs b/tests/ImageSharp.Tests/Image/ImageFramesCollectionTests.cs new file mode 100644 index 0000000000..acd1eec202 --- /dev/null +++ b/tests/ImageSharp.Tests/Image/ImageFramesCollectionTests.cs @@ -0,0 +1,219 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using SixLabors.ImageSharp.Advanced; +using Xunit; + +namespace SixLabors.ImageSharp.Tests +{ + public class ImageFramesCollectionTests + { + [Fact] + public void ImageFramesaLwaysHaveOneFrame() + { + var collection = new ImageFrameCollection(10, 10); + Assert.Equal(1, collection.Count); + } + + [Fact] + public void AddNewFrame_FramesMustHaveSameSize() + { + var collection = new ImageFrameCollection(10, 10); + + ArgumentException ex = Assert.Throws(() => + { + collection.Add(new ImageFrame(1, 1)); + }); + + Assert.Equal("Frame must have the same dimensions as the image.\r\nParameter name: frame", ex.Message); + } + + [Fact] + public void AddNewFrame_FramesNotBeNull() + { + var collection = new ImageFrameCollection(10, 10); + + ArgumentNullException ex = Assert.Throws(() => + { + collection.Add(null); + }); + + Assert.Equal("Value cannot be null.\r\nParameter name: frame", ex.Message); + } + + [Fact] + public void InsertNewFrame_FramesMustHaveSameSize() + { + var collection = new ImageFrameCollection(10, 10); + + ArgumentException ex = Assert.Throws(() => + { + collection.Insert(1, new ImageFrame(1, 1)); + }); + + Assert.Equal("Frame must have the same dimensions as the image.\r\nParameter name: frame", ex.Message); + } + + [Fact] + public void InsertNewFrame_FramesNotBeNull() + { + var collection = new ImageFrameCollection(10, 10); + + ArgumentNullException ex = Assert.Throws(() => + { + collection.Insert(1, null); + }); + + Assert.Equal("Value cannot be null.\r\nParameter name: frame", ex.Message); + } + + [Fact] + public void SetFrameAtIndex_FramesMustHaveSameSize() + { + var collection = new ImageFrameCollection(10, 10); + + ArgumentException ex = Assert.Throws(() => + { + collection[0] = new ImageFrame(1, 1); + }); + + Assert.Equal("Frame must have the same dimensions as the image.\r\nParameter name: frame", ex.Message); + } + + [Fact] + public void SetFrameAtIndex_FramesNotBeNull() + { + var collection = new ImageFrameCollection(10, 10); + + ArgumentNullException ex = Assert.Throws(() => + { + collection[0] = null; + }); + + Assert.Equal("Value cannot be null.\r\nParameter name: frame", ex.Message); + } + + [Fact] + public void Constructor_FramesMustHaveSameSize() + { + + ArgumentException ex = Assert.Throws(() => + { + var collection = new ImageFrameCollection(new[] { + new ImageFrame(10,10), + new ImageFrame(1,1), + }); + }); + + Assert.Equal("Frame must have the same dimensions as the image.\r\nParameter name: frame", ex.Message); + } + + [Fact] + public void RemoveAtFrame_ThrowIfRemovingLastFrame() + { + var collection = new ImageFrameCollection(new[] { + new ImageFrame(10,10) + }); + + InvalidOperationException ex = Assert.Throws(() => + { + collection.RemoveAt(0); + }); + Assert.Equal("Cannot remove last frame.", ex.Message); + } + + [Fact] + public void RemoveAtFrame_CanRemoveFrameZeroIfMultipleFramesExist() + { + + var collection = new ImageFrameCollection(new[] { + new ImageFrame(10,10), + new ImageFrame(10,10), + }); + + collection.RemoveAt(0); + Assert.Equal(1, collection.Count); + } + + [Fact] + public void RemoveFrame_ThrowIfRemovingLastFrame() + { + var collection = new ImageFrameCollection(new[] { + new ImageFrame(10,10) + }); + + InvalidOperationException ex = Assert.Throws(() => + { + collection.Remove(collection[0]); + }); + Assert.Equal("Cannot remove last frame.", ex.Message); + } + + [Fact] + public void RemoveFrame_CanRemoveFrameZeroIfMultipleFramesExist() + { + + var collection = new ImageFrameCollection(new[] { + new ImageFrame(10,10), + new ImageFrame(10,10), + }); + + collection.Remove(collection[0]); + Assert.Equal(1, collection.Count); + } + + [Fact] + public void RootFrameIsFrameAtIndexZero() + { + var collection = new ImageFrameCollection(new[] { + new ImageFrame(10,10), + new ImageFrame(10,10), + }); + + Assert.Equal(collection.RootFrame, collection[0]); + } + + [Fact] + public void ConstructorPopulatesFrames() + { + var collection = new ImageFrameCollection(new[] { + new ImageFrame(10,10), + new ImageFrame(10,10), + }); + + Assert.Equal(2, collection.Count); + } + + [Fact] + public void DisposeClearsCollection() + { + var collection = new ImageFrameCollection(new[] { + new ImageFrame(10,10), + new ImageFrame(10,10), + }); + + collection.Dispose(); + + Assert.Equal(0, collection.Count); + } + + [Fact] + public void Dispose_DisposesAllInnerFrames() + { + var collection = new ImageFrameCollection(new[] { + new ImageFrame(10,10), + new ImageFrame(10,10), + }); + + IPixelSource[] framesSnapShot = collection.OfType>().ToArray(); + collection.Dispose(); + + Assert.All(framesSnapShot, f => + { + // the pixel source of the frame is null after its been disposed. + Assert.Null(f.PixelBuffer); + }); + } + } +}