From f047392bf0efe8e9a04122c316b973c922973974 Mon Sep 17 00:00:00 2001 From: Stefan Nikolei Date: Thu, 9 Feb 2023 13:37:01 +0100 Subject: [PATCH 1/4] Remove nullable disable from various files --- src/ImageSharp/Common/Helpers/Guard.cs | 7 ++--- src/ImageSharp/ImageExtensions.cs | 3 +-- src/ImageSharp/ImageFrame{TPixel}.cs | 26 +++++++++---------- src/ImageSharp/IndexedImageFrame{TPixel}.cs | 3 --- .../Extensions/ProcessingExtensions.cs | 9 +++---- .../Linear/AffineTransformProcessor.cs | 2 +- .../Linear/ProjectiveTransformProcessor.cs | 2 +- .../Transforms/Resize/ResizeProcessor.cs | 2 +- 8 files changed, 24 insertions(+), 30 deletions(-) diff --git a/src/ImageSharp/Common/Helpers/Guard.cs b/src/ImageSharp/Common/Helpers/Guard.cs index 95211ccda8..a485bf9b36 100644 --- a/src/ImageSharp/Common/Helpers/Guard.cs +++ b/src/ImageSharp/Common/Helpers/Guard.cs @@ -1,9 +1,9 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -#nullable disable using System.Runtime.CompilerServices; using SixLabors.ImageSharp; +using SixLabors.ImageSharp.Processing.Processors.Transforms; namespace SixLabors; @@ -17,13 +17,14 @@ internal static partial class Guard /// The type of the value. /// is not a value type. [MethodImpl(InliningOptions.ShortMethod)] - public static void MustBeValueType(TValue value, string parameterName) + public static void SamplerMustBeValueType(TValue value, [CallerArgumentExpression("value")] string? parameterName = null) + where TValue : IResampler { if (value.GetType().IsValueType) { return; } - ThrowHelper.ThrowArgumentException("Type must be a struct.", parameterName); + ThrowHelper.ThrowArgumentException("Type must be a struct.", parameterName!); } } diff --git a/src/ImageSharp/ImageExtensions.cs b/src/ImageSharp/ImageExtensions.cs index 04930a2689..cf970b3166 100644 --- a/src/ImageSharp/ImageExtensions.cs +++ b/src/ImageSharp/ImageExtensions.cs @@ -1,6 +1,5 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -#nullable disable using System.Globalization; using System.Text; @@ -180,6 +179,6 @@ public static partial class ImageExtensions // Always available. stream.TryGetBuffer(out ArraySegment buffer); - return $"data:{format.DefaultMimeType};base64,{Convert.ToBase64String(buffer.Array, 0, (int)stream.Length)}"; + return $"data:{format.DefaultMimeType};base64,{Convert.ToBase64String(buffer.Array ?? Array.Empty(), 0, (int)stream.Length)}"; } } diff --git a/src/ImageSharp/ImageFrame{TPixel}.cs b/src/ImageSharp/ImageFrame{TPixel}.cs index fb4ee6ae5f..becd69a0eb 100644 --- a/src/ImageSharp/ImageFrame{TPixel}.cs +++ b/src/ImageSharp/ImageFrame{TPixel}.cs @@ -1,6 +1,5 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -#nullable disable using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -183,7 +182,7 @@ public sealed class ImageFrame : ImageFrame, IPixelSource try { - var accessor = new PixelAccessor(this.PixelBuffer); + PixelAccessor accessor = new(this.PixelBuffer); processPixels(accessor); } finally @@ -211,8 +210,8 @@ public sealed class ImageFrame : ImageFrame, IPixelSource try { - var accessor1 = new PixelAccessor(this.PixelBuffer); - var accessor2 = new PixelAccessor(frame2.PixelBuffer); + PixelAccessor accessor1 = new(this.PixelBuffer); + PixelAccessor accessor2 = new(frame2.PixelBuffer); processPixels(accessor1, accessor2); } finally @@ -247,9 +246,9 @@ public sealed class ImageFrame : ImageFrame, IPixelSource try { - var accessor1 = new PixelAccessor(this.PixelBuffer); - var accessor2 = new PixelAccessor(frame2.PixelBuffer); - var accessor3 = new PixelAccessor(frame3.PixelBuffer); + PixelAccessor accessor1 = new(this.PixelBuffer); + PixelAccessor accessor2 = new(frame2.PixelBuffer); + PixelAccessor accessor3 = new(frame3.PixelBuffer); processPixels(accessor1, accessor2, accessor3); } finally @@ -342,8 +341,7 @@ public sealed class ImageFrame : ImageFrame, IPixelSource if (disposing) { - this.PixelBuffer?.Dispose(); - this.PixelBuffer = null; + this.PixelBuffer.Dispose(); } this.isDisposed = true; @@ -379,14 +377,14 @@ public sealed class ImageFrame : ImageFrame, IPixelSource /// /// The configuration providing initialization code which allows extending the library. /// The - internal ImageFrame Clone(Configuration configuration) => new ImageFrame(configuration, this); + internal ImageFrame Clone(Configuration configuration) => new(configuration, this); /// /// Returns a copy of the image frame in the given pixel format. /// /// The pixel format. /// The - internal ImageFrame CloneAs() + internal ImageFrame? CloneAs() where TPixel2 : unmanaged, IPixel => this.CloneAs(this.GetConfiguration()); /// @@ -400,11 +398,11 @@ public sealed class ImageFrame : ImageFrame, IPixelSource { if (typeof(TPixel2) == typeof(TPixel)) { - return this.Clone(configuration) as ImageFrame; + return (this.Clone(configuration) as ImageFrame)!; } - var target = new ImageFrame(configuration, this.Width, this.Height, this.Metadata.DeepClone()); - var operation = new RowIntervalOperation(this.PixelBuffer, target.PixelBuffer, configuration); + ImageFrame target = new(configuration, this.Width, this.Height, this.Metadata.DeepClone()); + RowIntervalOperation operation = new(this.PixelBuffer, target.PixelBuffer, configuration); ParallelRowIterator.IterateRowIntervals( configuration, diff --git a/src/ImageSharp/IndexedImageFrame{TPixel}.cs b/src/ImageSharp/IndexedImageFrame{TPixel}.cs index 741b3219e0..12a78a0239 100644 --- a/src/ImageSharp/IndexedImageFrame{TPixel}.cs +++ b/src/ImageSharp/IndexedImageFrame{TPixel}.cs @@ -1,6 +1,5 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -#nullable disable using System.Buffers; using System.Runtime.CompilerServices; @@ -109,8 +108,6 @@ public sealed class IndexedImageFrame : IPixelSource, IDisposable this.isDisposed = true; this.pixelBuffer.Dispose(); this.paletteOwner.Dispose(); - this.pixelBuffer = null; - this.paletteOwner = null; } } } diff --git a/src/ImageSharp/Processing/Extensions/ProcessingExtensions.cs b/src/ImageSharp/Processing/Extensions/ProcessingExtensions.cs index 575ce293ca..02e688f57f 100644 --- a/src/ImageSharp/Processing/Extensions/ProcessingExtensions.cs +++ b/src/ImageSharp/Processing/Extensions/ProcessingExtensions.cs @@ -1,6 +1,5 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -#nullable disable using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.PixelFormats; @@ -135,7 +134,7 @@ public static partial class ProcessingExtensions /// The operation is null. /// The source has been disposed. /// The processing operation failed. - public static Image Clone(this Image source, Action operation) + public static Image? Clone(this Image source, Action operation) => Clone(source, source.GetConfiguration(), operation); /// @@ -150,14 +149,14 @@ public static partial class ProcessingExtensions /// The source has been disposed. /// The processing operation failed. /// The new . - public static Image Clone(this Image source, Configuration configuration, Action operation) + public static Image? Clone(this Image source, Configuration configuration, Action operation) { Guard.NotNull(configuration, nameof(configuration)); Guard.NotNull(source, nameof(source)); Guard.NotNull(operation, nameof(operation)); source.EnsureNotDisposed(); - var visitor = new ProcessingVisitor(configuration, operation, false); + ProcessingVisitor visitor = new(configuration, operation, false); source.AcceptVisitor(visitor); return visitor.ResultImage; } @@ -282,7 +281,7 @@ public static partial class ProcessingExtensions this.mutate = mutate; } - public Image ResultImage { get; private set; } + public Image? ResultImage { get; private set; } public void Visit(Image image) where TPixel : unmanaged, IPixel diff --git a/src/ImageSharp/Processing/Processors/Transforms/Linear/AffineTransformProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/Linear/AffineTransformProcessor.cs index abb8beccdf..4a13e69174 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Linear/AffineTransformProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Linear/AffineTransformProcessor.cs @@ -19,7 +19,7 @@ public class AffineTransformProcessor : CloningImageProcessor public AffineTransformProcessor(Matrix3x2 matrix, IResampler sampler, Size targetDimensions) { Guard.NotNull(sampler, nameof(sampler)); - Guard.MustBeValueType(sampler, nameof(sampler)); + Guard.SamplerMustBeValueType(sampler); if (TransformUtils.IsDegenerate(matrix)) { diff --git a/src/ImageSharp/Processing/Processors/Transforms/Linear/ProjectiveTransformProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/Linear/ProjectiveTransformProcessor.cs index 9b0b979e69..55e3227fb7 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Linear/ProjectiveTransformProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Linear/ProjectiveTransformProcessor.cs @@ -19,7 +19,7 @@ public sealed class ProjectiveTransformProcessor : CloningImageProcessor public ProjectiveTransformProcessor(Matrix4x4 matrix, IResampler sampler, Size targetDimensions) { Guard.NotNull(sampler, nameof(sampler)); - Guard.MustBeValueType(sampler, nameof(sampler)); + Guard.SamplerMustBeValueType(sampler); if (TransformUtils.IsDegenerate(matrix)) { diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs index b7651c03f3..f056e434ed 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs @@ -17,7 +17,7 @@ public class ResizeProcessor : CloningImageProcessor { Guard.NotNull(options, nameof(options)); Guard.NotNull(options.Sampler, nameof(options.Sampler)); - Guard.MustBeValueType(options.Sampler, nameof(options.Sampler)); + Guard.SamplerMustBeValueType(options.Sampler); (Size size, Rectangle rectangle) = ResizeHelper.CalculateTargetLocationAndBounds(sourceSize, options); From 0e40d718953d5ad6d50e4df7f28c07dd0a9e885f Mon Sep 17 00:00:00 2001 From: Stefan Nikolei Date: Fri, 10 Feb 2023 06:42:43 +0100 Subject: [PATCH 2/4] Change Guard removed the generic constrained of IResampler and added notnull --- src/ImageSharp/Common/Helpers/Guard.cs | 5 ++--- .../Processors/Transforms/Linear/AffineTransformProcessor.cs | 2 +- .../Transforms/Linear/ProjectiveTransformProcessor.cs | 2 +- .../Processors/Transforms/Resize/ResizeProcessor.cs | 2 +- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp/Common/Helpers/Guard.cs b/src/ImageSharp/Common/Helpers/Guard.cs index a485bf9b36..96cd094cd8 100644 --- a/src/ImageSharp/Common/Helpers/Guard.cs +++ b/src/ImageSharp/Common/Helpers/Guard.cs @@ -3,7 +3,6 @@ using System.Runtime.CompilerServices; using SixLabors.ImageSharp; -using SixLabors.ImageSharp.Processing.Processors.Transforms; namespace SixLabors; @@ -17,8 +16,8 @@ internal static partial class Guard /// The type of the value. /// is not a value type. [MethodImpl(InliningOptions.ShortMethod)] - public static void SamplerMustBeValueType(TValue value, [CallerArgumentExpression("value")] string? parameterName = null) - where TValue : IResampler + public static void MustBeValueType(TValue value, [CallerArgumentExpression("value")] string? parameterName = null) + where TValue : notnull { if (value.GetType().IsValueType) { diff --git a/src/ImageSharp/Processing/Processors/Transforms/Linear/AffineTransformProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/Linear/AffineTransformProcessor.cs index 4a13e69174..30c279e593 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Linear/AffineTransformProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Linear/AffineTransformProcessor.cs @@ -19,7 +19,7 @@ public class AffineTransformProcessor : CloningImageProcessor public AffineTransformProcessor(Matrix3x2 matrix, IResampler sampler, Size targetDimensions) { Guard.NotNull(sampler, nameof(sampler)); - Guard.SamplerMustBeValueType(sampler); + Guard.MustBeValueType(sampler); if (TransformUtils.IsDegenerate(matrix)) { diff --git a/src/ImageSharp/Processing/Processors/Transforms/Linear/ProjectiveTransformProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/Linear/ProjectiveTransformProcessor.cs index 55e3227fb7..9e9507b737 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Linear/ProjectiveTransformProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Linear/ProjectiveTransformProcessor.cs @@ -19,7 +19,7 @@ public sealed class ProjectiveTransformProcessor : CloningImageProcessor public ProjectiveTransformProcessor(Matrix4x4 matrix, IResampler sampler, Size targetDimensions) { Guard.NotNull(sampler, nameof(sampler)); - Guard.SamplerMustBeValueType(sampler); + Guard.MustBeValueType(sampler); if (TransformUtils.IsDegenerate(matrix)) { diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs index f056e434ed..719622a28f 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs @@ -17,7 +17,7 @@ public class ResizeProcessor : CloningImageProcessor { Guard.NotNull(options, nameof(options)); Guard.NotNull(options.Sampler, nameof(options.Sampler)); - Guard.SamplerMustBeValueType(options.Sampler); + Guard.MustBeValueType(options.Sampler); (Size size, Rectangle rectangle) = ResizeHelper.CalculateTargetLocationAndBounds(sourceSize, options); From a04526ff888bb06149a7af9d702e80f7727c3a9f Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 21 Feb 2023 15:30:09 +1000 Subject: [PATCH 3/4] Minor cleanup --- src/ImageSharp/ImageFrame{TPixel}.cs | 9 ++++----- src/ImageSharp/IndexedImageFrame{TPixel}.cs | 4 ++-- .../Processing/Extensions/ProcessingExtensions.cs | 12 +++++++----- .../Processing/ProjectiveTransformBuilder.cs | 4 ++-- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/ImageSharp/ImageFrame{TPixel}.cs b/src/ImageSharp/ImageFrame{TPixel}.cs index becd69a0eb..3734402d30 100644 --- a/src/ImageSharp/ImageFrame{TPixel}.cs +++ b/src/ImageSharp/ImageFrame{TPixel}.cs @@ -144,7 +144,7 @@ public sealed class ImageFrame : ImageFrame, IPixelSource } /// - public Buffer2D PixelBuffer { get; private set; } + public Buffer2D PixelBuffer { get; } /// /// Gets or sets the pixel at the specified position. @@ -309,6 +309,7 @@ public sealed class ImageFrame : ImageFrame, IPixelSource /// Copies the pixels to a of the same size. /// /// The target pixel buffer accessor. + /// ImageFrame{TPixel}.CopyTo(): target must be of the same size! internal void CopyTo(Buffer2D target) { if (this.Size() != target.Size()) @@ -445,14 +446,12 @@ public sealed class ImageFrame : ImageFrame, IPixelSource } [MethodImpl(InliningOptions.ColdPath)] - private static void ThrowArgumentOutOfRangeException(string paramName) - { - throw new ArgumentOutOfRangeException(paramName); - } + private static void ThrowArgumentOutOfRangeException(string paramName) => throw new ArgumentOutOfRangeException(paramName); /// /// A implementing the clone logic for . /// + /// The type of the target pixel format. private readonly struct RowIntervalOperation : IRowIntervalOperation where TPixel2 : unmanaged, IPixel { diff --git a/src/ImageSharp/IndexedImageFrame{TPixel}.cs b/src/ImageSharp/IndexedImageFrame{TPixel}.cs index 12a78a0239..6807e77ad2 100644 --- a/src/ImageSharp/IndexedImageFrame{TPixel}.cs +++ b/src/ImageSharp/IndexedImageFrame{TPixel}.cs @@ -17,8 +17,8 @@ namespace SixLabors.ImageSharp; public sealed class IndexedImageFrame : IPixelSource, IDisposable where TPixel : unmanaged, IPixel { - private Buffer2D pixelBuffer; - private IMemoryOwner paletteOwner; + private readonly Buffer2D pixelBuffer; + private readonly IMemoryOwner paletteOwner; private bool isDisposed; /// diff --git a/src/ImageSharp/Processing/Extensions/ProcessingExtensions.cs b/src/ImageSharp/Processing/Extensions/ProcessingExtensions.cs index 02e688f57f..f830ddfd09 100644 --- a/src/ImageSharp/Processing/Extensions/ProcessingExtensions.cs +++ b/src/ImageSharp/Processing/Extensions/ProcessingExtensions.cs @@ -134,7 +134,7 @@ public static partial class ProcessingExtensions /// The operation is null. /// The source has been disposed. /// The processing operation failed. - public static Image? Clone(this Image source, Action operation) + public static Image Clone(this Image source, Action operation) => Clone(source, source.GetConfiguration(), operation); /// @@ -149,7 +149,7 @@ public static partial class ProcessingExtensions /// The source has been disposed. /// The processing operation failed. /// The new . - public static Image? Clone(this Image source, Configuration configuration, Action operation) + public static Image Clone(this Image source, Configuration configuration, Action operation) { Guard.NotNull(configuration, nameof(configuration)); Guard.NotNull(source, nameof(source)); @@ -158,7 +158,7 @@ public static partial class ProcessingExtensions ProcessingVisitor visitor = new(configuration, operation, false); source.AcceptVisitor(visitor); - return visitor.ResultImage; + return visitor.GetResultImage(); } /// @@ -274,6 +274,8 @@ public static partial class ProcessingExtensions private readonly bool mutate; + private Image? resultImage; + public ProcessingVisitor(Configuration configuration, Action operation, bool mutate) { this.configuration = configuration; @@ -281,7 +283,7 @@ public static partial class ProcessingExtensions this.mutate = mutate; } - public Image? ResultImage { get; private set; } + public Image GetResultImage() => this.resultImage!; public void Visit(Image image) where TPixel : unmanaged, IPixel @@ -290,7 +292,7 @@ public static partial class ProcessingExtensions this.configuration.ImageOperationsProvider.CreateImageProcessingContext(this.configuration, image, this.mutate); this.operation(operationsRunner); - this.ResultImage = operationsRunner.GetResultImage(); + this.resultImage = operationsRunner.GetResultImage(); } } } diff --git a/src/ImageSharp/Processing/ProjectiveTransformBuilder.cs b/src/ImageSharp/Processing/ProjectiveTransformBuilder.cs index 44d6d8e720..ad0888ad77 100644 --- a/src/ImageSharp/Processing/ProjectiveTransformBuilder.cs +++ b/src/ImageSharp/Processing/ProjectiveTransformBuilder.cs @@ -11,7 +11,7 @@ namespace SixLabors.ImageSharp.Processing; /// public class ProjectiveTransformBuilder { - private readonly List> matrixFactories = new List>(); + private readonly List> matrixFactories = new(); /// /// Prepends a matrix that performs a tapering projective transform. @@ -313,7 +313,7 @@ public class ProjectiveTransformBuilder Guard.MustBeGreaterThan(sourceRectangle.Height, 0, nameof(sourceRectangle)); // Translate the origin matrix to cater for source rectangle offsets. - var matrix = Matrix4x4.CreateTranslation(new Vector3(-sourceRectangle.Location, 0)); + Matrix4x4 matrix = Matrix4x4.CreateTranslation(new Vector3(-sourceRectangle.Location, 0)); Size size = sourceRectangle.Size; From 1a9db45a8500799934ae7645d9f50c68f5fdb1fa Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 21 Feb 2023 15:47:42 +1000 Subject: [PATCH 4/4] Update Buffer2D to allow disposal testing. --- src/ImageSharp/Memory/Buffer2D{T}.cs | 8 +++++++- .../Image/ImageFrameCollectionTests.Generic.cs | 8 ++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/ImageSharp/Memory/Buffer2D{T}.cs b/src/ImageSharp/Memory/Buffer2D{T}.cs index 8d6465389f..1173e02e17 100644 --- a/src/ImageSharp/Memory/Buffer2D{T}.cs +++ b/src/ImageSharp/Memory/Buffer2D{T}.cs @@ -55,6 +55,8 @@ public sealed class Buffer2D : IDisposable /// internal MemoryGroup FastMemoryGroup { get; private set; } + internal bool IsDisposed { get; private set; } + /// /// Gets a reference to the element at the specified position. /// @@ -79,7 +81,11 @@ public sealed class Buffer2D : IDisposable /// /// Disposes the instance /// - public void Dispose() => this.FastMemoryGroup.Dispose(); + public void Dispose() + { + this.FastMemoryGroup.Dispose(); + this.IsDisposed = true; + } /// /// Gets a to the row 'y' beginning from the pixel at the first pixel on that row. diff --git a/tests/ImageSharp.Tests/Image/ImageFrameCollectionTests.Generic.cs b/tests/ImageSharp.Tests/Image/ImageFrameCollectionTests.Generic.cs index fba19065b0..62d1ef4db9 100644 --- a/tests/ImageSharp.Tests/Image/ImageFrameCollectionTests.Generic.cs +++ b/tests/ImageSharp.Tests/Image/ImageFrameCollectionTests.Generic.cs @@ -182,12 +182,12 @@ public abstract partial class ImageFrameCollectionTests new[] { imageFrame1, imageFrame2 }); IPixelSource[] framesSnapShot = collection.OfType>().ToArray(); + + Assert.All(framesSnapShot, f => Assert.False(f.PixelBuffer.IsDisposed)); + collection.Dispose(); - Assert.All( - framesSnapShot, - f => // The pixel source of the frame is null after its been disposed. - Assert.Null(f.PixelBuffer)); + Assert.All(framesSnapShot, f => Assert.True(f.PixelBuffer.IsDisposed)); } [Theory]