-
-
Notifications
You must be signed in to change notification settings - Fork 889
Make processors public, refactor cloning. #1011
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1796862
75d27b5
ab856a3
464598c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| // Copyright (c) Six Labors and contributors. | ||
| // Licensed under the Apache License, Version 2.0. | ||
|
|
||
| using SixLabors.ImageSharp.PixelFormats; | ||
| using SixLabors.Primitives; | ||
|
|
||
| namespace SixLabors.ImageSharp.Processing.Processors | ||
| { | ||
| /// <summary> | ||
| /// The base class for all cloning image processors. | ||
| /// </summary> | ||
| public abstract class CloningImageProcessor : ICloningImageProcessor | ||
| { | ||
| /// <inheritdoc/> | ||
| public abstract ICloningImageProcessor<TPixel> CreatePixelSpecificCloningProcessor<TPixel>(Image<TPixel> source, Rectangle sourceRectangle) | ||
| where TPixel : struct, IPixel<TPixel>; | ||
|
|
||
| /// <inheritdoc/> | ||
| IImageProcessor<TPixel> IImageProcessor.CreatePixelSpecificProcessor<TPixel>(Image<TPixel> source, Rectangle sourceRectangle) | ||
JimBobSquarePants marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| => this.CreatePixelSpecificCloningProcessor(source, sourceRectangle); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,17 +2,21 @@ | |
| // Licensed under the Apache License, Version 2.0. | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using SixLabors.ImageSharp.Advanced; | ||
| using SixLabors.ImageSharp.PixelFormats; | ||
| using SixLabors.Primitives; | ||
|
|
||
| namespace SixLabors.ImageSharp.Processing.Processors | ||
| { | ||
| /// <summary> | ||
| /// Allows the application of processing algorithms to a clone of the original image. | ||
| /// The base class for all pixel specific cloning image processors. | ||
| /// Allows the application of processing algorithms to the image. | ||
| /// The image is cloned before operating upon and the buffers swapped upon completion. | ||
| /// </summary> | ||
| /// <typeparam name="TPixel">The pixel format.</typeparam> | ||
| internal abstract class CloningImageProcessor<TPixel> : ICloningImageProcessor<TPixel> | ||
| public abstract class CloningImageProcessor<TPixel> : ICloningImageProcessor<TPixel> | ||
| where TPixel : struct, IPixel<TPixel> | ||
| { | ||
| /// <summary> | ||
|
|
@@ -38,21 +42,17 @@ protected CloningImageProcessor(Image<TPixel> source, Rectangle sourceRectangle) | |
| protected Rectangle SourceRectangle { get; } | ||
|
|
||
| /// <summary> | ||
| /// Gets the <see cref="ImageSharp.Configuration"/> instance to use when performing operations. | ||
| /// Gets the <see cref="Configuration"/> instance to use when performing operations. | ||
| /// </summary> | ||
| protected Configuration Configuration { get; } | ||
|
|
||
| /// <inheritdoc/> | ||
| public Image<TPixel> CloneAndApply() | ||
| Image<TPixel> ICloningImageProcessor<TPixel>.CloneAndExecute() | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hidden so not to clutter child classes. |
||
| { | ||
| try | ||
| { | ||
| Image<TPixel> clone = this.CreateDestination(); | ||
|
|
||
| if (clone.Frames.Count != this.Source.Frames.Count) | ||
| { | ||
| throw new ImageProcessingException($"An error occurred when processing the image using {this.GetType().Name}. The processor changed the number of frames."); | ||
| } | ||
| Image<TPixel> clone = this.CreateTarget(); | ||
| this.CheckFrameCount(this.Source, clone); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if the checks are necessary. The centralized cloning logic should guarantee that the frame counts are matching.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I actually thought that but was too lazy to check and just merged the code into a single method
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's an iterative way to delete bloated code 😉 |
||
|
|
||
| Configuration configuration = this.Source.GetConfiguration(); | ||
| this.BeforeImageApply(clone); | ||
|
|
@@ -84,17 +84,24 @@ public Image<TPixel> CloneAndApply() | |
| } | ||
|
|
||
| /// <inheritdoc/> | ||
| public void Apply() | ||
| void IImageProcessor<TPixel>.Execute() | ||
| { | ||
| using (Image<TPixel> cloned = this.CloneAndApply()) | ||
| // Create an interim clone of the source image to operate on. | ||
| // Doing this allows for the application of transforms that will alter | ||
| // the dimensions of the image. | ||
| Image<TPixel> clone = default; | ||
| try | ||
| { | ||
| // we now need to move the pixel data/size data from one image base to another | ||
| if (cloned.Frames.Count != this.Source.Frames.Count) | ||
| { | ||
| throw new ImageProcessingException($"An error occurred when processing the image using {this.GetType().Name}. The processor changed the number of frames."); | ||
| } | ||
| clone = ((ICloningImageProcessor<TPixel>)this).CloneAndExecute(); | ||
|
|
||
| this.Source.SwapOrCopyPixelsBuffersFrom(cloned); | ||
| // We now need to move the pixel data/size data from the clone to the source. | ||
| this.CheckFrameCount(this.Source, clone); | ||
| this.Source.SwapOrCopyPixelsBuffersFrom(clone); | ||
| } | ||
| finally | ||
| { | ||
| // Dispose of the clone now that we have swapped the pixel/size data. | ||
| clone?.Dispose(); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -106,10 +113,10 @@ public void Dispose() | |
| } | ||
|
|
||
| /// <summary> | ||
| /// Generates a deep clone of the source image that operations should be applied to. | ||
| /// Gets the size of the target image. | ||
| /// </summary> | ||
| /// <returns>The cloned image.</returns> | ||
| protected virtual Image<TPixel> CreateDestination() => this.Source.Clone(); | ||
| /// <returns>The <see cref="Size"/>.</returns> | ||
| protected abstract Size GetTargetSize(); | ||
|
|
||
| /// <summary> | ||
| /// This method is called before the process is applied to prepare the processor. | ||
|
|
@@ -160,5 +167,30 @@ protected virtual void AfterImageApply(Image<TPixel> destination) | |
| protected virtual void Dispose(bool disposing) | ||
| { | ||
| } | ||
|
|
||
| private Image<TPixel> CreateTarget() | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method was identical in every implementation. The only different was where the target size came from. |
||
| { | ||
| Image<TPixel> source = this.Source; | ||
| Size targetSize = this.GetTargetSize(); | ||
|
|
||
| // We will always be creating the clone even for mutate because we may need to resize the canvas | ||
| IEnumerable<ImageFrame<TPixel>> frames = source.Frames.Select<ImageFrame<TPixel>, ImageFrame<TPixel>>( | ||
| x => new ImageFrame<TPixel>( | ||
| source.GetConfiguration(), | ||
| targetSize.Width, | ||
| targetSize.Height, | ||
| x.Metadata.DeepClone())); | ||
|
|
||
| // Use the overload to prevent an extra frame being added | ||
| return new Image<TPixel>(this.Configuration, source.Metadata.DeepClone(), frames); | ||
| } | ||
|
|
||
| private void CheckFrameCount(Image<TPixel> a, Image<TPixel> b) | ||
| { | ||
| if (a.Frames.Count != b.Frames.Count) | ||
| { | ||
| throw new ImageProcessingException($"An error occurred when processing the image using {this.GetType().Name}. The processor changed the number of frames."); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| // Copyright (c) Six Labors and contributors. | ||
| // Licensed under the Apache License, Version 2.0. | ||
|
|
||
| using SixLabors.ImageSharp.PixelFormats; | ||
| using SixLabors.Primitives; | ||
|
|
||
| namespace SixLabors.ImageSharp.Processing.Processors | ||
| { | ||
| /// <summary> | ||
| /// Defines an algorithm to alter the pixels of a cloned image. | ||
| /// </summary> | ||
| public interface ICloningImageProcessor : IImageProcessor | ||
| { | ||
| /// <summary> | ||
| /// Creates a pixel specific <see cref="ICloningImageProcessor{TPixel}"/> that is capable of executing | ||
| /// the processing algorithm on an <see cref="Image{TPixel}"/>. | ||
| /// </summary> | ||
| /// <typeparam name="TPixel">The pixel type.</typeparam> | ||
| /// <param name="source">The source image. Cannot be null.</param> | ||
| /// <param name="sourceRectangle"> | ||
| /// The <see cref="Rectangle"/> structure that specifies the portion of the image object to draw. | ||
| /// </param> | ||
| /// <returns>The <see cref="ICloningImageProcessor{TPixel}"/></returns> | ||
| ICloningImageProcessor<TPixel> CreatePixelSpecificCloningProcessor<TPixel>(Image<TPixel> source, Rectangle sourceRectangle) | ||
| where TPixel : struct, IPixel<TPixel>; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spotted these when removing the AOT CreateDestination method. No longer required as not virtual.