From 548fc417113976c95ccfbfb2458b3082730a4cbb Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 13 Apr 2017 02:24:59 +0200 Subject: [PATCH] fixed indexer and copy operations --- src/ImageSharp/Colors/Color.BulkOperations.cs | 10 +- src/ImageSharp/Common/Memory/BufferSpan.cs | 158 ++---------------- src/ImageSharp/Common/Memory/BufferSpan{T}.cs | 16 +- .../Colors/BulkPixelOperationsTests.cs | 4 +- .../Common/BufferSpanTests.cs | 35 +++- tests/ImageSharp.Tests/Common/TestStructs.cs | 2 + 6 files changed, 59 insertions(+), 166 deletions(-) diff --git a/src/ImageSharp/Colors/Color.BulkOperations.cs b/src/ImageSharp/Colors/Color.BulkOperations.cs index 08d70330d..d5c55ba81 100644 --- a/src/ImageSharp/Colors/Color.BulkOperations.cs +++ b/src/ImageSharp/Colors/Color.BulkOperations.cs @@ -90,7 +90,7 @@ namespace ImageSharp vf.CopyTo(fTemp, i); } - BufferSpan.Copy(tempBuf, (BufferSpan)destVectors, unpackedRawCount); + BufferSpan.Copy(tempBuf.Span.AsBytes(), destVectors.AsBytes(), unpackedRawCount); } } @@ -156,18 +156,18 @@ namespace ImageSharp } /// - internal override void PackFromXyzwBytes( + internal override unsafe void PackFromXyzwBytes( BufferSpan sourceBytes, BufferSpan destColors, int count) { - BufferSpan.Copy(sourceBytes, destColors, count); + BufferSpan.Copy(sourceBytes, destColors.AsBytes(), count * sizeof(Color)); } /// - internal override void ToXyzwBytes(BufferSpan sourceColors, BufferSpan destBytes, int count) + internal override unsafe void ToXyzwBytes(BufferSpan sourceColors, BufferSpan destBytes, int count) { - BufferSpan.Copy(sourceColors, destBytes, count); + BufferSpan.Copy(sourceColors.AsBytes(), destBytes, count * sizeof(Color)); } /// diff --git a/src/ImageSharp/Common/Memory/BufferSpan.cs b/src/ImageSharp/Common/Memory/BufferSpan.cs index 6dad393c3..5db1cd7da 100644 --- a/src/ImageSharp/Common/Memory/BufferSpan.cs +++ b/src/ImageSharp/Common/Memory/BufferSpan.cs @@ -14,11 +14,6 @@ namespace ImageSharp /// internal static class BufferSpan { - /// - /// It's worth to use Marshal.Copy() or Buffer.BlockCopy() over this size. - /// - private const int ByteCountThreshold = 1024; - /// /// Copy 'count' number of elements of the same type from 'source' to 'dest' /// @@ -27,69 +22,29 @@ namespace ImageSharp /// The destination . /// The number of elements to copy [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void Copy(BufferSpan source, BufferSpan destination, int count) + public static unsafe void Copy(BufferSpan source, BufferSpan destination, int count) where T : struct { - CopyImpl(source, destination, count); - } - - /// - /// Copy 'countInSource' elements of from 'source' into the raw byte buffer 'destination'. - /// - /// The element type. - /// The source buffer of elements to copy from. - /// The destination buffer. - /// The number of elements to copy from 'source' - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void Copy(BufferSpan source, BufferSpan destination, int countInSource) - where T : struct - { - CopyImpl(source, destination, countInSource); - } - - /// - /// Copy 'countInDest' number of elements into 'dest' from a raw byte buffer defined by 'source'. - /// - /// The element type. - /// The raw source buffer to copy from"/> - /// The destination buffer"/> - /// The number of elements to copy. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static unsafe void Copy(BufferSpan source, BufferSpan destination, int countInDest) - where TDest : struct - { - // TODO: Refactor this method when Unsafe.CopyBlock(ref T, ref T) gets available! - int byteCount = SizeOf(countInDest); - - if (PerTypeValues.IsPrimitiveType) - { - Buffer.BlockCopy(source.Array, source.ByteOffset, destination.Array, destination.ByteOffset, byteCount); - return; - } + ref byte srcRef = ref Unsafe.As(ref source.DangerousGetPinnableReference()); + ref byte destRef = ref Unsafe.As(ref destination.DangerousGetPinnableReference()); - ref byte destRef = ref Unsafe.As(ref destination.DangerousGetPinnableReference()); + int byteCount = Unsafe.SizeOf() * count; - fixed (void* pinnedDest = &destRef) + // TODO: Use unfixed Unsafe.CopyBlock(ref T, ref T, int) for small blocks, when it gets available! + fixed (byte* pSrc = &srcRef) + fixed (byte* pDest = &destRef) { -#if !NETSTANDARD1_1 - ref byte srcRef = ref source.DangerousGetPinnableReference(); - fixed (void* pinnedSrc = &srcRef) - { - Buffer.MemoryCopy(pinnedSrc, pinnedDest, byteCount, byteCount); - return; - } +#if NETSTANDARD1_1 + Unsafe.CopyBlock(pDest, pSrc, (uint) byteCount); #else - if (byteCount > ByteCountThreshold) + if (byteCount > 512) { - IntPtr ptr = (IntPtr)pinnedDest; - Marshal.Copy(source.Array, source.Start, ptr, byteCount); + int destLength = destination.Length * Unsafe.SizeOf(); + Buffer.MemoryCopy(pSrc, pDest, destLength, byteCount); } - - ref byte srcRef = ref source.DangerousGetPinnableReference(); - - fixed (void* pinnedSrc = &srcRef) + else { - Unsafe.CopyBlock(pinnedSrc, pinnedDest, (uint)byteCount); + Unsafe.CopyBlock(pDest, pSrc, (uint)byteCount); } #endif } @@ -115,90 +70,5 @@ namespace ImageSharp public static uint USizeOf(int count) where T : struct => (uint)SizeOf(count); - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static unsafe void CopyImpl(BufferSpan source, BufferSpan destination, int countInSource) - where T : struct - where TDest : struct - { - // TODO: Use Unsafe.CopyBlock(ref T, ref T) for small buffers when it gets available! - int byteCount = SizeOf(countInSource); - - if (PerTypeValues.IsPrimitiveType && PerTypeValues.IsPrimitiveType) - { - Buffer.BlockCopy(source.Array, source.ByteOffset, destination.Array, destination.ByteOffset, byteCount); - return; - } - - ref byte destRef = ref Unsafe.As(ref destination.DangerousGetPinnableReference()); - - fixed (void* pinnedDest = &destRef) - { -#if !NETSTANDARD1_1 - ref byte srcRef = ref Unsafe.As(ref source.DangerousGetPinnableReference()); - fixed (void* pinnedSrc = &srcRef) - { - Buffer.MemoryCopy(pinnedSrc, pinnedDest, byteCount, byteCount); - return; - } -#else - if (byteCount > ByteCountThreshold) - { - IntPtr ptr = (IntPtr)pinnedDest; - if (Unsafe.SizeOf() == sizeof(long)) - { - Marshal.Copy(Unsafe.As(source.Array), source.Start, ptr, countInSource); - return; - } - else if (Unsafe.SizeOf() == sizeof(int)) - { - Marshal.Copy(Unsafe.As(source.Array), source.Start, ptr, countInSource); - return; - } - else if (Unsafe.SizeOf() == sizeof(short)) - { - Marshal.Copy(Unsafe.As(source.Array), source.Start, ptr, countInSource); - return; - } - else if (Unsafe.SizeOf() == sizeof(byte)) - { - Marshal.Copy(Unsafe.As(source.Array), source.Start, ptr, countInSource); - return; - } - } - - ref byte srcRef = ref Unsafe.As(ref source.DangerousGetPinnableReference()); - - fixed (void* pinnedSrc = &srcRef) - { - Unsafe.CopyBlock(pinnedSrc, pinnedDest, (uint)byteCount); - } -#endif - } - } - - /// - /// Per-type static value cache for type 'T' - /// - /// The type - internal class PerTypeValues - { - /// - /// Gets a value indicating whether 'T' is a primitive type. - /// - public static readonly bool IsPrimitiveType = - typeof(T) == typeof(byte) || - typeof(T) == typeof(char) || - typeof(T) == typeof(short) || - typeof(T) == typeof(ushort) || - typeof(T) == typeof(int) || - typeof(T) == typeof(uint) || - typeof(T) == typeof(float) || - typeof(T) == typeof(double) || - typeof(T) == typeof(long) || - typeof(T) == typeof(ulong) || - typeof(T) == typeof(IntPtr) || - typeof(T) == typeof(UIntPtr); - } } } \ No newline at end of file diff --git a/src/ImageSharp/Common/Memory/BufferSpan{T}.cs b/src/ImageSharp/Common/Memory/BufferSpan{T}.cs index 23b8bece7..d61f7f77c 100644 --- a/src/ImageSharp/Common/Memory/BufferSpan{T}.cs +++ b/src/ImageSharp/Common/Memory/BufferSpan{T}.cs @@ -106,23 +106,23 @@ namespace ImageSharp get { DebugGuard.MustBeLessThan(index, this.Length, nameof(index)); - ref T arrayRef = ref this.DangerousGetPinnableReference(); - return ref Unsafe.Add(ref arrayRef, this.Start); + ref T startRef = ref this.DangerousGetPinnableReference(); + return ref Unsafe.Add(ref startRef, index); } } /// /// Converts generic to a of bytes - /// setting it's to correct value. + /// setting it's and to correct values. /// - /// The to convert + /// The span of bytes [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static explicit operator BufferSpan(BufferSpan source) + public BufferSpan AsBytes() { BufferSpan result = default(BufferSpan); - result.Array = Unsafe.As(source.Array); - result.Start = source.Start * Unsafe.SizeOf(); - result.Length = source.Length * Unsafe.SizeOf(); + result.Array = Unsafe.As(this.Array); + result.Start = this.Start * Unsafe.SizeOf(); + result.Length = this.Length * Unsafe.SizeOf(); return result; } diff --git a/tests/ImageSharp.Tests/Colors/BulkPixelOperationsTests.cs b/tests/ImageSharp.Tests/Colors/BulkPixelOperationsTests.cs index 60e25aa04..fe005ad01 100644 --- a/tests/ImageSharp.Tests/Colors/BulkPixelOperationsTests.cs +++ b/tests/ImageSharp.Tests/Colors/BulkPixelOperationsTests.cs @@ -18,7 +18,7 @@ namespace ImageSharp.Tests.Colors } // For 4.6 test runner MemberData does not work without redeclaring the public field in the derived test class: - public static new TheoryData ArraySizesData => new TheoryData { 7, 16, 1111 }; + //public static new TheoryData ArraySizesData => new TheoryData { 7, 16, 1111 }; [Fact] public void IsSpecialImplementation() @@ -66,7 +66,7 @@ namespace ImageSharp.Tests.Colors { } - public static new TheoryData ArraySizesData => new TheoryData { 7, 16, 1111 }; + //public static new TheoryData ArraySizesData => new TheoryData { 7, 16, 1111 }; } [Theory] diff --git a/tests/ImageSharp.Tests/Common/BufferSpanTests.cs b/tests/ImageSharp.Tests/Common/BufferSpanTests.cs index f2fcb2539..58a217b18 100644 --- a/tests/ImageSharp.Tests/Common/BufferSpanTests.cs +++ b/tests/ImageSharp.Tests/Common/BufferSpanTests.cs @@ -31,13 +31,14 @@ namespace ImageSharp.Tests.Common using (PinnedBuffer colorBuf = new PinnedBuffer(fooz)) { BufferSpan orig = colorBuf.Slice(1); - BufferSpan asBytes = (BufferSpan)orig; + BufferSpan asBytes = orig.AsBytes(); Assert.Equal(asBytes.Start, sizeof(Foo)); + Assert.Equal(orig.Length * Unsafe.SizeOf(), asBytes.Length); Assert.SameRefs(ref orig.DangerousGetPinnableReference(), ref asBytes.DangerousGetPinnableReference()); } } - + public class Construct { [Fact] @@ -186,6 +187,26 @@ namespace ImageSharp.Tests.Common Assert.Equal(new Foo(666, 666), a[start + index]); } + + [Theory] + [InlineData(10, 0, 0, 5)] + [InlineData(10, 1, 1, 5)] + [InlineData(10, 1, 1, 6)] + [InlineData(10, 1, 1, 7)] + public void AsBytes_Read(int length, int start, int index, int byteOffset) + { + Foo[] a = Foo.CreateArray(length); + BufferSpan span = new BufferSpan(a, start); + + BufferSpan bytes = span.AsBytes(); + + byte actual = bytes[index * Unsafe.SizeOf() + byteOffset]; + + ref byte baseRef = ref Unsafe.As(ref a[0]); + byte expected = Unsafe.Add(ref baseRef, (start + index) * Unsafe.SizeOf() + byteOffset); + + Assert.Equal(expected, actual); + } } [Theory] @@ -310,7 +331,7 @@ namespace ImageSharp.Tests.Common BufferSpan apSource = new BufferSpan(source, 1); BufferSpan apDest = new BufferSpan(dest, sizeof(Foo)); - BufferSpan.Copy(apSource, apDest, count - 1); + BufferSpan.Copy(apSource.AsBytes(), apDest, (count - 1)*sizeof(Foo)); AssertNotDefault(source, 1); @@ -333,7 +354,7 @@ namespace ImageSharp.Tests.Common BufferSpan apSource = new BufferSpan(source, 1); BufferSpan apDest = new BufferSpan(dest, sizeof(AlignedFoo)); - BufferSpan.Copy(apSource, apDest, count - 1); + BufferSpan.Copy(apSource.AsBytes(), apDest, (count - 1) * sizeof(AlignedFoo)); AssertNotDefault(source, 1); @@ -356,7 +377,7 @@ namespace ImageSharp.Tests.Common BufferSpan apSource = new BufferSpan(source); BufferSpan apDest = new BufferSpan(dest); - BufferSpan.Copy(apSource, apDest, count); + BufferSpan.Copy(apSource.AsBytes(), apDest, count*sizeof(int)); AssertNotDefault(source, 1); @@ -377,7 +398,7 @@ namespace ImageSharp.Tests.Common BufferSpan apSource = new BufferSpan(source); BufferSpan apDest = new BufferSpan(dest); - BufferSpan.Copy(apSource, apDest, count); + BufferSpan.Copy(apSource, apDest.AsBytes(), count*sizeof(Foo)); AssertNotDefault(source, sizeof(Foo) + 1); AssertNotDefault(dest, 1); @@ -396,7 +417,7 @@ namespace ImageSharp.Tests.Common using (PinnedBuffer colorBuf = new PinnedBuffer(colors)) using (PinnedBuffer byteBuf = new PinnedBuffer(colors.Length * 4)) { - BufferSpan.Copy(colorBuf, byteBuf, colorBuf.Length); + BufferSpan.Copy(colorBuf.Span.AsBytes(), byteBuf, colorBuf.Length*sizeof(Color)); byte[] a = byteBuf.Array; diff --git a/tests/ImageSharp.Tests/Common/TestStructs.cs b/tests/ImageSharp.Tests/Common/TestStructs.cs index 9e762bbd1..f7f09bea2 100644 --- a/tests/ImageSharp.Tests/Common/TestStructs.cs +++ b/tests/ImageSharp.Tests/Common/TestStructs.cs @@ -25,6 +25,8 @@ namespace ImageSharp.Tests.Common } return result; } + + public override string ToString() => $"({this.A},{this.B})"; }