From b61366ac005a9e7b08731a1a4db16e161bd4bb3d Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 13 Aug 2020 15:49:45 +0200 Subject: [PATCH 01/18] Add ByteMemoryManager type --- src/ImageSharp/Memory/ByteMemoryManager{T}.cs | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 src/ImageSharp/Memory/ByteMemoryManager{T}.cs diff --git a/src/ImageSharp/Memory/ByteMemoryManager{T}.cs b/src/ImageSharp/Memory/ByteMemoryManager{T}.cs new file mode 100644 index 000000000..3a9eb34c2 --- /dev/null +++ b/src/ImageSharp/Memory/ByteMemoryManager{T}.cs @@ -0,0 +1,67 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. +using System; +using System.Buffers; +using System.Runtime.InteropServices; + +namespace SixLabors.ImageSharp.Memory +{ + /// + /// A custom that can wrap of instances + /// and cast them to be for any arbitrary unmanaged value type. + /// + /// The value type to use when casting the wrapped instance. + internal sealed class ByteMemoryManager : MemoryManager + where T : unmanaged + { + /// + /// The wrapped of instance. + /// + private readonly Memory memory; + + /// + /// Initializes a new instance of the class. + /// + /// The of instance to wrap. + public ByteMemoryManager(Memory memory) + { + this.memory = memory; + } + + /// + protected override void Dispose(bool disposing) + { + } + + /// + public override Span GetSpan() + { + if (MemoryMarshal.TryGetArray(this.memory, out ArraySegment arraySegment)) + { + return MemoryMarshal.Cast(arraySegment.AsSpan()); + } + + if (MemoryMarshal.TryGetMemoryManager>(this.memory, out MemoryManager memoryManager)) + { + return MemoryMarshal.Cast(memoryManager.GetSpan()); + } + + // This should never be reached, as Memory can currently only be wrapping + // either a byte[] array or a MemoryManager instance in this case. + ThrowHelper.ThrowArgumentException("The input Memory instance was not valid.", nameof(this.memory)); + + return default; + } + + /// + public override MemoryHandle Pin(int elementIndex = 0) + { + return this.memory.Pin(); + } + + /// + public override void Unpin() + { + } + } +} From cd394109840e0a823b65cd569349279c235bf210 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 13 Aug 2020 15:56:24 +0200 Subject: [PATCH 02/18] Add Image.WrapMemory from Memory --- src/ImageSharp/Image.WrapMemory.cs | 65 ++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/src/ImageSharp/Image.WrapMemory.cs b/src/ImageSharp/Image.WrapMemory.cs index 2d3c29ed4..34bdeafdc 100644 --- a/src/ImageSharp/Image.WrapMemory.cs +++ b/src/ImageSharp/Image.WrapMemory.cs @@ -150,5 +150,70 @@ namespace SixLabors.ImageSharp int height) where TPixel : unmanaged, IPixel => WrapMemory(Configuration.Default, pixelMemoryOwner, width, height); + + /// + /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, + /// allowing to view/manipulate it as an ImageSharp instance. + /// + /// The pixel type + /// The + /// The byte memory representing the pixel data. + /// The width of the memory image. + /// The height of the memory image. + /// The . + /// The configuration is null. + /// The metadata is null. + /// An instance + public static Image WrapMemory( + Configuration configuration, + Memory byteMemory, + int width, + int height, + ImageMetadata metadata) + where TPixel : unmanaged, IPixel + { + Guard.NotNull(configuration, nameof(configuration)); + Guard.NotNull(metadata, nameof(metadata)); + + var memoryManager = new ByteMemoryManager(byteMemory); + var memorySource = MemoryGroup.Wrap(memoryManager.Memory); + return new Image(configuration, memorySource, width, height, metadata); + } + + /// + /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, + /// allowing to view/manipulate it as an ImageSharp instance. + /// + /// The pixel type + /// The + /// The byte memory representing the pixel data. + /// The width of the memory image. + /// The height of the memory image. + /// The configuration is null. + /// An instance. + public static Image WrapMemory( + Configuration configuration, + Memory byteMemory, + int width, + int height) + where TPixel : unmanaged, IPixel + => WrapMemory(configuration, byteMemory, width, height, new ImageMetadata()); + + /// + /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, + /// allowing to view/manipulate it as an ImageSharp instance. + /// The memory is being observed, the caller remains responsible for managing it's lifecycle. + /// + /// The pixel type. + /// The byte memory representing the pixel data. + /// The width of the memory image. + /// The height of the memory image. + /// An instance. + public static Image WrapMemory( + Memory byteMemory, + int width, + int height) + where TPixel : unmanaged, IPixel + => WrapMemory(Configuration.Default, byteMemory, width, height); } } From 129977e4654553bdd1b0fde12a16cb8e8093c3d3 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 13 Aug 2020 16:12:42 +0200 Subject: [PATCH 03/18] Add tests for new Image.WrapMemory APIs --- .../Image/ImageTests.WrapMemory.cs | 107 +++++++++++++++++- 1 file changed, 106 insertions(+), 1 deletion(-) diff --git a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs index 2b30d9459..c0cd3f56a 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs @@ -6,10 +6,10 @@ using System.Buffers; using System.Drawing; using System.Drawing.Imaging; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.Common.Helpers; -using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.PixelFormats; using Xunit; @@ -80,6 +80,52 @@ namespace SixLabors.ImageSharp.Tests } } + public sealed class CastMemoryManager : MemoryManager + where TFrom : unmanaged + where TTo : unmanaged + { + private readonly Memory memory; + + public CastMemoryManager(Memory memory) + { + this.memory = memory; + } + + /// + protected override void Dispose(bool disposing) + { + } + + /// + public override Span GetSpan() + { + if (MemoryMarshal.TryGetArray(this.memory, out ArraySegment arraySegment)) + { + return MemoryMarshal.Cast(arraySegment.AsSpan()); + } + + if (MemoryMarshal.TryGetMemoryManager>(this.memory, out MemoryManager memoryManager)) + { + return MemoryMarshal.Cast(memoryManager.GetSpan()); + } + + ThrowHelper.ThrowArgumentException("The input Memory instance was not valid.", nameof(this.memory)); + + return default; + } + + /// + public override MemoryHandle Pin(int elementIndex = 0) + { + return this.memory.Pin(); + } + + /// + public override void Unpin() + { + } + } + [Fact] public void WrapMemory_CreatedImageIsCorrect() { @@ -173,6 +219,65 @@ namespace SixLabors.ImageSharp.Tests } } + [Fact] + public void WrapMemory_FromBytes_CreatedImageIsCorrect() + { + Configuration cfg = Configuration.Default.Clone(); + var metaData = new ImageMetadata(); + + var array = new byte[25 * Unsafe.SizeOf()]; + var memory = new Memory(array); + + using (var image = Image.WrapMemory(cfg, memory, 5, 5, metaData)) + { + Assert.True(image.TryGetSinglePixelSpan(out Span imageSpan)); + ref Rgba32 pixel0 = ref imageSpan[0]; + Assert.True(Unsafe.AreSame(ref Unsafe.As(ref array[0]), ref pixel0)); + + Assert.Equal(cfg, image.GetConfiguration()); + Assert.Equal(metaData, image.Metadata); + } + } + + [Fact] + public void WrapSystemDrawingBitmap_FromBytes_WhenObserved() + { + if (ShouldSkipBitmapTest) + { + return; + } + + using (var bmp = new Bitmap(51, 23)) + { + using (var memoryManager = new BitmapMemoryManager(bmp)) + { + Memory pixelMemory = memoryManager.Memory; + Memory byteMemory = new CastMemoryManager(pixelMemory).Memory; + Bgra32 bg = Color.Red; + Bgra32 fg = Color.Green; + + using (var image = Image.WrapMemory(byteMemory, bmp.Width, bmp.Height)) + { + Assert.Equal(pixelMemory, image.GetRootFramePixelBuffer().GetSingleMemory()); + Assert.True(image.TryGetSinglePixelSpan(out Span imageSpan)); + imageSpan.Fill(bg); + for (var i = 10; i < 20; i++) + { + image.GetPixelRowSpan(i).Slice(10, 10).Fill(fg); + } + } + + Assert.False(memoryManager.IsDisposed); + } + + string fn = System.IO.Path.Combine( + TestEnvironment.ActualOutputDirectoryFullPath, + $"{nameof(this.WrapSystemDrawingBitmap_WhenObserved)}.bmp"); + + bmp.Save(fn, ImageFormat.Bmp); + } + } + private static bool ShouldSkipBitmapTest => !TestEnvironment.Is64BitProcess || (TestHelpers.ImageSharpBuiltAgainst != "netcoreapp3.1" && TestHelpers.ImageSharpBuiltAgainst != "netcoreapp2.1"); } From 8f4417cf61c92cf400d50f6b4bb05022be7882b6 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 13 Aug 2020 16:30:27 +0200 Subject: [PATCH 04/18] Remove unnecessary indirection in ByteMemoryManager --- src/ImageSharp/Memory/ByteMemoryManager{T}.cs | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/src/ImageSharp/Memory/ByteMemoryManager{T}.cs b/src/ImageSharp/Memory/ByteMemoryManager{T}.cs index 3a9eb34c2..924230fc8 100644 --- a/src/ImageSharp/Memory/ByteMemoryManager{T}.cs +++ b/src/ImageSharp/Memory/ByteMemoryManager{T}.cs @@ -36,21 +36,7 @@ namespace SixLabors.ImageSharp.Memory /// public override Span GetSpan() { - if (MemoryMarshal.TryGetArray(this.memory, out ArraySegment arraySegment)) - { - return MemoryMarshal.Cast(arraySegment.AsSpan()); - } - - if (MemoryMarshal.TryGetMemoryManager>(this.memory, out MemoryManager memoryManager)) - { - return MemoryMarshal.Cast(memoryManager.GetSpan()); - } - - // This should never be reached, as Memory can currently only be wrapping - // either a byte[] array or a MemoryManager instance in this case. - ThrowHelper.ThrowArgumentException("The input Memory instance was not valid.", nameof(this.memory)); - - return default; + return MemoryMarshal.Cast(this.memory.Span); } /// From b9dc71d37534810e03120dc94c26bb8dccdf2e85 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 13 Aug 2020 16:46:05 +0200 Subject: [PATCH 05/18] Fix bug in Pin() method, minor code tweaks --- src/ImageSharp/Memory/ByteMemoryManager{T}.cs | 2 +- .../Image/ImageTests.WrapMemory.cs | 16 ++-------------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/ImageSharp/Memory/ByteMemoryManager{T}.cs b/src/ImageSharp/Memory/ByteMemoryManager{T}.cs index 924230fc8..70d1b1c30 100644 --- a/src/ImageSharp/Memory/ByteMemoryManager{T}.cs +++ b/src/ImageSharp/Memory/ByteMemoryManager{T}.cs @@ -42,7 +42,7 @@ namespace SixLabors.ImageSharp.Memory /// public override MemoryHandle Pin(int elementIndex = 0) { - return this.memory.Pin(); + return this.memory.Slice(elementIndex).Pin(); } /// diff --git a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs index c0cd3f56a..06ee069c6 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs @@ -99,25 +99,13 @@ namespace SixLabors.ImageSharp.Tests /// public override Span GetSpan() { - if (MemoryMarshal.TryGetArray(this.memory, out ArraySegment arraySegment)) - { - return MemoryMarshal.Cast(arraySegment.AsSpan()); - } - - if (MemoryMarshal.TryGetMemoryManager>(this.memory, out MemoryManager memoryManager)) - { - return MemoryMarshal.Cast(memoryManager.GetSpan()); - } - - ThrowHelper.ThrowArgumentException("The input Memory instance was not valid.", nameof(this.memory)); - - return default; + return MemoryMarshal.Cast(this.memory.Span); } /// public override MemoryHandle Pin(int elementIndex = 0) { - return this.memory.Pin(); + return this.memory.Slice(elementIndex).Pin(); } /// From 77a6d08a463265897bc1239de6d1413cbc7ead10 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 13 Aug 2020 17:15:16 +0200 Subject: [PATCH 06/18] Fix relative offsetting in Pin() methods --- src/ImageSharp/Memory/ByteMemoryManager{T}.cs | 6 +++++- tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs | 10 +++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Memory/ByteMemoryManager{T}.cs b/src/ImageSharp/Memory/ByteMemoryManager{T}.cs index 70d1b1c30..223709df6 100644 --- a/src/ImageSharp/Memory/ByteMemoryManager{T}.cs +++ b/src/ImageSharp/Memory/ByteMemoryManager{T}.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. using System; using System.Buffers; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; namespace SixLabors.ImageSharp.Memory @@ -42,7 +43,10 @@ namespace SixLabors.ImageSharp.Memory /// public override MemoryHandle Pin(int elementIndex = 0) { - return this.memory.Slice(elementIndex).Pin(); + // We need to adjust the offset into the wrapped byte segment, + // as the input index refers to the target-cast memory of T. + // We just have to shift this index by the byte size of T. + return this.memory.Slice(elementIndex * Unsafe.SizeOf()).Pin(); } /// diff --git a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs index 06ee069c6..ee8e4b97a 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs @@ -105,7 +105,15 @@ namespace SixLabors.ImageSharp.Tests /// public override MemoryHandle Pin(int elementIndex = 0) { - return this.memory.Slice(elementIndex).Pin(); + int byteOffset = elementIndex * Unsafe.SizeOf(); + int shiftedOffset = Math.DivRem(byteOffset, Unsafe.SizeOf(), out int remainder); + + if (remainder != 0) + { + ThrowHelper.ThrowArgumentException("The input index doesn't result in an aligned item access", nameof(elementIndex)); + } + + return this.memory.Slice(shiftedOffset).Pin(); } /// From 9642d0e14c9f6398abdb0299e5ea16d94bc0e57c Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 13 Aug 2020 17:45:38 +0200 Subject: [PATCH 07/18] Fix comparison in wrapping bytes test --- .../ImageSharp.Tests/Image/ImageTests.WrapMemory.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs index ee8e4b97a..9e1ebbf85 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs @@ -254,8 +254,16 @@ namespace SixLabors.ImageSharp.Tests using (var image = Image.WrapMemory(byteMemory, bmp.Width, bmp.Height)) { - Assert.Equal(pixelMemory, image.GetRootFramePixelBuffer().GetSingleMemory()); - Assert.True(image.TryGetSinglePixelSpan(out Span imageSpan)); + Span pixelSpan = pixelMemory.Span; + Span imageSpan = image.GetRootFramePixelBuffer().GetSingleMemory().Span; + + // We can't compare the two Memory instances directly as they wrap different memory managers. + // To check that the underlying data matches, we can just manually check their lenth, and the + // fact that a reference to the first pixel in both spans is actually the same memory location. + Assert.Equal(pixelSpan.Length, imageSpan.Length); + Assert.True(Unsafe.AreSame(ref pixelSpan.GetPinnableReference(), ref imageSpan.GetPinnableReference())); + + Assert.True(image.TryGetSinglePixelSpan(out imageSpan)); imageSpan.Fill(bg); for (var i = 10; i < 20; i++) { From 0e5ddabb7cd65392315331648a26fe34ac0d90de Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 13 Aug 2020 19:31:04 +0200 Subject: [PATCH 08/18] Replace Default.Clone with CreateDefaultConfiguration --- tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs index 9e1ebbf85..bb22f59a3 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs @@ -125,7 +125,7 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void WrapMemory_CreatedImageIsCorrect() { - Configuration cfg = Configuration.Default.Clone(); + var cfg = Configuration.CreateDefaultInstance(); var metaData = new ImageMetadata(); var array = new Rgba32[25]; @@ -218,7 +218,7 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void WrapMemory_FromBytes_CreatedImageIsCorrect() { - Configuration cfg = Configuration.Default.Clone(); + var cfg = Configuration.CreateDefaultInstance(); var metaData = new ImageMetadata(); var array = new byte[25 * Unsafe.SizeOf()]; From 575402d3312be739e0e16d76e3d550589774c2e9 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 25 Aug 2020 22:35:10 +0200 Subject: [PATCH 09/18] Add input size validation in Image.WrapMemory --- src/ImageSharp/Image.WrapMemory.cs | 5 ++ .../Image/ImageTests.WrapMemory.cs | 51 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/src/ImageSharp/Image.WrapMemory.cs b/src/ImageSharp/Image.WrapMemory.cs index 34bdeafdc..081ed22a1 100644 --- a/src/ImageSharp/Image.WrapMemory.cs +++ b/src/ImageSharp/Image.WrapMemory.cs @@ -38,6 +38,7 @@ namespace SixLabors.ImageSharp { Guard.NotNull(configuration, nameof(configuration)); Guard.NotNull(metadata, nameof(metadata)); + Guard.IsTrue(pixelMemory.Length == width * height, nameof(pixelMemory), "The length of the input memory doesn't match the specified image size"); var memorySource = MemoryGroup.Wrap(pixelMemory); return new Image(configuration, memorySource, width, height, metadata); @@ -105,6 +106,7 @@ namespace SixLabors.ImageSharp { Guard.NotNull(configuration, nameof(configuration)); Guard.NotNull(metadata, nameof(metadata)); + Guard.IsTrue(pixelMemoryOwner.Memory.Length == width * height, nameof(pixelMemoryOwner), "The length of the input memory doesn't match the specified image size"); var memorySource = MemoryGroup.Wrap(pixelMemoryOwner); return new Image(configuration, memorySource, width, height, metadata); @@ -176,6 +178,9 @@ namespace SixLabors.ImageSharp Guard.NotNull(metadata, nameof(metadata)); var memoryManager = new ByteMemoryManager(byteMemory); + + Guard.IsTrue(memoryManager.Memory.Length == width * height, nameof(byteMemory), "The length of the input memory doesn't match the specified image size"); + var memorySource = MemoryGroup.Wrap(memoryManager.Memory); return new Image(configuration, memorySource, width, height, metadata); } diff --git a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs index bb22f59a3..7dc7dbb30 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs @@ -282,6 +282,57 @@ namespace SixLabors.ImageSharp.Tests } } + [Theory] + [InlineData(0, 5, 5)] + [InlineData(20, 5, 5)] + [InlineData(26, 5, 5)] + [InlineData(2, 1, 1)] + [InlineData(1023, 32, 32)] + public void WrapMemory_MemoryOfT_InvalidSize(int size, int height, int width) + { + var array = new Rgba32[size]; + var memory = new Memory(array); + + Assert.Throws(() => Image.WrapMemory(memory, height, width)); + } + + private class TestMemoryOwner : IMemoryOwner + { + public Memory Memory { get; set; } + + public void Dispose() + { + } + } + + [Theory] + [InlineData(0, 5, 5)] + [InlineData(20, 5, 5)] + [InlineData(26, 5, 5)] + [InlineData(2, 1, 1)] + [InlineData(1023, 32, 32)] + public void WrapMemory_IMemoryOwnerOfT_InvalidSize(int size, int height, int width) + { + var array = new Rgba32[size]; + var memory = new TestMemoryOwner { Memory = array }; + + Assert.Throws(() => Image.WrapMemory(memory, height, width)); + } + + [Theory] + [InlineData(0, 5, 5)] + [InlineData(20, 5, 5)] + [InlineData(26, 5, 5)] + [InlineData(2, 1, 1)] + [InlineData(1023, 32, 32)] + public void WrapMemory_MemoryOfByte_InvalidSize(int size, int height, int width) + { + var array = new byte[size * Unsafe.SizeOf()]; + var memory = new Memory(array); + + Assert.Throws(() => Image.WrapMemory(memory, height, width)); + } + private static bool ShouldSkipBitmapTest => !TestEnvironment.Is64BitProcess || (TestHelpers.ImageSharpBuiltAgainst != "netcoreapp3.1" && TestHelpers.ImageSharpBuiltAgainst != "netcoreapp2.1"); } From 5397ab328fa51c5a0d07924b9bdec9773fd0d2d3 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 25 Aug 2020 22:39:21 +0200 Subject: [PATCH 10/18] Minor optimizations, improve XML docs and annotations --- .../Memory/MemoryOwnerExtensions.cs | 37 +++++++++++++++++-- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Memory/MemoryOwnerExtensions.cs b/src/ImageSharp/Memory/MemoryOwnerExtensions.cs index 98fd40e65..0cff94110 100644 --- a/src/ImageSharp/Memory/MemoryOwnerExtensions.cs +++ b/src/ImageSharp/Memory/MemoryOwnerExtensions.cs @@ -3,6 +3,7 @@ using System; using System.Buffers; +using System.Diagnostics.Contracts; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -13,12 +14,29 @@ namespace SixLabors.ImageSharp.Memory /// internal static class MemoryOwnerExtensions { + /// + /// Gets a from an instance. + /// + /// The buffer + /// The + [Pure] + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Span GetSpan(this IMemoryOwner buffer) - => buffer.Memory.Span; + { + return buffer.Memory.Span; + } + /// + /// Gets the length of an internal buffer. + /// + /// The buffer + /// The length of the buffer + [Pure] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static int Length(this IMemoryOwner buffer) - => buffer.GetSpan().Length; + { + return buffer.Memory.Length; + } /// /// Gets a to an offsetted position inside the buffer. @@ -26,6 +44,7 @@ namespace SixLabors.ImageSharp.Memory /// The buffer /// The start /// The + [Pure] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Span Slice(this IMemoryOwner buffer, int start) { @@ -39,6 +58,7 @@ namespace SixLabors.ImageSharp.Memory /// The start /// The length of the slice /// The + [Pure] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Span Slice(this IMemoryOwner buffer, int start, int length) { @@ -55,8 +75,17 @@ namespace SixLabors.ImageSharp.Memory buffer.GetSpan().Clear(); } + /// + /// Gets a reference to the first item in the internal buffer for an instance. + /// + /// The buffer + /// A reference to the first item within the memory wrapped by + [Pure] + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static ref T GetReference(this IMemoryOwner buffer) - where T : struct => - ref MemoryMarshal.GetReference(buffer.GetSpan()); + where T : struct + { + return ref MemoryMarshal.GetReference(buffer.GetSpan()); + } } } From 88a93fafe2b4f7db2086561e6195d3998533be9c Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 25 Aug 2020 22:41:43 +0200 Subject: [PATCH 11/18] Remove unnecessary Memory.Span access --- src/ImageSharp/Processing/Processors/Dithering/OrderedDither.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Processing/Processors/Dithering/OrderedDither.cs b/src/ImageSharp/Processing/Processors/Dithering/OrderedDither.cs index b11411e32..448eb3833 100644 --- a/src/ImageSharp/Processing/Processors/Dithering/OrderedDither.cs +++ b/src/ImageSharp/Processing/Processors/Dithering/OrderedDither.cs @@ -262,7 +262,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Dithering this.source = source; this.bounds = bounds; this.scale = processor.DitherScale; - this.bitDepth = ImageMaths.GetBitsNeededForColorDepth(processor.Palette.Span.Length); + this.bitDepth = ImageMaths.GetBitsNeededForColorDepth(processor.Palette.Length); } [MethodImpl(InliningOptions.ShortMethod)] From 16c9dba5bc4ed336d375efd9270f0a4f8bd54413 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Wed, 26 Aug 2020 01:25:02 +0200 Subject: [PATCH 12/18] Simplify XML docs for WrapMemory APIs --- src/ImageSharp/Image.WrapMemory.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/ImageSharp/Image.WrapMemory.cs b/src/ImageSharp/Image.WrapMemory.cs index 081ed22a1..d89c44dc5 100644 --- a/src/ImageSharp/Image.WrapMemory.cs +++ b/src/ImageSharp/Image.WrapMemory.cs @@ -17,7 +17,7 @@ namespace SixLabors.ImageSharp { /// /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, - /// allowing to view/manipulate it as an ImageSharp instance. + /// allowing to view/manipulate it as an instance. /// /// The pixel type /// The @@ -46,7 +46,7 @@ namespace SixLabors.ImageSharp /// /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, - /// allowing to view/manipulate it as an ImageSharp instance. + /// allowing to view/manipulate it as an instance. /// /// The pixel type /// The @@ -65,7 +65,7 @@ namespace SixLabors.ImageSharp /// /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, - /// allowing to view/manipulate it as an ImageSharp instance. + /// allowing to view/manipulate it as an instance. /// The memory is being observed, the caller remains responsible for managing it's lifecycle. /// /// The pixel type. @@ -82,7 +82,7 @@ namespace SixLabors.ImageSharp /// /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, - /// allowing to view/manipulate it as an ImageSharp instance. + /// allowing to view/manipulate it as an instance. /// The ownership of the is being transferred to the new instance, /// meaning that the caller is not allowed to dispose . /// It will be disposed together with the result image. @@ -114,7 +114,7 @@ namespace SixLabors.ImageSharp /// /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, - /// allowing to view/manipulate it as an ImageSharp instance. + /// allowing to view/manipulate it as an instance. /// The ownership of the is being transferred to the new instance, /// meaning that the caller is not allowed to dispose . /// It will be disposed together with the result image. @@ -136,7 +136,7 @@ namespace SixLabors.ImageSharp /// /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, - /// allowing to view/manipulate it as an ImageSharp instance. + /// allowing to view/manipulate it as an instance. /// The ownership of the is being transferred to the new instance, /// meaning that the caller is not allowed to dispose . /// It will be disposed together with the result image. @@ -155,7 +155,7 @@ namespace SixLabors.ImageSharp /// /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, - /// allowing to view/manipulate it as an ImageSharp instance. + /// allowing to view/manipulate it as an instance. /// /// The pixel type /// The @@ -187,7 +187,7 @@ namespace SixLabors.ImageSharp /// /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, - /// allowing to view/manipulate it as an ImageSharp instance. + /// allowing to view/manipulate it as an instance. /// /// The pixel type /// The @@ -206,7 +206,7 @@ namespace SixLabors.ImageSharp /// /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, - /// allowing to view/manipulate it as an ImageSharp instance. + /// allowing to view/manipulate it as an instance. /// The memory is being observed, the caller remains responsible for managing it's lifecycle. /// /// The pixel type. From c061df958422b527edb5de248ad26fdb73f33e6f Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sun, 30 Aug 2020 13:04:03 +0200 Subject: [PATCH 13/18] Added unit tests for bokeh blur constructor --- .../Processors/Convolution/BokehBlurTest.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/ImageSharp.Tests/Processing/Processors/Convolution/BokehBlurTest.cs b/tests/ImageSharp.Tests/Processing/Processors/Convolution/BokehBlurTest.cs index 490a6ea49..50b8782e4 100644 --- a/tests/ImageSharp.Tests/Processing/Processors/Convolution/BokehBlurTest.cs +++ b/tests/ImageSharp.Tests/Processing/Processors/Convolution/BokehBlurTest.cs @@ -35,6 +35,19 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Convolution 0.02565295+0.01611732j 0.0153483+0.01605112j 0.00698622+0.01370844j 0.00135338+0.00998296j -0.00152245+0.00604545j -0.00227282+0.002851j ]]"; + [Theory] + [InlineData(-10, 2, 3f)] + [InlineData(-1, 2, 3f)] + [InlineData(0, 2, 3f)] + [InlineData(20, -1, 3f)] + [InlineData(20, -0, 3f)] + [InlineData(20, 4, -10f)] + [InlineData(20, 4, 0f)] + public void VerifyBokehBlurProcessorArguments_Fail(int radius, int components, float gamma) + { + Assert.Throws(() => new BokehBlurProcessor(radius, components, gamma)); + } + [Fact] public void VerifyComplexComponents() { From 141abca768c45352a24b2d31bab7c37ba86079a3 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sun, 30 Aug 2020 13:07:06 +0200 Subject: [PATCH 14/18] Added checks to bokeh blur constructor --- .../Processing/Processors/Convolution/BokehBlurProcessor.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor.cs b/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor.cs index 27eca523c..d329641e5 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor.cs @@ -47,6 +47,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution /// public BokehBlurProcessor(int radius, int components, float gamma) { + Guard.MustBeGreaterThan(radius, 0, nameof(radius)); + Guard.MustBeBetweenOrEqualTo(components, 1, 6, nameof(components)); Guard.MustBeGreaterThanOrEqualTo(gamma, 1, nameof(gamma)); this.Radius = radius; From c69a41fe626e651298a0b56d107fef66d0aa9009 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sun, 30 Aug 2020 13:07:36 +0200 Subject: [PATCH 15/18] Skipped checks in default bokeh blur constructor --- .../Processing/Processors/Convolution/BokehBlurProcessor.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor.cs b/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor.cs index d329641e5..8a4c703e0 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor.cs @@ -29,8 +29,10 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution /// Initializes a new instance of the class. /// public BokehBlurProcessor() - : this(DefaultRadius, DefaultComponents, DefaultGamma) { + this.Radius = DefaultRadius; + this.Components = DefaultComponents; + this.Gamma = DefaultGamma; } /// From f0e8a0f6f502aa2c81352308cc5dd4d9d0aa3083 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 1 Sep 2020 00:23:53 +0100 Subject: [PATCH 16/18] Add missing SaveAsync method --- .../Advanced/AdvancedImageExtensions.cs | 4 +- src/ImageSharp/ImageExtensions.cs | 63 ++++++++++++++++--- .../Image/ImageTests.SaveAsync.cs | 31 +++++++++ 3 files changed, 90 insertions(+), 8 deletions(-) diff --git a/src/ImageSharp/Advanced/AdvancedImageExtensions.cs b/src/ImageSharp/Advanced/AdvancedImageExtensions.cs index c5abbda61..54a773be0 100644 --- a/src/ImageSharp/Advanced/AdvancedImageExtensions.cs +++ b/src/ImageSharp/Advanced/AdvancedImageExtensions.cs @@ -23,7 +23,9 @@ namespace SixLabors.ImageSharp.Advanced /// /// The source image. /// The target file path to save the image to. - /// The matching encoder. + /// The file path is null. + /// No encoder available for provided path. + /// The matching . public static IImageEncoder DetectEncoder(this Image source, string filePath) { Guard.NotNull(filePath, nameof(filePath)); diff --git a/src/ImageSharp/ImageExtensions.cs b/src/ImageSharp/ImageExtensions.cs index d40c5c271..75cd32106 100644 --- a/src/ImageSharp/ImageExtensions.cs +++ b/src/ImageSharp/ImageExtensions.cs @@ -18,27 +18,29 @@ namespace SixLabors.ImageSharp public static partial class ImageExtensions { /// - /// Writes the image to the given stream using the currently loaded image format. + /// Writes the image to the given file path using an encoder detected from the path. /// /// The source image. /// The file path to save the image to. /// The path is null. + /// No encoder available for provided path. public static void Save(this Image source, string path) => source.Save(path, source.DetectEncoder(path)); /// - /// Writes the image to the given stream using the currently loaded image format. + /// Writes the image to the given file path using an encoder detected from the path. /// /// The source image. /// The file path to save the image to. /// The token to monitor for cancellation requests. /// The path is null. + /// No encoder available for provided path. /// A representing the asynchronous operation. public static Task SaveAsync(this Image source, string path, CancellationToken cancellationToken = default) => source.SaveAsync(path, source.DetectEncoder(path), cancellationToken); /// - /// Writes the image to the given stream using the currently loaded image format. + /// Writes the image to the given file path using the given image encoder. /// /// The source image. /// The file path to save the image to. @@ -56,7 +58,7 @@ namespace SixLabors.ImageSharp } /// - /// Writes the image to the given stream using the currently loaded image format. + /// Writes the image to the given file path using the given image encoder. /// /// The source image. /// The file path to save the image to. @@ -73,12 +75,15 @@ namespace SixLabors.ImageSharp { Guard.NotNull(path, nameof(path)); Guard.NotNull(encoder, nameof(encoder)); - using Stream fs = source.GetConfiguration().FileSystem.Create(path); - await source.SaveAsync(fs, encoder, cancellationToken).ConfigureAwait(false); + + using (Stream fs = source.GetConfiguration().FileSystem.Create(path)) + { + await source.SaveAsync(fs, encoder, cancellationToken).ConfigureAwait(false); + } } /// - /// Writes the image to the given stream using the currently loaded image format. + /// Writes the image to the given stream using the given image format. /// /// The source image. /// The stream to save the image to. @@ -115,6 +120,50 @@ namespace SixLabors.ImageSharp source.Save(stream, encoder); } + /// + /// Writes the image to the given stream using the given image format. + /// + /// The source image. + /// The stream to save the image to. + /// The format to save the image in. + /// The token to monitor for cancellation requests. + /// The stream is null. + /// The format is null. + /// The stream is not writable. + /// No encoder available for provided format. + /// A representing the asynchronous operation. + public static Task SaveAsync( + this Image source, + Stream stream, + IImageFormat format, + CancellationToken cancellationToken = default) + { + Guard.NotNull(stream, nameof(stream)); + Guard.NotNull(format, nameof(format)); + + if (!stream.CanWrite) + { + throw new NotSupportedException("Cannot write to the stream."); + } + + IImageEncoder encoder = source.GetConfiguration().ImageFormatsManager.FindEncoder(format); + + if (encoder is null) + { + var sb = new StringBuilder(); + sb.AppendLine("No encoder was found for the provided mime type. Registered encoders include:"); + + foreach (KeyValuePair val in source.GetConfiguration().ImageFormatsManager.ImageEncoders) + { + sb.AppendFormat(" - {0} : {1}{2}", val.Key.Name, val.Value.GetType().Name, Environment.NewLine); + } + + throw new NotSupportedException(sb.ToString()); + } + + return source.SaveAsync(stream, encoder, cancellationToken); + } + /// /// Returns a Base64 encoded string from the given image. /// The result is prepended with a Data URI diff --git a/tests/ImageSharp.Tests/Image/ImageTests.SaveAsync.cs b/tests/ImageSharp.Tests/Image/ImageTests.SaveAsync.cs index 40c3b65b5..4e6b002d0 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.SaveAsync.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.SaveAsync.cs @@ -72,6 +72,37 @@ namespace SixLabors.ImageSharp.Tests } } + [Theory] + [InlineData("test.png", "image/png")] + [InlineData("test.tga", "image/tga")] + [InlineData("test.bmp", "image/bmp")] + [InlineData("test.jpg", "image/jpeg")] + [InlineData("test.gif", "image/gif")] + public async Task SaveStreamWithMime(string filename, string mimeType) + { + using (var image = new Image(5, 5)) + { + string ext = Path.GetExtension(filename); + IImageFormat format = image.GetConfiguration().ImageFormatsManager.FindFormatByFileExtension(ext); + Assert.Equal(mimeType, format.DefaultMimeType); + + using (var stream = new MemoryStream()) + { + var asyncStream = new AsyncStreamWrapper(stream, () => false); + await image.SaveAsync(asyncStream, format); + + stream.Position = 0; + + (Image Image, IImageFormat Format) imf = await Image.LoadWithFormatAsync(stream); + + Assert.Equal(format, imf.Format); + Assert.Equal(mimeType, imf.Format.DefaultMimeType); + + imf.Image.Dispose(); + } + } + } + [Fact] public async Task ThrowsWhenDisposed() { From 1aa26a0e17709f7a6d51f07c2fb5c87b85445fed Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 1 Sep 2020 07:48:17 +0100 Subject: [PATCH 17/18] Fix Codecov --- .github/workflows/build-and-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 412b1d807..0e093a834 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -64,7 +64,7 @@ jobs: XUNIT_PATH: .\tests\ImageSharp.Tests # Required for xunit - name: Update Codecov - uses: codecov/codecov-action@v1.0.7 + uses: codecov/codecov-action@v1 if: matrix.options.codecov == true && startsWith(github.repository, 'SixLabors') with: flags: unittests From d6e3f9fbbc02715c14139075805d9d84a68dc885 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 1 Sep 2020 13:21:56 +0200 Subject: [PATCH 18/18] Remove [Pure] attributes --- src/ImageSharp/Memory/MemoryOwnerExtensions.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/ImageSharp/Memory/MemoryOwnerExtensions.cs b/src/ImageSharp/Memory/MemoryOwnerExtensions.cs index 0cff94110..c2551ccf2 100644 --- a/src/ImageSharp/Memory/MemoryOwnerExtensions.cs +++ b/src/ImageSharp/Memory/MemoryOwnerExtensions.cs @@ -3,7 +3,6 @@ using System; using System.Buffers; -using System.Diagnostics.Contracts; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -19,7 +18,6 @@ namespace SixLabors.ImageSharp.Memory /// /// The buffer /// The - [Pure] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Span GetSpan(this IMemoryOwner buffer) { @@ -31,7 +29,6 @@ namespace SixLabors.ImageSharp.Memory /// /// The buffer /// The length of the buffer - [Pure] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static int Length(this IMemoryOwner buffer) { @@ -44,7 +41,6 @@ namespace SixLabors.ImageSharp.Memory /// The buffer /// The start /// The - [Pure] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Span Slice(this IMemoryOwner buffer, int start) { @@ -58,7 +54,6 @@ namespace SixLabors.ImageSharp.Memory /// The start /// The length of the slice /// The - [Pure] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Span Slice(this IMemoryOwner buffer, int start, int length) { @@ -80,7 +75,6 @@ namespace SixLabors.ImageSharp.Memory /// /// The buffer /// A reference to the first item within the memory wrapped by - [Pure] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static ref T GetReference(this IMemoryOwner buffer) where T : struct