Correctly decode images (#1055)

When decoding from SKData, it is not immediate. Rather, the data is passed to
a lazy image that is decoded when the pixels are actually needed. Thus,
passing fixed the byte[] array will result in invalid images if the array
is moved in memory or changed later on. So, we create a copy.

The other cases where the codec was use to construct a bitmap first are also
wrong. SKBitmap constructs differently to SKImage, and since SKImage is just
going to create a copy anyway, just copy the encoded data once, and pass the
data to SKImage. This way, we are all in with SKImage, and we get the expected
results.

binding/Binding/SKImage.cs

@@ -188,10 +188,8 @@ namespace SkiaSharp
 			if (data.Length == 0)
 				throw new ArgumentException ("The data buffer was empty.");
 
-			fixed (byte* b = data) {
-				using (var skdata = SKData.Create ((IntPtr)b, data.Length)) {
-					return FromEncodedData (skdata);
-				}
+			using (var skdata = SKData.CreateCopy (data)) {
+				return FromEncodedData (skdata);
 			}
 		}
 
@@ -202,10 +200,8 @@ namespace SkiaSharp
 			if (data.Length == 0)
 				throw new ArgumentException ("The data buffer was empty.");
 
-			fixed (byte* b = data) {
-				using (var skdata = SKData.Create ((IntPtr)b, data.Length)) {
-					return FromEncodedData (skdata);
-				}
+			using (var skdata = SKData.CreateCopy (data)) {
+				return FromEncodedData (skdata);
 			}
 		}
 
@@ -214,19 +210,10 @@ namespace SkiaSharp
 			if (data == null)
 				throw new ArgumentNullException (nameof (data));
 
-			using (var codec = SKCodec.Create (data)) {
-				if (codec == null)
+			using (var skdata = SKData.Create (data)) {
+				if (skdata == null)
 					return null;
-
-				var bitmap = SKBitmap.Decode (codec, codec.Info);
-				if (bitmap == null)
-					return null;
-
-				bitmap.SetImmutable ();
-				return FromPixels (bitmap.PeekPixels (), delegate {
-					bitmap.Dispose ();
-					bitmap = null;
-				});
+				return FromEncodedData (skdata);
 			}
 		}
 
@@ -235,19 +222,10 @@ namespace SkiaSharp
 			if (data == null)
 				throw new ArgumentNullException (nameof (data));
 
-			using (var codec = SKCodec.Create (data)) {
-				if (codec == null)
+			using (var skdata = SKData.Create (data)) {
+				if (skdata == null)
 					return null;
-
-				var bitmap = SKBitmap.Decode (codec, codec.Info);
-				if (bitmap == null)
-					return null;
-
-				bitmap.SetImmutable ();
-				return FromPixels (bitmap.PeekPixels (), delegate {
-					bitmap.Dispose ();
-					bitmap = null;
-				});
+				return FromEncodedData (skdata);
 			}
 		}
 
@@ -256,19 +234,10 @@ namespace SkiaSharp
 			if (filename == null)
 				throw new ArgumentNullException (nameof (filename));
 
-			using (var codec = SKCodec.Create (filename)) {
-				if (codec == null)
+			using (var skdata = SKData.Create (filename)) {
+				if (skdata == null)
 					return null;
-
-				var bitmap = SKBitmap.Decode (codec, codec.Info);
-				if (bitmap == null)
-					return null;
-
-				bitmap.SetImmutable ();
-				return FromPixels (bitmap.PeekPixels (), delegate {
-					bitmap.Dispose ();
-					bitmap = null;
-				});
+				return FromEncodedData (skdata);
 			}
 		}
 

tests/Tests/SKImageTest.cs

@@ -525,6 +525,181 @@ namespace SkiaSharp.Tests
 			}
 		}
 
+		[Trait(CategoryKey, GpuCategory)]
+		[SkippableFact]
+		public void DecodingWithDataAndDrawingOnGPUCreatesCorrectImage()
+		{
+			var info = new SKImageInfo(120, 120);
+			var path = Path.Combine(PathToImages, "vimeo_icon_dark.png");
+
+			using (var ctx = CreateGlContext())
+			{
+				ctx.MakeCurrent();
+
+				using (var grContext = GRContext.CreateGl())
+				using (var surface = SKSurface.Create(grContext, true, info))
+				{
+					var canvas = surface.Canvas;
+
+					canvas.Clear(SKColors.Crimson);
+
+					using (var data = SKData.Create(path))
+					using (var image = SKImage.FromEncodedData(data))
+					{
+						canvas.DrawImage(image, 0, 0);
+					}
+
+					using (var bmp = new SKBitmap(info))
+					{
+						surface.ReadPixels(info, bmp.GetPixels(), info.RowBytes, 0, 0);
+
+						Assert.Equal(SKColors.Crimson, bmp.GetPixel(3, 3));
+						Assert.Equal(SKColors.Crimson, bmp.GetPixel(70, 50));
+						Assert.Equal(new SKColor(23, 35, 34), bmp.GetPixel(40, 40));
+					}
+				}
+			}
+		}
+
+		[Trait(CategoryKey, GpuCategory)]
+		[SkippableFact]
+		public void DecodingWithBitmapAndDrawingOnGPUCreatesCorrectImage()
+		{
+			var info = new SKImageInfo(120, 120);
+			var path = Path.Combine(PathToImages, "vimeo_icon_dark.png");
+
+			using (var ctx = CreateGlContext())
+			{
+				ctx.MakeCurrent();
+
+				using (var grContext = GRContext.CreateGl())
+				using (var surface = SKSurface.Create(grContext, true, info))
+				{
+					var canvas = surface.Canvas;
+
+					canvas.Clear(SKColors.Crimson);
+
+					using (var bitmap = SKBitmap.Decode(path))
+					using (var image = SKImage.FromBitmap(bitmap))
+					{
+						canvas.DrawImage(image, 0, 0);
+					}
+
+					using (var bmp = new SKBitmap(info))
+					{
+						surface.ReadPixels(info, bmp.GetPixels(), info.RowBytes, 0, 0);
+
+						Assert.Equal(SKColors.Crimson, bmp.GetPixel(3, 3));
+						Assert.Equal(SKColors.Crimson, bmp.GetPixel(70, 50));
+						Assert.Equal(new SKColor(23, 35, 34), bmp.GetPixel(40, 40));
+					}
+				}
+			}
+		}
+
+		[Trait(CategoryKey, GpuCategory)]
+		[SkippableFact]
+		public void DecodingWithPathAndDrawingOnGPUCreatesCorrectImage()
+		{
+			var info = new SKImageInfo(120, 120);
+			var path = Path.Combine(PathToImages, "vimeo_icon_dark.png");
+
+			using (var ctx = CreateGlContext())
+			{
+				ctx.MakeCurrent();
+
+				using (var grContext = GRContext.CreateGl())
+				using (var surface = SKSurface.Create(grContext, true, info))
+				{
+					var canvas = surface.Canvas;
+
+					canvas.Clear(SKColors.Crimson);
+
+					using (var image = SKImage.FromEncodedData(path))
+					{
+						canvas.DrawImage(image, 0, 0);
+					}
+
+					using (var bmp = new SKBitmap(info))
+					{
+						surface.ReadPixels(info, bmp.GetPixels(), info.RowBytes, 0, 0);
+
+						Assert.Equal(SKColors.Crimson, bmp.GetPixel(3, 3));
+						Assert.Equal(SKColors.Crimson, bmp.GetPixel(70, 50));
+						Assert.Equal(new SKColor(23, 35, 34), bmp.GetPixel(40, 40));
+					}
+				}
+			}
+		}
+
+		[SkippableFact]
+		public void DecodingWithDataCreatesCorrectImage()
+		{
+			var info = new SKImageInfo(120, 120);
+			var path = Path.Combine(PathToImages, "vimeo_icon_dark.png");
+
+			using (var bmp = new SKBitmap(info))
+			using (var canvas = new SKCanvas(bmp))
+			{
+				canvas.Clear(SKColors.Crimson);
+
+				using (var data = SKData.Create(path))
+				using (var image = SKImage.FromEncodedData(data))
+				{
+					canvas.DrawImage(image, 0, 0);
+				}
+
+				Assert.Equal(SKColors.Crimson, bmp.GetPixel(3, 3));
+				Assert.Equal(SKColors.Crimson, bmp.GetPixel(70, 50));
+				Assert.Equal(new SKColor(23, 35, 34), bmp.GetPixel(40, 40));
+			}
+		}
+
+		[SkippableFact]
+		public void DecodingWithBitmapCreatesCorrectImage()
+		{
+			var info = new SKImageInfo(120, 120);
+			var path = Path.Combine(PathToImages, "vimeo_icon_dark.png");
+
+			using (var bmp = new SKBitmap(info))
+			using (var canvas = new SKCanvas(bmp))
+			{
+				canvas.Clear(SKColors.Crimson);
+
+				using (var bitmap = SKBitmap.Decode(path))
+				using (var image = SKImage.FromBitmap(bitmap))
+				{
+					canvas.DrawImage(image, 0, 0);
+				}
+
+				Assert.Equal(SKColors.Crimson, bmp.GetPixel(3, 3));
+				Assert.Equal(SKColors.Crimson, bmp.GetPixel(70, 50));
+				Assert.Equal(new SKColor(23, 35, 34), bmp.GetPixel(40, 40));
+			}
+		}
+
+		[SkippableFact]
+		public void DecodingWithPathCreatesCorrectImage()
+		{
+			var info = new SKImageInfo(120, 120);
+			var path = Path.Combine(PathToImages, "vimeo_icon_dark.png");
+
+			using (var bmp = new SKBitmap(info))
+			using (var canvas = new SKCanvas(bmp))
+			{
+				canvas.Clear(SKColors.Crimson);
+
+				using (var image = SKImage.FromEncodedData(path))
+				{
+					canvas.DrawImage(image, 0, 0);
+				}
+
+				Assert.Equal(SKColors.Crimson, bmp.GetPixel(3, 3));
+				Assert.Equal(SKColors.Crimson, bmp.GetPixel(70, 50));
+				Assert.Equal(new SKColor(23, 35, 34), bmp.GetPixel(40, 40));
+			}
+		}
+
 		[Obsolete]
 		[SkippableFact]
 		public void EncodeWithSimpleSerializer()