Renamed *.Advanced.ImageExtensions to AdvancedImageExtensions,

new ImageExtensions.SavePixelData() overloads,
better tests

src/ImageSharp/Advanced/ImageExtensions.cs → src/ImageSharp/Advanced/AdvancedImageExtensions.cs

@@ -10,7 +10,7 @@ namespace SixLabors.ImageSharp.Advanced
     /// <summary>
     /// Extension methods over Image{TPixel}
     /// </summary>
-    public static class ImageExtensions
+    public static class AdvancedImageExtensions
     {
         /// <summary>
         /// Returns a reference to the 0th element of the Pixel buffer,

src/ImageSharp/Image/ImageExtensions.cs

@@ -112,7 +112,7 @@ namespace SixLabors.ImageSharp
         }
 
         /// <summary>
-        /// Saves the raw image to the given bytes.
+        ///  Saves the raw image pixels to a byte array in row-major order.
         /// </summary>
         /// <typeparam name="TPixel">The Pixel format.</typeparam>
         /// <param name="source">The source image</param>
@@ -123,7 +123,7 @@ namespace SixLabors.ImageSharp
          => source.GetPixelSpan().AsBytes().ToArray();
 
         /// <summary>
-        /// Saves the raw image to the given bytes.
+        /// Saves the raw image pixels to the given byte array in row-major order.
         /// </summary>
         /// <typeparam name="TPixel">The Pixel format.</typeparam>
         /// <param name="source">The source image</param>
@@ -131,26 +131,21 @@ namespace SixLabors.ImageSharp
         /// <exception cref="System.ArgumentNullException">Thrown if the stream is null.</exception>
         public static void SavePixelData<TPixel>(this ImageFrame<TPixel> source, byte[] buffer)
             where TPixel : struct, IPixel<TPixel>
-            => SavePixelData(source, new Span<byte>(buffer));
+            => SavePixelData(source, buffer.AsSpan().NonPortableCast<byte, TPixel>());
 
         /// <summary>
-        /// Saves the raw image to the given bytes.
+        /// Saves the raw image pixels to the given TPixel array in row-major order.
         /// </summary>
         /// <typeparam name="TPixel">The Pixel format.</typeparam>
         /// <param name="source">The source image</param>
         /// <param name="buffer">The buffer to save the raw pixel data to.</param>
         /// <exception cref="System.ArgumentNullException">Thrown if the stream is null.</exception>
-        private static void SavePixelData<TPixel>(this ImageFrame<TPixel> source, Span<byte> buffer)
+        public static void SavePixelData<TPixel>(this ImageFrame<TPixel> source, TPixel[] buffer)
             where TPixel : struct, IPixel<TPixel>
-        {
-            Span<byte> byteBuffer = source.GetPixelSpan().AsBytes();
-            Guard.MustBeGreaterThanOrEqualTo(buffer.Length, byteBuffer.Length, nameof(buffer));
-
-            byteBuffer.CopyTo(buffer);
-        }
+            => SavePixelData(source, new Span<TPixel>(buffer));
 
         /// <summary>
-        /// Saves the raw image to the given bytes.
+        /// Saves the raw image pixels to a byte array in row-major order.
         /// </summary>
         /// <typeparam name="TPixel">The Pixel format.</typeparam>
         /// <param name="source">The source image</param>
@@ -161,7 +156,7 @@ namespace SixLabors.ImageSharp
          => source.Frames.RootFrame.SavePixelData();
 
         /// <summary>
-        /// Saves the raw image to the given bytes.
+        /// Saves the raw image pixels to the given byte array in row-major order.
         /// </summary>
         /// <typeparam name="TPixel">The Pixel format.</typeparam>
         /// <param name="source">The source image</param>
@@ -172,13 +167,13 @@ namespace SixLabors.ImageSharp
             => source.Frames.RootFrame.SavePixelData(buffer);
 
         /// <summary>
-        /// Saves the raw image to the given bytes.
+        /// Saves the raw image pixels to the given TPixel array in row-major order.
         /// </summary>
         /// <typeparam name="TPixel">The Pixel format.</typeparam>
         /// <param name="source">The source image</param>
         /// <param name="buffer">The buffer to save the raw pixel data to.</param>
         /// <exception cref="System.ArgumentNullException">Thrown if the stream is null.</exception>
-        private static void SavePixelData<TPixel>(this Image<TPixel> source, Span<byte> buffer)
+        public static void SavePixelData<TPixel>(this Image<TPixel> source, TPixel[] buffer)
             where TPixel : struct, IPixel<TPixel>
             => source.Frames.RootFrame.SavePixelData(buffer);
 
@@ -200,5 +195,32 @@ namespace SixLabors.ImageSharp
                 return $"data:{format.DefaultMimeType};base64,{Convert.ToBase64String(stream.ToArray())}";
             }
         }
+
+        /// <summary>
+        /// Saves the raw image to the given bytes.
+        /// </summary>
+        /// <typeparam name="TPixel">The Pixel format.</typeparam>
+        /// <param name="source">The source image</param>
+        /// <param name="buffer">The buffer to save the raw pixel data to.</param>
+        /// <exception cref="System.ArgumentNullException">Thrown if the stream is null.</exception>
+        internal static void SavePixelData<TPixel>(this Image<TPixel> source, Span<byte> buffer)
+            where TPixel : struct, IPixel<TPixel>
+            => source.Frames.RootFrame.SavePixelData(buffer.NonPortableCast<byte, TPixel>());
+
+        /// <summary>
+        /// Saves the raw image to the given bytes.
+        /// </summary>
+        /// <typeparam name="TPixel">The Pixel format.</typeparam>
+        /// <param name="source">The source image</param>
+        /// <param name="buffer">The buffer to save the raw pixel data to.</param>
+        /// <exception cref="System.ArgumentNullException">Thrown if the stream is null.</exception>
+        internal static void SavePixelData<TPixel>(this ImageFrame<TPixel> source, Span<TPixel> buffer)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            Span<TPixel> sourceBuffer = source.GetPixelSpan();
+            Guard.MustBeGreaterThanOrEqualTo(buffer.Length, sourceBuffer.Length, nameof(buffer));
+
+            sourceBuffer.CopyTo(buffer);
+        }
     }
 }

tests/ImageSharp.Tests/Advanced/AdvancedImageExtensionsTests.cs

@@ -0,0 +1,38 @@
+// Copyright (c) Six Labors and contributors.
+// Licensed under the Apache License, Version 2.0.
+
+using Xunit;
+using SixLabors.ImageSharp.Advanced;
+using System.Runtime.CompilerServices;
+// ReSharper disable InconsistentNaming
+
+namespace SixLabors.ImageSharp.Tests.Advanced
+{
+    using SixLabors.ImageSharp.PixelFormats;
+
+    public class AdvancedImageExtensionsTests
+    {
+        [Theory]
+        [WithTestPatternImages(131, 127, PixelTypes.Rgba32 | PixelTypes.Bgr24)]
+        public unsafe void DangerousGetPinnableReference_CopyToBuffer<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            using (Image<TPixel> image = provider.GetImage())
+            {
+                TPixel[] targetBuffer = new TPixel[image.Width * image.Height];
+
+                ref byte source = ref Unsafe.As<TPixel, byte>(ref targetBuffer[0]);
+                ref byte dest = ref Unsafe.As<TPixel, byte>(ref image.DangerousGetPinnableReferenceToPixelBuffer());
+                
+                fixed (byte* targetPtr = &source)
+                fixed (byte* pixelBasePtr = &dest)
+                {
+                    uint dataSizeInBytes = (uint)(image.Width * image.Height * Unsafe.SizeOf<TPixel>());
+                    Unsafe.CopyBlock(targetPtr, pixelBasePtr, dataSizeInBytes);
+                }
+
+                image.ComparePixelBufferTo(targetBuffer);
+            }
+        }
+    }
+}

tests/ImageSharp.Tests/Advanced/ImageExtensionsTests.cs

@@ -1,39 +0,0 @@
-// Copyright (c) Six Labors and contributors.
-// Licensed under the Apache License, Version 2.0.
-
-using Xunit;
-using SixLabors.ImageSharp.Advanced;
-using System.Runtime.CompilerServices;
-
-namespace SixLabors.ImageSharp.Tests.Advanced
-{
-    public class ImageExtensionsTests
-    {
-        [Fact]
-        public unsafe void DangerousGetPinnableReference_CopyToBuffer()
-        {
-            var image = new Image<Rgba32>(128, 128);
-            for (int y = 0; y < image.Height; y++)
-                for (int x = 0; x < image.Width; x++)
-                {
-                    image[x, y] = new Rgba32(x, 255 - y, x + y);
-                }
-
-            Rgba32[] targetBuffer = new Rgba32[image.Width * image.Height];
-
-            fixed (Rgba32* targetPtr = targetBuffer)
-            fixed (Rgba32* pixelBasePtr = &image.DangerousGetPinnableReferenceToPixelBuffer())
-            {
-                uint dataSizeInBytes = (uint)(image.Width * image.Height * Unsafe.SizeOf<Rgba32>());
-                Unsafe.CopyBlock(targetPtr, pixelBasePtr, dataSizeInBytes);
-            }
-
-            for (int y = 0; y < image.Height; y++)
-                for (int x = 0; x < image.Width; x++)
-                {
-                    int linearIndex = y * image.Width + x;
-                    Assert.Equal(image[x, y], targetBuffer[linearIndex]);
-                }
-        }
-    }
-}

tests/ImageSharp.Tests/Image/ImageSaveTests.cs

@@ -9,9 +9,12 @@ using SixLabors.ImageSharp.IO;
 using SixLabors.ImageSharp.PixelFormats;
 using Moq;
 using Xunit;
+// ReSharper disable InconsistentNaming
 
 namespace SixLabors.ImageSharp.Tests
 {
+    using System.Runtime.CompilerServices;
+
     /// <summary>
     /// Tests the <see cref="Image"/> class.
     /// </summary>
@@ -47,76 +50,39 @@ namespace SixLabors.ImageSharp.Tests
             this.Image = new Image<Rgba32>(config, 1, 1);
         }
 
-        [Fact]
-        public void SavePixelData_Rgba32()
+        [Theory]
+        [WithTestPatternImages(13, 19, PixelTypes.Rgba32 | PixelTypes.Bgr24)]
+        public void SavePixelData_ToPixelStructArray<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
         {
-            using (var img = new Image<Rgba32>(2, 2))
+            using (Image<TPixel> image = provider.GetImage())
             {
-                img[0, 0] = Rgba32.White;
-                img[1, 0] = Rgba32.Black;
+                TPixel[] buffer = new TPixel[image.Width*image.Height];
+                image.SavePixelData(buffer);
 
-                img[0, 1] = Rgba32.Red;
-                img[1, 1] = Rgba32.Blue;
-                var buffer = new byte[2 * 2 * 4]; // width * height * bytes per pixel
-                img.SavePixelData(buffer);
-
-                Assert.Equal(255, buffer[0]); // 0, 0, R
-                Assert.Equal(255, buffer[1]); // 0, 0, G
-                Assert.Equal(255, buffer[2]); // 0, 0, B
-                Assert.Equal(255, buffer[3]); // 0, 0, A
-
-                Assert.Equal(0, buffer[4]); // 1, 0, R
-                Assert.Equal(0, buffer[5]); // 1, 0, G
-                Assert.Equal(0, buffer[6]); // 1, 0, B
-                Assert.Equal(255, buffer[7]); // 1, 0, A
-
-                Assert.Equal(255, buffer[8]); // 0, 1, R
-                Assert.Equal(0, buffer[9]); // 0, 1, G
-                Assert.Equal(0, buffer[10]); // 0, 1, B
-                Assert.Equal(255, buffer[11]); // 0, 1, A
-
-                Assert.Equal(0, buffer[12]); // 1, 1, R
-                Assert.Equal(0, buffer[13]); // 1, 1, G
-                Assert.Equal(255, buffer[14]); // 1, 1, B
-                Assert.Equal(255, buffer[15]); // 1, 1, A
+                image.ComparePixelBufferTo(buffer);
+
+                // TODO: We need a separate test-case somewhere ensuring that image pixels are stored in row-major order!
             }
         }
 
-
-        [Fact]
-        public void SavePixelData_Bgr24()
+        [Theory]
+        [WithTestPatternImages(19, 13, PixelTypes.Rgba32 | PixelTypes.Bgr24)]
+        public void SavePixelData_ToByteArray<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
         {
-            using (var img = new Image<Bgr24>(2, 2))
+            using (Image<TPixel> image = provider.GetImage())
             {
-                img[0, 0] = NamedColors<Bgr24>.White;
-                img[1, 0] = NamedColors<Bgr24>.Black;
-
-                img[0, 1] = NamedColors<Bgr24>.Red;
-                img[1, 1] = NamedColors<Bgr24>.Blue;
-
-                var buffer = new byte[2 * 2 * 3]; // width * height * bytes per pixel
-                img.SavePixelData(buffer);
+                byte[] buffer = new byte[image.Width*image.Height*Unsafe.SizeOf<TPixel>()];
 
-                Assert.Equal(255, buffer[0]); // 0, 0, B
-                Assert.Equal(255, buffer[1]); // 0, 0, G
-                Assert.Equal(255, buffer[2]); // 0, 0, R
+                image.SavePixelData(buffer);
 
-                Assert.Equal(0, buffer[3]); // 1, 0, B
-                Assert.Equal(0, buffer[4]); // 1, 0, G
-                Assert.Equal(0, buffer[5]); // 1, 0, R
-
-                Assert.Equal(0, buffer[6]); // 0, 1, B
-                Assert.Equal(0, buffer[7]); // 0, 1,   G
-                Assert.Equal(255, buffer[8]); // 0, 1,  R
-
-                Assert.Equal(255, buffer[9]); // 1, 1,  B
-                Assert.Equal(0, buffer[10]); // 1, 1,  G
-                Assert.Equal(0, buffer[11]); // 1, 1, R
+                image.ComparePixelBufferTo(buffer.AsSpan().NonPortableCast<byte, TPixel>());
             }
         }
-
+        
         [Fact]
-        public void SavePixelData_Rgba32_Buffer_must_be_bigger()
+        public void SavePixelData_Rgba32_WhenBufferIsTooSmall_Throws()
         {
             using (var img = new Image<Rgba32>(2, 2))
             {

tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs

@@ -17,6 +17,8 @@ namespace SixLabors.ImageSharp.Tests
     using SixLabors.ImageSharp.Advanced;
     using SixLabors.ImageSharp.Memory;
 
+    using Xunit;
+
     public static class TestImageExtensions
     {
         /// <summary>
@@ -81,7 +83,7 @@ namespace SixLabors.ImageSharp.Tests
                 grayscale,
                 appendPixelTypeToFileName);
         }
-
+        
         /// <summary>
         /// Compares the image against the expected Reference output, throws an exception if the images are not similar enough.
         /// The output file should be named identically to the output produced by <see cref="DebugSave{TPixel}(Image{TPixel}, ITestImageProvider, object, string, bool)"/>.
@@ -153,6 +155,24 @@ namespace SixLabors.ImageSharp.Tests
             }
         }
 
+        public static Image<TPixel> ComparePixelBufferTo<TPixel>(
+            this Image<TPixel> image,
+            Span<TPixel> expectedPixels)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            Span<TPixel> actual = image.GetPixelSpan();
+
+            Assert.True(expectedPixels.Length == actual.Length, "Buffer sizes are not equal!");
+
+            for (int i = 0; i < expectedPixels.Length; i++)
+            {
+                Assert.True(expectedPixels[i].Equals(actual[i]), $"Pixels are different on position {i}!" );
+            }
+            
+            return image;
+        }
+
+
         public static Image<TPixel> CompareToOriginal<TPixel>(
             this Image<TPixel> image,
             ITestImageProvider provider)
@@ -160,7 +180,7 @@ namespace SixLabors.ImageSharp.Tests
         {
             return CompareToOriginal(image, provider, ImageComparer.Tolerant());
         }
-
+        
         public static Image<TPixel> CompareToOriginal<TPixel>(
             this Image<TPixel> image,
             ITestImageProvider provider,