From b173a70a00eedef6905b0e5db09f93746e462c6a Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 27 Aug 2019 09:58:16 +1000 Subject: [PATCH] Implement IDisposable in IImageProcessor instances. (#990) * Implement IDisposable and ensure inheritance calls base * add ImageProcessingContextTests and move some other test classes * loosen up tests and leave TODO notes --- ...> DefaultImageProcessorContext{TPixel}.cs} | 19 +- ...InternalImageProcessingContext{TPixel}.cs} | 0 ...or.cs => CloningImageProcessor{TPixel}.cs} | 21 +++ .../Convolution/BoxBlurProcessor{TPixel}.cs | 9 +- .../EdgeDetector2DProcessor{TPixel}.cs | 9 +- .../EdgeDetectorCompassProcessor{TPixel}.cs | 12 +- .../EdgeDetectorProcessor{TPixel}.cs | 9 +- .../GaussianBlurProcessor{TPixel}.cs | 9 +- .../GaussianSharpenProcessor{TPixel}.cs | 7 +- .../PaletteDitherProcessor{TPixel}.cs | 4 +- .../Filters/LomographProcessor{TPixel}.cs | 5 +- .../Filters/PolaroidProcessor{TPixel}.cs | 1 + ...r.cs => ICloningImageProcessor{TPixel}.cs} | 0 .../Processors/IImageProcessor{TPixel}.cs | 9 +- .../Processors/ImageProcessorExtensions.cs | 9 +- ...Processor.cs => ImageProcessor{TPixel}.cs} | 19 ++ .../Transforms/AutoOrientProcessor{TPixel}.cs | 2 + .../EntropyCropProcessor{TPixel}.cs | 3 + .../Resize/ResizeProcessor{TPixel}.cs | 30 ++- .../Transforms/TransformProcessor.cs | 5 +- .../Drawing/Paths/DrawPathCollection.cs | 1 + .../Drawing/Paths/FillPath.cs | 1 + .../Drawing/Paths/FillPathCollection.cs | 1 + .../Drawing/Paths/FillPolygon.cs | 1 + .../Drawing/Paths/FillRectangle.cs | 2 + .../ImageSharp.Tests/Drawing/Text/DrawText.cs | 1 + .../BaseImageOperationsExtensionTest.cs | 3 +- .../FakeImageOperationsProvider.cs | 9 +- .../Processing/Filters/LomographTest.cs | 2 + .../{ => Processing}/ImageOperationTests.cs | 2 +- .../Processing/ImageProcessingContextTests.cs | 171 ++++++++++++++++++ .../Processors/Convolution/BokehBlurTest.cs | 18 +- 32 files changed, 344 insertions(+), 50 deletions(-) rename src/ImageSharp/Processing/{DefaultImageProcessorContext.cs => DefaultImageProcessorContext{TPixel}.cs} (76%) rename src/ImageSharp/Processing/{IInternalImageProcessingContext.cs => IInternalImageProcessingContext{TPixel}.cs} (100%) rename src/ImageSharp/Processing/Processors/{CloningImageProcessor.cs => CloningImageProcessor{TPixel}.cs} (90%) rename src/ImageSharp/Processing/Processors/{ICloningImageProcessor.cs => ICloningImageProcessor{TPixel}.cs} (100%) rename src/ImageSharp/Processing/Processors/{ImageProcessor.cs => ImageProcessor{TPixel}.cs} (89%) rename tests/ImageSharp.Tests/{ => Processing}/BaseImageOperationsExtensionTest.cs (97%) rename tests/ImageSharp.Tests/{ => Processing}/FakeImageOperationsProvider.cs (96%) rename tests/ImageSharp.Tests/{ => Processing}/ImageOperationTests.cs (99%) create mode 100644 tests/ImageSharp.Tests/Processing/ImageProcessingContextTests.cs diff --git a/src/ImageSharp/Processing/DefaultImageProcessorContext.cs b/src/ImageSharp/Processing/DefaultImageProcessorContext{TPixel}.cs similarity index 76% rename from src/ImageSharp/Processing/DefaultImageProcessorContext.cs rename to src/ImageSharp/Processing/DefaultImageProcessorContext{TPixel}.cs index 7b6fbebd12..a6e3dc03a0 100644 --- a/src/ImageSharp/Processing/DefaultImageProcessorContext.cs +++ b/src/ImageSharp/Processing/DefaultImageProcessorContext{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 SixLabors.ImageSharp.Advanced; @@ -67,16 +67,25 @@ namespace SixLabors.ImageSharp.Processing // This will only work if the first processor applied is the cloning one thus // realistically for this optimization to work the resize must the first processor // applied any only up processors will take the double data path. - if (processor.CreatePixelSpecificProcessor(this.source, rectangle) is ICloningImageProcessor cloningImageProcessor) + using (IImageProcessor specificProcessor = processor.CreatePixelSpecificProcessor(this.source, rectangle)) { - this.destination = cloningImageProcessor.CloneAndApply(); - return this; + // TODO: if 'specificProcessor' is not an ICloningImageProcessor we are unnecessarily disposing and recreating it. + // This should be solved in a future refactor. + if (specificProcessor is ICloningImageProcessor cloningImageProcessor) + { + this.destination = cloningImageProcessor.CloneAndApply(); + return this; + } } this.destination = this.source.Clone(); } - processor.CreatePixelSpecificProcessor(this.destination, rectangle).Apply(); + using (IImageProcessor specificProcessor = processor.CreatePixelSpecificProcessor(this.destination, rectangle)) + { + specificProcessor.Apply(); + } + return this; } diff --git a/src/ImageSharp/Processing/IInternalImageProcessingContext.cs b/src/ImageSharp/Processing/IInternalImageProcessingContext{TPixel}.cs similarity index 100% rename from src/ImageSharp/Processing/IInternalImageProcessingContext.cs rename to src/ImageSharp/Processing/IInternalImageProcessingContext{TPixel}.cs diff --git a/src/ImageSharp/Processing/Processors/CloningImageProcessor.cs b/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs similarity index 90% rename from src/ImageSharp/Processing/Processors/CloningImageProcessor.cs rename to src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs index 518fdf0cb1..8b3e1eb965 100644 --- a/src/ImageSharp/Processing/Processors/CloningImageProcessor.cs +++ b/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs @@ -15,6 +15,8 @@ namespace SixLabors.ImageSharp.Processing.Processors internal abstract class CloningImageProcessor : ICloningImageProcessor where TPixel : struct, IPixel { + private bool isDisposed; + /// /// Initializes a new instance of the class. /// @@ -54,6 +56,7 @@ namespace SixLabors.ImageSharp.Processing.Processors throw new ImageProcessingException($"An error occurred when processing the image using {this.GetType().Name}. The processor changed the number of frames."); } + Configuration configuration = this.Source.GetConfiguration(); this.BeforeImageApply(clone); for (int i = 0; i < this.Source.Frames.Count; i++) @@ -97,6 +100,12 @@ namespace SixLabors.ImageSharp.Processing.Processors } } + /// + public void Dispose() + { + this.Dispose(true); + } + /// /// Generates a deep clone of the source image that operations should be applied to. /// @@ -144,5 +153,17 @@ namespace SixLabors.ImageSharp.Processing.Processors protected virtual void AfterImageApply(Image destination) { } + + /// + /// Disposes the object and frees resources for the Garbage Collector. + /// + /// 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/Convolution/BoxBlurProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/BoxBlurProcessor{TPixel}.cs index b9ca2df0b5..77110e642d 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/BoxBlurProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/BoxBlurProcessor{TPixel}.cs @@ -42,8 +42,13 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution public DenseMatrix KernelY { get; } /// - protected override void OnFrameApply(ImageFrame source) => - new Convolution2PassProcessor(this.KernelX, this.KernelY, false, this.Source, this.SourceRectangle).Apply(source); + protected override void OnFrameApply(ImageFrame source) + { + using (var processor = new Convolution2PassProcessor(this.KernelX, this.KernelY, false, this.Source, this.SourceRectangle)) + { + processor.Apply(source); + } + } /// /// Create a 1 dimensional Box kernel. diff --git a/src/ImageSharp/Processing/Processors/Convolution/EdgeDetector2DProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/EdgeDetector2DProcessor{TPixel}.cs index 26a789cfc7..7b070f99a3 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/EdgeDetector2DProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/EdgeDetector2DProcessor{TPixel}.cs @@ -56,10 +56,17 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution { new GrayscaleBt709Processor(1F).Apply(this.Source, this.SourceRectangle); } + + base.BeforeImageApply(); } /// protected override void OnFrameApply(ImageFrame source) - => new Convolution2DProcessor(this.KernelX, this.KernelY, true, this.Source, this.SourceRectangle).Apply(source); + { + using (var processor = new Convolution2DProcessor(this.KernelX, this.KernelY, true, this.Source, this.SourceRectangle)) + { + processor.Apply(source); + } + } } } diff --git a/src/ImageSharp/Processing/Processors/Convolution/EdgeDetectorCompassProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/EdgeDetectorCompassProcessor{TPixel}.cs index 543014f6f1..b7119ef44b 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/EdgeDetectorCompassProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/EdgeDetectorCompassProcessor{TPixel}.cs @@ -47,6 +47,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution { new GrayscaleBt709Processor(1F).Apply(this.Source, this.SourceRectangle); } + + base.BeforeImageApply(); } /// @@ -68,7 +70,10 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution // we need a clean copy for each pass to start from using (ImageFrame cleanCopy = source.Clone()) { - new ConvolutionProcessor(kernels[0], true, this.Source, this.SourceRectangle).Apply(source); + using (var processor = new ConvolutionProcessor(kernels[0], true, this.Source, this.SourceRectangle)) + { + processor.Apply(source); + } if (kernels.Length == 1) { @@ -97,7 +102,10 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution { using (ImageFrame pass = cleanCopy.Clone()) { - new ConvolutionProcessor(kernels[i], true, this.Source, this.SourceRectangle).Apply(pass); + using (var processor = new ConvolutionProcessor(kernels[i], true, this.Source, this.SourceRectangle)) + { + processor.Apply(pass); + } Buffer2D passPixels = pass.PixelBuffer; Buffer2D targetPixels = source.PixelBuffer; diff --git a/src/ImageSharp/Processing/Processors/Convolution/EdgeDetectorProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/EdgeDetectorProcessor{TPixel}.cs index 7ee9d68293..fc762cf1bb 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/EdgeDetectorProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/EdgeDetectorProcessor{TPixel}.cs @@ -43,10 +43,17 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution { new GrayscaleBt709Processor(1F).Apply(this.Source, this.SourceRectangle); } + + base.BeforeImageApply(); } /// protected override void OnFrameApply(ImageFrame source) - => new ConvolutionProcessor(this.KernelXY, true, this.Source, this.SourceRectangle).Apply(source); + { + using (var processor = new ConvolutionProcessor(this.KernelXY, true, this.Source, this.SourceRectangle)) + { + processor.Apply(source); + } + } } } diff --git a/src/ImageSharp/Processing/Processors/Convolution/GaussianBlurProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/GaussianBlurProcessor{TPixel}.cs index da7924d031..bbf36ea5e8 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/GaussianBlurProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/GaussianBlurProcessor{TPixel}.cs @@ -39,7 +39,12 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution public DenseMatrix KernelY { get; } /// - protected override void OnFrameApply(ImageFrame source) => - new Convolution2PassProcessor(this.KernelX, this.KernelY, false, this.Source, this.SourceRectangle).Apply(source); + protected override void OnFrameApply(ImageFrame source) + { + using (var processor = new Convolution2PassProcessor(this.KernelX, this.KernelY, false, this.Source, this.SourceRectangle)) + { + processor.Apply(source); + } + } } } diff --git a/src/ImageSharp/Processing/Processors/Convolution/GaussianSharpenProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/GaussianSharpenProcessor{TPixel}.cs index 0d99ceb9c0..dab55b2328 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/GaussianSharpenProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/GaussianSharpenProcessor{TPixel}.cs @@ -40,6 +40,11 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution /// protected override void OnFrameApply(ImageFrame source) - => new Convolution2PassProcessor(this.KernelX, this.KernelY, false, this.Source, this.SourceRectangle).Apply(source); + { + using (var processor = new Convolution2PassProcessor(this.KernelX, this.KernelY, false, this.Source, this.SourceRectangle)) + { + processor.Apply(source); + } + } } } diff --git a/src/ImageSharp/Processing/Processors/Dithering/PaletteDitherProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Dithering/PaletteDitherProcessor{TPixel}.cs index 291e6ac0ec..9f817267fe 100644 --- a/src/ImageSharp/Processing/Processors/Dithering/PaletteDitherProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Dithering/PaletteDitherProcessor{TPixel}.cs @@ -44,8 +44,6 @@ namespace SixLabors.ImageSharp.Processing.Processors.Dithering /// protected override void BeforeFrameApply(ImageFrame source) { - base.BeforeFrameApply(source); - // Lazy init palette: if (this.palette is null) { @@ -64,6 +62,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Dithering (Span)this.paletteVector, PixelConversionModifiers.Scale); } + + base.BeforeFrameApply(source); } /// diff --git a/src/ImageSharp/Processing/Processors/Filters/LomographProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Filters/LomographProcessor{TPixel}.cs index 52f0b37678..ab832a2759 100644 --- a/src/ImageSharp/Processing/Processors/Filters/LomographProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Filters/LomographProcessor{TPixel}.cs @@ -27,9 +27,10 @@ namespace SixLabors.ImageSharp.Processing.Processors.Filters } /// - protected override void AfterFrameApply(ImageFrame source) + protected override void AfterImageApply() { - new VignetteProcessor(VeryDarkGreen).CreatePixelSpecificProcessor(this.Source, this.SourceRectangle).Apply(); + new VignetteProcessor(VeryDarkGreen).Apply(this.Source, this.SourceRectangle); + base.AfterImageApply(); } } } diff --git a/src/ImageSharp/Processing/Processors/Filters/PolaroidProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Filters/PolaroidProcessor{TPixel}.cs index 5aad1005bb..0be5bbb0de 100644 --- a/src/ImageSharp/Processing/Processors/Filters/PolaroidProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Filters/PolaroidProcessor{TPixel}.cs @@ -33,6 +33,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Filters { new VignetteProcessor(VeryDarkOrange).Apply(this.Source, this.SourceRectangle); new GlowProcessor(LightOrange, this.Source.Width / 4F).Apply(this.Source, this.SourceRectangle); + base.AfterImageApply(); } } } diff --git a/src/ImageSharp/Processing/Processors/ICloningImageProcessor.cs b/src/ImageSharp/Processing/Processors/ICloningImageProcessor{TPixel}.cs similarity index 100% rename from src/ImageSharp/Processing/Processors/ICloningImageProcessor.cs rename to src/ImageSharp/Processing/Processors/ICloningImageProcessor{TPixel}.cs diff --git a/src/ImageSharp/Processing/Processors/IImageProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/IImageProcessor{TPixel}.cs index e2bc1b0ffb..3d6e0d765e 100644 --- a/src/ImageSharp/Processing/Processors/IImageProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/IImageProcessor{TPixel}.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. +using System; using SixLabors.ImageSharp.PixelFormats; using SixLabors.Primitives; @@ -10,16 +11,16 @@ namespace SixLabors.ImageSharp.Processing.Processors /// Implements an algorithm to alter the pixels of an image. /// /// The pixel format. - public interface IImageProcessor + public interface IImageProcessor : IDisposable where TPixel : struct, IPixel { /// - /// Applies the process to the specified portion of the specified . + /// Applies the process to the specified portion of the specified . /// - /// + /// /// The target is null. /// - /// + /// /// The target doesn't fit the dimension of the image. /// void Apply(); diff --git a/src/ImageSharp/Processing/Processors/ImageProcessorExtensions.cs b/src/ImageSharp/Processing/Processors/ImageProcessorExtensions.cs index 9eb10b6e43..53eedfd207 100644 --- a/src/ImageSharp/Processing/Processors/ImageProcessorExtensions.cs +++ b/src/ImageSharp/Processing/Processors/ImageProcessorExtensions.cs @@ -10,8 +10,7 @@ namespace SixLabors.ImageSharp.Processing.Processors { public static void Apply(this IImageProcessor processor, Image source, Rectangle sourceRectangle) { - var visitor = new ApplyVisitor(processor, sourceRectangle); - source.AcceptVisitor(visitor); + source.AcceptVisitor(new ApplyVisitor(processor, sourceRectangle)); } private class ApplyVisitor : IImageVisitor @@ -29,8 +28,10 @@ namespace SixLabors.ImageSharp.Processing.Processors public void Visit(Image image) where TPixel : struct, IPixel { - IImageProcessor processorImpl = this.processor.CreatePixelSpecificProcessor(image, this.sourceRectangle); - processorImpl.Apply(); + using (IImageProcessor processorImpl = this.processor.CreatePixelSpecificProcessor(image, this.sourceRectangle)) + { + processorImpl.Apply(); + } } } } diff --git a/src/ImageSharp/Processing/Processors/ImageProcessor.cs b/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs similarity index 89% rename from src/ImageSharp/Processing/Processors/ImageProcessor.cs rename to src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs index 39f3dd6360..8ac8cd67bc 100644 --- a/src/ImageSharp/Processing/Processors/ImageProcessor.cs +++ b/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs @@ -15,6 +15,8 @@ namespace SixLabors.ImageSharp.Processing.Processors internal abstract class ImageProcessor : IImageProcessor where TPixel : struct, IPixel { + private bool isDisposed; + /// /// Initializes a new instance of the class. /// @@ -92,6 +94,11 @@ namespace SixLabors.ImageSharp.Processing.Processors } } + /// + public virtual void Dispose() + { + } + /// /// This method is called before the process is applied to prepare the processor. /// @@ -128,5 +135,17 @@ namespace SixLabors.ImageSharp.Processing.Processors protected virtual void AfterImageApply() { } + + /// + /// Disposes the object and frees resources for the Garbage Collector. + /// + /// 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/Transforms/AutoOrientProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Transforms/AutoOrientProcessor{TPixel}.cs index e68a24b727..a5170c96a1 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/AutoOrientProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/AutoOrientProcessor{TPixel}.cs @@ -68,6 +68,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms default: break; } + + base.BeforeImageApply(); } /// diff --git a/src/ImageSharp/Processing/Processors/Transforms/EntropyCropProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Transforms/EntropyCropProcessor{TPixel}.cs index 6129e69ef4..b74fbb0ab7 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/EntropyCropProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/EntropyCropProcessor{TPixel}.cs @@ -35,6 +35,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms { Rectangle rectangle; + // TODO: This is clunky. We should add behavior enum to ExtractFrame. // All frames have be the same size so we only need to calculate the correct dimensions for the first frame using (var temp = new Image(this.Configuration, this.Source.Metadata.DeepClone(), new[] { this.Source.Frames.RootFrame.Clone() })) { @@ -51,6 +52,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms } new CropProcessor(rectangle, this.Source.Size()).Apply(this.Source, this.SourceRectangle); + + base.BeforeImageApply(); } /// diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs index 2033fd38c0..e16d4801ec 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs @@ -24,12 +24,13 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms internal class ResizeProcessor : TransformProcessor where TPixel : struct, IPixel { + private readonly ResizeProcessor parameterSource; + private bool isDisposed; + // The following fields are not immutable but are optionally created on demand. private ResizeKernelMap horizontalKernelMap; private ResizeKernelMap verticalKernelMap; - private readonly ResizeProcessor parameterSource; - public ResizeProcessor(ResizeProcessor parameterSource, Image source, Rectangle sourceRectangle) : base(source, sourceRectangle) { @@ -109,6 +110,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms sourceRectangle.Height, memoryAllocator); } + + base.BeforeImageApply(destination); } /// @@ -189,15 +192,24 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms } } - protected override void AfterImageApply(Image destination) + /// + protected override void Dispose(bool disposing) { - base.AfterImageApply(destination); + if (this.isDisposed) + { + return; + } + + if (disposing) + { + this.horizontalKernelMap?.Dispose(); + this.horizontalKernelMap = null; + this.verticalKernelMap?.Dispose(); + this.verticalKernelMap = null; + } - // TODO: An exception in the processing chain can leave these buffers undisposed. We should consider making image processors IDisposable! - this.horizontalKernelMap?.Dispose(); - this.horizontalKernelMap = null; - this.verticalKernelMap?.Dispose(); - this.verticalKernelMap = null; + this.isDisposed = true; + base.Dispose(disposing); } } } diff --git a/src/ImageSharp/Processing/Processors/Transforms/TransformProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/TransformProcessor.cs index ee501adf31..1f0c6ebc86 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/TransformProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/TransformProcessor.cs @@ -25,6 +25,9 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// protected override void AfterImageApply(Image destination) - => TransformProcessorHelpers.UpdateDimensionalMetadata(destination); + { + TransformProcessorHelpers.UpdateDimensionalMetadata(destination); + base.AfterImageApply(destination); + } } } diff --git a/tests/ImageSharp.Tests/Drawing/Paths/DrawPathCollection.cs b/tests/ImageSharp.Tests/Drawing/Paths/DrawPathCollection.cs index ee95bc743b..3691b54ce3 100644 --- a/tests/ImageSharp.Tests/Drawing/Paths/DrawPathCollection.cs +++ b/tests/ImageSharp.Tests/Drawing/Paths/DrawPathCollection.cs @@ -6,6 +6,7 @@ using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Primitives; using SixLabors.ImageSharp.Processing; using SixLabors.ImageSharp.Processing.Processors.Drawing; +using SixLabors.ImageSharp.Tests.Processing; using SixLabors.Shapes; using Xunit; diff --git a/tests/ImageSharp.Tests/Drawing/Paths/FillPath.cs b/tests/ImageSharp.Tests/Drawing/Paths/FillPath.cs index d12441ac2f..160ff22a3e 100644 --- a/tests/ImageSharp.Tests/Drawing/Paths/FillPath.cs +++ b/tests/ImageSharp.Tests/Drawing/Paths/FillPath.cs @@ -6,6 +6,7 @@ using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Primitives; using SixLabors.ImageSharp.Processing; using SixLabors.ImageSharp.Processing.Processors.Drawing; +using SixLabors.ImageSharp.Tests.Processing; using SixLabors.Shapes; using Xunit; diff --git a/tests/ImageSharp.Tests/Drawing/Paths/FillPathCollection.cs b/tests/ImageSharp.Tests/Drawing/Paths/FillPathCollection.cs index 17bb7e6ee5..b76ee8ffcd 100644 --- a/tests/ImageSharp.Tests/Drawing/Paths/FillPathCollection.cs +++ b/tests/ImageSharp.Tests/Drawing/Paths/FillPathCollection.cs @@ -6,6 +6,7 @@ using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Primitives; using SixLabors.ImageSharp.Processing; using SixLabors.ImageSharp.Processing.Processors.Drawing; +using SixLabors.ImageSharp.Tests.Processing; using SixLabors.Shapes; using Xunit; diff --git a/tests/ImageSharp.Tests/Drawing/Paths/FillPolygon.cs b/tests/ImageSharp.Tests/Drawing/Paths/FillPolygon.cs index 22b741ec1e..c62a871481 100644 --- a/tests/ImageSharp.Tests/Drawing/Paths/FillPolygon.cs +++ b/tests/ImageSharp.Tests/Drawing/Paths/FillPolygon.cs @@ -6,6 +6,7 @@ using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Primitives; using SixLabors.ImageSharp.Processing; using SixLabors.ImageSharp.Processing.Processors.Drawing; +using SixLabors.ImageSharp.Tests.Processing; using SixLabors.Shapes; using Xunit; diff --git a/tests/ImageSharp.Tests/Drawing/Paths/FillRectangle.cs b/tests/ImageSharp.Tests/Drawing/Paths/FillRectangle.cs index 6cb052aa8f..17a2b87c0e 100644 --- a/tests/ImageSharp.Tests/Drawing/Paths/FillRectangle.cs +++ b/tests/ImageSharp.Tests/Drawing/Paths/FillRectangle.cs @@ -5,6 +5,8 @@ using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Primitives; using SixLabors.ImageSharp.Processing; using SixLabors.ImageSharp.Processing.Processors.Drawing; +using SixLabors.ImageSharp.Tests.Processing; + using Xunit; namespace SixLabors.ImageSharp.Tests.Drawing.Paths diff --git a/tests/ImageSharp.Tests/Drawing/Text/DrawText.cs b/tests/ImageSharp.Tests/Drawing/Text/DrawText.cs index 8b45bd5240..e6866c6579 100644 --- a/tests/ImageSharp.Tests/Drawing/Text/DrawText.cs +++ b/tests/ImageSharp.Tests/Drawing/Text/DrawText.cs @@ -5,6 +5,7 @@ using System.Numerics; using SixLabors.Fonts; using SixLabors.ImageSharp.Processing; using SixLabors.ImageSharp.Processing.Processors.Text; +using SixLabors.ImageSharp.Tests.Processing; using SixLabors.Primitives; using Xunit; diff --git a/tests/ImageSharp.Tests/BaseImageOperationsExtensionTest.cs b/tests/ImageSharp.Tests/Processing/BaseImageOperationsExtensionTest.cs similarity index 97% rename from tests/ImageSharp.Tests/BaseImageOperationsExtensionTest.cs rename to tests/ImageSharp.Tests/Processing/BaseImageOperationsExtensionTest.cs index d298f61976..140af9563a 100644 --- a/tests/ImageSharp.Tests/BaseImageOperationsExtensionTest.cs +++ b/tests/ImageSharp.Tests/Processing/BaseImageOperationsExtensionTest.cs @@ -4,9 +4,10 @@ using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Processing; using SixLabors.Primitives; + using Xunit; -namespace SixLabors.ImageSharp.Tests +namespace SixLabors.ImageSharp.Tests.Processing { public abstract class BaseImageOperationsExtensionTest { diff --git a/tests/ImageSharp.Tests/FakeImageOperationsProvider.cs b/tests/ImageSharp.Tests/Processing/FakeImageOperationsProvider.cs similarity index 96% rename from tests/ImageSharp.Tests/FakeImageOperationsProvider.cs rename to tests/ImageSharp.Tests/Processing/FakeImageOperationsProvider.cs index fb4afc28d1..38d53c7b6c 100644 --- a/tests/ImageSharp.Tests/FakeImageOperationsProvider.cs +++ b/tests/ImageSharp.Tests/Processing/FakeImageOperationsProvider.cs @@ -3,14 +3,15 @@ using System.Collections.Generic; using System.Linq; + using SixLabors.ImageSharp.Advanced; -using SixLabors.Memory; using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Processing; using SixLabors.ImageSharp.Processing.Processors; +using SixLabors.Memory; using SixLabors.Primitives; -namespace SixLabors.ImageSharp.Tests +namespace SixLabors.ImageSharp.Tests.Processing { internal class FakeImageOperationsProvider : IImageProcessingContextFactory { @@ -19,7 +20,7 @@ namespace SixLabors.ImageSharp.Tests public bool HasCreated(Image source) where TPixel : struct, IPixel { - return Created(source).Any(); + return this.Created(source).Any(); } public IEnumerable> Created(Image source) where TPixel : struct, IPixel { @@ -29,7 +30,7 @@ namespace SixLabors.ImageSharp.Tests public IEnumerable.AppliedOperation> AppliedOperations(Image source) where TPixel : struct, IPixel { - return Created(source) + return this.Created(source) .SelectMany(x => x.Applied); } diff --git a/tests/ImageSharp.Tests/Processing/Filters/LomographTest.cs b/tests/ImageSharp.Tests/Processing/Filters/LomographTest.cs index a9aecaa9ca..65e04fbcc7 100644 --- a/tests/ImageSharp.Tests/Processing/Filters/LomographTest.cs +++ b/tests/ImageSharp.Tests/Processing/Filters/LomographTest.cs @@ -1,6 +1,8 @@ // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. +using SixLabors.ImageSharp.Tests.Processing; + using Xunit; namespace SixLabors.ImageSharp.Tests diff --git a/tests/ImageSharp.Tests/ImageOperationTests.cs b/tests/ImageSharp.Tests/Processing/ImageOperationTests.cs similarity index 99% rename from tests/ImageSharp.Tests/ImageOperationTests.cs rename to tests/ImageSharp.Tests/Processing/ImageOperationTests.cs index d2dc95cfe8..1b5c16538a 100644 --- a/tests/ImageSharp.Tests/ImageOperationTests.cs +++ b/tests/ImageSharp.Tests/Processing/ImageOperationTests.cs @@ -12,7 +12,7 @@ using SixLabors.ImageSharp.Processing.Processors; using Xunit; -namespace SixLabors.ImageSharp.Tests +namespace SixLabors.ImageSharp.Tests.Processing { /// /// Tests the configuration class. diff --git a/tests/ImageSharp.Tests/Processing/ImageProcessingContextTests.cs b/tests/ImageSharp.Tests/Processing/ImageProcessingContextTests.cs new file mode 100644 index 0000000000..589d595275 --- /dev/null +++ b/tests/ImageSharp.Tests/Processing/ImageProcessingContextTests.cs @@ -0,0 +1,171 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + +using Moq; + +using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Processing; +using SixLabors.ImageSharp.Processing.Processors; +using SixLabors.Primitives; + +using Xunit; + +namespace SixLabors.ImageSharp.Tests.Processing +{ + /// + /// Contains test cases for default implementation. + /// + public class ImageProcessingContextTests + { + private readonly Image image = new Image(10, 10); + + private readonly Mock processorDefinition; + + private readonly Mock> regularProcessorImpl; + + private readonly Mock> cloningProcessorImpl; + + private static readonly Rectangle Bounds = new Rectangle(3, 3, 5, 5); + + public ImageProcessingContextTests() + { + this.processorDefinition = new Mock(); + this.regularProcessorImpl = new Mock>(); + this.cloningProcessorImpl = new Mock>(); + } + + // bool throwException, bool useBounds + public static readonly TheoryData ProcessorTestData = new TheoryData() + { + { false, false }, + { false, true }, + { true, false }, + { true, true } + }; + [Theory] + [MemberData(nameof(ProcessorTestData))] + public void Mutate_RegularProcessor(bool throwException, bool useBounds) + { + this.SetupRegularProcessor(throwException); + + if (throwException) + { + Assert.Throws(() => this.MutateApply(useBounds)); + } + else + { + this.MutateApply(useBounds); + } + + this.regularProcessorImpl.Verify(p => p.Apply(), Times.Once()); + this.regularProcessorImpl.Verify(p => p.Dispose(), Times.Once()); + } + + [Theory] + [MemberData(nameof(ProcessorTestData))] + public void Clone_RegularProcessor(bool throwException, bool useBounds) + { + this.SetupRegularProcessor(throwException); + + if (throwException) + { + Assert.Throws(() => this.CloneApply(useBounds)); + } + else + { + this.CloneApply(useBounds); + } + + // TODO: This should be Times.Once(). See comments in DefaultImageProcessingContext.ApplyProcessor() + this.regularProcessorImpl.Verify(p => p.Apply(), Times.AtLeast(1)); + this.regularProcessorImpl.Verify(p => p.Dispose(), Times.AtLeast(1)); + } + + [Theory] + [MemberData(nameof(ProcessorTestData))] + public void Mutate_CloningProcessor(bool throwException, bool useBounds) + { + this.SetupCloningProcessor(throwException); + + if (throwException) + { + Assert.Throws(() => this.MutateApply(useBounds)); + } + else + { + this.MutateApply(useBounds); + } + + this.cloningProcessorImpl.Verify(p => p.Apply(), Times.Once()); + this.cloningProcessorImpl.Verify(p => p.Dispose(), Times.Once()); + } + + [Theory] + [MemberData(nameof(ProcessorTestData))] + public void Clone_CloningProcessor(bool throwException, bool useBounds) + { + this.SetupCloningProcessor(throwException); + + if (throwException) + { + Assert.Throws(() => this.CloneApply(useBounds)); + } + else + { + this.CloneApply(useBounds); + } + + this.cloningProcessorImpl.Verify(p => p.CloneAndApply(), Times.Once()); + this.cloningProcessorImpl.Verify(p => p.Dispose(), Times.Once()); + } + + private void MutateApply(bool useBounds) + { + if (useBounds) + { + this.image.Mutate(c => c.ApplyProcessor(this.processorDefinition.Object, Bounds)); + } + else + { + this.image.Mutate(c => c.ApplyProcessor(this.processorDefinition.Object)); + } + } + + private void CloneApply(bool useBounds) + { + if (useBounds) + { + this.image.Clone(c => c.ApplyProcessor(this.processorDefinition.Object, Bounds)).Dispose(); + } + else + { + this.image.Clone(c => c.ApplyProcessor(this.processorDefinition.Object)).Dispose(); + } + } + + private void SetupRegularProcessor(bool throwsException) + { + if (throwsException) + { + this.regularProcessorImpl.Setup(p => p.Apply()).Throws(new ImageProcessingException("Test")); + } + + this.processorDefinition + .Setup(p => p.CreatePixelSpecificProcessor(It.IsAny>(), It.IsAny())) + .Returns(this.regularProcessorImpl.Object); + } + + private void SetupCloningProcessor(bool throwsException) + { + if (throwsException) + { + this.cloningProcessorImpl.Setup(p => p.Apply()).Throws(new ImageProcessingException("Test")); + this.cloningProcessorImpl.Setup(p => p.CloneAndApply()).Throws(new ImageProcessingException("Test")); + } + + this.processorDefinition + .Setup(p => p.CreatePixelSpecificProcessor(It.IsAny>(), It.IsAny())) + .Returns(this.cloningProcessorImpl.Object); + } + } +} diff --git a/tests/ImageSharp.Tests/Processing/Processors/Convolution/BokehBlurTest.cs b/tests/ImageSharp.Tests/Processing/Processors/Convolution/BokehBlurTest.cs index a23de2b681..296b8d1d7f 100644 --- a/tests/ImageSharp.Tests/Processing/Processors/Convolution/BokehBlurTest.cs +++ b/tests/ImageSharp.Tests/Processing/Processors/Convolution/BokehBlurTest.cs @@ -59,16 +59,18 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Convolution using (var image = new Image(1, 1)) { var definition = new BokehBlurProcessor(10, BokehBlurProcessor.DefaultComponents, BokehBlurProcessor.DefaultGamma); - var processor = (BokehBlurProcessor)definition.CreatePixelSpecificProcessor(image, image.Bounds()); - Assert.Equal(components.Count, processor.Kernels.Count); - foreach ((Complex64[] a, Complex64[] b) in components.Zip(processor.Kernels, (a, b) => (a, b))) + using (var processor = (BokehBlurProcessor)definition.CreatePixelSpecificProcessor(image, image.Bounds())) { - Span spanA = a.AsSpan(), spanB = b.AsSpan(); - Assert.Equal(spanA.Length, spanB.Length); - for (int i = 0; i < spanA.Length; i++) + Assert.Equal(components.Count, processor.Kernels.Count); + foreach ((Complex64[] a, Complex64[] b) in components.Zip(processor.Kernels, (a, b) => (a, b))) { - Assert.True(Math.Abs(Math.Abs(spanA[i].Real) - Math.Abs(spanB[i].Real)) < 0.0001f); - Assert.True(Math.Abs(Math.Abs(spanA[i].Imaginary) - Math.Abs(spanB[i].Imaginary)) < 0.0001f); + Span spanA = a.AsSpan(), spanB = b.AsSpan(); + Assert.Equal(spanA.Length, spanB.Length); + for (int i = 0; i < spanA.Length; i++) + { + Assert.True(Math.Abs(Math.Abs(spanA[i].Real) - Math.Abs(spanB[i].Real)) < 0.0001f); + Assert.True(Math.Abs(Math.Abs(spanA[i].Imaginary) - Math.Abs(spanB[i].Imaginary)) < 0.0001f); + } } } }