From 5a33414d5f77f89b8665a15c87852559282d2bab Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 30 Aug 2017 00:16:48 +1000 Subject: [PATCH 1/4] Fix #301 Decode using tRNS chunk of present --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 203 +++++++++++++++--- tests/ImageSharp.Tests/FileTestBase.cs | 1 + .../Formats/Png/PngDecoderTests.cs | 21 +- 3 files changed, 186 insertions(+), 39 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index ad70a950a2..2cb38f3ff8 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -84,6 +84,16 @@ namespace SixLabors.ImageSharp.Formats.Png /// private readonly Configuration configuration; + /// + /// Gets the encoding to use + /// + private readonly Encoding textEncoding; + + /// + /// Gets or sets a value indicating whether the metadata should be ignored when the image is being decoded. + /// + private readonly bool ignoreMetadata; + /// /// The stream to decode from. /// @@ -155,14 +165,19 @@ namespace SixLabors.ImageSharp.Formats.Png private PngColorType pngColorType; /// - /// Gets the encoding to use + /// Represents any color in an Rgb24 encoded png that should be transparent /// - private Encoding textEncoding; + private Rgb24 rgb24Trans; /// - /// Gets or sets a value indicating whether the metadata should be ignored when the image is being decoded. + /// Represents any color in a Grayscale encoded png that should be transparent /// - private bool ignoreMetadata; + private byte intensityTrans; + + /// + /// Whether the image has transparency chunk and markers were decoded + /// + private bool hasTrans; /// /// Initializes a new instance of the class. @@ -232,6 +247,7 @@ namespace SixLabors.ImageSharp.Formats.Png byte[] alpha = new byte[currentChunk.Length]; Buffer.BlockCopy(currentChunk.Data, 0, alpha, 0, currentChunk.Length); this.paletteAlpha = alpha; + this.AssignTransparentMarkers(alpha); break; case PngChunkTypes.Text: this.ReadTextChunk(metadata, currentChunk.Data, currentChunk.Length); @@ -300,6 +316,11 @@ namespace SixLabors.ImageSharp.Formats.Png return result; } + /// + /// Returns a value indicating whether the given chunk is critical to decoding + /// + /// The chunk + /// The private static bool IsCriticalChunk(PngChunk chunk) { return @@ -309,6 +330,17 @@ namespace SixLabors.ImageSharp.Formats.Png chunk.Type == PngChunkTypes.End; } + /// + /// Reads an integer value from 2 consecutive bytes in LSB order + /// + /// The source buffer + /// THe offset + /// The + public static int ReadIntFrom2Bytes(byte[] buffer, int offset) + { + return ((buffer[offset] & 0xFF) << 16) | (buffer[offset + 1] & 0xFF); + } + /// /// Reads the data chunk containing physical dimension data. /// @@ -590,10 +622,19 @@ namespace SixLabors.ImageSharp.Formats.Png case PngColorType.Grayscale: int factor = 255 / ((int)Math.Pow(2, this.header.BitDepth) - 1); Span newScanline1 = ToArrayByBitsLength(scanlineBuffer, this.bytesPerScanline, this.header.BitDepth); + for (int x = 0; x < this.header.Width; x++) { byte intensity = (byte)(newScanline1[x] * factor); - color.PackFromRgba32(new Rgba32(intensity, intensity, intensity)); + if (this.hasTrans && intensity == this.intensityTrans) + { + color.PackFromRgba32(new Rgba32(intensity, intensity, intensity, 0)); + } + else + { + color.PackFromRgba32(new Rgba32(intensity, intensity, intensity)); + } + rowSpan[x] = color; } @@ -622,19 +663,60 @@ namespace SixLabors.ImageSharp.Formats.Png case PngColorType.Rgb: - if (this.header.BitDepth == 16) + if (!this.hasTrans) { - int length = this.header.Width * 3; - using (var compressed = new Buffer(length)) + if (this.header.BitDepth == 16) { - // TODO: Should we use pack from vector here instead? - this.From16BitTo8Bit(scanlineBuffer, compressed, length); - PixelOperations.Instance.PackFromRgb24Bytes(compressed, rowSpan, this.header.Width); + int length = this.header.Width * 3; + using (var compressed = new Buffer(length)) + { + // TODO: Should we use pack from vector here instead? + this.From16BitTo8Bit(scanlineBuffer, compressed, length); + PixelOperations.Instance.PackFromRgb24Bytes(compressed, rowSpan, this.header.Width); + } + } + else + { + PixelOperations.Instance.PackFromRgb24Bytes(scanlineBuffer, rowSpan, this.header.Width); } } else { - PixelOperations.Instance.PackFromRgb24Bytes(scanlineBuffer, rowSpan, this.header.Width); + if (this.header.BitDepth == 16) + { + int length = this.header.Width * 3; + using (var compressed = new Buffer(length)) + { + // TODO: Should we use pack from vector here instead? + this.From16BitTo8Bit(scanlineBuffer, compressed, length); + + Span rgb24Span = compressed.Span.NonPortableCast(); + for (int x = 0; x < this.header.Width; x++) + { + ref Rgb24 rgb24 = ref rgb24Span[x]; + var rgba32 = default(Rgba32); + rgba32.Rgb = rgb24; + rgba32.A = (byte)(rgb24.Equals(this.rgb24Trans) ? 0 : 255); + + color.PackFromRgba32(rgba32); + rowSpan[x] = color; + } + } + } + else + { + Span rgb24Span = scanlineBuffer.NonPortableCast(); + for (int x = 0; x < this.header.Width; x++) + { + ref Rgb24 rgb24 = ref rgb24Span[x]; + var rgba32 = default(Rgba32); + rgba32.Rgb = rgb24; + rgba32.A = (byte)(rgb24.Equals(this.rgb24Trans) ? 0 : 255); + + color.PackFromRgba32(rgba32); + rowSpan[x] = color; + } + } } break; @@ -675,6 +757,33 @@ namespace SixLabors.ImageSharp.Formats.Png } } + /// + /// Decodes and assigns marker colors that identify transparent pixels in non indexed images + /// + /// The aplha tRNS array + private void AssignTransparentMarkers(byte[] alpha) + { + if (this.pngColorType == PngColorType.Rgb) + { + if (alpha.Length >= 6) + { + byte r = (byte)ReadIntFrom2Bytes(alpha, 0); + byte g = (byte)ReadIntFrom2Bytes(alpha, 2); + byte b = (byte)ReadIntFrom2Bytes(alpha, 4); + this.rgb24Trans = new Rgb24(r, g, b); + this.hasTrans = true; + } + } + else if (this.pngColorType == PngColorType.Grayscale) + { + if (alpha.Length >= 2) + { + this.intensityTrans = (byte)ReadIntFrom2Bytes(alpha, 0); + this.hasTrans = true; + } + } + } + /// /// Processes a scanline that uses a palette /// @@ -744,10 +853,19 @@ namespace SixLabors.ImageSharp.Formats.Png case PngColorType.Grayscale: int factor = 255 / ((int)Math.Pow(2, this.header.BitDepth) - 1); Span newScanline1 = ToArrayByBitsLength(scanlineBuffer, this.bytesPerScanline, this.header.BitDepth); + for (int x = pixelOffset, o = 0; x < this.header.Width; x += increment, o++) { byte intensity = (byte)(newScanline1[o] * factor); - color.PackFromRgba32(new Rgba32(intensity, intensity, intensity)); + if (this.hasTrans && intensity == this.intensityTrans) + { + color.PackFromRgba32(new Rgba32(intensity, intensity, intensity, 0)); + } + else + { + color.PackFromRgba32(new Rgba32(intensity, intensity, intensity)); + } + rowSpan[x] = color; } @@ -815,27 +933,60 @@ namespace SixLabors.ImageSharp.Formats.Png { // TODO: Should we use pack from vector here instead? this.From16BitTo8Bit(scanlineBuffer, compressed, length); - for (int x = pixelOffset, o = 0; x < this.header.Width; x += increment, o += 3) - { - rgba.R = compressed[o]; - rgba.G = compressed[o + 1]; - rgba.B = compressed[o + 2]; - color.PackFromRgba32(rgba); - rowSpan[x] = color; + if (this.hasTrans) + { + for (int x = pixelOffset, o = 0; x < this.header.Width; x += increment, o += 3) + { + rgba.R = compressed[o]; + rgba.G = compressed[o + 1]; + rgba.B = compressed[o + 2]; + rgba.A = (byte)(this.rgb24Trans.Equals(rgba.Rgb) ? 0 : 255); + + color.PackFromRgba32(rgba); + rowSpan[x] = color; + } + } + else + { + for (int x = pixelOffset, o = 0; x < this.header.Width; x += increment, o += 3) + { + rgba.R = compressed[o]; + rgba.G = compressed[o + 1]; + rgba.B = compressed[o + 2]; + + color.PackFromRgba32(rgba); + rowSpan[x] = color; + } } } } else { - for (int x = pixelOffset, o = 0; x < this.header.Width; x += increment, o += this.bytesPerPixel) + if (this.hasTrans) { - rgba.R = scanlineBuffer[o]; - rgba.G = scanlineBuffer[o + this.bytesPerSample]; - rgba.B = scanlineBuffer[o + (2 * this.bytesPerSample)]; + for (int x = pixelOffset, o = 0; x < this.header.Width; x += increment, o += this.bytesPerPixel) + { + rgba.R = scanlineBuffer[o]; + rgba.G = scanlineBuffer[o + this.bytesPerSample]; + rgba.B = scanlineBuffer[o + (2 * this.bytesPerSample)]; + rgba.A = (byte)(this.rgb24Trans.Equals(rgba.Rgb) ? 0 : 255); - color.PackFromRgba32(rgba); - rowSpan[x] = color; + color.PackFromRgba32(rgba); + rowSpan[x] = color; + } + } + else + { + for (int x = pixelOffset, o = 0; x < this.header.Width; x += increment, o += this.bytesPerPixel) + { + rgba.R = scanlineBuffer[o]; + rgba.G = scanlineBuffer[o + this.bytesPerSample]; + rgba.B = scanlineBuffer[o + (2 * this.bytesPerSample)]; + + color.PackFromRgba32(rgba); + rowSpan[x] = color; + } } } diff --git a/tests/ImageSharp.Tests/FileTestBase.cs b/tests/ImageSharp.Tests/FileTestBase.cs index 88d6188805..30e169c7d0 100644 --- a/tests/ImageSharp.Tests/FileTestBase.cs +++ b/tests/ImageSharp.Tests/FileTestBase.cs @@ -85,6 +85,7 @@ namespace SixLabors.ImageSharp.Tests // TestFile.Create(TestImages.Bmp.NegHeight), // Perf: Enable for local testing only // TestFile.Create(TestImages.Bmp.CoreHeader), // Perf: Enable for local testing only TestFile.Create(TestImages.Png.Splash), + // TestFile.Create(TestImages.Png.SnakeGame), // TestFile.Create(TestImages.Png.Cross), // Perf: Enable for local testing only // TestFile.Create(TestImages.Png.Bad.ChunkLength1), // Perf: Enable for local testing only // TestFile.Create(TestImages.Png.Bad.ChunkLength2), // Perf: Enable for local testing only diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 836fc5b5c6..bb31c3b1ad 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -24,22 +24,19 @@ namespace SixLabors.ImageSharp.Tests TestImages.Png.Splash, TestImages.Png.Indexed, TestImages.Png.FilterVar, TestImages.Png.Bad.ChunkLength1, - + TestImages.Png.VimImage1, TestImages.Png.VersioningImage1, TestImages.Png.VersioningImage2, - // BUG !!! Should work. TODO: Fix it !!!! - // TestImages.Png.SnakeGame + TestImages.Png.SnakeGame }; - + public static readonly string[] TestImages48Bpp = { TestImages.Png.Rgb48Bpp, - - // TODO: Re enable, when Decode_Interlaced is fixed!!!! - // TestImages.Png.Rgb48BppInterlaced + TestImages.Png.Rgb48BppInterlaced }; // This is a workaround for Mono-s decoder being incompatible with ours and GDI+. @@ -79,21 +76,19 @@ namespace SixLabors.ImageSharp.Tests } } } - + [Theory] [WithFile(TestImages.Png.Interlaced, PixelTypes.Rgba32)] public void Decode_Interlaced_DoesNotThrow(TestImageProvider provider) where TPixel : struct, IPixel { - // Ok, it's incorrect, but at least let's run our decoder on interlaced images! (Single-fact AAA :P) using (Image image = provider.GetImage(new PngDecoder())) { image.DebugSave(provider); } } - // BUG in decoding interlaced images !!! TODO: Fix it! - [Theory(Skip = "Bug in decoding interlaced images.")] + [Theory] [WithFile(TestImages.Png.Interlaced, PixelTypes.Rgba32)] public void Decode_Interlaced_ImageIsCorrect(TestImageProvider provider) where TPixel : struct, IPixel @@ -104,7 +99,7 @@ namespace SixLabors.ImageSharp.Tests image.CompareToOriginal(provider, ImageComparer.Exact); } } - + // TODO: We need to decode these into Rgba64 properly, and do 'CompareToOriginal' in a Rgba64 mode! (See #285) [Theory] [WithFileCollection(nameof(TestImages48Bpp), PixelTypes.Rgba32)] @@ -122,7 +117,7 @@ namespace SixLabors.ImageSharp.Tests } } } - + [Theory] [WithFile(TestImages.Png.Splash, PixelTypes)] public void Decoder_IsNotBoundToSinglePixelType(TestImageProvider provider) From 31b4fbdba6d47b01772e04af7613eeb841ffe7ba Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 29 Aug 2017 21:02:54 +0200 Subject: [PATCH 2/4] added test cases with images from #313 and #314 --- tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs | 4 +++- tests/ImageSharp.Tests/TestImages.cs | 3 +++ tests/Images/Input/Png/banner7-adam.png | 3 +++ tests/Images/Input/Png/banner8-index.png | 3 +++ 4 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 tests/Images/Input/Png/banner7-adam.png create mode 100644 tests/Images/Input/Png/banner8-index.png diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index bb31c3b1ad..fc759fb56e 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -29,7 +29,9 @@ namespace SixLabors.ImageSharp.Tests TestImages.Png.VersioningImage1, TestImages.Png.VersioningImage2, - TestImages.Png.SnakeGame + TestImages.Png.SnakeGame, + TestImages.Png.Banner7Adam7InterlaceMode, + TestImages.Png.Banner8Index, }; diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 3fdbecf50d..3b238a8756 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -47,6 +47,9 @@ namespace SixLabors.ImageSharp.Tests public const string VersioningImage1 = "Png/versioning-1_1.png"; public const string VersioningImage2 = "Png/versioning-1_2.png"; + public const string Banner7Adam7InterlaceMode = "Png/banner7-adam.png"; + public const string Banner8Index = "Png/banner8-index.png"; + public static class Bad { // Odd chunk lengths diff --git a/tests/Images/Input/Png/banner7-adam.png b/tests/Images/Input/Png/banner7-adam.png new file mode 100644 index 0000000000..b7bedd8884 --- /dev/null +++ b/tests/Images/Input/Png/banner7-adam.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:5eb7d02dfce0821a5210ed69c887516a39089baca2989252afd712e41e656586 +size 16488 diff --git a/tests/Images/Input/Png/banner8-index.png b/tests/Images/Input/Png/banner8-index.png new file mode 100644 index 0000000000..075ca688db --- /dev/null +++ b/tests/Images/Input/Png/banner8-index.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:6398956c686a1e280c5d29d8301aebee7269296598cebb01454ba0c9e7f279bc +size 2327 From 79151fd0d3ca4482728c8824ea5d2502781298b7 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 2 Sep 2017 08:37:55 +1000 Subject: [PATCH 3/4] Update Readme --- README.md | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index a5e8d33e6d..31ca5cef3b 100644 --- a/README.md +++ b/README.md @@ -88,9 +88,9 @@ On platforms supporting netstandard 1.3+ // Image.Load(string path) is a shortcut for our default type. Other pixel formats use Image.Load(string path)) using (Image image = Image.Load("foo.jpg")) { - image.Mutate(x=>x - .Resize(image.Width / 2, image.Height / 2) - .Grayscale()); + image.Mutate(x => x + .Resize(image.Width / 2, image.Height / 2) + .Grayscale()); image.Save("bar.jpg"); // automatic encoder selected based on extension. } ``` @@ -101,9 +101,9 @@ using (FileStream stream = File.OpenRead("foo.jpg")) using (FileStream output = File.OpenWrite("bar.jpg")) using (Image image = Image.Load(stream)) { - image.Mutate(x=>x - .Resize(image.Width / 2, image.Height / 2) - .Grayscale()); + image.Mutate(x => x + .Resize(image.Width / 2, image.Height / 2) + .Grayscale()); image.Save(output); } ``` @@ -118,7 +118,7 @@ using (Image image = new Image(400, 400)) } ``` -For optimized access within a loop it is recommended that the following methods are used. +For optimized synchronous access within a loop it is recommended that the following methods are used. 1. `image.GetRowSpan(y)` 2. `image.GetRowSpan(x, y)` @@ -142,9 +142,7 @@ Grand High Eternal Dictator Core Team - [Dirk Lemstra](https://github.com/dlemstra) -- [Jeavon Leopold](https://github.com/jeavon) - [Anton Firsov](https://github.com/antonfirsov) -- [Olivia Ifrim](https://github.com/olivif) - [Scott Williams](https://github.com/tocsoft) ### Backers From 7cc11466ef341f96d4dcbeaa02763cccbb32a8cb Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 2 Sep 2017 09:10:01 +1000 Subject: [PATCH 4/4] Move test image tocorrect folder --- .../TestImages/Formats => Images/Input}/Bmp/BITMAPV5HEADER.bmp | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/{ImageSharp.Tests/TestImages/Formats => Images/Input}/Bmp/BITMAPV5HEADER.bmp (100%) diff --git a/tests/ImageSharp.Tests/TestImages/Formats/Bmp/BITMAPV5HEADER.bmp b/tests/Images/Input/Bmp/BITMAPV5HEADER.bmp similarity index 100% rename from tests/ImageSharp.Tests/TestImages/Formats/Bmp/BITMAPV5HEADER.bmp rename to tests/Images/Input/Bmp/BITMAPV5HEADER.bmp