Expose a generic, writable span for the pixels (#1242)

* Expose a generic, writable span for the pixels * Fixes #1240 * "Cache" the info object in size so we don' have to hop the interop. * Raw pixels depend on the CPU endianness
mono · Apr 23, 2020 · 290b61e · 290b61e
1 parent 928b729
commit 290b61e
Show file tree

Hide file tree

Showing 4 changed files with 128 additions and 6 deletions.
diff --git a/binding/Binding/SKPixmap.cs b/binding/Binding/SKPixmap.cs
@@ -95,9 +95,14 @@ public SKImageInfo Info {
 
 		public int Height => Info.Height;
 
-		public SKSizeI Size => new SKSizeI (Width, Height);
+		public SKSizeI Size {
+			get {
+				var info = Info;
+				return new SKSizeI (info.Width, info.Height);
+			}
+		}
 
-		public SKRectI Rect => SKRectI.Create (Width, Height);
+		public SKRectI Rect => SKRectI.Create (Size);
 
 		public SKColorType ColorType => Info.ColorType;
 
@@ -119,9 +124,30 @@ public IntPtr GetPixels () =>
 		public IntPtr GetPixels (int x, int y) =>
 			(IntPtr)SkiaApi.sk_pixmap_get_pixels_with_xy (Handle, x, y);
 
-		public ReadOnlySpan<byte> GetPixelSpan ()
+		public ReadOnlySpan<byte> GetPixelSpan () =>
+			new ReadOnlySpan<byte> (SkiaApi.sk_pixmap_get_pixels (Handle), BytesSize);
+
+		public unsafe Span<T> GetPixelSpan<T> ()
+			where T : unmanaged
 		{
-			return new ReadOnlySpan<byte> ((void*)GetPixels (), BytesSize);
+			var info = Info;
+			if (info.IsEmpty)
+				return null;
+
+			var bpp = info.BytesPerPixel;
+			if (bpp <= 0)
+				return null;
+
+			// byte is always valid
+			if (typeof (T) == typeof (byte))
+				return new Span<T> (SkiaApi.sk_pixmap_get_writable_addr (Handle), info.BytesSize);
+
+			// other types need to make sure they fit
+			var size = sizeof (T);
+			if (bpp != size)
+				throw new ArgumentException ($"Size of T ({size}) is not the same as the size of each pixel ({bpp}).", nameof (T));
+
+			return new Span<T> (SkiaApi.sk_pixmap_get_writable_addr (Handle), info.Width * info.Height);
 		}
 
 		public SKColor GetPixelColor (int x, int y)

diff --git a/binding/Binding/SkiaApi.generated.cs b/binding/Binding/SkiaApi.generated.cs
@@ -2547,6 +2547,10 @@ internal unsafe partial class SkiaApi
 		[DllImport (SKIA, CallingConvention = CallingConvention.Cdecl)]
 		internal static extern /* size_t */ IntPtr sk_pixmap_get_row_bytes (sk_pixmap_t cpixmap);
 
+		// void* sk_pixmap_get_writable_addr(const sk_pixmap_t* cpixmap)
+		[DllImport (SKIA, CallingConvention = CallingConvention.Cdecl)]
+		internal static extern void* sk_pixmap_get_writable_addr (sk_pixmap_t cpixmap);
+
 		// sk_pixmap_t* sk_pixmap_new()
 		[DllImport (SKIA, CallingConvention = CallingConvention.Cdecl)]
 		internal static extern sk_pixmap_t sk_pixmap_new ();

diff --git a/externals/skia b/externals/skia
diff --git a/tests/Tests/SKPixmapTest.cs b/tests/Tests/SKPixmapTest.cs
@@ -1,11 +1,18 @@
 using System;
+using System.Collections.Generic;
 using System.Runtime.InteropServices;
 using Xunit;
 
 namespace SkiaSharp.Tests
 {
 	public class SKPixmapTest : SKTest
 	{
+		public static IEnumerable<object[]> GetAllColorTypes()
+		{
+			foreach (SKColorType ct in Enum.GetValues(typeof(SKColorType)))
+				yield return new object[] { ct };
+		}
+
 		[SkippableFact]
 		public void CanScalePixels()
 		{
@@ -48,7 +55,7 @@ public void ReadPixelSucceeds()
 
 			Assert.True(result);
 		}
-		
+
 		[SkippableFact]
 		public void WithMethodsDoNotModifySource()
 		{
@@ -176,5 +183,90 @@ public void EncodeWithWebpEncoder()
 
 			Assert.Equal(SKEncodedImageFormat.Webp, codec.EncodedFormat);
 		}
+
+		[SkippableFact]
+		public void MismatchingColorTypesThrow()
+		{
+			var info = new SKImageInfo(1, 1, SKColorType.Rgba8888);
+			using var bmp = new SKBitmap(info);
+			using var pixmap = bmp.PeekPixels();
+
+			Assert.Throws<ArgumentException>(() => pixmap.GetPixelSpan<ushort>());
+		}
+
+		[SkippableTheory]
+		[MemberData(nameof(GetAllColorTypes))]
+		public void ByteWorksForEverything(SKColorType colortype)
+		{
+			var info = new SKImageInfo(1, 1, colortype);
+			using var bmp = new SKBitmap(info);
+			using var pixmap = bmp.PeekPixels();
+
+			Assert.Equal(info.BytesSize, pixmap?.GetPixelSpan<byte>().Length ?? 0);
+		}
+
+		[SkippableTheory]
+		[InlineData(0x00000000)]
+		[InlineData(0xFF000000)]
+		[InlineData(0xFFFF0000)]
+		[InlineData(0xFF00FF00)]
+		[InlineData(0xFF0000FF)]
+		[InlineData(0xFFFFFFFF)]
+		public void GetPixelSpanReadsValuesCorrectly(uint color)
+		{
+			var rgb888 = (SKColor)color;
+
+			var info = new SKImageInfo(1, 1, SKColorType.Rgba8888);
+			using var bmp = new SKBitmap(info);
+			using var pixmap = bmp.PeekPixels();
+
+			pixmap.Erase(rgb888);
+
+			// no need for swizzle
+			Assert.Equal(rgb888, pixmap.GetPixelColor(0, 0));
+
+			// swizzle for some CPU endianness
+			if (BitConverter.IsLittleEndian)
+				rgb888 = new SKColor(rgb888.Blue, rgb888.Green, rgb888.Red, rgb888.Alpha);
+
+			Assert.Equal(rgb888, pixmap.GetPixelSpan<SKColor>()[0]);
+			Assert.Equal(rgb888, pixmap.GetPixelSpan<uint>()[0]);
+		}
+
+		[SkippableTheory]
+		[InlineData(0x00000000, 0x0000)]
+		[InlineData(0xFF000000, 0x0000)]
+		[InlineData(0xFFFF0000, 0xF800)]
+		[InlineData(0xFF00FF00, 0x07E0)]
+		[InlineData(0xFF0000FF, 0x001F)]
+		[InlineData(0xFFFFFFFF, 0xFFFF)]
+		public void GetPixelSpanReads565Correctly(uint rgb888, ushort rgb565)
+		{
+			var info = new SKImageInfo(1, 1, SKColorType.Rgb565);
+			using var bmp = new SKBitmap(info);
+			using var pixmap = bmp.PeekPixels();
+
+			pixmap.Erase(rgb888);
+
+			Assert.Equal(rgb565, pixmap.GetPixelSpan<ushort>()[0]);
+		}
+
+		[SkippableTheory]
+		[InlineData(0x00000000, 0)]
+		[InlineData(0xFF000000, 0)]
+		[InlineData(0xFFFF0000, 53)]
+		[InlineData(0xFF00FF00, 182)]
+		[InlineData(0xFF0000FF, 18)]
+		[InlineData(0xFFFFFFFF, 255)]
+		public void GetPixelSpanReadsGray8Correctly(uint rgb888, byte gray8)
+		{
+			var info = new SKImageInfo(1, 1, SKColorType.Gray8);
+			using var bmp = new SKBitmap(info);
+			using var pixmap = bmp.PeekPixels();
+
+			pixmap.Erase(rgb888);
+
+			Assert.Equal(gray8, pixmap.GetPixelSpan<byte>()[0]);
+		}
 	}
 }