From dc968678346de1e6a675fc19efb5b10667c5f4a6 Mon Sep 17 00:00:00 2001 From: Petar Tasev Date: Sun, 18 Apr 2021 11:56:19 -0700 Subject: [PATCH 1/5] Added overloads to Image.WrapMemory for IMemoryOwner. --- src/ImageSharp/Image.WrapMemory.cs | 76 +++++++++++++++++++ src/ImageSharp/Memory/ByteMemoryOwner{T}.cs | 54 +++++++++++++ .../Image/ImageTests.WrapMemory.cs | 26 +++++++ 3 files changed, 156 insertions(+) create mode 100644 src/ImageSharp/Memory/ByteMemoryOwner{T}.cs diff --git a/src/ImageSharp/Image.WrapMemory.cs b/src/ImageSharp/Image.WrapMemory.cs index b0efdb60d..d7af873be 100644 --- a/src/ImageSharp/Image.WrapMemory.cs +++ b/src/ImageSharp/Image.WrapMemory.cs @@ -303,6 +303,82 @@ namespace SixLabors.ImageSharp where TPixel : unmanaged, IPixel => WrapMemory(Configuration.Default, byteMemory, width, height); + /// + /// Wraps an existing contiguous memory area of at least 'width' x 'height' pixels, + /// 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. + /// + /// The pixel type + /// The + /// The that is being transferred to the image + /// 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, + IMemoryOwner byteMemoryOwner, + int width, + int height, + ImageMetadata metadata) + where TPixel : unmanaged, IPixel + { + Guard.NotNull(configuration, nameof(configuration)); + Guard.NotNull(metadata, nameof(metadata)); + + var pixelMemoryOwner = new ByteMemoryOwner(byteMemoryOwner); + + Guard.IsTrue(pixelMemoryOwner.Memory.Length >= width * height, nameof(pixelMemoryOwner), "The length of the input memory is less than the specified image size"); + + var memorySource = MemoryGroup.Wrap(pixelMemoryOwner); + return new Image(configuration, memorySource, width, height, metadata); + } + + /// + /// Wraps an existing contiguous memory area of at least 'width' x 'height' pixels, + /// 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. + /// + /// The pixel type. + /// The + /// The that is being transferred to the image. + /// The width of the memory image. + /// The height of the memory image. + /// The configuration is null. + /// An instance + public static Image WrapMemory( + Configuration configuration, + IMemoryOwner byteMemoryOwner, + int width, + int height) + where TPixel : unmanaged, IPixel + => WrapMemory(configuration, byteMemoryOwner, width, height, new ImageMetadata()); + + /// + /// Wraps an existing contiguous memory area of at least 'width' x 'height' pixels, + /// 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. + /// + /// The pixel type + /// The that is being transferred to the image. + /// The width of the memory image. + /// The height of the memory image. + /// An instance. + public static Image WrapMemory( + IMemoryOwner byteMemoryOwner, + int width, + int height) + where TPixel : unmanaged, IPixel + => WrapMemory(Configuration.Default, byteMemoryOwner, width, height); + /// /// /// Wraps an existing contiguous memory area of at least 'width' x 'height' pixels allowing viewing/manipulation as diff --git a/src/ImageSharp/Memory/ByteMemoryOwner{T}.cs b/src/ImageSharp/Memory/ByteMemoryOwner{T}.cs new file mode 100644 index 000000000..bcf8eabf1 --- /dev/null +++ b/src/ImageSharp/Memory/ByteMemoryOwner{T}.cs @@ -0,0 +1,54 @@ +using System; +using System.Buffers; +using System.Collections.Generic; +using System.Text; + +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 ByteMemoryOwner : IMemoryOwner + where T : unmanaged + { + private readonly IMemoryOwner memoryOwner; + private readonly ByteMemoryManager memoryManager; + private bool disposedValue; + + /// + /// Initializes a new instance of the class. + /// + /// The of instance to wrap. + public ByteMemoryOwner(IMemoryOwner memoryOwner) + { + this.memoryOwner = memoryOwner; + this.memoryManager = new ByteMemoryManager(memoryOwner.Memory); + } + + /// + public Memory Memory => this.memoryManager.Memory; + + private void Dispose(bool disposing) + { + if (!this.disposedValue) + { + if (disposing) + { + this.memoryOwner.Dispose(); + } + + this.disposedValue = true; + } + } + + /// + public void Dispose() + { + // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method + this.Dispose(disposing: true); + GC.SuppressFinalize(this); + } + } +} diff --git a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs index 17c73cc83..1b40f43ab 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs @@ -413,6 +413,32 @@ namespace SixLabors.ImageSharp.Tests Image.WrapMemory(memory, height, width); } + [Theory] + [InlineData(0, 5, 5)] + [InlineData(20, 5, 5)] + [InlineData(1023, 32, 32)] + public void WrapMemory_IMemoryOwnerOfByte_InvalidSize(int size, int height, int width) + { + var array = new byte[size * Unsafe.SizeOf()]; + var memory = new TestMemoryOwner { Memory = array }; + + Assert.Throws(() => Image.WrapMemory(memory, height, width)); + } + + [Theory] + [InlineData(25, 5, 5)] + [InlineData(26, 5, 5)] + [InlineData(2, 1, 1)] + [InlineData(1024, 32, 32)] + [InlineData(2048, 32, 32)] + public void WrapMemory_IMemoryOwnerOfByte_ValidSize(int size, int height, int width) + { + var array = new byte[size * Unsafe.SizeOf()]; + var memory = new TestMemoryOwner { Memory = array }; + + Image.WrapMemory(memory, height, width); + } + [Theory] [InlineData(0, 5, 5)] [InlineData(20, 5, 5)] From 402782acf5dff895897124e66b6429d810dd9e1e Mon Sep 17 00:00:00 2001 From: Petar Tasev Date: Sun, 18 Apr 2021 12:00:33 -0700 Subject: [PATCH 2/5] Removed unused using statements. --- src/ImageSharp/Memory/ByteMemoryOwner{T}.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/ImageSharp/Memory/ByteMemoryOwner{T}.cs b/src/ImageSharp/Memory/ByteMemoryOwner{T}.cs index bcf8eabf1..a4761740a 100644 --- a/src/ImageSharp/Memory/ByteMemoryOwner{T}.cs +++ b/src/ImageSharp/Memory/ByteMemoryOwner{T}.cs @@ -1,7 +1,5 @@ using System; using System.Buffers; -using System.Collections.Generic; -using System.Text; namespace SixLabors.ImageSharp.Memory { From ae30a49357ee3eeadc305c3aee8fe5c3cdce97fc Mon Sep 17 00:00:00 2001 From: Petar Tasev Date: Sun, 18 Apr 2021 12:03:16 -0700 Subject: [PATCH 3/5] Added file header. --- src/ImageSharp/Memory/ByteMemoryOwner{T}.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ImageSharp/Memory/ByteMemoryOwner{T}.cs b/src/ImageSharp/Memory/ByteMemoryOwner{T}.cs index a4761740a..01262eb58 100644 --- a/src/ImageSharp/Memory/ByteMemoryOwner{T}.cs +++ b/src/ImageSharp/Memory/ByteMemoryOwner{T}.cs @@ -1,3 +1,6 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + using System; using System.Buffers; From 0871b85bdfd6df354d22f18456cb3390c567b8c6 Mon Sep 17 00:00:00 2001 From: Petar Tasev Date: Thu, 22 Apr 2021 19:34:48 -0700 Subject: [PATCH 4/5] Address code review comments. --- src/ImageSharp/Image.WrapMemory.cs | 2 +- src/ImageSharp/Memory/ByteMemoryOwner{T}.cs | 1 - .../Image/ImageTests.WrapMemory.cs | 29 +++++++++++++++---- 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/ImageSharp/Image.WrapMemory.cs b/src/ImageSharp/Image.WrapMemory.cs index d7af873be..115d51921 100644 --- a/src/ImageSharp/Image.WrapMemory.cs +++ b/src/ImageSharp/Image.WrapMemory.cs @@ -332,7 +332,7 @@ namespace SixLabors.ImageSharp var pixelMemoryOwner = new ByteMemoryOwner(byteMemoryOwner); - Guard.IsTrue(pixelMemoryOwner.Memory.Length >= width * height, nameof(pixelMemoryOwner), "The length of the input memory is less than the specified image size"); + Guard.IsTrue(pixelMemoryOwner.Memory.Length >= (long)width * height, nameof(pixelMemoryOwner), "The length of the input memory is less than the specified image size"); var memorySource = MemoryGroup.Wrap(pixelMemoryOwner); return new Image(configuration, memorySource, width, height, metadata); diff --git a/src/ImageSharp/Memory/ByteMemoryOwner{T}.cs b/src/ImageSharp/Memory/ByteMemoryOwner{T}.cs index 01262eb58..3cf62bbc1 100644 --- a/src/ImageSharp/Memory/ByteMemoryOwner{T}.cs +++ b/src/ImageSharp/Memory/ByteMemoryOwner{T}.cs @@ -49,7 +49,6 @@ namespace SixLabors.ImageSharp.Memory { // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method this.Dispose(disposing: true); - GC.SuppressFinalize(this); } } } diff --git a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs index 1b40f43ab..17be2fa2b 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs @@ -380,11 +380,11 @@ namespace SixLabors.ImageSharp.Tests private class TestMemoryOwner : IMemoryOwner { + public bool Disposed { get; private set; } + public Memory Memory { get; set; } - public void Dispose() - { - } + public void Dispose() => this.Disposed = true; } [Theory] @@ -433,10 +433,29 @@ namespace SixLabors.ImageSharp.Tests [InlineData(2048, 32, 32)] public void WrapMemory_IMemoryOwnerOfByte_ValidSize(int size, int height, int width) { - var array = new byte[size * Unsafe.SizeOf()]; + var random = new Random(); + var pixelSize = Unsafe.SizeOf(); + var array = new byte[size * pixelSize]; var memory = new TestMemoryOwner { Memory = array }; - Image.WrapMemory(memory, height, width); + using (var img = Image.WrapMemory(memory, width, height)) + { + Assert.Equal(width, img.Width); + Assert.Equal(height, img.Height); + + for (int i = 0; i < height; ++i) + { + var arrayIndex = pixelSize * width * i; + var expected = (byte)random.Next(0, 256); + + Span rowSpan = img.GetPixelRowSpan(i); + array[arrayIndex] = expected; + + Assert.Equal(expected, rowSpan[0].R); + } + } + + Assert.True(memory.Disposed); } [Theory] From 70c6616fa96dbe0f426d52650ff21f8b188d7d02 Mon Sep 17 00:00:00 2001 From: Petar Tasev Date: Fri, 23 Apr 2021 08:49:22 -0700 Subject: [PATCH 5/5] Changed test to compare mem locations, and added same test to MemOwner of TPixel. --- .../Image/ImageTests.WrapMemory.cs | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs index 17be2fa2b..bb75578a4 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs @@ -410,7 +410,24 @@ namespace SixLabors.ImageSharp.Tests var array = new Rgba32[size]; var memory = new TestMemoryOwner { Memory = array }; - Image.WrapMemory(memory, height, width); + using (var img = Image.WrapMemory(memory, width, height)) + { + Assert.Equal(width, img.Width); + Assert.Equal(height, img.Height); + + for (int i = 0; i < height; ++i) + { + var arrayIndex = width * i; + + Span rowSpan = img.GetPixelRowSpan(i); + ref Rgba32 r0 = ref rowSpan[0]; + ref Rgba32 r1 = ref array[arrayIndex]; + + Assert.True(Unsafe.AreSame(ref r0, ref r1)); + } + } + + Assert.True(memory.Disposed); } [Theory] @@ -433,7 +450,6 @@ namespace SixLabors.ImageSharp.Tests [InlineData(2048, 32, 32)] public void WrapMemory_IMemoryOwnerOfByte_ValidSize(int size, int height, int width) { - var random = new Random(); var pixelSize = Unsafe.SizeOf(); var array = new byte[size * pixelSize]; var memory = new TestMemoryOwner { Memory = array }; @@ -446,12 +462,12 @@ namespace SixLabors.ImageSharp.Tests for (int i = 0; i < height; ++i) { var arrayIndex = pixelSize * width * i; - var expected = (byte)random.Next(0, 256); Span rowSpan = img.GetPixelRowSpan(i); - array[arrayIndex] = expected; + ref Rgba32 r0 = ref rowSpan[0]; + ref Rgba32 r1 = ref Unsafe.As(ref array[arrayIndex]); - Assert.Equal(expected, rowSpan[0].R); + Assert.True(Unsafe.AreSame(ref r0, ref r1)); } }