From 699b6597156a5ceb34a1a6be9e872b61e429406b Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 17 Sep 2019 15:37:50 +1000 Subject: [PATCH 1/5] Refactor Image.Dispose --- src/ImageSharp/IImage.cs | 4 ++-- src/ImageSharp/Image.cs | 27 +++++++++++-------------- src/ImageSharp/ImageExtensions.cs | 13 +----------- src/ImageSharp/ImageFrame.cs | 12 ++++++++++- src/ImageSharp/ImageFrame{TPixel}.cs | 16 +++++++-------- src/ImageSharp/Image{TPixel}.cs | 30 +++++++++++++++++++++++++--- 6 files changed, 61 insertions(+), 41 deletions(-) diff --git a/src/ImageSharp/IImage.cs b/src/ImageSharp/IImage.cs index b9e2cee61..0d4dc3c9d 100644 --- a/src/ImageSharp/IImage.cs +++ b/src/ImageSharp/IImage.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System; @@ -11,4 +11,4 @@ namespace SixLabors.ImageSharp public interface IImage : IImageInfo, IDisposable { } -} \ No newline at end of file +} diff --git a/src/ImageSharp/Image.cs b/src/ImageSharp/Image.cs index 57f60f2e7..696f83662 100644 --- a/src/ImageSharp/Image.cs +++ b/src/ImageSharp/Image.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. +using System; using System.IO; using SixLabors.ImageSharp.Advanced; @@ -80,21 +81,11 @@ namespace SixLabors.ImageSharp /// Configuration IConfigurable.Configuration => this.Configuration; - /// - /// Gets a value indicating whether the image instance is disposed. - /// - public bool IsDisposed { get; private set; } - /// public void Dispose() { - if (this.IsDisposed) - { - return; - } - - this.IsDisposed = true; - this.DisposeImpl(); + this.Dispose(true); + GC.SuppressFinalize(this); } /// @@ -109,7 +100,7 @@ namespace SixLabors.ImageSharp Guard.NotNull(encoder, nameof(encoder)); this.EnsureNotDisposed(); - EncodeVisitor visitor = new EncodeVisitor(encoder, stream); + var visitor = new EncodeVisitor(encoder, stream); this.AcceptVisitor(visitor); } @@ -144,9 +135,15 @@ namespace SixLabors.ImageSharp protected void UpdateSize(Size size) => this.size = size; /// - /// Implements the Dispose logic. + /// Disposes the object and frees resources for the Garbage Collector. + /// + /// Whether to dispose of managed and unmanaged objects. + protected abstract void Dispose(bool disposing); + + /// + /// Throws if the image is disposed. /// - protected abstract void DisposeImpl(); + internal abstract void EnsureNotDisposed(); private class EncodeVisitor : IImageVisitor { diff --git a/src/ImageSharp/ImageExtensions.cs b/src/ImageSharp/ImageExtensions.cs index 6ea2b234c..6cdc948d4 100644 --- a/src/ImageSharp/ImageExtensions.cs +++ b/src/ImageSharp/ImageExtensions.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System; @@ -119,16 +119,5 @@ namespace SixLabors.ImageSharp return $"data:{format.DefaultMimeType};base64,{Convert.ToBase64String(stream.ToArray())}"; } } - - /// - /// Throws if the image is disposed. - /// - internal static void EnsureNotDisposed(this Image image) - { - if (image.IsDisposed) - { - throw new ObjectDisposedException(nameof(image), "Trying to execute an operation on a disposed image."); - } - } } } diff --git a/src/ImageSharp/ImageFrame.cs b/src/ImageSharp/ImageFrame.cs index f3fe1ed8d..91872b21d 100644 --- a/src/ImageSharp/ImageFrame.cs +++ b/src/ImageSharp/ImageFrame.cs @@ -74,7 +74,17 @@ namespace SixLabors.ImageSharp public Rectangle Bounds() => new Rectangle(0, 0, this.Width, this.Height); /// - public abstract void Dispose(); + public void Dispose() + { + this.Dispose(true); + GC.SuppressFinalize(this); + } + + /// + /// Disposes the object and frees resources for the Garbage Collector. + /// + /// Whether to dispose of managed and unmanaged objects. + protected abstract void Dispose(bool disposing); internal abstract void CopyPixelsTo(Span destination) where TDestinationPixel : struct, IPixel; diff --git a/src/ImageSharp/ImageFrame{TPixel}.cs b/src/ImageSharp/ImageFrame{TPixel}.cs index 5c9ff489e..0436eb9d2 100644 --- a/src/ImageSharp/ImageFrame{TPixel}.cs +++ b/src/ImageSharp/ImageFrame{TPixel}.cs @@ -21,7 +21,7 @@ namespace SixLabors.ImageSharp /// In all other cases it is the only frame of the image. /// /// The pixel format. - public sealed class ImageFrame : ImageFrame, IPixelSource, IDisposable + public sealed class ImageFrame : ImageFrame, IPixelSource where TPixel : struct, IPixel { private bool isDisposed; @@ -196,20 +196,20 @@ namespace SixLabors.ImageSharp this.UpdateSize(this.PixelBuffer.Size()); } - /// - /// Disposes the object and frees resources for the Garbage Collector. - /// - public override void Dispose() + /// + protected override void Dispose(bool disposing) { if (this.isDisposed) { return; } - this.PixelBuffer?.Dispose(); - this.PixelBuffer = null; + if (disposing) + { + this.PixelBuffer?.Dispose(); + this.PixelBuffer = null; + } - // Note disposing is done. this.isDisposed = true; } diff --git a/src/ImageSharp/Image{TPixel}.cs b/src/ImageSharp/Image{TPixel}.cs index a7ea58652..6dbfde20c 100644 --- a/src/ImageSharp/Image{TPixel}.cs +++ b/src/ImageSharp/Image{TPixel}.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System; @@ -18,9 +18,11 @@ namespace SixLabors.ImageSharp /// For generic -s the pixel type is known at compile time. /// /// The pixel format. - public sealed class Image : Image + public class Image : Image where TPixel : struct, IPixel { + private bool isDisposed; + /// /// Initializes a new instance of the class /// with the height and the width of the image. @@ -185,7 +187,29 @@ namespace SixLabors.ImageSharp } /// - protected override void DisposeImpl() => this.Frames.Dispose(); + protected override void Dispose(bool disposing) + { + if (this.isDisposed) + { + return; + } + + if (disposing) + { + this.Frames.Dispose(); + } + + this.isDisposed = true; + } + + /// + internal override void EnsureNotDisposed() + { + if (this.isDisposed) + { + throw new ObjectDisposedException("Trying to execute an operation on a disposed image."); + } + } /// internal override void AcceptVisitor(IImageVisitor visitor) From 1efaba60c3d05204f7f3c13643b29ed90393630b Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 17 Sep 2019 15:45:58 +1000 Subject: [PATCH 2/5] Refactor IImageProcessor --- .../Processors/CloningImageProcessor{TPixel}.cs | 7 +------ .../Processing/Processors/ImageProcessor{TPixel}.cs | 8 ++------ 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs index 8b3e1eb96..c2c869052 100644 --- a/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs @@ -15,8 +15,6 @@ namespace SixLabors.ImageSharp.Processing.Processors internal abstract class CloningImageProcessor : ICloningImageProcessor where TPixel : struct, IPixel { - private bool isDisposed; - /// /// Initializes a new instance of the class. /// @@ -104,6 +102,7 @@ namespace SixLabors.ImageSharp.Processing.Processors public void Dispose() { this.Dispose(true); + GC.SuppressFinalize(this); } /// @@ -160,10 +159,6 @@ namespace SixLabors.ImageSharp.Processing.Processors /// Whether to dispose managed and unmanaged objects. protected virtual void Dispose(bool disposing) { - if (!this.isDisposed) - { - this.isDisposed = true; - } } } } diff --git a/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs index 8ac8cd67b..55b4d3dc7 100644 --- a/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs @@ -15,8 +15,6 @@ namespace SixLabors.ImageSharp.Processing.Processors internal abstract class ImageProcessor : IImageProcessor where TPixel : struct, IPixel { - private bool isDisposed; - /// /// Initializes a new instance of the class. /// @@ -97,6 +95,8 @@ namespace SixLabors.ImageSharp.Processing.Processors /// public virtual void Dispose() { + this.Dispose(true); + GC.SuppressFinalize(this); } /// @@ -142,10 +142,6 @@ namespace SixLabors.ImageSharp.Processing.Processors /// Whether to dispose managed and unmanaged objects. protected virtual void Dispose(bool disposing) { - if (!this.isDisposed) - { - this.isDisposed = true; - } } } } From b49f2ab3e8961097eec432dc6f31862be3635060 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 17 Sep 2019 16:45:05 +1000 Subject: [PATCH 3/5] Expose visitor through advanced namespace. --- .../Processors/Drawing/DrawImageProcessor.cs | 1 + .../Advanced/AdvancedImageExtensions.cs | 9 +++++ .../{ => Advanced}/IImageVisitor.cs | 10 +++--- src/ImageSharp/Image.cs | 8 ++--- src/ImageSharp/Image{TPixel}.cs | 4 +-- .../Extensions/ProcessingExtensions.cs | 33 ++++++++++++------- .../Processors/ImageProcessorExtensions.cs | 1 + 7 files changed, 43 insertions(+), 23 deletions(-) rename src/ImageSharp/{ => Advanced}/IImageVisitor.cs (64%) diff --git a/src/ImageSharp.Drawing/Processing/Processors/Drawing/DrawImageProcessor.cs b/src/ImageSharp.Drawing/Processing/Processors/Drawing/DrawImageProcessor.cs index dc55112c9..e217fd9a6 100644 --- a/src/ImageSharp.Drawing/Processing/Processors/Drawing/DrawImageProcessor.cs +++ b/src/ImageSharp.Drawing/Processing/Processors/Drawing/DrawImageProcessor.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. +using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.PixelFormats; using SixLabors.Primitives; diff --git a/src/ImageSharp/Advanced/AdvancedImageExtensions.cs b/src/ImageSharp/Advanced/AdvancedImageExtensions.cs index 4b1d4222c..22e6d47e9 100644 --- a/src/ImageSharp/Advanced/AdvancedImageExtensions.cs +++ b/src/ImageSharp/Advanced/AdvancedImageExtensions.cs @@ -15,6 +15,15 @@ namespace SixLabors.ImageSharp.Advanced /// public static class AdvancedImageExtensions { + /// + /// Accepts a to implement a double-dispatch pattern in order to + /// apply pixel-specific operations on non-generic instances + /// + /// The source. + /// The visitor. + public static void AcceptVisitor(this Image source, IImageVisitor visitor) + => source.Accept(visitor); + /// /// Gets the configuration for the image. /// diff --git a/src/ImageSharp/IImageVisitor.cs b/src/ImageSharp/Advanced/IImageVisitor.cs similarity index 64% rename from src/ImageSharp/IImageVisitor.cs rename to src/ImageSharp/Advanced/IImageVisitor.cs index 971c4d37c..ba8b13e2e 100644 --- a/src/ImageSharp/IImageVisitor.cs +++ b/src/ImageSharp/Advanced/IImageVisitor.cs @@ -3,13 +3,13 @@ using SixLabors.ImageSharp.PixelFormats; -namespace SixLabors.ImageSharp +namespace SixLabors.ImageSharp.Advanced { /// - /// A visitor to implement double-dispatch pattern in order to apply pixel-specific operations - /// on non-generic instances. The operation is dispatched by . + /// A visitor to implement a double-dispatch pattern in order to apply pixel-specific operations + /// on non-generic instances. /// - internal interface IImageVisitor + public interface IImageVisitor { /// /// Provides a pixel-specific implementation for a given operation. @@ -19,4 +19,4 @@ namespace SixLabors.ImageSharp void Visit(Image image) where TPixel : struct, IPixel; } -} \ No newline at end of file +} diff --git a/src/ImageSharp/Image.cs b/src/ImageSharp/Image.cs index 57f60f2e7..dbdbfbd8f 100644 --- a/src/ImageSharp/Image.cs +++ b/src/ImageSharp/Image.cs @@ -109,8 +109,7 @@ namespace SixLabors.ImageSharp Guard.NotNull(encoder, nameof(encoder)); this.EnsureNotDisposed(); - EncodeVisitor visitor = new EncodeVisitor(encoder, stream); - this.AcceptVisitor(visitor); + this.AcceptVisitor(new EncodeVisitor(encoder, stream)); } /// @@ -131,11 +130,12 @@ namespace SixLabors.ImageSharp where TPixel2 : struct, IPixel; /// - /// Accept a . + /// Accepts a . /// Implemented by invoking /// with the pixel type of the image. /// - internal abstract void AcceptVisitor(IImageVisitor visitor); + /// The visitor. + protected internal abstract void Accept(IImageVisitor visitor); /// /// Update the size of the image after mutation. diff --git a/src/ImageSharp/Image{TPixel}.cs b/src/ImageSharp/Image{TPixel}.cs index a7ea58652..994a2c586 100644 --- a/src/ImageSharp/Image{TPixel}.cs +++ b/src/ImageSharp/Image{TPixel}.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System; @@ -188,7 +188,7 @@ namespace SixLabors.ImageSharp protected override void DisposeImpl() => this.Frames.Dispose(); /// - internal override void AcceptVisitor(IImageVisitor visitor) + protected internal override void Accept(IImageVisitor visitor) { this.EnsureNotDisposed(); diff --git a/src/ImageSharp/Processing/Extensions/ProcessingExtensions.cs b/src/ImageSharp/Processing/Extensions/ProcessingExtensions.cs index 48876251e..40b1c439e 100644 --- a/src/ImageSharp/Processing/Extensions/ProcessingExtensions.cs +++ b/src/ImageSharp/Processing/Extensions/ProcessingExtensions.cs @@ -25,8 +25,7 @@ namespace SixLabors.ImageSharp.Processing Guard.NotNull(operation, nameof(operation)); source.EnsureNotDisposed(); - var visitor = new ProcessingVisitor(operation, true); - source.AcceptVisitor(visitor); + source.AcceptVisitor(new ProcessingVisitor(operation, true)); } /// @@ -42,8 +41,10 @@ namespace SixLabors.ImageSharp.Processing Guard.NotNull(operation, nameof(operation)); source.EnsureNotDisposed(); - IInternalImageProcessingContext operationsRunner = source.GetConfiguration().ImageOperationsProvider - .CreateImageProcessingContext(source, true); + IInternalImageProcessingContext operationsRunner + = source.GetConfiguration() + .ImageOperationsProvider.CreateImageProcessingContext(source, true); + operation(operationsRunner); } @@ -60,8 +61,10 @@ namespace SixLabors.ImageSharp.Processing Guard.NotNull(operations, nameof(operations)); source.EnsureNotDisposed(); - IInternalImageProcessingContext operationsRunner = source.GetConfiguration().ImageOperationsProvider - .CreateImageProcessingContext(source, true); + IInternalImageProcessingContext operationsRunner + = source.GetConfiguration() + .ImageOperationsProvider.CreateImageProcessingContext(source, true); + operationsRunner.ApplyProcessors(operations); } @@ -96,8 +99,10 @@ namespace SixLabors.ImageSharp.Processing Guard.NotNull(operation, nameof(operation)); source.EnsureNotDisposed(); - IInternalImageProcessingContext operationsRunner = source.GetConfiguration().ImageOperationsProvider - .CreateImageProcessingContext(source, false); + IInternalImageProcessingContext operationsRunner + = source.GetConfiguration() + .ImageOperationsProvider.CreateImageProcessingContext(source, false); + operation(operationsRunner); return operationsRunner.GetResultImage(); } @@ -116,8 +121,10 @@ namespace SixLabors.ImageSharp.Processing Guard.NotNull(operations, nameof(operations)); source.EnsureNotDisposed(); - IInternalImageProcessingContext operationsRunner = source.GetConfiguration().ImageOperationsProvider - .CreateImageProcessingContext(source, false); + IInternalImageProcessingContext operationsRunner + = source.GetConfiguration() + .ImageOperationsProvider.CreateImageProcessingContext(source, false); + operationsRunner.ApplyProcessors(operations); return operationsRunner.GetResultImage(); } @@ -157,8 +164,10 @@ namespace SixLabors.ImageSharp.Processing public void Visit(Image image) where TPixel : struct, IPixel { - IInternalImageProcessingContext operationsRunner = image.GetConfiguration() - .ImageOperationsProvider.CreateImageProcessingContext(image, this.mutate); + IInternalImageProcessingContext operationsRunner = + image.GetConfiguration() + .ImageOperationsProvider.CreateImageProcessingContext(image, this.mutate); + this.operation(operationsRunner); this.ResultImage = operationsRunner.GetResultImage(); } diff --git a/src/ImageSharp/Processing/Processors/ImageProcessorExtensions.cs b/src/ImageSharp/Processing/Processors/ImageProcessorExtensions.cs index 53eedfd20..feb4c9f19 100644 --- a/src/ImageSharp/Processing/Processors/ImageProcessorExtensions.cs +++ b/src/ImageSharp/Processing/Processors/ImageProcessorExtensions.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. +using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.PixelFormats; using SixLabors.Primitives; From 24cd39a62fdfd3674437cf0a0aa968d42c605b41 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 18 Sep 2019 08:36:04 +1000 Subject: [PATCH 4/5] reseal Image{TPixel} --- src/ImageSharp/Image{TPixel}.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Image{TPixel}.cs b/src/ImageSharp/Image{TPixel}.cs index 6dbfde20c..181e818ee 100644 --- a/src/ImageSharp/Image{TPixel}.cs +++ b/src/ImageSharp/Image{TPixel}.cs @@ -18,7 +18,7 @@ namespace SixLabors.ImageSharp /// For generic -s the pixel type is known at compile time. /// /// The pixel format. - public class Image : Image + public sealed class Image : Image where TPixel : struct, IPixel { private bool isDisposed; From 985cfbda087c98a5913289d4ecb55c3ba63a71b3 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 18 Sep 2019 08:45:03 +1000 Subject: [PATCH 5/5] Remove protected. --- src/ImageSharp/Image.cs | 16 ++++++++-------- src/ImageSharp/Image{TPixel}.cs | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/ImageSharp/Image.cs b/src/ImageSharp/Image.cs index dbdbfbd8f..9030ae6af 100644 --- a/src/ImageSharp/Image.cs +++ b/src/ImageSharp/Image.cs @@ -129,14 +129,6 @@ namespace SixLabors.ImageSharp public abstract Image CloneAs(Configuration configuration) where TPixel2 : struct, IPixel; - /// - /// Accepts a . - /// Implemented by invoking - /// with the pixel type of the image. - /// - /// The visitor. - protected internal abstract void Accept(IImageVisitor visitor); - /// /// Update the size of the image after mutation. /// @@ -148,6 +140,14 @@ namespace SixLabors.ImageSharp /// protected abstract void DisposeImpl(); + /// + /// Accepts a . + /// Implemented by invoking + /// with the pixel type of the image. + /// + /// The visitor. + internal abstract void Accept(IImageVisitor visitor); + private class EncodeVisitor : IImageVisitor { private readonly IImageEncoder encoder; diff --git a/src/ImageSharp/Image{TPixel}.cs b/src/ImageSharp/Image{TPixel}.cs index 994a2c586..3d92c3be3 100644 --- a/src/ImageSharp/Image{TPixel}.cs +++ b/src/ImageSharp/Image{TPixel}.cs @@ -187,17 +187,17 @@ namespace SixLabors.ImageSharp /// protected override void DisposeImpl() => this.Frames.Dispose(); + /// + public override string ToString() => $"Image<{typeof(TPixel).Name}>: {this.Width}x{this.Height}"; + /// - protected internal override void Accept(IImageVisitor visitor) + internal override void Accept(IImageVisitor visitor) { this.EnsureNotDisposed(); visitor.Visit(this); } - /// - public override string ToString() => $"Image<{typeof(TPixel).Name}>: {this.Width}x{this.Height}"; - /// /// Switches the buffers used by the image and the pixelSource meaning that the Image will "own" the buffer from the pixelSource and the pixelSource will now own the Images buffer. ///