From 49fb759f2918a385f99b9af89743166cd7ff0a6d Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Mon, 1 Oct 2018 15:55:35 -0700 Subject: [PATCH 01/21] 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 3b67146b96..3ac13eb58a 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/21] 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 3ac13eb58a..fa509336e7 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/21] 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 ff25e26b7a..48c866f671 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 fa509336e7..a24b69160a 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/21] 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 a24b69160a..112dc72623 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 ec22f1bb42..0523502b05 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/21] 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 0000000000..e6b800e86a --- /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/21] 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 112dc72623..812175612a 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/21] 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 812175612a..8344d7b544 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 549cb3fe09..5f95499088 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/21] 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 0523502b05..ea43ba96a5 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/21] 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 0000000000..39a9676b63 --- /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 525cc8bd1c..7f3c9945ad 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/21] 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 39a9676b63..c1c151611a 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 7f3c9945ad..a86d8173cb 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/21] 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 c1c151611a..07fc688d50 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 8344d7b544..8401f4e98f 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 a86d8173cb..a46d83707e 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; From 77f6c1738fbbdfcb3f2273a617cb845d60306399 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 4 Oct 2018 08:27:00 +0100 Subject: [PATCH 12/21] Fix #721 --- src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs | 3 ++- .../ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs | 5 +++-- tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/External | 2 +- tests/Images/Input/Jpg/issues/Issue721-InvalidAPP0.jpg | 3 +++ 5 files changed, 10 insertions(+), 4 deletions(-) create mode 100644 tests/Images/Input/Jpg/issues/Issue721-InvalidAPP0.jpg diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 011b6100dc..e3b0b4bdc6 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -510,7 +510,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg /// The remaining bytes in the segment block. private void ProcessApplicationHeaderMarker(int remaining) { - if (remaining < 5) + // We can only decode JFif identifiers. + if (remaining < JFifMarker.Length) { // Skip the application header length this.InputStream.Skip(remaining); diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs index a14c4735f5..2f041e3ab4 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs @@ -25,7 +25,8 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg TestImages.Jpeg.Issues.MultiHuffmanBaseline394, TestImages.Jpeg.Issues.ExifDecodeOutOfRange694, TestImages.Jpeg.Issues.InvalidEOI695, - TestImages.Jpeg.Issues.ExifResizeOutOfRange696 + TestImages.Jpeg.Issues.ExifResizeOutOfRange696, + TestImages.Jpeg.Issues.InvalidAPP0721 }; public static string[] ProgressiveTestJpegs = @@ -54,7 +55,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg TestImages.Jpeg.Issues.MissingFF00ProgressiveBedroom159 }; - private static readonly Dictionary CustomToleranceValues = + private static readonly Dictionary CustomToleranceValues = new Dictionary { // Baseline: diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index acfad042eb..309fb1d4ab 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -155,6 +155,7 @@ namespace SixLabors.ImageSharp.Tests public const string ExifDecodeOutOfRange694 = "Jpg/issues/Issue694-Decode-Exif-OutOfRange.jpg"; public const string InvalidEOI695 = "Jpg/issues/Issue695-Invalid-EOI.jpg"; public const string ExifResizeOutOfRange696 = "Jpg/issues/Issue696-Resize-Exif-OutOfRange.jpg"; + public const string InvalidAPP0721 = "Jpg/issues/Issue721-InvalidAPP0.jpg"; } public static readonly string[] All = Baseline.All.Concat(Progressive.All).ToArray(); diff --git a/tests/Images/External b/tests/Images/External index 7f4d2d64c6..5f3cbd839f 160000 --- a/tests/Images/External +++ b/tests/Images/External @@ -1 +1 @@ -Subproject commit 7f4d2d64c6b820ca2b6827e6a8540a1013305ccf +Subproject commit 5f3cbd839fbbffae615d294d1dabafdcabc64cf9 diff --git a/tests/Images/Input/Jpg/issues/Issue721-InvalidAPP0.jpg b/tests/Images/Input/Jpg/issues/Issue721-InvalidAPP0.jpg new file mode 100644 index 0000000000..adaea47e55 --- /dev/null +++ b/tests/Images/Input/Jpg/issues/Issue721-InvalidAPP0.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:9775b7b975422d48a192d54dce140c1df19a181da9889eb99ee7d4331078b460 +size 1225163 From 6f9875d946aa15d9f1a0a4885337d5ad1383826e Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 4 Oct 2018 20:46:08 +0100 Subject: [PATCH 13/21] Decode components in correct order + cleanup + optimizations. --- .../ColorSpaceGenerator.csproj | 1 + .../Jpeg/Components/Decoder/FastACTables.cs | 27 +++++++---------- .../Jpeg/Components/Decoder/HuffmanTable.cs | 3 +- .../Jpeg/Components/Decoder/JpegFrame.cs | 6 ++++ .../Jpeg/Components/Decoder/ScanDecoder.cs | 30 ++++++++----------- .../Formats/Jpeg/JpegDecoderCore.cs | 14 +++++---- .../Formats/Jpg/JpegDecoderTests.Images.cs | 3 ++ tests/ImageSharp.Tests/TestImages.cs | 3 ++ tests/Images/External | 2 +- ...e723-Ordered-Interleaved-Progressive-A.jpg | 3 ++ ...e723-Ordered-Interleaved-Progressive-B.jpg | 3 ++ ...e723-Ordered-Interleaved-Progressive-C.jpg | 3 ++ 12 files changed, 55 insertions(+), 43 deletions(-) create mode 100644 ColorSpaceGenerator/ColorSpaceGenerator.csproj create mode 100644 tests/Images/Input/Jpg/issues/Issue723-Ordered-Interleaved-Progressive-A.jpg create mode 100644 tests/Images/Input/Jpg/issues/Issue723-Ordered-Interleaved-Progressive-B.jpg create mode 100644 tests/Images/Input/Jpg/issues/Issue723-Ordered-Interleaved-Progressive-C.jpg diff --git a/ColorSpaceGenerator/ColorSpaceGenerator.csproj b/ColorSpaceGenerator/ColorSpaceGenerator.csproj new file mode 100644 index 0000000000..7727272f8f --- /dev/null +++ b/ColorSpaceGenerator/ColorSpaceGenerator.csproj @@ -0,0 +1 @@ + diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/FastACTables.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/FastACTables.cs index 26bcde8e51..a7ec93eaf7 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/FastACTables.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/FastACTables.cs @@ -20,29 +20,22 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// Initializes a new instance of the class. /// /// The memory allocator used to allocate memory for image processing operations. - public FastACTables(MemoryAllocator memoryAllocator) - { - this.tables = memoryAllocator.Allocate2D(512, 4, AllocationOptions.Clean); - } + public FastACTables(MemoryAllocator memoryAllocator) => this.tables = memoryAllocator.Allocate2D(512, 4, AllocationOptions.Clean); /// /// Gets the representing the table at the index in the collection. /// /// The table index. /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public ReadOnlySpan GetTableSpan(int index) - { - return this.tables.GetRowSpan(index); - } + [MethodImpl(InliningOptions.ShortMethod)] + public ReadOnlySpan GetTableSpan(int index) => this.tables.GetRowSpan(index); /// - /// Gets a reference to the first element of the AC table indexed by /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public ref short GetAcTableReference(JpegComponent component) - { - return ref this.tables.GetRowSpan(component.ACHuffmanTableId)[0]; - } + /// Gets a reference to the first element of the AC table indexed by + /// + /// The frame component. + [MethodImpl(InliningOptions.ShortMethod)] + public ref short GetAcTableReference(JpegComponent component) => ref this.tables.GetRowSpan(component.ACHuffmanTableId)[0]; /// /// Builds a lookup table for fast AC entropy scan decoding. @@ -67,7 +60,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder int magbits = rs & 15; int len = huffman.Sizes[fast]; - if (magbits > 0 && len + magbits <= FastBits) + if (magbits != 0 && len + magbits <= FastBits) { // Magnitude code followed by receive_extend code int k = ((i << len) & ((1 << FastBits) - 1)) >> (FastBits - magbits); @@ -80,7 +73,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder // if the result is small enough, we can fit it in fastAC table if (k >= -128 && k <= 127) { - fastAC[i] = (short)((k * 256) + (run * 16) + (len + magbits)); + fastAC[i] = (short)((k << 8) + (run << 4) + (len + magbits)); } } } diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs index 0138164ed2..a6bf1bd953 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs @@ -64,8 +64,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder byte l = count[i]; for (short j = 0; j < l; j++) { - sizesRef[x] = i; - x++; + sizesRef[x++] = i; } } diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs index da089fa44a..36a3dc2d26 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs @@ -45,6 +45,12 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// public byte[] ComponentIds { get; set; } + /// + /// Gets or sets the order in which to process the components + /// in interleaved mode. + /// + public byte[] ComponentOrder { get; set; } + /// /// Gets or sets the frame component collection /// diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/ScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/ScanDecoder.cs index 8c525335bc..6741ccdac2 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/ScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/ScanDecoder.cs @@ -34,9 +34,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder // The restart interval. private readonly int restartInterval; - // The current component index. - private readonly int componentIndex; - // The number of interleaved components. private readonly int componentsLength; @@ -87,7 +84,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// The DC Huffman tables. /// The AC Huffman tables. /// The fast AC decoding tables. - /// The component index within the array. /// The length of the components. Different to the array length. /// The reset interval. /// The spectral selection start. @@ -100,7 +96,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder HuffmanTables dcHuffmanTables, HuffmanTables acHuffmanTables, FastACTables fastACTables, - int componentIndex, int componentsLength, int restartInterval, int spectralStart, @@ -117,7 +112,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder this.components = frame.Components; this.marker = JpegConstants.Markers.XFF; this.markerPosition = 0; - this.componentIndex = componentIndex; this.componentsLength = componentsLength; this.restartInterval = restartInterval; this.spectralStart = spectralStart; @@ -176,7 +170,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder // Scan an interleaved mcu... process components in order for (int k = 0; k < this.componentsLength; k++) { - JpegComponent component = this.components[k]; + int order = this.frame.ComponentOrder[k]; + JpegComponent component = this.components[order]; ref HuffmanTable dcHuffmanTable = ref this.dcHuffmanTables[component.DCHuffmanTableId]; ref HuffmanTable acHuffmanTable = ref this.acHuffmanTables[component.ACHuffmanTableId]; @@ -223,14 +218,13 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder } /// - /// Non-interleaved data, we just need to process one block at a ti - /// in trivial scanline order - /// number of blocks to do just depends on how many actual "pixels" - /// component has, independent of interleaved MCU blocking and such + /// Non-interleaved data, we just need to process one block at a time in trivial scanline order + /// number of blocks to do just depends on how many actual "pixels" each component has, + /// independent of interleaved MCU blocking and such. /// private void ParseBaselineDataNonInterleaved() { - JpegComponent component = this.components[this.componentIndex]; + JpegComponent component = this.components[this.frame.ComponentOrder[0]]; int w = component.WidthInBlocks; int h = component.HeightInBlocks; @@ -295,7 +289,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder // Scan an interleaved mcu... process components in order for (int k = 0; k < this.componentsLength; k++) { - JpegComponent component = this.components[k]; + int order = this.frame.ComponentOrder[k]; + JpegComponent component = this.components[order]; ref HuffmanTable dcHuffmanTable = ref this.dcHuffmanTables[component.DCHuffmanTableId]; int h = component.HorizontalSamplingFactor; int v = component.VerticalSamplingFactor; @@ -344,7 +339,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// private void ParseProgressiveDataNonInterleaved() { - JpegComponent component = this.components[this.componentIndex]; + JpegComponent component = this.components[this.frame.ComponentOrder[0]]; int w = component.WidthInBlocks; int h = component.HeightInBlocks; @@ -729,8 +724,9 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder } uint k = LRot(this.codeBuffer, n); - this.codeBuffer = k & ~Bmask[n]; - k &= Bmask[n]; + uint mask = Bmask[n]; + this.codeBuffer = k & ~mask; + k &= mask; this.codeBits -= n; return (int)k; } @@ -839,7 +835,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder // that way we don't need to shift inside the loop. uint temp = this.codeBuffer >> 16; int k; - for (k = FastBits + 1; ; k++) + for (k = FastBits + 1; ; ++k) { if (temp < table.MaxCode[k]) { diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index e3b0b4bdc6..22d9cbdee4 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -747,11 +747,12 @@ namespace SixLabors.ImageSharp.Formats.Jpeg if (!metadataOnly) { // No need to pool this. They max out at 4 - this.Frame.ComponentIds = new byte[this.Frame.ComponentCount]; - this.Frame.Components = new JpegComponent[this.Frame.ComponentCount]; + this.Frame.ComponentIds = new byte[this.ComponentCount]; + this.Frame.ComponentOrder = new byte[this.ComponentCount]; + this.Frame.Components = new JpegComponent[this.ComponentCount]; this.ColorSpace = this.DeduceJpegColorSpace(); - for (int i = 0; i < this.Frame.ComponentCount; i++) + for (int i = 0; i < this.ComponentCount; i++) { byte hv = this.temp[index + 1]; int h = hv >> 4; @@ -823,10 +824,10 @@ namespace SixLabors.ImageSharp.Formats.Jpeg codeLengths.GetSpan(), huffmanValues.GetSpan()); - if (huffmanTableSpec >> 4 != 0) + if (tableType != 0) { // Build a table that decodes both magnitude and value of small ACs in one go. - this.fastACTables.BuildACTableLut(huffmanTableSpec & 15, this.acHuffmanTables); + this.fastACTables.BuildACTableLut(tableIndex, this.acHuffmanTables); } } } @@ -867,6 +868,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg if (selector == id) { componentIndex = j; + break; } } @@ -879,6 +881,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg int tableSpec = this.InputStream.ReadByte(); component.DCHuffmanTableId = tableSpec >> 4; component.ACHuffmanTableId = tableSpec & 15; + this.Frame.ComponentOrder[i] = (byte)componentIndex; } this.InputStream.Read(this.temp, 0, 3); @@ -893,7 +896,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg this.dcHuffmanTables, this.acHuffmanTables, this.fastACTables, - componentIndex, selectorsCount, this.resetInterval, spectralStart, diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs index 2f041e3ab4..6bc559978c 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs @@ -44,6 +44,9 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg TestImages.Jpeg.Issues.BadRstProgressive518, TestImages.Jpeg.Issues.MissingFF00ProgressiveBedroom159, TestImages.Jpeg.Issues.DhtHasWrongLength624, + TestImages.Jpeg.Issues.OrderedInterleavedProgressive723A, + TestImages.Jpeg.Issues.OrderedInterleavedProgressive723B, + TestImages.Jpeg.Issues.OrderedInterleavedProgressive723C }; /// diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 309fb1d4ab..fdf586c430 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -156,6 +156,9 @@ namespace SixLabors.ImageSharp.Tests public const string InvalidEOI695 = "Jpg/issues/Issue695-Invalid-EOI.jpg"; public const string ExifResizeOutOfRange696 = "Jpg/issues/Issue696-Resize-Exif-OutOfRange.jpg"; public const string InvalidAPP0721 = "Jpg/issues/Issue721-InvalidAPP0.jpg"; + public const string OrderedInterleavedProgressive723A = "Jpg/issues/Issue723-Ordered-Interleaved-Progressive-A.jpg"; + public const string OrderedInterleavedProgressive723B = "Jpg/issues/Issue723-Ordered-Interleaved-Progressive-B.jpg"; + public const string OrderedInterleavedProgressive723C = "Jpg/issues/Issue723-Ordered-Interleaved-Progressive-C.jpg"; } public static readonly string[] All = Baseline.All.Concat(Progressive.All).ToArray(); diff --git a/tests/Images/External b/tests/Images/External index 5f3cbd839f..c0627f384c 160000 --- a/tests/Images/External +++ b/tests/Images/External @@ -1 +1 @@ -Subproject commit 5f3cbd839fbbffae615d294d1dabafdcabc64cf9 +Subproject commit c0627f384c1d3d2f8d914c9578ae31354c35fd2c diff --git a/tests/Images/Input/Jpg/issues/Issue723-Ordered-Interleaved-Progressive-A.jpg b/tests/Images/Input/Jpg/issues/Issue723-Ordered-Interleaved-Progressive-A.jpg new file mode 100644 index 0000000000..13cbb5aa12 --- /dev/null +++ b/tests/Images/Input/Jpg/issues/Issue723-Ordered-Interleaved-Progressive-A.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:3909297b3b3427e9836906e324ca6de64614f6da6290e2fa9bb1a0280b118f72 +size 42798 diff --git a/tests/Images/Input/Jpg/issues/Issue723-Ordered-Interleaved-Progressive-B.jpg b/tests/Images/Input/Jpg/issues/Issue723-Ordered-Interleaved-Progressive-B.jpg new file mode 100644 index 0000000000..11657617e8 --- /dev/null +++ b/tests/Images/Input/Jpg/issues/Issue723-Ordered-Interleaved-Progressive-B.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:4db53948af79218d106a7574f7159468fd2831828ae44e33009f57dc9ab9c07b +size 36937 diff --git a/tests/Images/Input/Jpg/issues/Issue723-Ordered-Interleaved-Progressive-C.jpg b/tests/Images/Input/Jpg/issues/Issue723-Ordered-Interleaved-Progressive-C.jpg new file mode 100644 index 0000000000..c1c92e0dc8 --- /dev/null +++ b/tests/Images/Input/Jpg/issues/Issue723-Ordered-Interleaved-Progressive-C.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:cf1c68bec39d39f9caabbf66cf361078a684cb3ae3a1b30509ad12484cae4783 +size 46799 From bc5010bfbb898c9fb2df8abd28e1717c5443519e Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 5 Oct 2018 10:11:05 +0100 Subject: [PATCH 14/21] Clean up Huffman table code. --- .../Jpeg/Components/Decoder/FastACTables.cs | 8 +- .../Jpeg/Components/Decoder/HuffmanTable.cs | 104 ++++++++---------- 2 files changed, 52 insertions(+), 60 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/FastACTables.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/FastACTables.cs index a7ec93eaf7..bfae7fd756 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/FastACTables.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/FastACTables.cs @@ -46,19 +46,19 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder { const int FastBits = ScanDecoder.FastBits; Span fastAC = this.tables.GetRowSpan(index); - ref HuffmanTable huffman = ref acHuffmanTables[index]; + ref HuffmanTable huffmanTable = ref acHuffmanTables[index]; int i; for (i = 0; i < (1 << FastBits); i++) { - byte fast = huffman.Lookahead[i]; + byte fast = huffmanTable.Lookahead[i]; fastAC[i] = 0; if (fast < byte.MaxValue) { - int rs = huffman.Values[fast]; + int rs = huffmanTable.Values[fast]; int run = (rs >> 4) & 15; int magbits = rs & 15; - int len = huffman.Sizes[fast]; + int len = huffmanTable.Sizes[fast]; if (magbits != 0 && len + magbits <= FastBits) { diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs index a6bf1bd953..ffc4ce9829 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs @@ -46,87 +46,79 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// Initializes a new instance of the struct. /// /// The to use for buffer allocations. - /// The code lengths + /// The code lengths /// The huffman values - public HuffmanTable(MemoryAllocator memoryAllocator, ReadOnlySpan count, ReadOnlySpan values) + public HuffmanTable(MemoryAllocator memoryAllocator, ReadOnlySpan codeLengths, ReadOnlySpan values) { const int Length = 257; using (IMemoryOwner huffcode = memoryAllocator.Allocate(Length)) { ref short huffcodeRef = ref MemoryMarshal.GetReference(huffcode.GetSpan()); + ref byte codeLengthsRef = ref MemoryMarshal.GetReference(codeLengths); // Figure C.1: make table of Huffman code length for each symbol - fixed (short* sizesRef = this.Sizes.Data) + ref short sizesRef = ref this.Sizes.Data[0]; + short x = 0; + + for (short i = 1; i < 17; i++) { - short x = 0; - for (short i = 1; i < 17; i++) + byte length = Unsafe.Add(ref codeLengthsRef, i); + for (short j = 0; j < length; j++) { - byte l = count[i]; - for (short j = 0; j < l; j++) - { - sizesRef[x++] = i; - } + Unsafe.Add(ref sizesRef, x++) = i; } + } - sizesRef[x] = 0; + Unsafe.Add(ref sizesRef, x) = 0; - // Figure C.2: generate the codes themselves - int k = 0; - fixed (int* valOffsetRef = this.ValOffset.Data) - fixed (uint* maxcodeRef = this.MaxCode.Data) + // Figure C.2: generate the codes themselves + int si = 0; + ref int valOffsetRef = ref this.ValOffset.Data[0]; + ref uint maxcodeRef = ref this.MaxCode.Data[0]; + + uint code = 0; + int k; + for (k = 1; k < 17; k++) + { + // Compute delta to add to code to compute symbol id. + Unsafe.Add(ref valOffsetRef, k) = (int)(si - code); + if (Unsafe.Add(ref sizesRef, si) == k) { - uint code = 0; - int j; - for (j = 1; j < 17; j++) + while (Unsafe.Add(ref sizesRef, si) == k) { - // Compute delta to add to code to compute symbol id. - valOffsetRef[j] = (int)(k - code); - if (sizesRef[k] == j) - { - while (sizesRef[k] == j) - { - Unsafe.Add(ref huffcodeRef, k++) = (short)code++; - } - } - - // Figure F.15: generate decoding tables for bit-sequential decoding. - // Compute largest code + 1 for this size. preshifted as need later. - maxcodeRef[j] = code << (16 - j); - code <<= 1; + Unsafe.Add(ref huffcodeRef, si++) = (short)code++; } - - maxcodeRef[j] = 0xFFFFFFFF; } - // Generate non-spec lookup tables to speed up decoding. - fixed (byte* lookaheadRef = this.Lookahead.Data) - { - const int FastBits = ScanDecoder.FastBits; - var fast = new Span(lookaheadRef, 1 << FastBits); - fast.Fill(0xFF); // Flag for non-accelerated + // Figure F.15: generate decoding tables for bit-sequential decoding. + // Compute largest code + 1 for this size. preshifted as we need later. + Unsafe.Add(ref maxcodeRef, k) = code << (16 - k); + code <<= 1; + } + + Unsafe.Add(ref maxcodeRef, k) = 0xFFFFFFFF; - for (int i = 0; i < k; i++) + // Generate non-spec lookup tables to speed up decoding. + const int FastBits = ScanDecoder.FastBits; + ref byte fastRef = ref this.Lookahead.Data[0]; + new Span(Unsafe.AsPointer(ref fastRef), 1 << FastBits).Fill(0xFF); // Flag for non-accelerated + + for (int i = 0; i < si; i++) + { + int size = Unsafe.Add(ref sizesRef, i); + if (size <= FastBits) + { + int c = Unsafe.Add(ref huffcodeRef, i) << (FastBits - size); + int m = 1 << (FastBits - size); + for (int l = 0; l < m; l++) { - int s = sizesRef[i]; - if (s <= ScanDecoder.FastBits) - { - int c = Unsafe.Add(ref huffcodeRef, i) << (FastBits - s); - int m = 1 << (FastBits - s); - for (int j = 0; j < m; j++) - { - fast[c + j] = (byte)i; - } - } + Unsafe.Add(ref fastRef, c + l) = (byte)i; } } } } - fixed (byte* huffValRef = this.Values.Data) - { - var huffValSpan = new Span(huffValRef, 256); - values.CopyTo(huffValSpan); - } + values.CopyTo(new Span(Unsafe.AsPointer(ref this.Values.Data[0]), 256)); } } } \ No newline at end of file From c10ba1280f14f23cde716578c2435ab12842de61 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 5 Oct 2018 21:42:42 +0100 Subject: [PATCH 15/21] Remove fixed structs. 7.3 can index fixed arrays! --- .../Jpeg/Components/Decoder/FastACTables.cs | 2 +- .../Components/Decoder/FixedByteBuffer256.cs | 24 ------------------- .../Components/Decoder/FixedByteBuffer512.cs | 24 ------------------- .../Components/Decoder/FixedInt16Buffer257.cs | 24 ------------------- .../Components/Decoder/FixedInt32Buffer18.cs | 24 ------------------- .../Components/Decoder/FixedUInt32Buffer18.cs | 24 ------------------- .../Jpeg/Components/Decoder/HuffmanTable.cs | 24 +++++++++---------- .../Jpeg/Components/Decoder/ScanDecoder.cs | 4 ++-- 8 files changed, 15 insertions(+), 135 deletions(-) delete mode 100644 src/ImageSharp/Formats/Jpeg/Components/Decoder/FixedByteBuffer256.cs delete mode 100644 src/ImageSharp/Formats/Jpeg/Components/Decoder/FixedByteBuffer512.cs delete mode 100644 src/ImageSharp/Formats/Jpeg/Components/Decoder/FixedInt16Buffer257.cs delete mode 100644 src/ImageSharp/Formats/Jpeg/Components/Decoder/FixedInt32Buffer18.cs delete mode 100644 src/ImageSharp/Formats/Jpeg/Components/Decoder/FixedUInt32Buffer18.cs diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/FastACTables.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/FastACTables.cs index bfae7fd756..06b46746a6 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/FastACTables.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/FastACTables.cs @@ -42,7 +42,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// /// The table index. /// The collection of AC Huffman tables. - public void BuildACTableLut(int index, HuffmanTables acHuffmanTables) + public unsafe void BuildACTableLut(int index, HuffmanTables acHuffmanTables) { const int FastBits = ScanDecoder.FastBits; Span fastAC = this.tables.GetRowSpan(index); diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/FixedByteBuffer256.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/FixedByteBuffer256.cs deleted file mode 100644 index 1d26178e0c..0000000000 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/FixedByteBuffer256.cs +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright (c) Six Labors and contributors. -// Licensed under the Apache License, Version 2.0. - -using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; - -namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder -{ - [StructLayout(LayoutKind.Sequential)] - internal unsafe struct FixedByteBuffer256 - { - public fixed byte Data[256]; - - public byte this[int idx] - { - [MethodImpl(MethodImplOptions.AggressiveInlining)] - get - { - ref byte self = ref Unsafe.As(ref this); - return Unsafe.Add(ref self, idx); - } - } - } -} \ No newline at end of file diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/FixedByteBuffer512.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/FixedByteBuffer512.cs deleted file mode 100644 index 556e74fd58..0000000000 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/FixedByteBuffer512.cs +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright (c) Six Labors and contributors. -// Licensed under the Apache License, Version 2.0. - -using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; - -namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder -{ - [StructLayout(LayoutKind.Sequential)] - internal unsafe struct FixedByteBuffer512 - { - public fixed byte Data[1 << ScanDecoder.FastBits]; - - public byte this[int idx] - { - [MethodImpl(MethodImplOptions.AggressiveInlining)] - get - { - ref byte self = ref Unsafe.As(ref this); - return Unsafe.Add(ref self, idx); - } - } - } -} \ No newline at end of file diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/FixedInt16Buffer257.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/FixedInt16Buffer257.cs deleted file mode 100644 index a3b67a700b..0000000000 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/FixedInt16Buffer257.cs +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright (c) Six Labors and contributors. -// Licensed under the Apache License, Version 2.0. - -using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; - -namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder -{ - [StructLayout(LayoutKind.Sequential)] - internal unsafe struct FixedInt16Buffer257 - { - public fixed short Data[257]; - - public short this[int idx] - { - [MethodImpl(MethodImplOptions.AggressiveInlining)] - get - { - ref short self = ref Unsafe.As(ref this); - return Unsafe.Add(ref self, idx); - } - } - } -} \ No newline at end of file diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/FixedInt32Buffer18.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/FixedInt32Buffer18.cs deleted file mode 100644 index bba89f072f..0000000000 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/FixedInt32Buffer18.cs +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright (c) Six Labors and contributors. -// Licensed under the Apache License, Version 2.0. - -using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; - -namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder -{ - [StructLayout(LayoutKind.Sequential)] - internal unsafe struct FixedInt32Buffer18 - { - public fixed int Data[18]; - - public int this[int idx] - { - [MethodImpl(MethodImplOptions.AggressiveInlining)] - get - { - ref int self = ref Unsafe.As(ref this); - return Unsafe.Add(ref self, idx); - } - } - } -} \ No newline at end of file diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/FixedUInt32Buffer18.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/FixedUInt32Buffer18.cs deleted file mode 100644 index 1d3ca99338..0000000000 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/FixedUInt32Buffer18.cs +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright (c) Six Labors and contributors. -// Licensed under the Apache License, Version 2.0. - -using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; - -namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder -{ - [StructLayout(LayoutKind.Sequential)] - internal unsafe struct FixedUInt32Buffer18 - { - public fixed uint Data[18]; - - public uint this[int idx] - { - [MethodImpl(MethodImplOptions.AggressiveInlining)] - get - { - ref uint self = ref Unsafe.As(ref this); - return Unsafe.Add(ref self, idx); - } - } - } -} \ No newline at end of file diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs index ffc4ce9829..08e8604f17 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs @@ -20,27 +20,27 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// /// Gets the max code array /// - public FixedUInt32Buffer18 MaxCode; + public fixed uint MaxCode[18]; /// /// Gets the value offset array /// - public FixedInt32Buffer18 ValOffset; + public fixed int ValOffset[18]; /// /// Gets the huffman value array /// - public FixedByteBuffer256 Values; + public fixed byte Values[256]; /// /// Gets the lookahead array /// - public FixedByteBuffer512 Lookahead; + public fixed byte Lookahead[512]; /// /// Gets the sizes array /// - public FixedInt16Buffer257 Sizes; + public fixed short Sizes[257]; /// /// Initializes a new instance of the struct. @@ -57,7 +57,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder ref byte codeLengthsRef = ref MemoryMarshal.GetReference(codeLengths); // Figure C.1: make table of Huffman code length for each symbol - ref short sizesRef = ref this.Sizes.Data[0]; + ref short sizesRef = ref this.Sizes[0]; short x = 0; for (short i = 1; i < 17; i++) @@ -73,8 +73,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder // Figure C.2: generate the codes themselves int si = 0; - ref int valOffsetRef = ref this.ValOffset.Data[0]; - ref uint maxcodeRef = ref this.MaxCode.Data[0]; + ref int valOffsetRef = ref this.ValOffset[0]; + ref uint maxcodeRef = ref this.MaxCode[0]; uint code = 0; int k; @@ -91,7 +91,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder } // Figure F.15: generate decoding tables for bit-sequential decoding. - // Compute largest code + 1 for this size. preshifted as we need later. + // Compute largest code + 1 for this size. preshifted as we needit later. Unsafe.Add(ref maxcodeRef, k) = code << (16 - k); code <<= 1; } @@ -100,8 +100,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder // Generate non-spec lookup tables to speed up decoding. const int FastBits = ScanDecoder.FastBits; - ref byte fastRef = ref this.Lookahead.Data[0]; - new Span(Unsafe.AsPointer(ref fastRef), 1 << FastBits).Fill(0xFF); // Flag for non-accelerated + ref byte fastRef = ref this.Lookahead[0]; + Unsafe.InitBlockUnaligned(ref fastRef, 0xFF, 1 << FastBits); // Flag for non-accelerated for (int i = 0; i < si; i++) { @@ -118,7 +118,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder } } - values.CopyTo(new Span(Unsafe.AsPointer(ref this.Values.Data[0]), 256)); + values.CopyTo(new Span(Unsafe.AsPointer(ref this.Values[0]), 256)); } } } \ No newline at end of file diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/ScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/ScanDecoder.cs index 6741ccdac2..351e453484 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/ScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/ScanDecoder.cs @@ -800,7 +800,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder } [MethodImpl(InliningOptions.ShortMethod)] - private int DecodeHuffman(ref HuffmanTable table) + private unsafe int DecodeHuffman(ref HuffmanTable table) { this.CheckBits(); @@ -825,7 +825,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder } [MethodImpl(InliningOptions.ColdPath)] - private int DecodeHuffmanSlow(ref HuffmanTable table) + private unsafe int DecodeHuffmanSlow(ref HuffmanTable table) { // Naive test is to shift the code_buffer down so k bits are // valid, then test against MaxCode. To speed this up, we've From af4c9dd5836b233b1e2fe8c94fd7885452ce6af3 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 5 Oct 2018 22:01:56 +0100 Subject: [PATCH 16/21] Add comment --- src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs index 08e8604f17..3f40068f0f 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs @@ -118,6 +118,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder } } + // Ok to use pointer here as struct is declared in method stack space so are essentially pinned. values.CopyTo(new Span(Unsafe.AsPointer(ref this.Values[0]), 256)); } } From 6f791d3e85db92d7eb5e2c1ab099ca9647bde2a8 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 5 Oct 2018 22:28:48 +0100 Subject: [PATCH 17/21] No pointers! --- src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs index 3f40068f0f..24d570bf1c 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs @@ -118,8 +118,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder } } - // Ok to use pointer here as struct is declared in method stack space so are essentially pinned. - values.CopyTo(new Span(Unsafe.AsPointer(ref this.Values[0]), 256)); + Unsafe.CopyBlockUnaligned(ref this.Values[0], ref MemoryMarshal.GetReference(values), 256); } } } \ No newline at end of file From acb81023886f7574352f39c8d49f17e2e07da056 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 6 Oct 2018 13:08:15 +0100 Subject: [PATCH 18/21] Throw when crop rectangle exceeds source bounds. --- ColorSpaceGenerator/ColorSpaceGenerator.csproj | 1 - src/ImageSharp/Processing/CropExtensions.cs | 2 +- .../Processors/Transforms/CropProcessor.cs | 7 +++++-- .../Processors/Transforms/EntropyCropProcessor.cs | 2 +- .../BaseImageOperationsExtensionTest.cs | 8 +++++--- tests/ImageSharp.Tests/ImageOperationTests.cs | 13 +++++-------- .../Processing/Transforms/CropTest.cs | 8 ++++++++ 7 files changed, 25 insertions(+), 16 deletions(-) delete mode 100644 ColorSpaceGenerator/ColorSpaceGenerator.csproj diff --git a/ColorSpaceGenerator/ColorSpaceGenerator.csproj b/ColorSpaceGenerator/ColorSpaceGenerator.csproj deleted file mode 100644 index 7727272f8f..0000000000 --- a/ColorSpaceGenerator/ColorSpaceGenerator.csproj +++ /dev/null @@ -1 +0,0 @@ - diff --git a/src/ImageSharp/Processing/CropExtensions.cs b/src/ImageSharp/Processing/CropExtensions.cs index 34c754a08e..1c0d80afc9 100644 --- a/src/ImageSharp/Processing/CropExtensions.cs +++ b/src/ImageSharp/Processing/CropExtensions.cs @@ -35,6 +35,6 @@ namespace SixLabors.ImageSharp.Processing /// The public static IImageProcessingContext Crop(this IImageProcessingContext source, Rectangle cropRectangle) where TPixel : struct, IPixel - => source.ApplyProcessor(new CropProcessor(cropRectangle)); + => source.ApplyProcessor(new CropProcessor(cropRectangle, source.GetCurrentSize())); } } \ No newline at end of file diff --git a/src/ImageSharp/Processing/Processors/Transforms/CropProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/CropProcessor.cs index 8e6a826fd3..3b1d7e94dd 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/CropProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/CropProcessor.cs @@ -22,8 +22,11 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// Initializes a new instance of the class. /// /// The target cropped rectangle. - public CropProcessor(Rectangle cropRectangle) + /// The source image size. + public CropProcessor(Rectangle cropRectangle, Size sourceSize) { + // Check bounds here and throw if we are passed a rectangle exceeding our source bounds. + Guard.IsTrue(new Rectangle(Point.Empty, sourceSize).Contains(cropRectangle), nameof(cropRectangle), "Crop rectangle should be smaller than the source bounds."); this.CropRectangle = cropRectangle; } @@ -53,7 +56,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms return; } - var rect = Rectangle.Intersect(this.CropRectangle, sourceRectangle); + Rectangle rect = this.CropRectangle; // Copying is cheap, we should process more pixels per task: ParallelExecutionSettings parallelSettings = configuration.GetParallelSettings().MultiplyMinimumPixelsPerTask(4); diff --git a/src/ImageSharp/Processing/Processors/Transforms/EntropyCropProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/EntropyCropProcessor.cs index 8eeae5d1fc..6de717afd9 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/EntropyCropProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/EntropyCropProcessor.cs @@ -62,7 +62,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms rectangle = ImageMaths.GetFilteredBoundingRectangle(temp, 0); } - new CropProcessor(rectangle).Apply(source, sourceRectangle); + new CropProcessor(rectangle, source.Size()).Apply(source, sourceRectangle); } /// diff --git a/tests/ImageSharp.Tests/BaseImageOperationsExtensionTest.cs b/tests/ImageSharp.Tests/BaseImageOperationsExtensionTest.cs index 34b2f718ee..7adbefb346 100644 --- a/tests/ImageSharp.Tests/BaseImageOperationsExtensionTest.cs +++ b/tests/ImageSharp.Tests/BaseImageOperationsExtensionTest.cs @@ -14,7 +14,9 @@ namespace SixLabors.ImageSharp.Tests private readonly FakeImageOperationsProvider.FakeImageOperations internalOperations; protected readonly Rectangle rect; protected readonly GraphicsOptions options; - private Image source; + private readonly Image source; + + public Rectangle SourceBounds() => this.source.Bounds(); public BaseImageOperationsExtensionTest() { @@ -29,7 +31,7 @@ namespace SixLabors.ImageSharp.Tests { Assert.InRange(index, 0, this.internalOperations.Applied.Count - 1); - var operation = this.internalOperations.Applied[index]; + FakeImageOperationsProvider.FakeImageOperations.AppliedOperation operation = this.internalOperations.Applied[index]; return Assert.IsType(operation.Processor); } @@ -38,7 +40,7 @@ namespace SixLabors.ImageSharp.Tests { Assert.InRange(index, 0, this.internalOperations.Applied.Count - 1); - var operation = this.internalOperations.Applied[index]; + FakeImageOperationsProvider.FakeImageOperations.AppliedOperation operation = this.internalOperations.Applied[index]; Assert.Equal(rect, operation.Rectangle); return Assert.IsType(operation.Processor); diff --git a/tests/ImageSharp.Tests/ImageOperationTests.cs b/tests/ImageSharp.Tests/ImageOperationTests.cs index d73eea6870..869882f672 100644 --- a/tests/ImageSharp.Tests/ImageOperationTests.cs +++ b/tests/ImageSharp.Tests/ImageOperationTests.cs @@ -56,7 +56,7 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void CloneCallsImageOperationsProvider_Func_WithDuplicateImage() { - var returned = this.image.Clone(x => x.ApplyProcessor(this.processor)); + Image returned = this.image.Clone(x => x.ApplyProcessor(this.processor)); Assert.True(this.provider.HasCreated(returned)); Assert.Contains(this.processor, this.provider.AppliedOperations(returned).Select(x => x.Processor)); @@ -65,7 +65,7 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void CloneCallsImageOperationsProvider_ListOfProcessors_WithDuplicateImage() { - var returned = this.image.Clone(this.processor); + Image returned = this.image.Clone(this.processor); Assert.True(this.provider.HasCreated(returned)); Assert.Contains(this.processor, this.provider.AppliedOperations(returned).Select(x => x.Processor)); @@ -74,7 +74,7 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void CloneCallsImageOperationsProvider_Func_NotOnOrigional() { - var returned = this.image.Clone(x => x.ApplyProcessor(this.processor)); + Image returned = this.image.Clone(x => x.ApplyProcessor(this.processor)); Assert.False(this.provider.HasCreated(this.image)); Assert.DoesNotContain(this.processor, this.provider.AppliedOperations(this.image).Select(x => x.Processor)); } @@ -82,7 +82,7 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void CloneCallsImageOperationsProvider_ListOfProcessors_NotOnOrigional() { - var returned = this.image.Clone(this.processor); + Image returned = this.image.Clone(this.processor); Assert.False(this.provider.HasCreated(this.image)); Assert.DoesNotContain(this.processor, this.provider.AppliedOperations(this.image).Select(x => x.Processor)); } @@ -95,9 +95,6 @@ namespace SixLabors.ImageSharp.Tests Assert.Contains(this.processor, operations.Applied.Select(x => x.Processor)); } - public void Dispose() - { - this.image.Dispose(); - } + public void Dispose() => this.image.Dispose(); } } \ No newline at end of file diff --git a/tests/ImageSharp.Tests/Processing/Transforms/CropTest.cs b/tests/ImageSharp.Tests/Processing/Transforms/CropTest.cs index 154167f15f..6731debd36 100644 --- a/tests/ImageSharp.Tests/Processing/Transforms/CropTest.cs +++ b/tests/ImageSharp.Tests/Processing/Transforms/CropTest.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. +using System; using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Processing; using SixLabors.ImageSharp.Processing.Processors.Transforms; @@ -33,5 +34,12 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms Assert.Equal(cropRectangle, processor.CropRectangle); } + + [Fact] + public void CropRectangleWithInvalidBoundsThrowsException() + { + var cropRectangle = Rectangle.Inflate(this.SourceBounds(), 5, 5); + Assert.Throws(() => this.operations.Crop(cropRectangle)); + } } } \ No newline at end of file From 1ce14f875effd5290f8c6ff1079c133f9cf163ed Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 6 Oct 2018 13:28:27 +0100 Subject: [PATCH 19/21] Remove invalid test --- .../Processing/Processors/Transforms/CropTest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/ImageSharp.Tests/Processing/Processors/Transforms/CropTest.cs b/tests/ImageSharp.Tests/Processing/Processors/Transforms/CropTest.cs index 2f78915120..c01c3b1bd3 100644 --- a/tests/ImageSharp.Tests/Processing/Processors/Transforms/CropTest.cs +++ b/tests/ImageSharp.Tests/Processing/Processors/Transforms/CropTest.cs @@ -17,7 +17,6 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Transforms { [Theory] [WithTestPatternImages(70, 30, PixelTypes.Rgba32, 0, 0, 70, 30)] - [WithTestPatternImages(50, 50, PixelTypes.Rgba32, -1, -1, 100, 200)] [WithTestPatternImages(30, 70, PixelTypes.Rgba32, 7, 13, 20, 50)] public void Crop(TestImageProvider provider, int x, int y, int w, int h) where TPixel : struct, IPixel From 8f7ef87fb3ae70edd924d7668067a0fae2178aab Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Sat, 6 Oct 2018 16:27:27 +0200 Subject: [PATCH 20/21] new generator methods (cherry picked from commit b116368137d044251ddc3b6879a0b43f3f964494) --- .../PixelOperations{TPixel}.Generated.cs | 43 ++++++------ .../PixelOperations{TPixel}.Generated.tt | 65 +++++++++++++++++-- 2 files changed, 80 insertions(+), 28 deletions(-) diff --git a/src/ImageSharp/PixelFormats/Generated/PixelOperations{TPixel}.Generated.cs b/src/ImageSharp/PixelFormats/Generated/PixelOperations{TPixel}.Generated.cs index f644fbefb5..c000b26469 100644 --- a/src/ImageSharp/PixelFormats/Generated/PixelOperations{TPixel}.Generated.cs +++ b/src/ImageSharp/PixelFormats/Generated/PixelOperations{TPixel}.Generated.cs @@ -24,13 +24,13 @@ namespace SixLabors.ImageSharp.PixelFormats ref Rgba64 sourceRef = ref MemoryMarshal.GetReference(source); ref TPixel destRef = ref MemoryMarshal.GetReference(destPixels); - var rgba = new Rgba64(0, 0, 0, 65535); + var temp = NamedColors.Black; for (int i = 0; i < count; i++) { ref TPixel dp = ref Unsafe.Add(ref destRef, i); - rgba = Unsafe.Add(ref sourceRef, i); - dp.PackFromRgba64(rgba); + temp = Unsafe.Add(ref sourceRef, i); + dp.PackFromRgba64(temp); } } @@ -95,13 +95,13 @@ namespace SixLabors.ImageSharp.PixelFormats ref Rgb48 sourceRef = ref MemoryMarshal.GetReference(source); ref TPixel destRef = ref MemoryMarshal.GetReference(destPixels); - var rgb = default(Rgb48); + var temp = NamedColors.Black; for (int i = 0; i < count; i++) { ref TPixel dp = ref Unsafe.Add(ref destRef, i); - rgb = Unsafe.Add(ref sourceRef, i); - dp.PackFromRgb48(rgb); + temp = Unsafe.Add(ref sourceRef, i); + dp.PackFromRgb48(temp); } } @@ -166,13 +166,13 @@ namespace SixLabors.ImageSharp.PixelFormats ref Rgba32 sourceRef = ref MemoryMarshal.GetReference(source); ref TPixel destRef = ref MemoryMarshal.GetReference(destPixels); - var rgba = new Rgba32(0, 0, 0, 255); + var temp = NamedColors.Black; for (int i = 0; i < count; i++) { ref TPixel dp = ref Unsafe.Add(ref destRef, i); - rgba = Unsafe.Add(ref sourceRef, i); - dp.PackFromRgba32(rgba); + temp = Unsafe.Add(ref sourceRef, i); + dp.PackFromRgba32(temp); } } @@ -237,13 +237,13 @@ namespace SixLabors.ImageSharp.PixelFormats ref Bgra32 sourceRef = ref MemoryMarshal.GetReference(source); ref TPixel destRef = ref MemoryMarshal.GetReference(destPixels); - var bgra = new Bgra32(0, 0, 0, 255); + var temp = NamedColors.Black; for (int i = 0; i < count; i++) { ref TPixel dp = ref Unsafe.Add(ref destRef, i); - bgra = Unsafe.Add(ref sourceRef, i); - dp.PackFromBgra32(bgra); + temp = Unsafe.Add(ref sourceRef, i); + dp.PackFromBgra32(temp); } } @@ -308,13 +308,13 @@ namespace SixLabors.ImageSharp.PixelFormats ref Rgb24 sourceRef = ref MemoryMarshal.GetReference(source); ref TPixel destRef = ref MemoryMarshal.GetReference(destPixels); - var rgba = new Rgba32(0, 0, 0, 255); + var temp = NamedColors.Black; for (int i = 0; i < count; i++) { ref TPixel dp = ref Unsafe.Add(ref destRef, i); - rgba.Rgb = Unsafe.Add(ref sourceRef, i); - dp.PackFromRgba32(rgba); + temp.Rgb = Unsafe.Add(ref sourceRef, i); + dp.PackFromRgba32(temp); } } @@ -379,13 +379,13 @@ namespace SixLabors.ImageSharp.PixelFormats ref Bgr24 sourceRef = ref MemoryMarshal.GetReference(source); ref TPixel destRef = ref MemoryMarshal.GetReference(destPixels); - var rgba = new Rgba32(0, 0, 0, 255); + var temp = NamedColors.Black; for (int i = 0; i < count; i++) { ref TPixel dp = ref Unsafe.Add(ref destRef, i); - rgba.Bgr = Unsafe.Add(ref sourceRef, i); - dp.PackFromRgba32(rgba); + temp.Bgr = Unsafe.Add(ref sourceRef, i); + dp.PackFromRgba32(temp); } } @@ -450,13 +450,13 @@ namespace SixLabors.ImageSharp.PixelFormats ref Argb32 sourceRef = ref MemoryMarshal.GetReference(source); ref TPixel destRef = ref MemoryMarshal.GetReference(destPixels); - var argb = new Argb32(0, 0, 0, 255); + var temp = NamedColors.Black; for (int i = 0; i < count; i++) { ref TPixel dp = ref Unsafe.Add(ref destRef, i); - argb = Unsafe.Add(ref sourceRef, i); - dp.PackFromArgb32(argb); + temp = Unsafe.Add(ref sourceRef, i); + dp.PackFromArgb32(temp); } } @@ -509,4 +509,5 @@ namespace SixLabors.ImageSharp.PixelFormats } } + } \ No newline at end of file diff --git a/src/ImageSharp/PixelFormats/Generated/PixelOperations{TPixel}.Generated.tt b/src/ImageSharp/PixelFormats/Generated/PixelOperations{TPixel}.Generated.tt index 1a6ac60f58..0729d02086 100644 --- a/src/ImageSharp/PixelFormats/Generated/PixelOperations{TPixel}.Generated.tt +++ b/src/ImageSharp/PixelFormats/Generated/PixelOperations{TPixel}.Generated.tt @@ -10,6 +10,49 @@ <#@ import namespace="System.Runtime.InteropServices" #> <#@ output extension=".cs" #> <# + + void GeneratePackFromMethods(string pixelType, string tempPixelType, string assignToTempCode) + { + #> + + /// + /// Converts 'count' elements in 'source` span of data to a span of -s. + /// + /// The source of data. + /// The to the destination pixels. + /// The number of pixels to convert. + internal virtual void PackFrom<#=pixelType#>(ReadOnlySpan<<#=pixelType#>> source, Span destPixels, int count) + { + GuardSpans(source, nameof(source), destPixels, nameof(destPixels), count); + + ref <#=pixelType#> sourceRef = ref MemoryMarshal.GetReference(source); + ref TPixel destRef = ref MemoryMarshal.GetReference(destPixels); + + var temp = NamedColors<<#=tempPixelType#>>.Black; + + for (int i = 0; i < count; i++) + { + ref TPixel dp = ref Unsafe.Add(ref destRef, i); + <#=assignToTempCode#> + dp.PackFrom<#=tempPixelType#>(temp); + } + } + + /// + /// A helper for that expects a byte span. + /// The layout of the data in 'sourceBytes' must be compatible with layout. + /// + /// The to the source bytes. + /// The to the destination pixels. + /// The number of pixels to convert. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal void PackFrom<#=pixelType#>Bytes(ReadOnlySpan sourceBytes, Span destPixels, int count) + { + this.PackFrom<#=pixelType#>(MemoryMarshal.Cast>(sourceBytes), destPixels, count); + } + <# + } + void GenerateToDestFormatMethods(string pixelType) { #> @@ -276,28 +319,36 @@ namespace SixLabors.ImageSharp.PixelFormats { <# - GeneratePackFromMethodUsingPackFromRgba64("Rgba64", "rgba = Unsafe.Add(ref sourceRef, i);"); + // GeneratePackFromMethodUsingPackFromRgba64("Rgba64", "rgba = Unsafe.Add(ref sourceRef, i);"); + + GeneratePackFromMethods("Rgba64", "Rgba64", "temp = Unsafe.Add(ref sourceRef, i);"); GenerateToDestFormatMethods("Rgba64"); - GeneratePackFromMethodUsingPackFromRgb48("Rgb48", "rgb = Unsafe.Add(ref sourceRef, i);"); + // GeneratePackFromMethodUsingPackFromRgb48("Rgb48", "rgb = Unsafe.Add(ref sourceRef, i);"); + GeneratePackFromMethods("Rgb48", "Rgb48", "temp = Unsafe.Add(ref sourceRef, i);"); GenerateToDestFormatMethods("Rgb48"); - GeneratePackFromMethodUsingPackFromRgba32("Rgba32", "rgba = Unsafe.Add(ref sourceRef, i);"); + GeneratePackFromMethods("Rgba32", "Rgba32", "temp = Unsafe.Add(ref sourceRef, i);"); GenerateToDestFormatMethods("Rgba32"); - GeneratePackFromMethodUsingPackFromBgra32("Bgra32", "bgra = Unsafe.Add(ref sourceRef, i);"); + // GeneratePackFromMethodUsingPackFromBgra32("Bgra32", "bgra = Unsafe.Add(ref sourceRef, i);"); + GeneratePackFromMethods("Bgra32", "Bgra32", "temp = Unsafe.Add(ref sourceRef, i);"); GenerateToDestFormatMethods("Bgra32"); - GeneratePackFromMethodUsingPackFromRgba32("Rgb24", "rgba.Rgb = Unsafe.Add(ref sourceRef, i);"); + // GeneratePackFromMethodUsingPackFromRgba32("Rgb24", "rgba.Rgb = Unsafe.Add(ref sourceRef, i);"); + GeneratePackFromMethods("Rgb24", "Rgba32", "temp.Rgb = Unsafe.Add(ref sourceRef, i);"); GenerateToDestFormatMethods("Rgb24"); - GeneratePackFromMethodUsingPackFromRgba32("Bgr24", "rgba.Bgr = Unsafe.Add(ref sourceRef, i);"); + // GeneratePackFromMethodUsingPackFromRgba32("Bgr24", "rgba.Bgr = Unsafe.Add(ref sourceRef, i);"); + GeneratePackFromMethods("Bgr24", "Rgba32", "temp.Bgr = Unsafe.Add(ref sourceRef, i);"); GenerateToDestFormatMethods("Bgr24"); - GeneratePackFromMethodUsingPackFromArgb32("Argb32", "argb = Unsafe.Add(ref sourceRef, i);"); + // GeneratePackFromMethodUsingPackFromArgb32("Argb32", "argb = Unsafe.Add(ref sourceRef, i);"); + GeneratePackFromMethods("Argb32", "Argb32", "temp = Unsafe.Add(ref sourceRef, i);"); GenerateToDestFormatMethods("Argb32"); #> } + } \ No newline at end of file From 006e9247201e0ce59c72ded46a48cc4d55bb90db Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Sat, 6 Oct 2018 16:50:40 +0200 Subject: [PATCH 21/21] drop old generators (cherry picked from commit 003bf21c50b8e6628ca68440b66a7d9edc0a2d7f) --- .../PixelOperations{TPixel}.Generated.cs | 7 + .../PixelOperations{TPixel}.Generated.tt | 220 +----------------- 2 files changed, 8 insertions(+), 219 deletions(-) diff --git a/src/ImageSharp/PixelFormats/Generated/PixelOperations{TPixel}.Generated.cs b/src/ImageSharp/PixelFormats/Generated/PixelOperations{TPixel}.Generated.cs index c000b26469..e8908fe05e 100644 --- a/src/ImageSharp/PixelFormats/Generated/PixelOperations{TPixel}.Generated.cs +++ b/src/ImageSharp/PixelFormats/Generated/PixelOperations{TPixel}.Generated.cs @@ -24,6 +24,7 @@ namespace SixLabors.ImageSharp.PixelFormats ref Rgba64 sourceRef = ref MemoryMarshal.GetReference(source); ref TPixel destRef = ref MemoryMarshal.GetReference(destPixels); + // For conversion methods writing only to RGB channels, we need to keep the alpha channel opaque! var temp = NamedColors.Black; for (int i = 0; i < count; i++) @@ -95,6 +96,7 @@ namespace SixLabors.ImageSharp.PixelFormats ref Rgb48 sourceRef = ref MemoryMarshal.GetReference(source); ref TPixel destRef = ref MemoryMarshal.GetReference(destPixels); + // For conversion methods writing only to RGB channels, we need to keep the alpha channel opaque! var temp = NamedColors.Black; for (int i = 0; i < count; i++) @@ -166,6 +168,7 @@ namespace SixLabors.ImageSharp.PixelFormats ref Rgba32 sourceRef = ref MemoryMarshal.GetReference(source); ref TPixel destRef = ref MemoryMarshal.GetReference(destPixels); + // For conversion methods writing only to RGB channels, we need to keep the alpha channel opaque! var temp = NamedColors.Black; for (int i = 0; i < count; i++) @@ -237,6 +240,7 @@ namespace SixLabors.ImageSharp.PixelFormats ref Bgra32 sourceRef = ref MemoryMarshal.GetReference(source); ref TPixel destRef = ref MemoryMarshal.GetReference(destPixels); + // For conversion methods writing only to RGB channels, we need to keep the alpha channel opaque! var temp = NamedColors.Black; for (int i = 0; i < count; i++) @@ -308,6 +312,7 @@ namespace SixLabors.ImageSharp.PixelFormats ref Rgb24 sourceRef = ref MemoryMarshal.GetReference(source); ref TPixel destRef = ref MemoryMarshal.GetReference(destPixels); + // For conversion methods writing only to RGB channels, we need to keep the alpha channel opaque! var temp = NamedColors.Black; for (int i = 0; i < count; i++) @@ -379,6 +384,7 @@ namespace SixLabors.ImageSharp.PixelFormats ref Bgr24 sourceRef = ref MemoryMarshal.GetReference(source); ref TPixel destRef = ref MemoryMarshal.GetReference(destPixels); + // For conversion methods writing only to RGB channels, we need to keep the alpha channel opaque! var temp = NamedColors.Black; for (int i = 0; i < count; i++) @@ -450,6 +456,7 @@ namespace SixLabors.ImageSharp.PixelFormats ref Argb32 sourceRef = ref MemoryMarshal.GetReference(source); ref TPixel destRef = ref MemoryMarshal.GetReference(destPixels); + // For conversion methods writing only to RGB channels, we need to keep the alpha channel opaque! var temp = NamedColors.Black; for (int i = 0; i < count; i++) diff --git a/src/ImageSharp/PixelFormats/Generated/PixelOperations{TPixel}.Generated.tt b/src/ImageSharp/PixelFormats/Generated/PixelOperations{TPixel}.Generated.tt index 0729d02086..5c762c7df1 100644 --- a/src/ImageSharp/PixelFormats/Generated/PixelOperations{TPixel}.Generated.tt +++ b/src/ImageSharp/PixelFormats/Generated/PixelOperations{TPixel}.Generated.tt @@ -28,6 +28,7 @@ ref <#=pixelType#> sourceRef = ref MemoryMarshal.GetReference(source); ref TPixel destRef = ref MemoryMarshal.GetReference(destPixels); + // For conversion methods writing only to RGB channels, we need to keep the alpha channel opaque! var temp = NamedColors<<#=tempPixelType#>>.Black; for (int i = 0; i < count; i++) @@ -94,216 +95,6 @@ <# } - void GeneratePackFromMethodUsingPackFromRgba64(string pixelType, string rgbaOperationCode) - { - #> - - /// - /// Converts 'count' elements in 'source` span of data to a span of -s. - /// - /// The source of data. - /// The to the destination pixels. - /// The number of pixels to convert. - internal virtual void PackFrom<#=pixelType#>(ReadOnlySpan<<#=pixelType#>> source, Span destPixels, int count) - { - GuardSpans(source, nameof(source), destPixels, nameof(destPixels), count); - - ref <#=pixelType#> sourceRef = ref MemoryMarshal.GetReference(source); - ref TPixel destRef = ref MemoryMarshal.GetReference(destPixels); - - var rgba = new Rgba64(0, 0, 0, 65535); - - for (int i = 0; i < count; i++) - { - ref TPixel dp = ref Unsafe.Add(ref destRef, i); - <#=rgbaOperationCode#> - dp.PackFromRgba64(rgba); - } - } - - /// - /// A helper for that expects a byte span. - /// The layout of the data in 'sourceBytes' must be compatible with layout. - /// - /// The to the source bytes. - /// The to the destination pixels. - /// The number of pixels to convert. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal void PackFrom<#=pixelType#>Bytes(ReadOnlySpan sourceBytes, Span destPixels, int count) - { - this.PackFrom<#=pixelType#>(MemoryMarshal.Cast>(sourceBytes), destPixels, count); - } - <# - } - - void GeneratePackFromMethodUsingPackFromRgb48(string pixelType, string rgbaOperationCode) - { - #> - - /// - /// Converts 'count' elements in 'source` span of data to a span of -s. - /// - /// The source of data. - /// The to the destination pixels. - /// The number of pixels to convert. - internal virtual void PackFrom<#=pixelType#>(ReadOnlySpan<<#=pixelType#>> source, Span destPixels, int count) - { - GuardSpans(source, nameof(source), destPixels, nameof(destPixels), count); - - ref <#=pixelType#> sourceRef = ref MemoryMarshal.GetReference(source); - ref TPixel destRef = ref MemoryMarshal.GetReference(destPixels); - - var rgb = default(Rgb48); - - for (int i = 0; i < count; i++) - { - ref TPixel dp = ref Unsafe.Add(ref destRef, i); - <#=rgbaOperationCode#> - dp.PackFromRgb48(rgb); - } - } - - /// - /// A helper for that expects a byte span. - /// The layout of the data in 'sourceBytes' must be compatible with layout. - /// - /// The to the source bytes. - /// The to the destination pixels. - /// The number of pixels to convert. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal void PackFrom<#=pixelType#>Bytes(ReadOnlySpan sourceBytes, Span destPixels, int count) - { - this.PackFrom<#=pixelType#>(MemoryMarshal.Cast>(sourceBytes), destPixels, count); - } - <# - } - - void GeneratePackFromMethodUsingPackFromRgba32(string pixelType, string rgbaOperationCode) - { - #> - - /// - /// Converts 'count' elements in 'source` span of data to a span of -s. - /// - /// The source of data. - /// The to the destination pixels. - /// The number of pixels to convert. - internal virtual void PackFrom<#=pixelType#>(ReadOnlySpan<<#=pixelType#>> source, Span destPixels, int count) - { - GuardSpans(source, nameof(source), destPixels, nameof(destPixels), count); - - ref <#=pixelType#> sourceRef = ref MemoryMarshal.GetReference(source); - ref TPixel destRef = ref MemoryMarshal.GetReference(destPixels); - - var rgba = new Rgba32(0, 0, 0, 255); - - for (int i = 0; i < count; i++) - { - ref TPixel dp = ref Unsafe.Add(ref destRef, i); - <#=rgbaOperationCode#> - dp.PackFromRgba32(rgba); - } - } - - /// - /// A helper for that expects a byte span. - /// The layout of the data in 'sourceBytes' must be compatible with layout. - /// - /// The to the source bytes. - /// The to the destination pixels. - /// The number of pixels to convert. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal void PackFrom<#=pixelType#>Bytes(ReadOnlySpan sourceBytes, Span destPixels, int count) - { - this.PackFrom<#=pixelType#>(MemoryMarshal.Cast>(sourceBytes), destPixels, count); - } - <# - } - - void GeneratePackFromMethodUsingPackFromArgb32(string pixelType, string argbOperationCode) - { - #> - - /// - /// Converts 'count' elements in 'source` span of data to a span of -s. - /// - /// The source of data. - /// The to the destination pixels. - /// The number of pixels to convert. - internal virtual void PackFrom<#=pixelType#>(ReadOnlySpan<<#=pixelType#>> source, Span destPixels, int count) - { - GuardSpans(source, nameof(source), destPixels, nameof(destPixels), count); - - ref <#=pixelType#> sourceRef = ref MemoryMarshal.GetReference(source); - ref TPixel destRef = ref MemoryMarshal.GetReference(destPixels); - - var argb = new Argb32(0, 0, 0, 255); - - for (int i = 0; i < count; i++) - { - ref TPixel dp = ref Unsafe.Add(ref destRef, i); - <#=argbOperationCode#> - dp.PackFromArgb32(argb); - } - } - - /// - /// A helper for that expects a byte span. - /// The layout of the data in 'sourceBytes' must be compatible with layout. - /// - /// The to the source bytes. - /// The to the destination pixels. - /// The number of pixels to convert. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal void PackFrom<#=pixelType#>Bytes(ReadOnlySpan sourceBytes, Span destPixels, int count) - { - this.PackFrom<#=pixelType#>(MemoryMarshal.Cast>(sourceBytes), destPixels, count); - } - <# - } - - void GeneratePackFromMethodUsingPackFromBgra32(string pixelType, string bgraOperationCode) - { - #> - - /// - /// Converts 'count' elements in 'source` span of data to a span of -s. - /// - /// The source of data. - /// The to the destination pixels. - /// The number of pixels to convert. - internal virtual void PackFrom<#=pixelType#>(ReadOnlySpan<<#=pixelType#>> source, Span destPixels, int count) - { - GuardSpans(source, nameof(source), destPixels, nameof(destPixels), count); - - ref <#=pixelType#> sourceRef = ref MemoryMarshal.GetReference(source); - ref TPixel destRef = ref MemoryMarshal.GetReference(destPixels); - - var bgra = new Bgra32(0, 0, 0, 255); - - for (int i = 0; i < count; i++) - { - ref TPixel dp = ref Unsafe.Add(ref destRef, i); - <#=bgraOperationCode#> - dp.PackFromBgra32(bgra); - } - } - - /// - /// A helper for that expects a byte span. - /// The layout of the data in 'sourceBytes' must be compatible with layout. - /// - /// The to the source bytes. - /// The to the destination pixels. - /// The number of pixels to convert. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal void PackFrom<#=pixelType#>Bytes(ReadOnlySpan sourceBytes, Span destPixels, int count) - { - this.PackFrom<#=pixelType#>(MemoryMarshal.Cast>(sourceBytes), destPixels, count); - } - <# - } - #> // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. @@ -318,35 +109,26 @@ namespace SixLabors.ImageSharp.PixelFormats public partial class PixelOperations { <# - - // GeneratePackFromMethodUsingPackFromRgba64("Rgba64", "rgba = Unsafe.Add(ref sourceRef, i);"); - GeneratePackFromMethods("Rgba64", "Rgba64", "temp = Unsafe.Add(ref sourceRef, i);"); GenerateToDestFormatMethods("Rgba64"); - // GeneratePackFromMethodUsingPackFromRgb48("Rgb48", "rgb = Unsafe.Add(ref sourceRef, i);"); GeneratePackFromMethods("Rgb48", "Rgb48", "temp = Unsafe.Add(ref sourceRef, i);"); GenerateToDestFormatMethods("Rgb48"); GeneratePackFromMethods("Rgba32", "Rgba32", "temp = Unsafe.Add(ref sourceRef, i);"); GenerateToDestFormatMethods("Rgba32"); - // GeneratePackFromMethodUsingPackFromBgra32("Bgra32", "bgra = Unsafe.Add(ref sourceRef, i);"); GeneratePackFromMethods("Bgra32", "Bgra32", "temp = Unsafe.Add(ref sourceRef, i);"); GenerateToDestFormatMethods("Bgra32"); - // GeneratePackFromMethodUsingPackFromRgba32("Rgb24", "rgba.Rgb = Unsafe.Add(ref sourceRef, i);"); GeneratePackFromMethods("Rgb24", "Rgba32", "temp.Rgb = Unsafe.Add(ref sourceRef, i);"); GenerateToDestFormatMethods("Rgb24"); - // GeneratePackFromMethodUsingPackFromRgba32("Bgr24", "rgba.Bgr = Unsafe.Add(ref sourceRef, i);"); GeneratePackFromMethods("Bgr24", "Rgba32", "temp.Bgr = Unsafe.Add(ref sourceRef, i);"); GenerateToDestFormatMethods("Bgr24"); - // GeneratePackFromMethodUsingPackFromArgb32("Argb32", "argb = Unsafe.Add(ref sourceRef, i);"); GeneratePackFromMethods("Argb32", "Argb32", "temp = Unsafe.Add(ref sourceRef, i);"); GenerateToDestFormatMethods("Argb32"); - #> }