Png encoder now passes parallel test #24

Previous build failed test when using unmanaged buffer copying. via
PixelRow

src/ImageSharp/Formats/Png/Filters/AverageFilter.cs

@@ -43,8 +43,7 @@ namespace ImageSharp.Formats
         /// <param name="previousScanline">The previous scanline.</param>
         /// <param name="result">The filtered scanline result.</param>
         /// <param name="bytesPerPixel">The bytes per pixel.</param>
-        /// <param name="bytesPerScanline">The number of bytes per scanline</param>
+        public static void Encode(byte[] scanline, byte[] previousScanline, byte[] result, int bytesPerPixel)
-        public static void Encode(byte[] scanline, byte[] previousScanline, byte[] result, int bytesPerPixel, int bytesPerScanline)
         {
             // Average(x) = Raw(x) - floor((Raw(x-bpp)+Prior(x))/2)
             fixed (byte* scan = scanline)
@@ -53,7 +52,7 @@ namespace ImageSharp.Formats
             {
                 res[0] = 3;
 
-                for (int x = 0; x < bytesPerScanline; x++)
+                for (int x = 0; x < scanline.Length; x++)
                 {
                     byte left = (x - bytesPerPixel < 0) ? (byte)0 : scan[x - bytesPerPixel];
                     byte above = prev[x];

src/ImageSharp/Formats/Png/Filters/NoneFilter.cs

@@ -30,12 +30,11 @@ namespace ImageSharp.Formats
         /// </summary>
         /// <param name="scanline">The scanline to encode</param>
         /// <param name="result">The filtered scanline result.</param>
-        /// <param name="bytesPerScanline">The number of bytes per scanline</param>
+        public static void Encode(byte[] scanline, byte[] result)
-        public static void Encode(byte[] scanline, byte[] result, int bytesPerScanline)
         {
             // Insert a byte before the data.
             result[0] = 0;
-            Buffer.BlockCopy(scanline, 0, result, 1, bytesPerScanline);
+            Buffer.BlockCopy(scanline, 0, result, 1, scanline.Length);
         }
     }
 }

src/ImageSharp/Formats/Png/Filters/PaethFilter.cs

@@ -45,8 +45,7 @@ namespace ImageSharp.Formats
         /// <param name="previousScanline">The previous scanline.</param>
         /// <param name="result">The filtered scanline result.</param>
         /// <param name="bytesPerPixel">The bytes per pixel.</param>
-        /// <param name="bytesPerScanline">The number of bytes per scanline</param>
+        public static void Encode(byte[] scanline, byte[] previousScanline, byte[] result, int bytesPerPixel)
-        public static void Encode(byte[] scanline, byte[] previousScanline, byte[] result, int bytesPerPixel, int bytesPerScanline)
         {
             // Paeth(x) = Raw(x) - PaethPredictor(Raw(x-bpp), Prior(x), Prior(x - bpp))
             fixed (byte* scan = scanline)
@@ -55,7 +54,7 @@ namespace ImageSharp.Formats
             {
                 res[0] = 4;
 
-                for (int x = 0; x < bytesPerScanline; x++)
+                for (int x = 0; x < scanline.Length; x++)
                 {
                     byte left = (x - bytesPerPixel < 0) ? (byte)0 : scan[x - bytesPerPixel];
                     byte above = prev[x];

src/ImageSharp/Formats/Png/Filters/SubFilter.cs

@@ -36,8 +36,7 @@ namespace ImageSharp.Formats
         /// <param name="scanline">The scanline to encode</param>
         /// <param name="result">The filtered scanline result.</param>
         /// <param name="bytesPerPixel">The bytes per pixel.</param>
-        /// <param name="bytesPerScanline">The number of bytes per scanline</param>
+        public static void Encode(byte[] scanline, byte[] result, int bytesPerPixel)
-        public static void Encode(byte[] scanline, byte[] result, int bytesPerPixel, int bytesPerScanline)
         {
             // Sub(x) = Raw(x) - Raw(x-bpp)
             fixed (byte* scan = scanline)
@@ -45,7 +44,7 @@ namespace ImageSharp.Formats
             {
                 res[0] = 1;
 
-                for (int x = 0; x < bytesPerScanline; x++)
+                for (int x = 0; x < scanline.Length; x++)
                 {
                     byte priorRawByte = (x - bytesPerPixel < 0) ? (byte)0 : scan[x - bytesPerPixel];
 

src/ImageSharp/Formats/Png/Filters/UpFilter.cs

@@ -38,8 +38,7 @@ namespace ImageSharp.Formats
         /// <param name="scanline">The scanline to encode</param>
         /// <param name="previousScanline">The previous scanline.</param>
         /// <param name="result">The filtered scanline result.</param>
-        /// <param name="bytesPerScanline">The number of bytes per scanline</param>
+        public static void Encode(byte[] scanline, byte[] previousScanline, byte[] result)
-        public static void Encode(byte[] scanline, byte[] previousScanline, byte[] result, int bytesPerScanline)
         {
             // Up(x) = Raw(x) - Prior(x)
             fixed (byte* scan = scanline)
@@ -48,7 +47,7 @@ namespace ImageSharp.Formats
             {
                 res[0] = 2;
 
-                for (int x = 0; x < bytesPerScanline; x++)
+                for (int x = 0; x < scanline.Length; x++)
                 {
                     byte above = prev[x];
 

src/ImageSharp/Formats/Png/PngEncoderCore.cs

@@ -316,11 +316,11 @@ namespace ImageSharp.Formats
             where TColor : struct, IPackedPixel<TPacked>
             where TPacked : struct
         {
-            // TODO: This should be possible one row at a time
+            // We can use the optimized PixelAccessor here and copy the bytes in unmanaged memory.
             int bpp = this.bytesPerPixel;
-            for (int x = 0; x < this.width; x++)
+            using (PixelRow<TColor, TPacked> pixelRow = new PixelRow<TColor, TPacked>(this.width, rawScanline, bpp == 4 ? ComponentOrder.XYZW : ComponentOrder.XYZ))
             {
-                pixels[x, row].ToBytes(rawScanline, x * this.bytesPerPixel, bpp == 4 ? ComponentOrder.XYZW : ComponentOrder.XYZ);
+                pixels.CopyTo(pixelRow, row);
             }
         }
 
@@ -335,16 +335,15 @@ namespace ImageSharp.Formats
         /// <param name="previousScanline">The previous scanline.</param>
         /// <param name="rawScanline">The raw scanline.</param>
         /// <param name="result">The filtered scanline result.</param>
-        /// <param name="bytesPerScanline">The number of bytes per scanline.</param>
         /// <returns>The <see cref="T:byte[]"/></returns>
-        private byte[] EncodePixelRow<TColor, TPacked>(PixelAccessor<TColor, TPacked> pixels, int row, byte[] previousScanline, byte[] rawScanline, byte[] result, int bytesPerScanline)
+        private byte[] EncodePixelRow<TColor, TPacked>(PixelAccessor<TColor, TPacked> pixels, int row, byte[] previousScanline, byte[] rawScanline, byte[] result)
             where TColor : struct, IPackedPixel<TPacked>
             where TPacked : struct
         {
             switch (this.PngColorType)
             {
                 case PngColorType.Palette:
-                    Buffer.BlockCopy(this.palettePixelData, row * bytesPerScanline, rawScanline, 0, bytesPerScanline);
+                    Buffer.BlockCopy(this.palettePixelData, row * rawScanline.Length, rawScanline, 0, rawScanline.Length);
                     break;
                 case PngColorType.Grayscale:
                 case PngColorType.GrayscaleWithAlpha:
@@ -355,7 +354,7 @@ namespace ImageSharp.Formats
                     break;
             }
 
-            return this.GetOptimalFilteredScanline(rawScanline, previousScanline, result, bytesPerScanline);
+            return this.GetOptimalFilteredScanline(rawScanline, previousScanline, result);
         }
 
         /// <summary>
@@ -365,25 +364,24 @@ namespace ImageSharp.Formats
         /// <param name="rawScanline">The raw scanline</param>
         /// <param name="previousScanline">The previous scanline</param>
         /// <param name="result">The filtered scanline result.</param>
-        /// <param name="bytesPerScanline">The number of bytes per scanline</param>
         /// <returns>The <see cref="T:byte[]"/></returns>
-        private byte[] GetOptimalFilteredScanline(byte[] rawScanline, byte[] previousScanline, byte[] result, int bytesPerScanline)
+        private byte[] GetOptimalFilteredScanline(byte[] rawScanline, byte[] previousScanline, byte[] result)
         {
             // Palette images don't compress well with adaptive filtering.
             if (this.PngColorType == PngColorType.Palette)
             {
-                NoneFilter.Encode(rawScanline, result, bytesPerScanline);
+                NoneFilter.Encode(rawScanline, result);
                 return result;
             }
 
-            SubFilter.Encode(rawScanline, this.sub, this.bytesPerPixel, bytesPerScanline);
+            SubFilter.Encode(rawScanline, this.sub, this.bytesPerPixel);
-            int currentTotalVariation = this.CalculateTotalVariation(this.sub, bytesPerScanline);
+            int currentTotalVariation = this.CalculateTotalVariation(this.sub);
             int lowestTotalVariation = currentTotalVariation;
 
             result = this.sub;
 
-            UpFilter.Encode(rawScanline, previousScanline, this.up, bytesPerScanline);
+            UpFilter.Encode(rawScanline, previousScanline, this.up);
-            currentTotalVariation = this.CalculateTotalVariation(this.up, bytesPerScanline);
+            currentTotalVariation = this.CalculateTotalVariation(this.up);
 
             if (currentTotalVariation < lowestTotalVariation)
             {
@@ -391,8 +389,8 @@ namespace ImageSharp.Formats
                 result = this.up;
             }
 
-            AverageFilter.Encode(rawScanline, previousScanline, this.average, this.bytesPerPixel, bytesPerScanline);
+            AverageFilter.Encode(rawScanline, previousScanline, this.average, this.bytesPerPixel);
-            currentTotalVariation = this.CalculateTotalVariation(this.average, bytesPerScanline);
+            currentTotalVariation = this.CalculateTotalVariation(this.average);
 
             if (currentTotalVariation < lowestTotalVariation)
             {
@@ -400,8 +398,8 @@ namespace ImageSharp.Formats
                 result = this.average;
             }
 
-            PaethFilter.Encode(rawScanline, previousScanline, this.paeth, this.bytesPerPixel, bytesPerScanline);
+            PaethFilter.Encode(rawScanline, previousScanline, this.paeth, this.bytesPerPixel);
-            currentTotalVariation = this.CalculateTotalVariation(this.paeth, bytesPerScanline);
+            currentTotalVariation = this.CalculateTotalVariation(this.paeth);
 
             if (currentTotalVariation < lowestTotalVariation)
             {
@@ -415,16 +413,15 @@ namespace ImageSharp.Formats
         /// Calculates the total variation of given byte array. Total variation is the sum of the absolute values of
         /// neighbor differences.
         /// </summary>
-        /// <param name="input">The scanline bytes</param>
+        /// <param name="scanline">The scanline bytes</param>
-        /// <param name="bytesPerScanline">The number of bytes per scanline</param>
         /// <returns>The <see cref="int"/></returns>
-        private int CalculateTotalVariation(byte[] input, int bytesPerScanline)
+        private int CalculateTotalVariation(byte[] scanline)
         {
             int totalVariation = 0;
 
-            for (int i = 1; i < bytesPerScanline; i++)
+            for (int i = 1; i < scanline.Length; i++)
             {
-                totalVariation += Math.Abs(input[i] - input[i - 1]);
+                totalVariation += Math.Abs(scanline[i] - scanline[i - 1]);
             }
 
             return totalVariation;
@@ -618,17 +615,17 @@ namespace ImageSharp.Formats
             where TPacked : struct
         {
             int bytesPerScanline = this.width * this.bytesPerPixel;
-            byte[] previousScanline = ArrayPool<byte>.Shared.Rent(bytesPerScanline);
+            byte[] previousScanline = new byte[bytesPerScanline];
-            byte[] rawScanline = ArrayPool<byte>.Shared.Rent(bytesPerScanline);
+            byte[] rawScanline = new byte[bytesPerScanline];
             int resultLength = bytesPerScanline + 1;
-            byte[] result = ArrayPool<byte>.Shared.Rent(resultLength);
+            byte[] result = new byte[resultLength];
 
             if (this.PngColorType != PngColorType.Palette)
             {
-                this.sub = ArrayPool<byte>.Shared.Rent(resultLength);
+                this.sub = new byte[resultLength];
-                this.up = ArrayPool<byte>.Shared.Rent(resultLength);
+                this.up = new byte[resultLength];
-                this.average = ArrayPool<byte>.Shared.Rent(resultLength);
+                this.average = new byte[resultLength];
-                this.paeth = ArrayPool<byte>.Shared.Rent(resultLength);
+                this.paeth = new byte[resultLength];
             }
 
             byte[] buffer;
@@ -641,7 +638,7 @@ namespace ImageSharp.Formats
                 {
                     for (int y = 0; y < this.height; y++)
                     {
-                        deflateStream.Write(this.EncodePixelRow(pixels, y, previousScanline, rawScanline, result, bytesPerScanline), 0, resultLength);
+                        deflateStream.Write(this.EncodePixelRow(pixels, y, previousScanline, rawScanline, result), 0, resultLength);
 
                         Swap(ref rawScanline, ref previousScanline);
                     }
@@ -653,17 +650,6 @@ namespace ImageSharp.Formats
             finally
             {
                 memoryStream?.Dispose();
-                ArrayPool<byte>.Shared.Return(previousScanline);
-                ArrayPool<byte>.Shared.Return(rawScanline);
-                ArrayPool<byte>.Shared.Return(result);
-
-                if (this.PngColorType != PngColorType.Palette)
-                {
-                    ArrayPool<byte>.Shared.Return(this.sub);
-                    ArrayPool<byte>.Shared.Return(this.up);
-                    ArrayPool<byte>.Shared.Return(this.average);
-                    ArrayPool<byte>.Shared.Return(this.paeth);
-                }
             }
 
             // Store the chunks in repeated 64k blocks.
@@ -743,4 +729,4 @@ namespace ImageSharp.Formats
             WriteInteger(stream, (uint)crc32.Value);
         }
     }
-}
+}

src/ImageSharp/Image/PixelRow.cs

@@ -39,6 +39,32 @@ namespace ImageSharp
         /// </remarks>
         private bool isDisposed;
 
+        /// <summary>
+        /// Initializes a new instance of the <see cref="PixelRow{TColor,TPacked}"/> class.
+        /// </summary>
+        /// <param name="width">The width.</param>
+        /// <param name="bytes">The bytes.</param>
+        /// <param name="componentOrder">The component order.</param>
+        /// <exception cref="ArgumentOutOfRangeException">
+        /// Thrown if <paramref name="bytes"></paramref> is the incorrect length.
+        /// </exception>
+        public PixelRow(int width, byte[] bytes, ComponentOrder componentOrder)
+        {
+            if (bytes.Length != width * GetComponentCount(componentOrder))
+            {
+                throw new ArgumentOutOfRangeException($"Invalid byte array length. Length {bytes.Length}; Should be {width * GetComponentCount(componentOrder)}.");
+            }
+
+            this.Width = width;
+            this.ComponentOrder = componentOrder;
+            this.Bytes = bytes;
+            this.pixelsHandle = GCHandle.Alloc(this.Bytes, GCHandleType.Pinned);
+
+            // TODO: Why is Resharper warning us about an impure method call?
+            this.dataPointer = this.pixelsHandle.AddrOfPinnedObject();
+            this.PixelBase = (byte*)this.dataPointer.ToPointer();
+        }
+
         /// <summary>
         /// Initializes a new instance of the <see cref="PixelRow{TColor,TPacked}"/> class.
         /// </summary>

tests/ImageSharp.Tests/Formats/Png/PngTests.cs

@@ -44,7 +44,7 @@ namespace ImageSharp.Tests
 
                         using (FileStream output = File.OpenWrite($"{path}/{file.FileNameWithoutExtension}.png"))
                         {
-                            image.Save(output, new PngFormat());
+                            image.SaveAsPng(output);
                         }
                     });
         }