From c76897104771e577750361df28ab7f8ff30025bc Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Fri, 15 Sep 2017 07:50:48 +0100 Subject: [PATCH 1/9] Expose advanced pixel reference API. --- src/ImageSharp/Advanced/ImageExtensions.cs | 44 +++++++++++++++++++--- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/src/ImageSharp/Advanced/ImageExtensions.cs b/src/ImageSharp/Advanced/ImageExtensions.cs index f4043b5ad..a517ea5b3 100644 --- a/src/ImageSharp/Advanced/ImageExtensions.cs +++ b/src/ImageSharp/Advanced/ImageExtensions.cs @@ -12,13 +12,35 @@ namespace SixLabors.ImageSharp.Advanced /// internal static class ImageExtensions { + /// + /// Gets a reference to the pixel at the specified position. + /// + /// The source image frame + /// The x coordinate (row) + /// The y coordinate (position at row) + /// A reference to the element. + public static ref TPixel GetPixelReference(this ImageFrame source, int x, int y) + where TPixel : struct, IPixel + => ref GetPixelReference((IPixelSource)source, x, y); + + /// + /// Gets a reference to the pixel at the specified position. + /// + /// The source image frame + /// The x coordinate (row) + /// The y coordinate (position at row) + /// A reference to the element. + public static ref TPixel GetPixelReference(this Image source, int x, int y) + where TPixel : struct, IPixel + => ref source.Frames.RootFrame.GetPixelReference(x, y); + /// /// Gets the representation of the pixels as an area of contiguous memory in the given pixel format. /// /// The type of the pixel. /// The source. /// The - public static Span GetPixelSpan(this ImageFrame source) + internal static Span GetPixelSpan(this ImageFrame source) where TPixel : struct, IPixel => GetSpan(source); @@ -29,7 +51,7 @@ namespace SixLabors.ImageSharp.Advanced /// The source. /// The row. /// The - public static Span GetPixelRowSpan(this ImageFrame source, int row) + internal static Span GetPixelRowSpan(this ImageFrame source, int row) where TPixel : struct, IPixel => GetSpan(source, row); @@ -39,7 +61,7 @@ namespace SixLabors.ImageSharp.Advanced /// The type of the pixel. /// The source. /// The - public static Span GetPixelSpan(this Image source) + internal static Span GetPixelSpan(this Image source) where TPixel : struct, IPixel => source.Frames.RootFrame.GetPixelSpan(); @@ -50,7 +72,7 @@ namespace SixLabors.ImageSharp.Advanced /// The source. /// The row. /// The - public static Span GetPixelRowSpan(this Image source, int row) + internal static Span GetPixelRowSpan(this Image source, int row) where TPixel : struct, IPixel => source.Frames.RootFrame.GetPixelRowSpan(row); @@ -60,7 +82,7 @@ namespace SixLabors.ImageSharp.Advanced /// The Pixel format. /// The source image /// Returns the configuration. - public static Configuration GetConfiguration(this Image source) + internal static Configuration GetConfiguration(this Image source) where TPixel : struct, IPixel => GetConfiguration((IConfigurable)source); @@ -107,5 +129,17 @@ namespace SixLabors.ImageSharp.Advanced /// Returns the bounds of the image private static Configuration GetConfiguration(IConfigurable source) => source?.Configuration ?? Configuration.Default; + + /// + /// Gets a reference to the pixel at the specified position. + /// + /// The source image frame + /// The x coordinate (row) + /// The y coordinate (position at row) + /// A reference to the element. + private static ref TPixel GetPixelReference(IPixelSource source, int x, int y) + where TPixel : struct, IPixel + => ref source.PixelBuffer[x, y]; + } } From ed7f9b020b96ea133cca14261f3b77594218d5a9 Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Fri, 15 Sep 2017 08:02:47 +0100 Subject: [PATCH 2/9] fix style cop issues --- src/ImageSharp/Advanced/ImageExtensions.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ImageSharp/Advanced/ImageExtensions.cs b/src/ImageSharp/Advanced/ImageExtensions.cs index a517ea5b3..fa299f811 100644 --- a/src/ImageSharp/Advanced/ImageExtensions.cs +++ b/src/ImageSharp/Advanced/ImageExtensions.cs @@ -140,6 +140,5 @@ namespace SixLabors.ImageSharp.Advanced private static ref TPixel GetPixelReference(IPixelSource source, int x, int y) where TPixel : struct, IPixel => ref source.PixelBuffer[x, y]; - } } From 0afea0f9d66a2e85b3173d9d66010aea0f671cc5 Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Fri, 15 Sep 2017 19:18:41 +0100 Subject: [PATCH 3/9] rename api to DangerousGetPinnableReferenceToPixelBuffer --- src/ImageSharp/Advanced/ImageExtensions.cs | 33 ++++++++++------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/src/ImageSharp/Advanced/ImageExtensions.cs b/src/ImageSharp/Advanced/ImageExtensions.cs index fa299f811..63a66de83 100644 --- a/src/ImageSharp/Advanced/ImageExtensions.cs +++ b/src/ImageSharp/Advanced/ImageExtensions.cs @@ -13,26 +13,24 @@ namespace SixLabors.ImageSharp.Advanced internal static class ImageExtensions { /// - /// Gets a reference to the pixel at the specified position. + /// Returns a reference to the 0th element of the Pixel buffer. + /// Such a reference can be used for pinning but must never be dereferenced. /// /// The source image frame - /// The x coordinate (row) - /// The y coordinate (position at row) - /// A reference to the element. - public static ref TPixel GetPixelReference(this ImageFrame source, int x, int y) + /// A pinnable reference the first root of the pixel buffer. + public static ref TPixel DangerousGetPinnableReferenceToPixelBuffer(this ImageFrame source) where TPixel : struct, IPixel - => ref GetPixelReference((IPixelSource)source, x, y); + => ref DangerousGetPinnableReferenceToPixelBuffer((IPixelSource)source); /// - /// Gets a reference to the pixel at the specified position. + /// Returns a reference to the 0th element of the Pixel buffer. + /// Such a reference can be used for pinning but must never be dereferenced. /// - /// The source image frame - /// The x coordinate (row) - /// The y coordinate (position at row) - /// A reference to the element. - public static ref TPixel GetPixelReference(this Image source, int x, int y) + /// The source image + /// A pinnable reference the first root of the pixel buffer. + public static ref TPixel DangerousGetPinnableReferenceToPixelBuffer(this Image source) where TPixel : struct, IPixel - => ref source.Frames.RootFrame.GetPixelReference(x, y); + => ref source.Frames.RootFrame.DangerousGetPinnableReferenceToPixelBuffer(); /// /// Gets the representation of the pixels as an area of contiguous memory in the given pixel format. @@ -131,14 +129,13 @@ namespace SixLabors.ImageSharp.Advanced => source?.Configuration ?? Configuration.Default; /// - /// Gets a reference to the pixel at the specified position. + /// Returns a reference to the 0th element of the Pixel buffer. + /// Such a reference can be used for pinning but must never be dereferenced. /// /// The source image frame - /// The x coordinate (row) - /// The y coordinate (position at row) /// A reference to the element. - private static ref TPixel GetPixelReference(IPixelSource source, int x, int y) + private static ref TPixel DangerousGetPinnableReferenceToPixelBuffer(IPixelSource source) where TPixel : struct, IPixel - => ref source.PixelBuffer[x, y]; + => ref source.PixelBuffer.Span.DangerousGetPinnableReference(); } } From 2fa2f1d459b4ce17f67eb55c2a06829e1f9c72e5 Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Fri, 15 Sep 2017 21:18:44 +0100 Subject: [PATCH 4/9] make class public --- src/ImageSharp/Advanced/ImageExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Advanced/ImageExtensions.cs b/src/ImageSharp/Advanced/ImageExtensions.cs index 63a66de83..19e37689d 100644 --- a/src/ImageSharp/Advanced/ImageExtensions.cs +++ b/src/ImageSharp/Advanced/ImageExtensions.cs @@ -10,7 +10,7 @@ namespace SixLabors.ImageSharp.Advanced /// /// Extension methods over Image{TPixel} /// - internal static class ImageExtensions + public static class ImageExtensions { /// /// Returns a reference to the 0th element of the Pixel buffer. From c1526747557597930b055b5cdc2a98675445fa33 Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Fri, 15 Sep 2017 21:26:06 +0100 Subject: [PATCH 5/9] fix style issues --- src/ImageSharp/Advanced/ImageExtensions.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ImageSharp/Advanced/ImageExtensions.cs b/src/ImageSharp/Advanced/ImageExtensions.cs index 19e37689d..aaff68312 100644 --- a/src/ImageSharp/Advanced/ImageExtensions.cs +++ b/src/ImageSharp/Advanced/ImageExtensions.cs @@ -16,6 +16,7 @@ namespace SixLabors.ImageSharp.Advanced /// Returns a reference to the 0th element of the Pixel buffer. /// Such a reference can be used for pinning but must never be dereferenced. /// + /// The Pixel format. /// The source image frame /// A pinnable reference the first root of the pixel buffer. public static ref TPixel DangerousGetPinnableReferenceToPixelBuffer(this ImageFrame source) @@ -26,6 +27,7 @@ namespace SixLabors.ImageSharp.Advanced /// Returns a reference to the 0th element of the Pixel buffer. /// Such a reference can be used for pinning but must never be dereferenced. /// + /// The Pixel format. /// The source image /// A pinnable reference the first root of the pixel buffer. public static ref TPixel DangerousGetPinnableReferenceToPixelBuffer(this Image source) From 2945aaef9afe8310d44c150fd1fd2a203ac26e4a Mon Sep 17 00:00:00 2001 From: Anton Firsov Date: Sat, 16 Sep 2017 00:15:39 +0200 Subject: [PATCH 6/9] Updated docs on DangerousGetPinnableReferenceToPixelBuffer() --- src/ImageSharp/Advanced/ImageExtensions.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Advanced/ImageExtensions.cs b/src/ImageSharp/Advanced/ImageExtensions.cs index aaff68312..8ab245aee 100644 --- a/src/ImageSharp/Advanced/ImageExtensions.cs +++ b/src/ImageSharp/Advanced/ImageExtensions.cs @@ -13,8 +13,9 @@ namespace SixLabors.ImageSharp.Advanced public static class ImageExtensions { /// - /// Returns a reference to the 0th element of the Pixel buffer. - /// Such a reference can be used for pinning but must never be dereferenced. + /// Returns a reference to the 0th element of the Pixel buffer, + /// allowing direct manipulation of pixel data through unsafe operations. + /// The pixel buffer is a contigous memory area containing Width*Height TPixel elements layed out in row-major order. /// /// The Pixel format. /// The source image frame @@ -24,8 +25,9 @@ namespace SixLabors.ImageSharp.Advanced => ref DangerousGetPinnableReferenceToPixelBuffer((IPixelSource)source); /// - /// Returns a reference to the 0th element of the Pixel buffer. - /// Such a reference can be used for pinning but must never be dereferenced. + /// Returns a reference to the 0th element of the Pixel buffer, + /// allowing direct manipulation of pixel data through unsafe operations. + /// The pixel buffer is a contigous memory area containing Width*Height TPixel elements layed out in row-major order. /// /// The Pixel format. /// The source image From fef1366b501e0397c41ec8f1d785506cd1e7ecb5 Mon Sep 17 00:00:00 2001 From: Eric Mellino Date: Sat, 16 Sep 2017 03:04:09 -0700 Subject: [PATCH 7/9] Add a test case for DangerousGetPinnableReferenceToPixelBuffer. --- .../Advanced/ImageExtensionsTests.cs | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 tests/ImageSharp.Tests/Advanced/ImageExtensionsTests.cs diff --git a/tests/ImageSharp.Tests/Advanced/ImageExtensionsTests.cs b/tests/ImageSharp.Tests/Advanced/ImageExtensionsTests.cs new file mode 100644 index 000000000..3bd6bf19b --- /dev/null +++ b/tests/ImageSharp.Tests/Advanced/ImageExtensionsTests.cs @@ -0,0 +1,39 @@ +// 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(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()); + 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]); + } + } + } +} From 5dc06612b2cf0d231a97d7e124f3bf12bf12e0cc Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Sat, 16 Sep 2017 17:08:04 +0200 Subject: [PATCH 8/9] Renamed *.Advanced.ImageExtensions to AdvancedImageExtensions, new ImageExtensions.SavePixelData() overloads, better tests --- ...tensions.cs => AdvancedImageExtensions.cs} | 2 +- src/ImageSharp/Image/ImageExtensions.cs | 52 ++++++++---- .../Advanced/AdvancedImageExtensionsTests.cs | 38 +++++++++ .../Advanced/ImageExtensionsTests.cs | 39 --------- .../ImageSharp.Tests/Image/ImageSaveTests.cs | 80 ++++++------------- .../TestUtilities/TestImageExtensions.cs | 24 +++++- 6 files changed, 121 insertions(+), 114 deletions(-) rename src/ImageSharp/Advanced/{ImageExtensions.cs => AdvancedImageExtensions.cs} (99%) create mode 100644 tests/ImageSharp.Tests/Advanced/AdvancedImageExtensionsTests.cs delete mode 100644 tests/ImageSharp.Tests/Advanced/ImageExtensionsTests.cs diff --git a/src/ImageSharp/Advanced/ImageExtensions.cs b/src/ImageSharp/Advanced/AdvancedImageExtensions.cs similarity index 99% rename from src/ImageSharp/Advanced/ImageExtensions.cs rename to src/ImageSharp/Advanced/AdvancedImageExtensions.cs index 8ab245aee..511e66c64 100644 --- a/src/ImageSharp/Advanced/ImageExtensions.cs +++ b/src/ImageSharp/Advanced/AdvancedImageExtensions.cs @@ -10,7 +10,7 @@ namespace SixLabors.ImageSharp.Advanced /// /// Extension methods over Image{TPixel} /// - public static class ImageExtensions + public static class AdvancedImageExtensions { /// /// Returns a reference to the 0th element of the Pixel buffer, diff --git a/src/ImageSharp/Image/ImageExtensions.cs b/src/ImageSharp/Image/ImageExtensions.cs index c5b3d6f31..c4de1c298 100644 --- a/src/ImageSharp/Image/ImageExtensions.cs +++ b/src/ImageSharp/Image/ImageExtensions.cs @@ -112,7 +112,7 @@ namespace SixLabors.ImageSharp } /// - /// Saves the raw image to the given bytes. + /// Saves the raw image pixels to a byte array in row-major order. /// /// The Pixel format. /// The source image @@ -123,7 +123,7 @@ namespace SixLabors.ImageSharp => source.GetPixelSpan().AsBytes().ToArray(); /// - /// Saves the raw image to the given bytes. + /// Saves the raw image pixels to the given byte array in row-major order. /// /// The Pixel format. /// The source image @@ -131,26 +131,21 @@ namespace SixLabors.ImageSharp /// Thrown if the stream is null. public static void SavePixelData(this ImageFrame source, byte[] buffer) where TPixel : struct, IPixel - => SavePixelData(source, new Span(buffer)); + => SavePixelData(source, buffer.AsSpan().NonPortableCast()); /// - /// Saves the raw image to the given bytes. + /// Saves the raw image pixels to the given TPixel array in row-major order. /// /// The Pixel format. /// The source image /// The buffer to save the raw pixel data to. /// Thrown if the stream is null. - private static void SavePixelData(this ImageFrame source, Span buffer) + public static void SavePixelData(this ImageFrame source, TPixel[] buffer) where TPixel : struct, IPixel - { - Span byteBuffer = source.GetPixelSpan().AsBytes(); - Guard.MustBeGreaterThanOrEqualTo(buffer.Length, byteBuffer.Length, nameof(buffer)); - - byteBuffer.CopyTo(buffer); - } + => SavePixelData(source, new Span(buffer)); /// - /// Saves the raw image to the given bytes. + /// Saves the raw image pixels to a byte array in row-major order. /// /// The Pixel format. /// The source image @@ -161,7 +156,7 @@ namespace SixLabors.ImageSharp => source.Frames.RootFrame.SavePixelData(); /// - /// Saves the raw image to the given bytes. + /// Saves the raw image pixels to the given byte array in row-major order. /// /// The Pixel format. /// The source image @@ -172,13 +167,13 @@ namespace SixLabors.ImageSharp => source.Frames.RootFrame.SavePixelData(buffer); /// - /// Saves the raw image to the given bytes. + /// Saves the raw image pixels to the given TPixel array in row-major order. /// /// The Pixel format. /// The source image /// The buffer to save the raw pixel data to. /// Thrown if the stream is null. - private static void SavePixelData(this Image source, Span buffer) + public static void SavePixelData(this Image source, TPixel[] buffer) where TPixel : struct, IPixel => source.Frames.RootFrame.SavePixelData(buffer); @@ -200,5 +195,32 @@ namespace SixLabors.ImageSharp return $"data:{format.DefaultMimeType};base64,{Convert.ToBase64String(stream.ToArray())}"; } } + + /// + /// Saves the raw image to the given bytes. + /// + /// The Pixel format. + /// The source image + /// The buffer to save the raw pixel data to. + /// Thrown if the stream is null. + internal static void SavePixelData(this Image source, Span buffer) + where TPixel : struct, IPixel + => source.Frames.RootFrame.SavePixelData(buffer.NonPortableCast()); + + /// + /// Saves the raw image to the given bytes. + /// + /// The Pixel format. + /// The source image + /// The buffer to save the raw pixel data to. + /// Thrown if the stream is null. + internal static void SavePixelData(this ImageFrame source, Span buffer) + where TPixel : struct, IPixel + { + Span sourceBuffer = source.GetPixelSpan(); + Guard.MustBeGreaterThanOrEqualTo(buffer.Length, sourceBuffer.Length, nameof(buffer)); + + sourceBuffer.CopyTo(buffer); + } } } diff --git a/tests/ImageSharp.Tests/Advanced/AdvancedImageExtensionsTests.cs b/tests/ImageSharp.Tests/Advanced/AdvancedImageExtensionsTests.cs new file mode 100644 index 000000000..4291e775d --- /dev/null +++ b/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(TestImageProvider provider) + where TPixel : struct, IPixel + { + using (Image image = provider.GetImage()) + { + TPixel[] targetBuffer = new TPixel[image.Width * image.Height]; + + ref byte source = ref Unsafe.As(ref targetBuffer[0]); + ref byte dest = ref Unsafe.As(ref image.DangerousGetPinnableReferenceToPixelBuffer()); + + fixed (byte* targetPtr = &source) + fixed (byte* pixelBasePtr = &dest) + { + uint dataSizeInBytes = (uint)(image.Width * image.Height * Unsafe.SizeOf()); + Unsafe.CopyBlock(targetPtr, pixelBasePtr, dataSizeInBytes); + } + + image.ComparePixelBufferTo(targetBuffer); + } + } + } +} diff --git a/tests/ImageSharp.Tests/Advanced/ImageExtensionsTests.cs b/tests/ImageSharp.Tests/Advanced/ImageExtensionsTests.cs deleted file mode 100644 index 3bd6bf19b..000000000 --- a/tests/ImageSharp.Tests/Advanced/ImageExtensionsTests.cs +++ /dev/null @@ -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(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()); - 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]); - } - } - } -} diff --git a/tests/ImageSharp.Tests/Image/ImageSaveTests.cs b/tests/ImageSharp.Tests/Image/ImageSaveTests.cs index 36d3b3c05..5b672059c 100644 --- a/tests/ImageSharp.Tests/Image/ImageSaveTests.cs +++ b/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; + /// /// Tests the class. /// @@ -47,76 +50,39 @@ namespace SixLabors.ImageSharp.Tests this.Image = new Image(config, 1, 1); } - [Fact] - public void SavePixelData_Rgba32() + [Theory] + [WithTestPatternImages(13, 19, PixelTypes.Rgba32 | PixelTypes.Bgr24)] + public void SavePixelData_ToPixelStructArray(TestImageProvider provider) + where TPixel : struct, IPixel { - using (var img = new Image(2, 2)) + using (Image 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(TestImageProvider provider) + where TPixel : struct, IPixel { - using (var img = new Image(2, 2)) + using (Image image = provider.GetImage()) { - img[0, 0] = NamedColors.White; - img[1, 0] = NamedColors.Black; - - img[0, 1] = NamedColors.Red; - img[1, 1] = NamedColors.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()]; - 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()); } } - + [Fact] - public void SavePixelData_Rgba32_Buffer_must_be_bigger() + public void SavePixelData_Rgba32_WhenBufferIsTooSmall_Throws() { using (var img = new Image(2, 2)) { diff --git a/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs b/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs index b3dd763c7..b367ecbd9 100644 --- a/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs +++ b/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 { /// @@ -81,7 +83,7 @@ namespace SixLabors.ImageSharp.Tests grayscale, appendPixelTypeToFileName); } - + /// /// 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 . @@ -153,6 +155,24 @@ namespace SixLabors.ImageSharp.Tests } } + public static Image ComparePixelBufferTo( + this Image image, + Span expectedPixels) + where TPixel : struct, IPixel + { + Span 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 CompareToOriginal( this Image image, ITestImageProvider provider) @@ -160,7 +180,7 @@ namespace SixLabors.ImageSharp.Tests { return CompareToOriginal(image, provider, ImageComparer.Tolerant()); } - + public static Image CompareToOriginal( this Image image, ITestImageProvider provider, From 63fa1fdbe1c44fb72a96ceb3bde316637da97528 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Sat, 16 Sep 2017 17:14:23 +0200 Subject: [PATCH 9/9] unrelated to PR: Skipping AVX2-tests if the CPU does not support AVX2 --- src/ImageSharp/Common/Extensions/SimdUtils.cs | 4 +-- .../PixelFormats/Rgba32.PixelOperations.cs | 2 +- .../ImageSharp.Tests/Common/SimdUtilsTests.cs | 30 +++++++++++++++++++ 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/ImageSharp/Common/Extensions/SimdUtils.cs b/src/ImageSharp/Common/Extensions/SimdUtils.cs index cb80a672a..d6b2fad09 100644 --- a/src/ImageSharp/Common/Extensions/SimdUtils.cs +++ b/src/ImageSharp/Common/Extensions/SimdUtils.cs @@ -17,11 +17,11 @@ namespace SixLabors.ImageSharp /// /// Indicates AVX2 architecture where both float and integer registers are of size 256 byte. /// - public static readonly bool IsAvx2 = Vector.Count == 8 && Vector.Count == 8; + public static readonly bool IsAvx2CompatibleArchitecture = Vector.Count == 8 && Vector.Count == 8; internal static void GuardAvx2(string operation) { - if (!IsAvx2) + if (!IsAvx2CompatibleArchitecture) { throw new NotSupportedException($"{operation} is supported only on AVX2 CPU!"); } diff --git a/src/ImageSharp/PixelFormats/Rgba32.PixelOperations.cs b/src/ImageSharp/PixelFormats/Rgba32.PixelOperations.cs index 6f4f93d87..552ac0a01 100644 --- a/src/ImageSharp/PixelFormats/Rgba32.PixelOperations.cs +++ b/src/ImageSharp/PixelFormats/Rgba32.PixelOperations.cs @@ -120,7 +120,7 @@ namespace SixLabors.ImageSharp { GuardSpans(sourceVectors, nameof(sourceVectors), destColors, nameof(destColors), count); - if (!SimdUtils.IsAvx2) + if (!SimdUtils.IsAvx2CompatibleArchitecture) { base.PackFromVector4(sourceVectors, destColors, count); return; diff --git a/tests/ImageSharp.Tests/Common/SimdUtilsTests.cs b/tests/ImageSharp.Tests/Common/SimdUtilsTests.cs index 44762a243..e5f2fd5e7 100644 --- a/tests/ImageSharp.Tests/Common/SimdUtilsTests.cs +++ b/tests/ImageSharp.Tests/Common/SimdUtilsTests.cs @@ -109,6 +109,16 @@ namespace SixLabors.ImageSharp.Tests.Common AssertEvenRoundIsCorrect(r, v); } + private bool SkipOnNonAvx2([CallerMemberName] string testCaseName = null) + { + if (!SimdUtils.IsAvx2CompatibleArchitecture) + { + this.Output.WriteLine("Skipping AVX2 specific test case: " + testCaseName); + return true; + } + return false; + } + [Theory] [InlineData(1, 0)] [InlineData(1, 8)] @@ -116,6 +126,11 @@ namespace SixLabors.ImageSharp.Tests.Common [InlineData(3, 128)] public void BulkConvertNormalizedFloatToByte_WithRoundedData(int seed, int count) { + if (this.SkipOnNonAvx2()) + { + return; + } + float[] orig = new Random(seed).GenerateRandomRoundedFloatArray(count, 0, 256); float[] normalized = orig.Select(f => f / 255f).ToArray(); @@ -135,6 +150,11 @@ namespace SixLabors.ImageSharp.Tests.Common [InlineData(3, 128)] public void BulkConvertNormalizedFloatToByte_WithNonRoundedData(int seed, int count) { + if (this.SkipOnNonAvx2()) + { + return; + } + float[] source = new Random(seed).GenerateRandomFloatArray(count, 0, 1f); byte[] dest = new byte[count]; @@ -155,6 +175,11 @@ namespace SixLabors.ImageSharp.Tests.Common [InlineData(3, 128)] public void BulkConvertNormalizedFloatToByteClampOverflows(int seed, int count) { + if (this.SkipOnNonAvx2()) + { + return; + } + float[] orig = new Random(seed).GenerateRandomRoundedFloatArray(count, -50, 444); float[] normalized = orig.Select(f => f / 255f).ToArray(); @@ -185,6 +210,11 @@ namespace SixLabors.ImageSharp.Tests.Common [Fact] private void BulkConvertNormalizedFloatToByte_Step() { + if (this.SkipOnNonAvx2()) + { + return; + } + float[] source = {0, 7, 42, 255, 0.5f, 1.1f, 2.6f, 16f}; byte[] expected = source.Select(f => (byte)Math.Round(f)).ToArray();