From c9e17d896644503b8e0db2c40de859214ef4cd0a Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Sun, 5 Mar 2017 17:56:30 +0100 Subject: [PATCH] PixelAccessor and PixelArea using PinnedBuffer --- .../Colors/PackedPixel/BulkPixelOperations.cs | 168 ------------ .../BulkPixelOperations{TColor}.cs | 256 ++++++++++++++++++ .../Common/Memory/ArrayPointer{T}.cs | 32 +-- src/ImageSharp/Common/Memory/PinnedBuffer.cs | 89 ++++-- .../Common/Memory/PixelDataPool{T}.cs | 60 ++++ src/ImageSharp/Image/ImageBase{TColor}.cs | 10 +- src/ImageSharp/Image/PixelAccessor{TColor}.cs | 139 ++++------ src/ImageSharp/Image/PixelArea{TColor}.cs | 108 ++------ src/ImageSharp/Image/PixelPool{TColor}.cs | 43 --- .../ImageSharp.Sandbox46.csproj | 6 + .../Common/PinnedBufferTests.cs | 25 ++ .../ImageSharp.Tests/Image/PixelPoolTests.cs | 49 +++- 12 files changed, 552 insertions(+), 433 deletions(-) delete mode 100644 src/ImageSharp/Colors/PackedPixel/BulkPixelOperations.cs create mode 100644 src/ImageSharp/Colors/PackedPixel/BulkPixelOperations{TColor}.cs create mode 100644 src/ImageSharp/Common/Memory/PixelDataPool{T}.cs delete mode 100644 src/ImageSharp/Image/PixelPool{TColor}.cs diff --git a/src/ImageSharp/Colors/PackedPixel/BulkPixelOperations.cs b/src/ImageSharp/Colors/PackedPixel/BulkPixelOperations.cs deleted file mode 100644 index ffa28fc132..0000000000 --- a/src/ImageSharp/Colors/PackedPixel/BulkPixelOperations.cs +++ /dev/null @@ -1,168 +0,0 @@ -namespace ImageSharp -{ - using System; - using System.Numerics; - using System.Runtime.CompilerServices; - - public unsafe class BulkPixelOperations - where TColor : struct, IPixel - { - public static BulkPixelOperations Instance { get; } = default(TColor).BulkOperations; - - private static readonly int ColorSize = Unsafe.SizeOf(); - - internal virtual void PackFromVector4( - ArrayPointer sourceVectors, - ArrayPointer destColors, - int count) - { - Vector4* sp = (Vector4*)sourceVectors.PointerAtOffset; - byte* dp = (byte*)destColors; - - for (int i = 0; i < count; i++) - { - Vector4 v = Unsafe.Read(sp); - TColor c = default(TColor); - c.PackFromVector4(v); - Unsafe.Write(dp, c); - - sp++; - dp += ColorSize; - } - } - - internal virtual void PackToVector4( - ArrayPointer sourceColors, - ArrayPointer destVectors, - int count) - { - byte* sp = (byte*)sourceColors; - Vector4* dp = (Vector4*)destVectors.PointerAtOffset; - - for (int i = 0; i < count; i++) - { - TColor c = Unsafe.Read(sp); - *dp = c.ToVector4(); - sp += ColorSize; - dp++; - } - } - - internal virtual void PackFromXyzBytes( - ArrayPointer sourceBytes, - ArrayPointer destColors, - int count) - { - byte* sp = (byte*)sourceBytes; - byte* dp = (byte*)destColors.PointerAtOffset; - - for (int i = 0; i < count; i++) - { - TColor c = default(TColor); - c.PackFromBytes(sp[0], sp[1], sp[2], 255); - Unsafe.Write(dp, c); - sp += 3; - dp += ColorSize; - } - } - - internal virtual void PackToXyzBytes( - ArrayPointer sourceColors, - ArrayPointer destBytes, int count) - { - byte* sp = (byte*)sourceColors; - byte[] dest = destBytes.Array; - - for (int i = destBytes.Offset; i < destBytes.Offset + count*3; i+=3) - { - TColor c = Unsafe.Read(sp); - c.ToXyzBytes(dest, i); - sp += ColorSize; - } - } - - internal virtual void PackFromXyzwBytes(ArrayPointer sourceBytes, ArrayPointer destColors, int count) - { - byte* sp = (byte*)sourceBytes; - byte* dp = (byte*)destColors.PointerAtOffset; - - for (int i = 0; i < count; i++) - { - TColor c = default(TColor); - c.PackFromBytes(sp[0], sp[1], sp[2], sp[3]); - Unsafe.Write(dp, c); - sp += 4; - dp += ColorSize; - } - } - - internal virtual void PackToXyzwBytes(ArrayPointer sourceColors, ArrayPointer destBytes, int count) - { - byte* sp = (byte*)sourceColors; - byte[] dest = destBytes.Array; - - for (int i = destBytes.Offset; i < destBytes.Offset + count * 4; i += 4) - { - TColor c = Unsafe.Read(sp); - c.ToXyzwBytes(dest, i); - sp += ColorSize; - } - } - - internal virtual void PackFromZyxBytes(ArrayPointer sourceBytes, ArrayPointer destColors, int count) - { - byte* sp = (byte*)sourceBytes; - byte* dp = (byte*)destColors.PointerAtOffset; - - for (int i = 0; i < count; i++) - { - TColor c = default(TColor); - c.PackFromBytes(sp[2], sp[1], sp[0], 255); - Unsafe.Write(dp, c); - sp += 3; - dp += ColorSize; - } - } - - internal virtual void PackToZyxBytes(ArrayPointer sourceColors, ArrayPointer destBytes, int count) - { - byte* sp = (byte*)sourceColors; - byte[] dest = destBytes.Array; - - for (int i = destBytes.Offset; i < destBytes.Offset + count * 3; i += 3) - { - TColor c = Unsafe.Read(sp); - c.ToZyxBytes(dest, i); - sp += ColorSize; - } - } - - internal virtual void PackFromZyxwBytes(ArrayPointer sourceBytes, ArrayPointer destColors, int count) - { - byte* sp = (byte*)sourceBytes; - byte* dp = (byte*)destColors.PointerAtOffset; - - for (int i = 0; i < count; i++) - { - TColor c = default(TColor); - c.PackFromBytes(sp[2], sp[1], sp[0], sp[3]); - Unsafe.Write(dp, c); - sp += 4; - dp += ColorSize; - } - } - - internal virtual void PackToZyxwBytes(ArrayPointer sourceColors, ArrayPointer destBytes, int count) - { - byte* sp = (byte*)sourceColors; - byte[] dest = destBytes.Array; - - for (int i = destBytes.Offset; i < destBytes.Offset + count * 4; i += 4) - { - TColor c = Unsafe.Read(sp); - c.ToZyxwBytes(dest, i); - sp += ColorSize; - } - } - } -} \ No newline at end of file diff --git a/src/ImageSharp/Colors/PackedPixel/BulkPixelOperations{TColor}.cs b/src/ImageSharp/Colors/PackedPixel/BulkPixelOperations{TColor}.cs new file mode 100644 index 0000000000..557d59a16c --- /dev/null +++ b/src/ImageSharp/Colors/PackedPixel/BulkPixelOperations{TColor}.cs @@ -0,0 +1,256 @@ +// +// Copyright (c) James Jackson-South and contributors. +// Licensed under the Apache License, Version 2.0. +// + +namespace ImageSharp +{ + using System.Numerics; + using System.Runtime.CompilerServices; + + /// + /// A stateless class implementing Strategy Pattern for batched pixel-data conversion operations + /// for pixel buffers of type . + /// + /// The pixel format. + public unsafe class BulkPixelOperations + where TColor : struct, IPixel + { + /// + /// The size of in bytes + /// + private static readonly int ColorSize = Unsafe.SizeOf(); + + /// + /// Gets the global instance for the pixel type + /// + public static BulkPixelOperations Instance { get; } = default(TColor).BulkOperations; + + /// + /// Bulk version of + /// + /// The to the source vectors. + /// The to the destination colors. + /// The number of pixels to convert. + internal virtual void PackFromVector4( + ArrayPointer sourceVectors, + ArrayPointer destColors, + int count) + { + Vector4* sp = (Vector4*)sourceVectors.PointerAtOffset; + byte* dp = (byte*)destColors; + + for (int i = 0; i < count; i++) + { + Vector4 v = Unsafe.Read(sp); + TColor c = default(TColor); + c.PackFromVector4(v); + Unsafe.Write(dp, c); + + sp++; + dp += ColorSize; + } + } + + /// + /// Bulk version of . + /// + /// The to the source colors. + /// The to the destination vectors. + /// The number of pixels to convert. + internal virtual void PackToVector4( + ArrayPointer sourceColors, + ArrayPointer destVectors, + int count) + { + byte* sp = (byte*)sourceColors; + Vector4* dp = (Vector4*)destVectors.PointerAtOffset; + + for (int i = 0; i < count; i++) + { + TColor c = Unsafe.Read(sp); + *dp = c.ToVector4(); + sp += ColorSize; + dp++; + } + } + + /// + /// Bulk version of that converts data in . + /// + /// + /// + /// + internal virtual void PackFromXyzBytes( + ArrayPointer sourceBytes, + ArrayPointer destColors, + int count) + { + byte* sp = (byte*)sourceBytes; + byte* dp = (byte*)destColors.PointerAtOffset; + + for (int i = 0; i < count; i++) + { + TColor c = default(TColor); + c.PackFromBytes(sp[0], sp[1], sp[2], 255); + Unsafe.Write(dp, c); + sp += 3; + dp += ColorSize; + } + } + + /// + /// Bulk version of . + /// + /// + /// + /// + internal virtual void PackToXyzBytes(ArrayPointer sourceColors, ArrayPointer destBytes, int count) + { + byte* sp = (byte*)sourceColors; + byte[] dest = destBytes.Array; + + for (int i = destBytes.Offset; i < destBytes.Offset + count * 3; i += 3) + { + TColor c = Unsafe.Read(sp); + c.ToXyzBytes(dest, i); + sp += ColorSize; + } + } + + /// + /// Bulk version of that converts data in . + /// + /// + /// + /// + internal virtual void PackFromXyzwBytes( + ArrayPointer sourceBytes, + ArrayPointer destColors, + int count) + { + byte* sp = (byte*)sourceBytes; + byte* dp = (byte*)destColors.PointerAtOffset; + + for (int i = 0; i < count; i++) + { + TColor c = default(TColor); + c.PackFromBytes(sp[0], sp[1], sp[2], sp[3]); + Unsafe.Write(dp, c); + sp += 4; + dp += ColorSize; + } + } + + /// + /// Bulk version of . + /// + /// + /// + /// + internal virtual void PackToXyzwBytes( + ArrayPointer sourceColors, + ArrayPointer destBytes, + int count) + { + byte* sp = (byte*)sourceColors; + byte[] dest = destBytes.Array; + + for (int i = destBytes.Offset; i < destBytes.Offset + count * 4; i += 4) + { + TColor c = Unsafe.Read(sp); + c.ToXyzwBytes(dest, i); + sp += ColorSize; + } + } + + /// + /// Bulk version of that converts data in . + /// + /// + /// + /// + internal virtual void PackFromZyxBytes( + ArrayPointer sourceBytes, + ArrayPointer destColors, + int count) + { + byte* sp = (byte*)sourceBytes; + byte* dp = (byte*)destColors.PointerAtOffset; + + for (int i = 0; i < count; i++) + { + TColor c = default(TColor); + c.PackFromBytes(sp[2], sp[1], sp[0], 255); + Unsafe.Write(dp, c); + sp += 3; + dp += ColorSize; + } + } + + /// + /// Bulk version of . + /// + /// + /// + /// + internal virtual void PackToZyxBytes(ArrayPointer sourceColors, ArrayPointer destBytes, int count) + { + byte* sp = (byte*)sourceColors; + byte[] dest = destBytes.Array; + + for (int i = destBytes.Offset; i < destBytes.Offset + count * 3; i += 3) + { + TColor c = Unsafe.Read(sp); + c.ToZyxBytes(dest, i); + sp += ColorSize; + } + } + + /// + /// Bulk version of that converts data in . + /// + /// + /// + /// + internal virtual void PackFromZyxwBytes( + ArrayPointer sourceBytes, + ArrayPointer destColors, + int count) + { + byte* sp = (byte*)sourceBytes; + byte* dp = (byte*)destColors.PointerAtOffset; + + for (int i = 0; i < count; i++) + { + TColor c = default(TColor); + c.PackFromBytes(sp[2], sp[1], sp[0], sp[3]); + Unsafe.Write(dp, c); + sp += 4; + dp += ColorSize; + } + } + + /// + /// Bulk version of . + /// + /// + /// + /// + internal virtual void PackToZyxwBytes( + ArrayPointer sourceColors, + ArrayPointer destBytes, + int count) + { + byte* sp = (byte*)sourceColors; + byte[] dest = destBytes.Array; + + for (int i = destBytes.Offset; i < destBytes.Offset + count * 4; i += 4) + { + TColor c = Unsafe.Read(sp); + c.ToZyxwBytes(dest, i); + sp += ColorSize; + } + } + } +} \ No newline at end of file diff --git a/src/ImageSharp/Common/Memory/ArrayPointer{T}.cs b/src/ImageSharp/Common/Memory/ArrayPointer{T}.cs index 8f99327ba2..e0b728095e 100644 --- a/src/ImageSharp/Common/Memory/ArrayPointer{T}.cs +++ b/src/ImageSharp/Common/Memory/ArrayPointer{T}.cs @@ -68,27 +68,12 @@ namespace ImageSharp /// public IntPtr PointerAtOffset { get; private set; } - /// - /// Forms a slice out of the given ArrayPointer, beginning at 'offset'. - /// - /// The offset in number of elements - /// The offseted (sliced) ArrayPointer - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public ArrayPointer Slice(int offset) - { - ArrayPointer result = default(ArrayPointer); - result.Array = this.Array; - result.Offset = this.Offset + offset; - result.PointerAtOffset = this.PointerAtOffset + (Unsafe.SizeOf() * offset); - return result; - } - /// /// Convertes instance to a raw 'void*' pointer /// /// The to convert [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static explicit operator void*(ArrayPointer arrayPointer) + public static explicit operator void* (ArrayPointer arrayPointer) { return (void*)arrayPointer.PointerAtOffset; } @@ -102,5 +87,20 @@ namespace ImageSharp { return (byte*)arrayPointer.PointerAtOffset; } + + /// + /// Forms a slice out of the given ArrayPointer, beginning at 'offset'. + /// + /// The offset in number of elements + /// The offseted (sliced) ArrayPointer + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public ArrayPointer Slice(int offset) + { + ArrayPointer result = default(ArrayPointer); + result.Array = this.Array; + result.Offset = this.Offset + offset; + result.PointerAtOffset = this.PointerAtOffset + (Unsafe.SizeOf() * offset); + return result; + } } } \ No newline at end of file diff --git a/src/ImageSharp/Common/Memory/PinnedBuffer.cs b/src/ImageSharp/Common/Memory/PinnedBuffer.cs index c6e0c7c6f9..201d93b56a 100644 --- a/src/ImageSharp/Common/Memory/PinnedBuffer.cs +++ b/src/ImageSharp/Common/Memory/PinnedBuffer.cs @@ -5,25 +5,23 @@ namespace ImageSharp using System.Runtime.InteropServices; /// - /// Manages a pinned buffer of 'T' as a Disposable resource. + /// Manages a pinned buffer of value type data 'T' as a Disposable resource. /// The backing array is either pooled or comes from the outside. - /// TODO: Should replace the pinning/dispose logic in several classes like or ! /// /// The value type. internal class PinnedBuffer : IDisposable where T : struct { + /// + /// A handle that allows to access the managed as an unmanaged memory by pinning. + /// private GCHandle handle; - private bool isBufferRented; - - private bool isDisposed; - /// - /// TODO: Consider reusing functionality of + /// A value indicating wether this instance should return the array to the pool. /// - private static readonly ArrayPool ArrayPool = ArrayPool.Create(); - + private bool isPoolingOwner; + /// /// Initializes a new instance of the class. /// @@ -31,8 +29,8 @@ namespace ImageSharp public PinnedBuffer(int count) { this.Count = count; - this.Array = ArrayPool.Rent(count); - this.isBufferRented = true; + this.Array = PixelDataPool.Rent(count); + this.isPoolingOwner = true; this.Pin(); } @@ -48,7 +46,34 @@ namespace ImageSharp } /// - /// The count of "relevant" elements. Usually be smaller than 'Array.Length' when is pooled. + /// Initializes a new instance of the class. + /// + /// The count of "relevant" elements in 'array'. + /// The array to pin. + public PinnedBuffer(int count, T[] array) + { + if (array.Length < count) + { + throw new ArgumentException("Can't initialize a PinnedBuffer with array.Length < count", nameof(array)); + } + this.Count = count; + this.Array = array; + this.Pin(); + } + + ~PinnedBuffer() + { + this.UnPin(); + } + + /// + /// Gets a value indicating whether this instance is disposed, or has lost ownership of . + /// + public bool IsDisposedOrLostArrayOwnership { get; private set; } + + + /// + /// Gets the count of "relevant" elements. Usually be smaller than 'Array.Length' when is pooled. /// public int Count { get; private set; } @@ -58,7 +83,7 @@ namespace ImageSharp public T[] Array { get; private set; } /// - /// Pointer to the pinned . + /// Gets a pointer to the pinned . /// public IntPtr Pointer { get; private set; } @@ -67,16 +92,16 @@ namespace ImageSharp /// public void Dispose() { - if (this.isDisposed) + if (this.IsDisposedOrLostArrayOwnership) { return; } - this.isDisposed = true; + this.IsDisposedOrLostArrayOwnership = true; this.UnPin(); - if (this.isBufferRented) + if (this.isPoolingOwner) { - ArrayPool.Return(this.Array, true); + PixelDataPool.Return(this.Array); } this.Array = null; @@ -85,12 +110,37 @@ namespace ImageSharp GC.SuppressFinalize(this); } + /// + /// Unpins and makes the object "quasi-disposed" so the array is no longer owned by this object. + /// If is rented, it's the callers responsibility to return it to it's pool. (Most likely ) + /// + /// The unpinned + public T[] UnPinAndTakeArrayOwnership() + { + if (this.IsDisposedOrLostArrayOwnership) + { + throw new InvalidOperationException("UnPinAndTakeArrayOwnership() is invalid: either PinnedBuffer is disposed or UnPinAndTakeArrayOwnership() has been called multiple times!"); + } + + this.IsDisposedOrLostArrayOwnership = true; + this.UnPin(); + T[] array = this.Array; + this.Array = null; + return array; + } + + /// + /// Pins . + /// private void Pin() { this.handle = GCHandle.Alloc(this.Array, GCHandleType.Pinned); this.Pointer = this.handle.AddrOfPinnedObject(); } + /// + /// Unpins . + /// private void UnPin() { if (this.Pointer == IntPtr.Zero || !this.handle.IsAllocated) @@ -100,10 +150,5 @@ namespace ImageSharp this.handle.Free(); this.Pointer = IntPtr.Zero; } - - ~PinnedBuffer() - { - this.UnPin(); - } } } \ No newline at end of file diff --git a/src/ImageSharp/Common/Memory/PixelDataPool{T}.cs b/src/ImageSharp/Common/Memory/PixelDataPool{T}.cs new file mode 100644 index 0000000000..f6f6a10425 --- /dev/null +++ b/src/ImageSharp/Common/Memory/PixelDataPool{T}.cs @@ -0,0 +1,60 @@ +// +// Copyright (c) James Jackson-South and contributors. +// Licensed under the Apache License, Version 2.0. +// + +namespace ImageSharp +{ + using System; + using System.Buffers; + + /// + /// Provides a resource pool that enables reusing instances of value type arrays . + /// will always return arrays initialized with 'default(T)' + /// + /// The value type. + public static class PixelDataPool + where T : struct + { + /// + /// The used to pool data. + /// + private static readonly ArrayPool ArrayPool = ArrayPool.Create(CalculateMaxArrayLength(), 50); + + /// + /// Rents the pixel array from the pool. + /// + /// The minimum length of the array to return. + /// The + public static T[] Rent(int minimumLength) + { + return ArrayPool.Rent(minimumLength); + } + + /// + /// Returns the rented pixel array back to the pool. + /// + /// The array to return to the buffer pool. + public static void Return(T[] array) + { + ArrayPool.Return(array, true); + } + + /// + /// Heuristically calculates a reasonable maxArrayLength value for the backing . + /// + /// The maxArrayLength value + internal static int CalculateMaxArrayLength() + { + if (typeof(IPixel).IsAssignableFrom(typeof(T))) + { + const int MaximumExpectedImageSize = 16384; + return MaximumExpectedImageSize * MaximumExpectedImageSize; + } + else + { + return int.MaxValue; + } + } + } +} \ No newline at end of file diff --git a/src/ImageSharp/Image/ImageBase{TColor}.cs b/src/ImageSharp/Image/ImageBase{TColor}.cs index e4b4485c7f..9bd760805a 100644 --- a/src/ImageSharp/Image/ImageBase{TColor}.cs +++ b/src/ImageSharp/Image/ImageBase{TColor}.cs @@ -162,13 +162,15 @@ namespace ImageSharp internal void SwapPixelsBuffers(PixelAccessor pixelSource) { Guard.NotNull(pixelSource, nameof(pixelSource)); - Guard.IsTrue(pixelSource.PooledMemory, nameof(pixelSource.PooledMemory), "pixelSource must be using pooled memory"); + + // TODO: This check was useful. We can introduce a bool PixelAccessor.IsBoundToImage to re-introduce it. + // Guard.IsTrue(pixelSource.PooledMemory, nameof(pixelSource.PooledMemory), "pixelSource must be using pooled memory"); int newWidth = pixelSource.Width; int newHeight = pixelSource.Height; // Push my memory into the accessor (which in turn unpins the old puffer ready for the images use) - TColor[] newPixels = pixelSource.ReturnCurrentPixelsAndReplaceThemInternally(this.Width, this.Height, this.pixelBuffer, true); + TColor[] newPixels = pixelSource.ReturnCurrentPixelsAndReplaceThemInternally(this.Width, this.Height, this.pixelBuffer); this.Width = newWidth; this.Height = newHeight; this.pixelBuffer = newPixels; @@ -222,7 +224,7 @@ namespace ImageSharp /// private void RentPixels() { - this.pixelBuffer = PixelPool.RentPixels(this.Width * this.Height); + this.pixelBuffer = PixelDataPool.Rent(this.Width * this.Height); } /// @@ -230,7 +232,7 @@ namespace ImageSharp /// private void ReturnPixels() { - PixelPool.ReturnPixels(this.pixelBuffer); + PixelDataPool.Return(this.pixelBuffer); this.pixelBuffer = null; } } diff --git a/src/ImageSharp/Image/PixelAccessor{TColor}.cs b/src/ImageSharp/Image/PixelAccessor{TColor}.cs index b31ada10bb..3a3f0f5c7e 100644 --- a/src/ImageSharp/Image/PixelAccessor{TColor}.cs +++ b/src/ImageSharp/Image/PixelAccessor{TColor}.cs @@ -18,21 +18,11 @@ namespace ImageSharp public unsafe class PixelAccessor : IDisposable where TColor : struct, IPixel { - /// - /// The pointer to the pixel buffer. - /// - private IntPtr dataPointer; - /// /// The position of the first pixel in the image. /// private byte* pixelsBase; - - /// - /// Provides a way to access the pixels from unmanaged memory. - /// - private GCHandle pixelsHandle; - + /// /// A value indicating whether this instance of the given entity has been disposed. /// @@ -45,9 +35,9 @@ namespace ImageSharp private bool isDisposed; /// - /// The pixel buffer + /// The containing the pixel data. /// - private TColor[] pixelBuffer; + private PinnedBuffer pixelBuffer; /// /// Initializes a new instance of the class. @@ -59,7 +49,7 @@ namespace ImageSharp Guard.MustBeGreaterThan(image.Width, 0, "image width"); Guard.MustBeGreaterThan(image.Height, 0, "image height"); - this.SetPixelBufferUnsafe(image.Width, image.Height, image.Pixels, false); + this.SetPixelBufferUnsafe(image.Width, image.Height, image.Pixels); this.ParallelOptions = image.Configuration.ParallelOptions; } @@ -70,7 +60,7 @@ namespace ImageSharp /// The height of the image represented by the pixel buffer. /// The pixel buffer. public PixelAccessor(int width, int height, TColor[] pixels) - : this(width, height, pixels, false) + : this(width, height, new PinnedBuffer(width * height, pixels)) { } @@ -80,7 +70,7 @@ namespace ImageSharp /// The width of the image represented by the pixel buffer. /// The height of the image represented by the pixel buffer. public PixelAccessor(int width, int height) - : this(width, height, PixelPool.RentPixels(width * height), true) + : this(width, height, new PinnedBuffer(width * height)) { } @@ -90,19 +80,18 @@ namespace ImageSharp /// The width of the image represented by the pixel buffer. /// The height of the image represented by the pixel buffer. /// The pixel buffer. - /// if set to true then the is from the thus should be returned once disposed. - private PixelAccessor(int width, int height, TColor[] pixels, bool pooledMemory) + private PixelAccessor(int width, int height, PinnedBuffer pixels) { Guard.NotNull(pixels, nameof(pixels)); Guard.MustBeGreaterThan(width, 0, nameof(width)); Guard.MustBeGreaterThan(height, 0, nameof(height)); - if (!(pixels.Length >= width * height)) - { - throw new ArgumentException($"Pixel array must have the length of at least {width * height}."); - } + //if (!(pixels.Length >= width * height)) + //{ + // throw new ArgumentException($"Pixel array must have the length of at least {width * height}."); + //} - this.SetPixelBufferUnsafe(width, height, pixels, pooledMemory); + this.SetPixelBufferUnsafe(width, height, pixels); this.ParallelOptions = Configuration.Default.ParallelOptions; } @@ -114,21 +103,16 @@ namespace ImageSharp { this.Dispose(); } - - /// - /// Gets a value indicating whether the current pixel buffer is from a pooled source. - /// - public bool PooledMemory { get; private set; } - + /// /// Gets the pixel buffer array. /// - public TColor[] PixelBuffer => this.pixelBuffer; + public TColor[] PixelBuffer => this.pixelBuffer.Array; /// /// Gets the pointer to the pixel buffer. /// - public IntPtr DataPointer => this.dataPointer; + public IntPtr DataPointer => this.pixelBuffer.Pointer; /// /// Gets the size of a single pixel in the number of bytes. @@ -246,24 +230,18 @@ namespace ImageSharp { return; } - - this.UnPinPixels(); - + // Note disposing is done. this.isDisposed = true; + this.pixelBuffer.Dispose(); + // This object will be cleaned up by the Dispose method. // Therefore, you should call GC.SuppressFinalize to // take this object off the finalization queue // and prevent finalization code for this object // from executing a second time. GC.SuppressFinalize(this); - - if (this.PooledMemory) - { - PixelPool.ReturnPixels(this.pixelBuffer); - this.pixelBuffer = null; - } } /// @@ -280,13 +258,12 @@ namespace ImageSharp /// The width. /// The height. /// The pixels. - /// If set to true this indicates that the pixel buffer is from a pooled source. /// Returns the old pixel data thats has gust been replaced. - /// If is true then caller is responsible for ensuring is called. - internal TColor[] ReturnCurrentPixelsAndReplaceThemInternally(int width, int height, TColor[] pixels, bool pooledMemory) + /// If is true then caller is responsible for ensuring is called. + internal TColor[] ReturnCurrentPixelsAndReplaceThemInternally(int width, int height, TColor[] pixels) { - TColor[] oldPixels = this.pixelBuffer; - this.SetPixelBufferUnsafe(width, height, pixels, pooledMemory); + TColor[] oldPixels = this.pixelBuffer.UnPinAndTakeArrayOwnership(); + this.SetPixelBufferUnsafe(width, height, pixels); return oldPixels; } @@ -514,53 +491,57 @@ namespace ImageSharp return this.pixelsBase + (((y * this.Width) + x) * Unsafe.SizeOf()); } + private void SetPixelBufferUnsafe(int width, int height, TColor[] pixels) + { + this.SetPixelBufferUnsafe(width, height, new PinnedBuffer(width * height, pixels)); + } + /// /// Sets the pixel buffer in an unsafe manor this should not be used unless you know what its doing!!! /// /// The width. /// The height. - /// The pixels. - /// If set to true this indicates that the pixel buffer is from a pooled source. - private void SetPixelBufferUnsafe(int width, int height, TColor[] pixels, bool pooledMemory) + /// The pixel buffer + private void SetPixelBufferUnsafe(int width, int height, PinnedBuffer pixels) { this.pixelBuffer = pixels; - this.PooledMemory = pooledMemory; + this.pixelsBase = (byte*)pixels.Pointer; + this.Width = width; this.Height = height; - this.PinPixels(); this.PixelSize = Unsafe.SizeOf(); this.RowStride = this.Width * this.PixelSize; } - /// - /// Pins the pixels data. - /// - private void PinPixels() - { - // unpin any old pixels just incase - this.UnPinPixels(); - - this.pixelsHandle = GCHandle.Alloc(this.pixelBuffer, GCHandleType.Pinned); - this.dataPointer = this.pixelsHandle.AddrOfPinnedObject(); - this.pixelsBase = (byte*)this.dataPointer.ToPointer(); - } - - /// - /// Unpins pixels data. - /// - private void UnPinPixels() - { - if (this.pixelsBase != null) - { - if (this.pixelsHandle.IsAllocated) - { - this.pixelsHandle.Free(); - } - - this.dataPointer = IntPtr.Zero; - this.pixelsBase = null; - } - } + ///// + ///// Pins the pixels data. + ///// + //private void PinPixels() + //{ + // // unpin any old pixels just incase + // this.UnPinPixels(); + + // this.pixelsHandle = GCHandle.Alloc(this.pixelBuffer, GCHandleType.Pinned); + // this.dataPointer = this.pixelsHandle.AddrOfPinnedObject(); + // this.pixelsBase = (byte*)this.dataPointer.ToPointer(); + //} + + ///// + ///// Unpins pixels data. + ///// + //private void UnPinPixels() + //{ + // if (this.pixelsBase != null) + // { + // if (this.pixelsHandle.IsAllocated) + // { + // this.pixelsHandle.Free(); + // } + + // this.dataPointer = IntPtr.Zero; + // this.pixelsBase = null; + // } + //} /// /// Copy an area of pixels to the image. diff --git a/src/ImageSharp/Image/PixelArea{TColor}.cs b/src/ImageSharp/Image/PixelArea{TColor}.cs index 77b648ca56..25840167eb 100644 --- a/src/ImageSharp/Image/PixelArea{TColor}.cs +++ b/src/ImageSharp/Image/PixelArea{TColor}.cs @@ -18,21 +18,6 @@ namespace ImageSharp public sealed unsafe class PixelArea : IDisposable where TColor : struct, IPixel { - /// - /// True if was rented from by the constructor - /// - private readonly bool isBufferRented; - - /// - /// Provides a way to access the pixels from unmanaged memory. - /// - private readonly GCHandle pixelsHandle; - - /// - /// The pointer to the pixel buffer. - /// - private IntPtr dataPointer; - /// /// A value indicating whether this instance of the given entity has been disposed. /// @@ -44,6 +29,11 @@ namespace ImageSharp /// private bool isDisposed; + /// + /// The underlying buffer containing the raw pixel data. + /// + private PinnedBuffer byteBuffer; + /// /// Initializes a new instance of the class. /// @@ -76,14 +66,11 @@ namespace ImageSharp this.Height = height; this.ComponentOrder = componentOrder; this.RowStride = width * GetComponentCount(componentOrder); - this.Bytes = bytes; - this.Length = bytes.Length; - this.isBufferRented = false; - this.pixelsHandle = GCHandle.Alloc(this.Bytes, GCHandleType.Pinned); + this.Length = bytes.Length; // TODO: Is this the right value for Length? - // TODO: Why is Resharper warning us about an impure method call? - this.dataPointer = this.pixelsHandle.AddrOfPinnedObject(); - this.PixelBase = (byte*)this.dataPointer.ToPointer(); + this.byteBuffer = new PinnedBuffer(bytes); + + this.PixelBase = (byte*)this.byteBuffer.Pointer; } /// @@ -132,27 +119,15 @@ namespace ImageSharp this.ComponentOrder = componentOrder; this.RowStride = (width * GetComponentCount(componentOrder)) + padding; this.Length = this.RowStride * height; - this.Bytes = BytesPool.Rent(this.Length); - this.isBufferRented = true; - this.pixelsHandle = GCHandle.Alloc(this.Bytes, GCHandleType.Pinned); - // TODO: Why is Resharper warning us about an impure method call? - this.dataPointer = this.pixelsHandle.AddrOfPinnedObject(); - this.PixelBase = (byte*)this.dataPointer.ToPointer(); + this.byteBuffer = new PinnedBuffer(this.Length); + this.PixelBase = (byte*)this.byteBuffer.Pointer; } - - /// - /// Finalizes an instance of the class. - /// - ~PixelArea() - { - this.Dispose(false); - } - + /// /// Gets the data in bytes. /// - public byte[] Bytes { get; } + public byte[] Bytes => this.byteBuffer.Array; /// /// Gets the length of the buffer. @@ -167,7 +142,7 @@ namespace ImageSharp /// /// Gets the pointer to the pixel buffer. /// - public IntPtr DataPointer => this.dataPointer; + public IntPtr DataPointer => this.byteBuffer.Pointer; /// /// Gets the height. @@ -188,26 +163,19 @@ namespace ImageSharp /// Gets the width. /// public int Width { get; } - - /// - /// Gets the pool used to rent bytes, when it's not coming from an external source. - /// - // TODO: Use own pool? - private static ArrayPool BytesPool => ArrayPool.Shared; - + /// /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources. /// public void Dispose() { - this.Dispose(true); + if (this.isDisposed) + { + return; + } - // This object will be cleaned up by the Dispose method. - // Therefore, you should call GC.SuppressFinalize to - // take this object off the finalization queue - // and prevent finalization code for this object - // from executing a second time. - GC.SuppressFinalize(this); + this.byteBuffer.Dispose(); + this.isDisposed = true; } /// @@ -281,38 +249,6 @@ namespace ImageSharp nameof(bytes), $"Invalid byte array length. Length {bytes.Length}; Should be {requiredLength}."); } - } - - /// - /// Disposes the object and frees resources for the Garbage Collector. - /// - /// If true, the object gets disposed. - private void Dispose(bool disposing) - { - if (this.isDisposed) - { - return; - } - - if (this.PixelBase == null) - { - return; - } - - if (this.pixelsHandle.IsAllocated) - { - this.pixelsHandle.Free(); - } - - if (disposing && this.isBufferRented) - { - BytesPool.Return(this.Bytes); - } - - this.dataPointer = IntPtr.Zero; - this.PixelBase = null; - - this.isDisposed = true; - } + } } } \ No newline at end of file diff --git a/src/ImageSharp/Image/PixelPool{TColor}.cs b/src/ImageSharp/Image/PixelPool{TColor}.cs deleted file mode 100644 index ea6dad6b12..0000000000 --- a/src/ImageSharp/Image/PixelPool{TColor}.cs +++ /dev/null @@ -1,43 +0,0 @@ -// -// Copyright (c) James Jackson-South and contributors. -// Licensed under the Apache License, Version 2.0. -// - -namespace ImageSharp -{ - using System; - using System.Buffers; - - // TODO: Consider refactoring this into a more general ClearPool, so we can use it in PinnedBuffer! - /// - /// Provides a resource pool that enables reusing instances of type . - /// - /// The pixel format. - public static class PixelPool - where TColor : struct, IPixel - { - /// - /// The used to pool data. TODO: Choose sensible default size and count - /// - private static readonly ArrayPool ArrayPool = ArrayPool.Create(int.MaxValue, 50); - - /// - /// Rents the pixel array from the pool. - /// - /// The minimum length of the array to return. - /// The - public static TColor[] RentPixels(int minimumLength) - { - return ArrayPool.Rent(minimumLength); - } - - /// - /// Returns the rented pixel array back to the pool. - /// - /// The array to return to the buffer pool. - public static void ReturnPixels(TColor[] array) - { - ArrayPool.Return(array, true); - } - } -} \ No newline at end of file diff --git a/tests/ImageSharp.Sandbox46/ImageSharp.Sandbox46.csproj b/tests/ImageSharp.Sandbox46/ImageSharp.Sandbox46.csproj index ad436793d4..094eedb180 100644 --- a/tests/ImageSharp.Sandbox46/ImageSharp.Sandbox46.csproj +++ b/tests/ImageSharp.Sandbox46/ImageSharp.Sandbox46.csproj @@ -212,6 +212,9 @@ Tests\Colors\BulkPixelOperationsTests.cs + + Tests\Common\PinnedBufferTests.cs + Tests\Drawing\PolygonTests.cs @@ -248,6 +251,9 @@ Tests\Formats\Jpg\YCbCrImageTests.cs + + Tests\Image\PixelPoolTests.cs + Tests\MetaData\ImagePropertyTests.cs diff --git a/tests/ImageSharp.Tests/Common/PinnedBufferTests.cs b/tests/ImageSharp.Tests/Common/PinnedBufferTests.cs index e0783f7165..c5eb2a5103 100644 --- a/tests/ImageSharp.Tests/Common/PinnedBufferTests.cs +++ b/tests/ImageSharp.Tests/Common/PinnedBufferTests.cs @@ -22,6 +22,7 @@ { using (PinnedBuffer buffer = new PinnedBuffer(count)) { + Assert.False(buffer.IsDisposedOrLostArrayOwnership); Assert.NotNull(buffer.Array); Assert.Equal(count, buffer.Count); Assert.True(buffer.Array.Length >= count); @@ -38,6 +39,7 @@ Foo[] array = new Foo[count]; using (PinnedBuffer buffer = new PinnedBuffer(array)) { + Assert.False(buffer.IsDisposedOrLostArrayOwnership); Assert.Equal(array, buffer.Array); Assert.Equal(count, buffer.Count); @@ -45,6 +47,15 @@ } } + [Fact] + public void Dispose() + { + PinnedBuffer buffer = new PinnedBuffer(42); + buffer.Dispose(); + + Assert.True(buffer.IsDisposedOrLostArrayOwnership); + } + [Fact] public void GetArrayPointer() { @@ -60,6 +71,20 @@ } } + [Fact] + public void UnPinAndTakeArrayOwnership() + { + Foo[] data = null; + using (PinnedBuffer buffer = new PinnedBuffer(42)) + { + data = buffer.UnPinAndTakeArrayOwnership(); + Assert.True(buffer.IsDisposedOrLostArrayOwnership); + } + + Assert.NotNull(data); + Assert.True(data.Length >= 42); + } + private static void VerifyPointer(PinnedBuffer buffer) { IntPtr ptr = (IntPtr)Unsafe.AsPointer(ref buffer.Array[0]); diff --git a/tests/ImageSharp.Tests/Image/PixelPoolTests.cs b/tests/ImageSharp.Tests/Image/PixelPoolTests.cs index 0b762cf7c3..001785d60c 100644 --- a/tests/ImageSharp.Tests/Image/PixelPoolTests.cs +++ b/tests/ImageSharp.Tests/Image/PixelPoolTests.cs @@ -1,4 +1,4 @@ -// +// // Copyright (c) James Jackson-South and contributors. // Licensed under the Apache License, Version 2.0. // @@ -10,54 +10,54 @@ namespace ImageSharp.Tests using Xunit; /// - /// Tests the class. + /// Tests the class. /// - public class PixelPoolTests + public class PixelDataPoolTests { [Fact] - public void PixelPoolRentsMinimumSize() + public void PixelDataPoolRentsMinimumSize() { - Color[] pixels = PixelPool.RentPixels(1024); + Color[] pixels = PixelDataPool.Rent(1024); Assert.True(pixels.Length >= 1024); } [Fact] - public void PixelPoolRentsEmptyArray() + public void PixelDataPoolRentsEmptyArray() { for (int i = 16; i < 1024; i += 16) { - Color[] pixels = PixelPool.RentPixels(i); + Color[] pixels = PixelDataPool.Rent(i); Assert.True(pixels.All(p => p == default(Color))); - PixelPool.ReturnPixels(pixels); + PixelDataPool.Return(pixels); } for (int i = 16; i < 1024; i += 16) { - Color[] pixels = PixelPool.RentPixels(i); + Color[] pixels = PixelDataPool.Rent(i); Assert.True(pixels.All(p => p == default(Color))); - PixelPool.ReturnPixels(pixels); + PixelDataPool.Return(pixels); } } [Fact] - public void PixelPoolDoesNotThrowWhenReturningNonPooled() + public void PixelDataPoolDoesNotThrowWhenReturningNonPooled() { Color[] pixels = new Color[1024]; - PixelPool.ReturnPixels(pixels); + PixelDataPool.Return(pixels); Assert.True(pixels.Length >= 1024); } [Fact] - public void PixelPoolCleansRentedArray() + public void PixelDataPoolCleansRentedArray() { - Color[] pixels = PixelPool.RentPixels(256); + Color[] pixels = PixelDataPool.Rent(256); for (int i = 0; i < pixels.Length; i++) { @@ -66,9 +66,28 @@ namespace ImageSharp.Tests Assert.True(pixels.All(p => p == Color.Azure)); - PixelPool.ReturnPixels(pixels); + PixelDataPool.Return(pixels); Assert.True(pixels.All(p => p == default(Color))); } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void CalculateMaxArrayLength(bool isRawData) + { + int max = isRawData ? PixelDataPool.CalculateMaxArrayLength() + : PixelDataPool.CalculateMaxArrayLength(); + + Assert.Equal(max < int.MaxValue, !isRawData); + } + + [Fact] + public void RentNonIPixelData() + { + byte[] data = PixelDataPool.Rent(16384); + + Assert.True(data.Length >= 16384); + } } } \ No newline at end of file