From 49fb759f2918a385f99b9af89743166cd7ff0a6d Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Mon, 1 Oct 2018 15:55:35 -0700 Subject: [PATCH 01/11] Unify PngDecoder buffer --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 51 ++++++++------------ 1 file changed, 20 insertions(+), 31 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 3b67146b9..3ac13eb58 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -38,19 +38,9 @@ namespace SixLabors.ImageSharp.Formats.Png }; /// - /// Reusable buffer for reading chunk types. + /// Reusable buffer. /// - private readonly byte[] chunkTypeBuffer = new byte[4]; - - /// - /// Reusable buffer for reading chunk lengths. - /// - private readonly byte[] chunkLengthBuffer = new byte[4]; - - /// - /// Reusable buffer for reading crc values. - /// - private readonly byte[] crcBuffer = new byte[4]; + private readonly byte[] buffer = new byte[4]; /// /// Reusable crc for validating chunks. @@ -1001,7 +991,7 @@ namespace SixLabors.ImageSharp.Formats.Png return 0; } - this.currentStream.Read(this.crcBuffer, 0, 4); + this.currentStream.Read(this.buffer, 0, 4); if (this.TryReadChunk(out PngChunk chunk)) { @@ -1086,13 +1076,17 @@ namespace SixLabors.ImageSharp.Formats.Png /// The . private void ValidateChunk(in PngChunk chunk) { + Span chunkType = stackalloc byte[4]; + + BinaryPrimitives.WriteUInt32BigEndian(chunkType, (uint)chunk.Type); + this.crc.Reset(); - this.crc.Update(this.chunkTypeBuffer); + this.crc.Update(chunkType); this.crc.Update(chunk.Data.GetSpan()); if (this.crc.Value != chunk.Crc) { - string chunkTypeName = Encoding.UTF8.GetString(this.chunkTypeBuffer, 0, 4); + string chunkTypeName = Encoding.UTF8.GetString(chunkType.ToArray(), 0, 4); throw new ImageFormatException($"CRC Error. PNG {chunkTypeName} chunk is corrupt!"); } @@ -1106,14 +1100,9 @@ namespace SixLabors.ImageSharp.Formats.Png /// private uint ReadChunkCrc() { - int numBytes = this.currentStream.Read(this.crcBuffer, 0, 4); - - if (numBytes >= 1 && numBytes <= 3) - { - throw new ImageFormatException("Image stream is not valid!"); - } - - return BinaryPrimitives.ReadUInt32BigEndian(this.crcBuffer); + return this.currentStream.Read(this.buffer, 0, 4) == 4 + ? BinaryPrimitives.ReadUInt32BigEndian(this.buffer) + : throw new ImageFormatException("Image stream is not valid!"); } /// @@ -1148,22 +1137,22 @@ namespace SixLabors.ImageSharp.Formats.Png /// private PngChunkType ReadChunkType() { - return this.currentStream.Read(this.chunkTypeBuffer, 0, 4) == 4 - ? (PngChunkType)BinaryPrimitives.ReadUInt32BigEndian(this.chunkTypeBuffer.AsSpan()) + return this.currentStream.Read(this.buffer, 0, 4) == 4 + ? (PngChunkType)BinaryPrimitives.ReadUInt32BigEndian(this.buffer) : throw new ImageFormatException("Invalid PNG data."); } /// - /// Calculates the length of the given chunk. + /// Attempts to read the length of the next chunk. /// - /// - /// Thrown if the input stream is not valid. - /// + /// + /// Whether the the length was read. + /// private bool TryReadChunkLength(out int result) { - if (this.currentStream.Read(this.chunkLengthBuffer, 0, 4) == 4) + if (this.currentStream.Read(this.buffer, 0, 4) == 4) { - result = BinaryPrimitives.ReadInt32BigEndian(this.chunkLengthBuffer); + result = BinaryPrimitives.ReadInt32BigEndian(this.buffer); return true; } From 7c88e011be4303296c61254d079074e510e5f916 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Mon, 1 Oct 2018 15:57:26 -0700 Subject: [PATCH 02/11] Remove extra line breaks --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 3ac13eb58..fa509336e 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -563,22 +563,18 @@ namespace SixLabors.ImageSharp.Formats.Png break; case FilterType.Sub: - SubFilter.Decode(scanlineSpan, this.bytesPerPixel); break; case FilterType.Up: - UpFilter.Decode(scanlineSpan, this.previousScanline.GetSpan()); break; case FilterType.Average: - AverageFilter.Decode(scanlineSpan, this.previousScanline.GetSpan(), this.bytesPerPixel); break; case FilterType.Paeth: - PaethFilter.Decode(scanlineSpan, this.previousScanline.GetSpan(), this.bytesPerPixel); break; @@ -637,22 +633,18 @@ namespace SixLabors.ImageSharp.Formats.Png break; case FilterType.Sub: - SubFilter.Decode(scanSpan, this.bytesPerPixel); break; case FilterType.Up: - UpFilter.Decode(scanSpan, prevSpan); break; case FilterType.Average: - AverageFilter.Decode(scanSpan, prevSpan, this.bytesPerPixel); break; case FilterType.Paeth: - PaethFilter.Decode(scanSpan, prevSpan, this.bytesPerPixel); break; @@ -705,7 +697,6 @@ namespace SixLabors.ImageSharp.Formats.Png switch (this.pngColorType) { case PngColorType.Grayscale: - PngScanlineProcessor.ProcessGrayscaleScanline( this.header, scanlineSpan, @@ -717,7 +708,6 @@ namespace SixLabors.ImageSharp.Formats.Png break; case PngColorType.GrayscaleWithAlpha: - PngScanlineProcessor.ProcessGrayscaleWithAlphaScanline( this.header, scanlineSpan, @@ -728,7 +718,6 @@ namespace SixLabors.ImageSharp.Formats.Png break; case PngColorType.Palette: - PngScanlineProcessor.ProcessPaletteScanline( this.header, scanlineSpan, @@ -739,7 +728,6 @@ namespace SixLabors.ImageSharp.Formats.Png break; case PngColorType.Rgb: - PngScanlineProcessor.ProcessRgbScanline( this.header, scanlineSpan, @@ -753,7 +741,6 @@ namespace SixLabors.ImageSharp.Formats.Png break; case PngColorType.RgbWithAlpha: - PngScanlineProcessor.ProcessRgbaScanline( this.header, scanlineSpan, @@ -789,7 +776,6 @@ namespace SixLabors.ImageSharp.Formats.Png switch (this.pngColorType) { case PngColorType.Grayscale: - PngScanlineProcessor.ProcessInterlacedGrayscaleScanline( this.header, scanlineSpan, @@ -803,7 +789,6 @@ namespace SixLabors.ImageSharp.Formats.Png break; case PngColorType.GrayscaleWithAlpha: - PngScanlineProcessor.ProcessInterlacedGrayscaleWithAlphaScanline( this.header, scanlineSpan, @@ -816,7 +801,6 @@ namespace SixLabors.ImageSharp.Formats.Png break; case PngColorType.Palette: - PngScanlineProcessor.ProcessInterlacedPaletteScanline( this.header, scanlineSpan, @@ -829,7 +813,6 @@ namespace SixLabors.ImageSharp.Formats.Png break; case PngColorType.Rgb: - PngScanlineProcessor.ProcessInterlacedRgbScanline( this.header, scanlineSpan, @@ -845,7 +828,6 @@ namespace SixLabors.ImageSharp.Formats.Png break; case PngColorType.RgbWithAlpha: - PngScanlineProcessor.ProcessInterlacedRgbaScanline( this.header, scanlineSpan, From c29a967e26bce334d62521fd4cb6daa56567efb9 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Mon, 1 Oct 2018 16:19:24 -0700 Subject: [PATCH 03/11] Move Png ColorType to Constants --- src/ImageSharp/Formats/Png/PngConstants.cs | 12 ++++++++++++ src/ImageSharp/Formats/Png/PngDecoderCore.cs | 17 +++-------------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngConstants.cs b/src/ImageSharp/Formats/Png/PngConstants.cs index ff25e26b7..48c866f67 100644 --- a/src/ImageSharp/Formats/Png/PngConstants.cs +++ b/src/ImageSharp/Formats/Png/PngConstants.cs @@ -41,5 +41,17 @@ namespace SixLabors.ImageSharp.Formats.Png /// The header bytes as a big endian coded ulong. /// public const ulong HeaderValue = 0x89504E470D0A1A0AUL; + + /// + /// The dictionary of available color types. + /// + public static readonly Dictionary ColorTypes = new Dictionary() + { + [PngColorType.Grayscale] = new byte[] { 1, 2, 4, 8, 16 }, + [PngColorType.Rgb] = new byte[] { 8, 16 }, + [PngColorType.Palette] = new byte[] { 1, 2, 4, 8 }, + [PngColorType.GrayscaleWithAlpha] = new byte[] { 8, 16 }, + [PngColorType.RgbWithAlpha] = new byte[] { 8, 16 } + }; } } \ No newline at end of file diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index fa509336e..a24b69160 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -25,18 +25,6 @@ namespace SixLabors.ImageSharp.Formats.Png /// internal sealed class PngDecoderCore { - /// - /// The dictionary of available color types. - /// - private static readonly Dictionary ColorTypes = new Dictionary() - { - [PngColorType.Grayscale] = new byte[] { 1, 2, 4, 8, 16 }, - [PngColorType.Rgb] = new byte[] { 8, 16 }, - [PngColorType.Palette] = new byte[] { 1, 2, 4, 8 }, - [PngColorType.GrayscaleWithAlpha] = new byte[] { 8, 16 }, - [PngColorType.RgbWithAlpha] = new byte[] { 8, 16 } - }; - /// /// Reusable buffer. /// @@ -858,6 +846,7 @@ namespace SixLabors.ImageSharp.Formats.Png ushort rc = BinaryPrimitives.ReadUInt16LittleEndian(alpha.Slice(0, 2)); ushort gc = BinaryPrimitives.ReadUInt16LittleEndian(alpha.Slice(2, 2)); ushort bc = BinaryPrimitives.ReadUInt16LittleEndian(alpha.Slice(4, 2)); + this.rgb48Trans = new Rgb48(rc, gc, bc); this.hasTrans = true; return; @@ -909,12 +898,12 @@ namespace SixLabors.ImageSharp.Formats.Png /// private void ValidateHeader() { - if (!ColorTypes.ContainsKey(this.header.ColorType)) + if (!PngConstants.ColorTypes.ContainsKey(this.header.ColorType)) { throw new NotSupportedException("Color type is not supported or not valid."); } - if (!ColorTypes[this.header.ColorType].Contains(this.header.BitDepth)) + if (!PngConstants.ColorTypes[this.header.ColorType].Contains(this.header.BitDepth)) { throw new NotSupportedException("Bit depth is not supported or not valid."); } From 666aaffaab7e6fb850d73cfc75f40dc7e281b1d4 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Mon, 1 Oct 2018 16:23:38 -0700 Subject: [PATCH 04/11] Move PngHeader validation to struct --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 33 ++------------------ src/ImageSharp/Formats/Png/PngHeader.cs | 29 +++++++++++++++++ 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index a24b69160..112dc7262 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -198,7 +198,6 @@ namespace SixLabors.ImageSharp.Formats.Png { case PngChunkType.Header: this.ReadHeaderChunk(pngMetaData, chunk.Data.Array); - this.ValidateHeader(); break; case PngChunkType.Physical: this.ReadPhysicalChunk(metaData, chunk.Data.GetSpan()); @@ -287,7 +286,6 @@ namespace SixLabors.ImageSharp.Formats.Png { case PngChunkType.Header: this.ReadHeaderChunk(pngMetaData, chunk.Data.Array); - this.ValidateHeader(); break; case PngChunkType.Physical: this.ReadPhysicalChunk(metaData, chunk.Data.GetSpan()); @@ -886,37 +884,10 @@ namespace SixLabors.ImageSharp.Formats.Png { this.header = PngHeader.Parse(data); + this.header.Validate(); + pngMetaData.BitDepth = (PngBitDepth)this.header.BitDepth; pngMetaData.ColorType = this.header.ColorType; - } - - /// - /// Validates the png header. - /// - /// - /// Thrown if the image does pass validation. - /// - private void ValidateHeader() - { - if (!PngConstants.ColorTypes.ContainsKey(this.header.ColorType)) - { - throw new NotSupportedException("Color type is not supported or not valid."); - } - - if (!PngConstants.ColorTypes[this.header.ColorType].Contains(this.header.BitDepth)) - { - throw new NotSupportedException("Bit depth is not supported or not valid."); - } - - if (this.header.FilterMethod != 0) - { - throw new NotSupportedException("The png specification only defines 0 as filter method."); - } - - if (this.header.InterlaceMethod != PngInterlaceMode.None && this.header.InterlaceMethod != PngInterlaceMode.Adam7) - { - throw new NotSupportedException("The png specification only defines 'None' and 'Adam7' as interlaced methods."); - } this.pngColorType = this.header.ColorType; } diff --git a/src/ImageSharp/Formats/Png/PngHeader.cs b/src/ImageSharp/Formats/Png/PngHeader.cs index ec22f1bb4..0523502b0 100644 --- a/src/ImageSharp/Formats/Png/PngHeader.cs +++ b/src/ImageSharp/Formats/Png/PngHeader.cs @@ -80,6 +80,35 @@ namespace SixLabors.ImageSharp.Formats.Png /// public PngInterlaceMode InterlaceMethod { get; } + /// + /// Validates the png header. + /// + /// + /// Thrown if the image does pass validation. + /// + public void Validate() + { + if (!PngConstants.ColorTypes.ContainsKey(this.ColorType)) + { + throw new NotSupportedException("Color type is not supported or not valid."); + } + + if (PngConstants.ColorTypes[this.ColorType].AsSpan().IndexOf(this.BitDepth) == -1) + { + throw new NotSupportedException("Bit depth is not supported or not valid."); + } + + if (this.FilterMethod != 0) + { + throw new NotSupportedException("The png specification only defines 0 as filter method."); + } + + if (this.InterlaceMethod != PngInterlaceMode.None && this.InterlaceMethod != PngInterlaceMode.Adam7) + { + throw new NotSupportedException("The png specification only defines 'None' and 'Adam7' as interlaced methods."); + } + } + /// /// Writes the header to the given buffer. /// From e203fb71212e9774dacc0e13024731c6198dd2f3 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Mon, 1 Oct 2018 16:48:44 -0700 Subject: [PATCH 05/11] Add GetString(ReadOnlySpan polyfill to Encoding --- .../Common/Extensions/EncoderExtensions.cs | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 src/ImageSharp/Common/Extensions/EncoderExtensions.cs diff --git a/src/ImageSharp/Common/Extensions/EncoderExtensions.cs b/src/ImageSharp/Common/Extensions/EncoderExtensions.cs new file mode 100644 index 000000000..e6b800e86 --- /dev/null +++ b/src/ImageSharp/Common/Extensions/EncoderExtensions.cs @@ -0,0 +1,34 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + +#if !NETCOREAPP2_1 +using System; +using System.Text; + +namespace SixLabors.ImageSharp +{ + /// + /// Extension methods for the type. + /// + internal static unsafe class EncoderExtensions + { + /// + /// Gets a string from the provided buffer data. + /// + /// The encoding. + /// The buffer. + /// The string. + public static string GetString(this Encoding encoding, ReadOnlySpan buffer) + { +#if NETSTANDARD1_1 + return encoding.GetString(buffer.ToArray()); +#else + fixed (byte* bytes = buffer) + { + return encoding.GetString(bytes, buffer.Length); + } +#endif + } + } +} +#endif \ No newline at end of file From 581f7049fd05db502d38715173d7b6ad342c024d Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Mon, 1 Oct 2018 16:51:46 -0700 Subject: [PATCH 06/11] Optimize ReadTextChunk --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 26 ++++++-------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 112dc7262..812175612 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -3,9 +3,7 @@ using System; using System.Buffers.Binary; -using System.Collections.Generic; using System.IO; -using System.Linq; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Text; @@ -230,7 +228,7 @@ namespace SixLabors.ImageSharp.Formats.Png this.AssignTransparentMarkers(alpha); break; case PngChunkType.Text: - this.ReadTextChunk(metaData, chunk.Data.Array, chunk.Length); + this.ReadTextChunk(metaData, chunk.Data.Array.AsSpan(0, chunk.Length)); break; case PngChunkType.Exif: if (!this.ignoreMetadata) @@ -297,7 +295,7 @@ namespace SixLabors.ImageSharp.Formats.Png this.SkipChunkDataAndCrc(chunk); break; case PngChunkType.Text: - this.ReadTextChunk(metaData, chunk.Data.Array, chunk.Length); + this.ReadTextChunk(metaData, chunk.Data.Array.AsSpan(0, chunk.Length)); break; case PngChunkType.End: this.isEndChunkReached = true; @@ -896,28 +894,18 @@ namespace SixLabors.ImageSharp.Formats.Png /// Reads a text chunk containing image properties from the data. /// /// The metadata to decode to. - /// The containing data. - /// The maximum length to read. - private void ReadTextChunk(ImageMetaData metadata, byte[] data, int length) + /// The containing the data. + private void ReadTextChunk(ImageMetaData metadata, ReadOnlySpan data) { if (this.ignoreMetadata) { return; } - int zeroIndex = 0; + int zeroIndex = data.IndexOf((byte)0); - for (int i = 0; i < length; i++) - { - if (data[i] == 0) - { - zeroIndex = i; - break; - } - } - - string name = this.textEncoding.GetString(data, 0, zeroIndex); - string value = this.textEncoding.GetString(data, zeroIndex + 1, length - zeroIndex - 1); + string name = this.textEncoding.GetString(data.Slice(0, zeroIndex)); + string value = this.textEncoding.GetString(data.Slice(zeroIndex + 1)); metadata.Properties.Add(new ImageProperty(name, value)); } From 25b6b33cb7f71e9215e6ec9868531127fa6ad527 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Mon, 1 Oct 2018 16:56:39 -0700 Subject: [PATCH 07/11] Use Encoding.GetString(ROS) polyfill --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 2 +- src/ImageSharp/MetaData/Profiles/Exif/ExifReader.cs | 13 +------------ 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 812175612..8344d7b54 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -1016,7 +1016,7 @@ namespace SixLabors.ImageSharp.Formats.Png if (this.crc.Value != chunk.Crc) { - string chunkTypeName = Encoding.UTF8.GetString(chunkType.ToArray(), 0, 4); + string chunkTypeName = Encoding.UTF8.GetString(chunkType); throw new ImageFormatException($"CRC Error. PNG {chunkTypeName} chunk is corrupt!"); } diff --git a/src/ImageSharp/MetaData/Profiles/Exif/ExifReader.cs b/src/ImageSharp/MetaData/Profiles/Exif/ExifReader.cs index 549cb3fe0..5f9549908 100644 --- a/src/ImageSharp/MetaData/Profiles/Exif/ExifReader.cs +++ b/src/ImageSharp/MetaData/Profiles/Exif/ExifReader.cs @@ -127,25 +127,14 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif private unsafe string ConvertToString(ReadOnlySpan buffer) { - Span nullChar = stackalloc byte[1] { 0 }; - - int nullCharIndex = buffer.IndexOf(nullChar); + int nullCharIndex = buffer.IndexOf((byte)0); if (nullCharIndex > -1) { buffer = buffer.Slice(0, nullCharIndex); } -#if NETSTANDARD1_1 - return Encoding.UTF8.GetString(buffer.ToArray(), 0, buffer.Length); -#elif NETCOREAPP2_1 return Encoding.UTF8.GetString(buffer); -#else - fixed (byte* pointer = &MemoryMarshal.GetReference(buffer)) - { - return Encoding.UTF8.GetString(pointer, buffer.Length); - } -#endif } /// From 94119841bf82f0a0e658439d129d97aa39559c99 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Mon, 1 Oct 2018 17:13:22 -0700 Subject: [PATCH 08/11] Add details to PNG header validation errors --- src/ImageSharp/Formats/Png/PngHeader.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngHeader.cs b/src/ImageSharp/Formats/Png/PngHeader.cs index 0523502b0..ea43ba96a 100644 --- a/src/ImageSharp/Formats/Png/PngHeader.cs +++ b/src/ImageSharp/Formats/Png/PngHeader.cs @@ -88,24 +88,25 @@ namespace SixLabors.ImageSharp.Formats.Png /// public void Validate() { - if (!PngConstants.ColorTypes.ContainsKey(this.ColorType)) + if (!PngConstants.ColorTypes.TryGetValue(this.ColorType, out byte[] supportedBitDepths)) { - throw new NotSupportedException("Color type is not supported or not valid."); + throw new NotSupportedException($"Invalid or unsupported color type. Was '{this.ColorType}'."); } - if (PngConstants.ColorTypes[this.ColorType].AsSpan().IndexOf(this.BitDepth) == -1) + if (supportedBitDepths.AsSpan().IndexOf(this.BitDepth) == -1) { - throw new NotSupportedException("Bit depth is not supported or not valid."); + throw new NotSupportedException($"Invalid or unsupported bit depth. Was '{this.BitDepth}'."); } if (this.FilterMethod != 0) { - throw new NotSupportedException("The png specification only defines 0 as filter method."); + throw new NotSupportedException($"Invalid filter method. Expected 0. Was '{this.FilterMethod}'."); } + // The png specification only defines 'None' and 'Adam7' as interlaced methods. if (this.InterlaceMethod != PngInterlaceMode.None && this.InterlaceMethod != PngInterlaceMode.Adam7) { - throw new NotSupportedException("The png specification only defines 'None' and 'Adam7' as interlaced methods."); + throw new NotSupportedException($"Invalid interlace method. Expected 'None' or 'Adam7'. Was '{this.InterlaceMethod}'."); } } From 96ad14fb6e6af49261882a45cb25d68f72e1df9d Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Mon, 1 Oct 2018 17:53:07 -0700 Subject: [PATCH 09/11] Breakout the PngPhysicalChunkData conversion and encoding functions from the encoder --- .../Png/Chunks/PngPhysicalChunkData.cs | 93 +++++++++++++++++++ src/ImageSharp/Formats/Png/PngEncoderCore.cs | 42 +-------- 2 files changed, 95 insertions(+), 40 deletions(-) create mode 100644 src/ImageSharp/Formats/Png/Chunks/PngPhysicalChunkData.cs diff --git a/src/ImageSharp/Formats/Png/Chunks/PngPhysicalChunkData.cs b/src/ImageSharp/Formats/Png/Chunks/PngPhysicalChunkData.cs new file mode 100644 index 000000000..39a9676b6 --- /dev/null +++ b/src/ImageSharp/Formats/Png/Chunks/PngPhysicalChunkData.cs @@ -0,0 +1,93 @@ +using System; +using System.Buffers.Binary; +using SixLabors.ImageSharp.Common.Helpers; +using SixLabors.ImageSharp.MetaData; + +namespace SixLabors.ImageSharp.Formats.Png +{ + /// + /// The pHYs chunk specifies the intended pixel size or aspect ratio for display of the image. + /// + internal readonly struct PngPhysicalChunkData + { + public const int Size = 9; + + public PngPhysicalChunkData(uint x, uint y, byte unitSpecifier) + { + this.XAxisPixelsPerUnit = x; + this.YAxisPixelsPerUnit = y; + this.UnitSpecifier = unitSpecifier; + } + + /// + /// Gets the number of pixels per unit on the X axis. + /// + public uint XAxisPixelsPerUnit { get; } + + /// + /// Gets the number of pixels per unit on the Y axis. + /// + public uint YAxisPixelsPerUnit { get; } + + /// + /// Gets the unit specifier. + /// 0: unit is unknown + /// 1: unit is the meter + /// When the unit specifier is 0, the pHYs chunk defines pixel aspect ratio only; the actual size of the pixels remains unspecified. + /// + public byte UnitSpecifier { get; } + + /// + /// Constructs the PngPhysicalChunkData from the provided metadata. + /// If the resolution units are not in meters, they are automatically convereted. + /// + /// The metadata. + /// The constructed PngPhysicalChunkData instance. + public static PngPhysicalChunkData FromMetadata(ImageMetaData meta) + { + byte unitSpecifier = 0; + uint x; + uint y; + + switch (meta.ResolutionUnits) + { + case PixelResolutionUnit.AspectRatio: + unitSpecifier = 0; // Unspecified + x = (uint)Math.Round(meta.HorizontalResolution); + y = (uint)Math.Round(meta.VerticalResolution); + break; + + case PixelResolutionUnit.PixelsPerInch: + unitSpecifier = 1; // Per meter + x = (uint)Math.Round(UnitConverter.InchToMeter(meta.HorizontalResolution)); + y = (uint)Math.Round(UnitConverter.InchToMeter(meta.VerticalResolution)); + break; + + case PixelResolutionUnit.PixelsPerCentimeter: + unitSpecifier = 1; // Per meter + x = (uint)Math.Round(UnitConverter.CmToMeter(meta.HorizontalResolution)); + y = (uint)Math.Round(UnitConverter.CmToMeter(meta.VerticalResolution)); + break; + + default: + unitSpecifier = 1; // Per meter + x = (uint)Math.Round(meta.HorizontalResolution); + y = (uint)Math.Round(meta.VerticalResolution); + break; + } + + return new PngPhysicalChunkData(x, y, unitSpecifier); + } + + /// + /// Writes the data to the given buffer. + /// + /// The buffer. + public void WriteTo(Span buffer) + { + BinaryPrimitives.WriteUInt32BigEndian(buffer.Slice(0, 4), this.XAxisPixelsPerUnit); + BinaryPrimitives.WriteUInt32BigEndian(buffer.Slice(4, 4), this.YAxisPixelsPerUnit); + buffer[8] = this.UnitSpecifier; + } + } +} diff --git a/src/ImageSharp/Formats/Png/PngEncoderCore.cs b/src/ImageSharp/Formats/Png/PngEncoderCore.cs index 525cc8bd1..7f3c9945a 100644 --- a/src/ImageSharp/Formats/Png/PngEncoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngEncoderCore.cs @@ -674,47 +674,9 @@ namespace SixLabors.ImageSharp.Formats.Png /// The image meta data. private void WritePhysicalChunk(Stream stream, ImageMetaData meta) { - // The pHYs chunk specifies the intended pixel size or aspect ratio for display of the image. It contains: - // Pixels per unit, X axis: 4 bytes (unsigned integer) - // Pixels per unit, Y axis: 4 bytes (unsigned integer) - // Unit specifier: 1 byte - // - // The following values are legal for the unit specifier: - // 0: unit is unknown - // 1: unit is the meter - // - // When the unit specifier is 0, the pHYs chunk defines pixel aspect ratio only; the actual size of the pixels remains unspecified. - Span hResolution = this.chunkDataBuffer.AsSpan(0, 4); - Span vResolution = this.chunkDataBuffer.AsSpan(4, 4); - - switch (meta.ResolutionUnits) - { - case PixelResolutionUnit.AspectRatio: - this.chunkDataBuffer[8] = 0; - BinaryPrimitives.WriteInt32BigEndian(hResolution, (int)Math.Round(meta.HorizontalResolution)); - BinaryPrimitives.WriteInt32BigEndian(vResolution, (int)Math.Round(meta.VerticalResolution)); - break; - - case PixelResolutionUnit.PixelsPerInch: - this.chunkDataBuffer[8] = 1; // Per meter - BinaryPrimitives.WriteInt32BigEndian(hResolution, (int)Math.Round(UnitConverter.InchToMeter(meta.HorizontalResolution))); - BinaryPrimitives.WriteInt32BigEndian(vResolution, (int)Math.Round(UnitConverter.InchToMeter(meta.VerticalResolution))); - break; - - case PixelResolutionUnit.PixelsPerCentimeter: - this.chunkDataBuffer[8] = 1; // Per meter - BinaryPrimitives.WriteInt32BigEndian(hResolution, (int)Math.Round(UnitConverter.CmToMeter(meta.HorizontalResolution))); - BinaryPrimitives.WriteInt32BigEndian(vResolution, (int)Math.Round(UnitConverter.CmToMeter(meta.VerticalResolution))); - break; - - default: - this.chunkDataBuffer[8] = 1; // Per meter - BinaryPrimitives.WriteInt32BigEndian(hResolution, (int)Math.Round(meta.HorizontalResolution)); - BinaryPrimitives.WriteInt32BigEndian(vResolution, (int)Math.Round(meta.VerticalResolution)); - break; - } + PngPhysicalChunkData.FromMetadata(meta).WriteTo(this.chunkDataBuffer); - this.WriteChunk(stream, PngChunkType.Physical, this.chunkDataBuffer, 0, 9); + this.WriteChunk(stream, PngChunkType.Physical, this.chunkDataBuffer, 0, PngPhysicalChunkData.Size); } /// From 05a0ca20d902f08a587da9e85e48620b0d3dcd8d Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Mon, 1 Oct 2018 18:00:33 -0700 Subject: [PATCH 10/11] Move PhysicalChunkData to Chunks namespace --- .../{PngPhysicalChunkData.cs => PhysicalChunkData.cs} | 8 ++++---- src/ImageSharp/Formats/Png/PngEncoderCore.cs | 5 ++--- 2 files changed, 6 insertions(+), 7 deletions(-) rename src/ImageSharp/Formats/Png/Chunks/{PngPhysicalChunkData.cs => PhysicalChunkData.cs} (93%) diff --git a/src/ImageSharp/Formats/Png/Chunks/PngPhysicalChunkData.cs b/src/ImageSharp/Formats/Png/Chunks/PhysicalChunkData.cs similarity index 93% rename from src/ImageSharp/Formats/Png/Chunks/PngPhysicalChunkData.cs rename to src/ImageSharp/Formats/Png/Chunks/PhysicalChunkData.cs index 39a9676b6..c1c151611 100644 --- a/src/ImageSharp/Formats/Png/Chunks/PngPhysicalChunkData.cs +++ b/src/ImageSharp/Formats/Png/Chunks/PhysicalChunkData.cs @@ -3,16 +3,16 @@ using System.Buffers.Binary; using SixLabors.ImageSharp.Common.Helpers; using SixLabors.ImageSharp.MetaData; -namespace SixLabors.ImageSharp.Formats.Png +namespace SixLabors.ImageSharp.Formats.Png.Chunks { /// /// The pHYs chunk specifies the intended pixel size or aspect ratio for display of the image. /// - internal readonly struct PngPhysicalChunkData + internal readonly struct PhysicalChunkData { public const int Size = 9; - public PngPhysicalChunkData(uint x, uint y, byte unitSpecifier) + public PhysicalChunkData(uint x, uint y, byte unitSpecifier) { this.XAxisPixelsPerUnit = x; this.YAxisPixelsPerUnit = y; @@ -43,7 +43,7 @@ namespace SixLabors.ImageSharp.Formats.Png /// /// The metadata. /// The constructed PngPhysicalChunkData instance. - public static PngPhysicalChunkData FromMetadata(ImageMetaData meta) + public static PhysicalChunkData FromMetadata(ImageMetaData meta) { byte unitSpecifier = 0; uint x; diff --git a/src/ImageSharp/Formats/Png/PngEncoderCore.cs b/src/ImageSharp/Formats/Png/PngEncoderCore.cs index 7f3c9945a..a86d8173c 100644 --- a/src/ImageSharp/Formats/Png/PngEncoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngEncoderCore.cs @@ -9,7 +9,6 @@ using System.Linq; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using SixLabors.ImageSharp.Advanced; -using SixLabors.ImageSharp.Common.Helpers; using SixLabors.ImageSharp.Formats.Png.Filters; using SixLabors.ImageSharp.Formats.Png.Zlib; using SixLabors.ImageSharp.Memory; @@ -674,9 +673,9 @@ namespace SixLabors.ImageSharp.Formats.Png /// The image meta data. private void WritePhysicalChunk(Stream stream, ImageMetaData meta) { - PngPhysicalChunkData.FromMetadata(meta).WriteTo(this.chunkDataBuffer); + PhysicalChunkData.FromMetadata(meta).WriteTo(this.chunkDataBuffer); - this.WriteChunk(stream, PngChunkType.Physical, this.chunkDataBuffer, 0, PngPhysicalChunkData.Size); + this.WriteChunk(stream, PngChunkType.Physical, this.chunkDataBuffer, 0, PhysicalChunkData.Size); } /// From 66c3f93890d40c12881522ad97e16099cf3a7060 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Mon, 1 Oct 2018 18:04:32 -0700 Subject: [PATCH 11/11] Add Parse method to PhysicalChunkData --- .../Formats/Png/Chunks/PhysicalChunkData.cs | 16 ++++++++++++- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 23 +++++-------------- src/ImageSharp/Formats/Png/PngEncoderCore.cs | 1 + 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/ImageSharp/Formats/Png/Chunks/PhysicalChunkData.cs b/src/ImageSharp/Formats/Png/Chunks/PhysicalChunkData.cs index c1c151611..07fc688d5 100644 --- a/src/ImageSharp/Formats/Png/Chunks/PhysicalChunkData.cs +++ b/src/ImageSharp/Formats/Png/Chunks/PhysicalChunkData.cs @@ -37,6 +37,20 @@ namespace SixLabors.ImageSharp.Formats.Png.Chunks /// public byte UnitSpecifier { get; } + /// + /// Parses the PhysicalChunkData from the given buffer. + /// + /// The data buffer. + /// The parsed PhysicalChunkData. + public static PhysicalChunkData Parse(ReadOnlySpan data) + { + uint hResolution = BinaryPrimitives.ReadUInt32BigEndian(data.Slice(0, 4)); + uint vResolution = BinaryPrimitives.ReadUInt32BigEndian(data.Slice(4, 4)); + byte unit = data[8]; + + return new PhysicalChunkData(hResolution, vResolution, unit); + } + /// /// Constructs the PngPhysicalChunkData from the provided metadata. /// If the resolution units are not in meters, they are automatically convereted. @@ -76,7 +90,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Chunks break; } - return new PngPhysicalChunkData(x, y, unitSpecifier); + return new PhysicalChunkData(x, y, unitSpecifier); } /// diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 8344d7b54..8401f4e98 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -8,6 +8,7 @@ using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Text; using SixLabors.ImageSharp.Advanced; +using SixLabors.ImageSharp.Formats.Png.Chunks; using SixLabors.ImageSharp.Formats.Png.Filters; using SixLabors.ImageSharp.Formats.Png.Zlib; using SixLabors.ImageSharp.Memory; @@ -376,26 +377,14 @@ namespace SixLabors.ImageSharp.Formats.Png /// The data containing physical data. private void ReadPhysicalChunk(ImageMetaData metadata, ReadOnlySpan data) { - // The pHYs chunk specifies the intended pixel size or aspect ratio for display of the image. It contains: - // Pixels per unit, X axis: 4 bytes (unsigned integer) - // Pixels per unit, Y axis: 4 bytes (unsigned integer) - // Unit specifier: 1 byte - // - // The following values are legal for the unit specifier: - // 0: unit is unknown - // 1: unit is the meter - // - // When the unit specifier is 0, the pHYs chunk defines pixel aspect ratio only; the actual size of the pixels remains unspecified. - int hResolution = BinaryPrimitives.ReadInt32BigEndian(data.Slice(0, 4)); - int vResolution = BinaryPrimitives.ReadInt32BigEndian(data.Slice(4, 4)); - byte unit = data[8]; - - metadata.ResolutionUnits = unit == byte.MinValue + var physicalChunk = PhysicalChunkData.Parse(data); + + metadata.ResolutionUnits = physicalChunk.UnitSpecifier == byte.MinValue ? PixelResolutionUnit.AspectRatio : PixelResolutionUnit.PixelsPerMeter; - metadata.HorizontalResolution = hResolution; - metadata.VerticalResolution = vResolution; + metadata.HorizontalResolution = physicalChunk.XAxisPixelsPerUnit; + metadata.VerticalResolution = physicalChunk.YAxisPixelsPerUnit; } /// diff --git a/src/ImageSharp/Formats/Png/PngEncoderCore.cs b/src/ImageSharp/Formats/Png/PngEncoderCore.cs index a86d8173c..a46d83707 100644 --- a/src/ImageSharp/Formats/Png/PngEncoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngEncoderCore.cs @@ -9,6 +9,7 @@ using System.Linq; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using SixLabors.ImageSharp.Advanced; +using SixLabors.ImageSharp.Formats.Png.Chunks; using SixLabors.ImageSharp.Formats.Png.Filters; using SixLabors.ImageSharp.Formats.Png.Zlib; using SixLabors.ImageSharp.Memory;