From a96e9113362410cb61a9919a3a5302ecb55f43e1 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 15 Nov 2016 16:05:06 +1100 Subject: [PATCH] Png encoder now passes parallel test #24 Previous build failed test when using unmanaged buffer copying. via PixelRow --- .../Formats/Png/Filters/AverageFilter.cs | 5 +- .../Formats/Png/Filters/NoneFilter.cs | 5 +- .../Formats/Png/Filters/PaethFilter.cs | 5 +- .../Formats/Png/Filters/SubFilter.cs | 5 +- .../Formats/Png/Filters/UpFilter.cs | 5 +- src/ImageSharp/Formats/Png/PngEncoderCore.cs | 72 ++++++++----------- src/ImageSharp/Image/PixelRow.cs | 26 +++++++ .../ImageSharp.Tests/Formats/Png/PngTests.cs | 2 +- 8 files changed, 66 insertions(+), 59 deletions(-) diff --git a/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs b/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs index 3855b289da..7a94ee32b2 100644 --- a/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs @@ -43,8 +43,7 @@ namespace ImageSharp.Formats /// The previous scanline. /// The filtered scanline result. /// The bytes per pixel. - /// The number of bytes per scanline - public static void Encode(byte[] scanline, byte[] previousScanline, byte[] result, int bytesPerPixel, int bytesPerScanline) + public static void Encode(byte[] scanline, byte[] previousScanline, byte[] result, int bytesPerPixel) { // 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]; diff --git a/src/ImageSharp/Formats/Png/Filters/NoneFilter.cs b/src/ImageSharp/Formats/Png/Filters/NoneFilter.cs index 426f9f1d9b..e5787a9442 100644 --- a/src/ImageSharp/Formats/Png/Filters/NoneFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/NoneFilter.cs @@ -30,12 +30,11 @@ namespace ImageSharp.Formats /// /// The scanline to encode /// The filtered scanline result. - /// The number of bytes per scanline - public static void Encode(byte[] scanline, byte[] result, int bytesPerScanline) + public static void Encode(byte[] scanline, byte[] result) { // Insert a byte before the data. result[0] = 0; - Buffer.BlockCopy(scanline, 0, result, 1, bytesPerScanline); + Buffer.BlockCopy(scanline, 0, result, 1, scanline.Length); } } } diff --git a/src/ImageSharp/Formats/Png/Filters/PaethFilter.cs b/src/ImageSharp/Formats/Png/Filters/PaethFilter.cs index 692ccfed8e..f257641e9d 100644 --- a/src/ImageSharp/Formats/Png/Filters/PaethFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/PaethFilter.cs @@ -45,8 +45,7 @@ namespace ImageSharp.Formats /// The previous scanline. /// The filtered scanline result. /// The bytes per pixel. - /// The number of bytes per scanline - public static void Encode(byte[] scanline, byte[] previousScanline, byte[] result, int bytesPerPixel, int bytesPerScanline) + public static void Encode(byte[] scanline, byte[] previousScanline, byte[] result, int bytesPerPixel) { // 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]; diff --git a/src/ImageSharp/Formats/Png/Filters/SubFilter.cs b/src/ImageSharp/Formats/Png/Filters/SubFilter.cs index 6cf5c6cdb4..a098a242ca 100644 --- a/src/ImageSharp/Formats/Png/Filters/SubFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/SubFilter.cs @@ -36,8 +36,7 @@ namespace ImageSharp.Formats /// The scanline to encode /// The filtered scanline result. /// The bytes per pixel. - /// The number of bytes per scanline - public static void Encode(byte[] scanline, byte[] result, int bytesPerPixel, int bytesPerScanline) + public static void Encode(byte[] scanline, byte[] result, int bytesPerPixel) { // 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]; diff --git a/src/ImageSharp/Formats/Png/Filters/UpFilter.cs b/src/ImageSharp/Formats/Png/Filters/UpFilter.cs index e4281fb736..9b8d78e811 100644 --- a/src/ImageSharp/Formats/Png/Filters/UpFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/UpFilter.cs @@ -38,8 +38,7 @@ namespace ImageSharp.Formats /// The scanline to encode /// The previous scanline. /// The filtered scanline result. - /// The number of bytes per scanline - public static void Encode(byte[] scanline, byte[] previousScanline, byte[] result, int bytesPerScanline) + public static void Encode(byte[] scanline, byte[] previousScanline, byte[] result) { // 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]; diff --git a/src/ImageSharp/Formats/Png/PngEncoderCore.cs b/src/ImageSharp/Formats/Png/PngEncoderCore.cs index 95f50cf276..bca1d0033f 100644 --- a/src/ImageSharp/Formats/Png/PngEncoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngEncoderCore.cs @@ -316,11 +316,11 @@ namespace ImageSharp.Formats where TColor : struct, IPackedPixel 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 pixelRow = new PixelRow(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 /// The previous scanline. /// The raw scanline. /// The filtered scanline result. - /// The number of bytes per scanline. /// The - private byte[] EncodePixelRow(PixelAccessor pixels, int row, byte[] previousScanline, byte[] rawScanline, byte[] result, int bytesPerScanline) + private byte[] EncodePixelRow(PixelAccessor pixels, int row, byte[] previousScanline, byte[] rawScanline, byte[] result) where TColor : struct, IPackedPixel 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); } /// @@ -365,25 +364,24 @@ namespace ImageSharp.Formats /// The raw scanline /// The previous scanline /// The filtered scanline result. - /// The number of bytes per scanline /// The - 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); - int currentTotalVariation = this.CalculateTotalVariation(this.sub, bytesPerScanline); + SubFilter.Encode(rawScanline, this.sub, this.bytesPerPixel); + int currentTotalVariation = this.CalculateTotalVariation(this.sub); int lowestTotalVariation = currentTotalVariation; result = this.sub; - UpFilter.Encode(rawScanline, previousScanline, this.up, bytesPerScanline); - currentTotalVariation = this.CalculateTotalVariation(this.up, bytesPerScanline); + UpFilter.Encode(rawScanline, previousScanline, this.up); + 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); - currentTotalVariation = this.CalculateTotalVariation(this.average, bytesPerScanline); + AverageFilter.Encode(rawScanline, previousScanline, this.average, this.bytesPerPixel); + 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); - currentTotalVariation = this.CalculateTotalVariation(this.paeth, bytesPerScanline); + PaethFilter.Encode(rawScanline, previousScanline, this.paeth, this.bytesPerPixel); + 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. /// - /// The scanline bytes - /// The number of bytes per scanline + /// The scanline bytes /// The - 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.Shared.Rent(bytesPerScanline); - byte[] rawScanline = ArrayPool.Shared.Rent(bytesPerScanline); + byte[] previousScanline = new byte[bytesPerScanline]; + byte[] rawScanline = new byte[bytesPerScanline]; int resultLength = bytesPerScanline + 1; - byte[] result = ArrayPool.Shared.Rent(resultLength); + byte[] result = new byte[resultLength]; if (this.PngColorType != PngColorType.Palette) { - this.sub = ArrayPool.Shared.Rent(resultLength); - this.up = ArrayPool.Shared.Rent(resultLength); - this.average = ArrayPool.Shared.Rent(resultLength); - this.paeth = ArrayPool.Shared.Rent(resultLength); + this.sub = new byte[resultLength]; + this.up = new byte[resultLength]; + this.average = new byte[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.Shared.Return(previousScanline); - ArrayPool.Shared.Return(rawScanline); - ArrayPool.Shared.Return(result); - - if (this.PngColorType != PngColorType.Palette) - { - ArrayPool.Shared.Return(this.sub); - ArrayPool.Shared.Return(this.up); - ArrayPool.Shared.Return(this.average); - ArrayPool.Shared.Return(this.paeth); - } } // Store the chunks in repeated 64k blocks. @@ -743,4 +729,4 @@ namespace ImageSharp.Formats WriteInteger(stream, (uint)crc32.Value); } } -} +} \ No newline at end of file diff --git a/src/ImageSharp/Image/PixelRow.cs b/src/ImageSharp/Image/PixelRow.cs index 4682aaaac3..d19e9ccfed 100644 --- a/src/ImageSharp/Image/PixelRow.cs +++ b/src/ImageSharp/Image/PixelRow.cs @@ -39,6 +39,32 @@ namespace ImageSharp /// private bool isDisposed; + /// + /// Initializes a new instance of the class. + /// + /// The width. + /// The bytes. + /// The component order. + /// + /// Thrown if is the incorrect length. + /// + 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(); + } + /// /// Initializes a new instance of the class. /// diff --git a/tests/ImageSharp.Tests/Formats/Png/PngTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngTests.cs index 04e28192aa..0df3e58184 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngTests.cs +++ b/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); } }); }