From d4fb78b098b50109a77e15b5980a75dd24afa83c Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 20 Oct 2020 18:42:30 +0100 Subject: [PATCH] Auto repair Png options to use Bit8. Fixes #935 --- .../Formats/Png/PngEncoderOptionsHelpers.cs | 22 ++++++++---------- .../Formats/Png/PngEncoderTests.cs | 11 +++++++++ tests/ImageSharp.Tests/TestImages.cs | 3 +++ tests/Images/Input/Png/issues/Issue_935.png | Bin 0 -> 870 bytes 4 files changed, 24 insertions(+), 12 deletions(-) create mode 100644 tests/Images/Input/Png/issues/Issue_935.png diff --git a/src/ImageSharp/Formats/Png/PngEncoderOptionsHelpers.cs b/src/ImageSharp/Formats/Png/PngEncoderOptionsHelpers.cs index b0311f0887..9342e09dfe 100644 --- a/src/ImageSharp/Formats/Png/PngEncoderOptionsHelpers.cs +++ b/src/ImageSharp/Formats/Png/PngEncoderOptionsHelpers.cs @@ -35,6 +35,15 @@ public static void AdjustOptions( options.ColorType ??= pngMetadata.ColorType ?? SuggestColorType(); options.BitDepth ??= pngMetadata.BitDepth ?? SuggestBitDepth(); + // Ensure bit depth and color type are a supported combination. + // Bit8 is the only bit depth supported by all color types. + byte bits = (byte)options.BitDepth; + byte[] validBitDepths = PngConstants.ColorTypes[options.ColorType.Value]; + if (Array.IndexOf(validBitDepths, bits) == -1) + { + options.BitDepth = PngBitDepth.Bit8; + } + options.InterlaceMethod ??= pngMetadata.InterlaceMethod; use16Bit = options.BitDepth == PngBitDepth.Bit16; @@ -44,12 +53,6 @@ public static void AdjustOptions( { options.ChunkFilter = PngChunkFilter.ExcludeAll; } - - // Ensure we are not allowing impossible combinations. - if (!PngConstants.ColorTypes.ContainsKey(options.ColorType.Value)) - { - throw new NotSupportedException("Color type is not supported or not valid."); - } } /// @@ -68,15 +71,10 @@ public static IndexedImageFrame CreateQuantizedFrame( return null; } - byte bits = (byte)options.BitDepth; - if (Array.IndexOf(PngConstants.ColorTypes[options.ColorType.Value], bits) == -1) - { - throw new NotSupportedException("Bit depth is not supported or not valid."); - } - // Use the metadata to determine what quantization depth to use if no quantizer has been set. if (options.Quantizer is null) { + byte bits = (byte)options.BitDepth; var maxColors = ImageMaths.GetColorCountForBitDepth(bits); options.Quantizer = new WuQuantizer(new QuantizerOptions { MaxColors = maxColors }); } diff --git a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs index 9ba956d722..465bed8a16 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs @@ -556,6 +556,17 @@ static void RunTest(string serialized) provider); } + [Fact] + public void EncodeFixesInvalidOptions() + { + // https://github.com/SixLabors/ImageSharp/issues/935 + using var ms = new MemoryStream(); + var testFile = TestFile.Create(TestImages.Png.Issue935); + using Image image = testFile.CreateRgba32Image(new PngDecoder()); + + image.Save(ms, new PngEncoder { ColorType = PngColorType.RgbWithAlpha }); + } + private static void TestPngEncoderCore( TestImageProvider provider, PngColorType pngColorType, diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index fd5296c375..dce36bb0fb 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -107,6 +107,9 @@ public static class Png public const string Issue1177_1 = "Png/issues/Issue_1177_1.png"; public const string Issue1177_2 = "Png/issues/Issue_1177_2.png"; + // Issue 935: https://github.com/SixLabors/ImageSharp/issues/935 + public const string Issue935 = "Png/issues/Issue_935.png"; + public static class Bad { public const string MissingDataChunk = "Png/xdtn0g01.png"; diff --git a/tests/Images/Input/Png/issues/Issue_935.png b/tests/Images/Input/Png/issues/Issue_935.png new file mode 100644 index 0000000000000000000000000000000000000000..30c1a8ff0fae5079941c32113c120ddf2e6d2239 GIT binary patch literal 870 zcmV-s1DX7ZP)^nEqI9JTib*`MB>s(jB4-bBuagBfW;9l(-!MQ;u)hB|7Hn-|l2AexA zogQpXI;R0_&N`=&ZN7#{1**eDn>ApxSpz1U8@+R;M_Vt}^P_#4K?U0BovA=Oy)!wN zoM(b%oO!B%lh3YC z)d%k2vIb7`4ftbMepJ9cm~7wy;xq6Fg|S`ltaqv+g|R*r#>!V1pDBeit#b;~i^{+Q zEgCy>+5Z-tR>Y>PK|@1zo@a>6g$;r#+X z-@|y?-tk8rEC1egU@8Z^JBXxTD4O2PcX-{Ty;q->{vi|~7IbO%Mf>Xysg>(V=88Q zNvW8PYtIL8Fa0umaMDTLNaecqja06y-AKjkdHEnve#u9S=Qt>pUi-g+AFS)rluCJq zI(u-{fn-y_Rfoh?$Io~yqw-TJXXwJ^41Ktm;pD=_3}+WEGJMnw8hWsyB9&qs)EOH; ws54GAu#KGHP$(1%g+ifFC=?2XLjSt{0)2RWclf+7Q~&?~07*qoM6N<$f(O}|egFUf literal 0 HcmV?d00001