From 6f9875d946aa15d9f1a0a4885337d5ad1383826e Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 4 Oct 2018 20:46:08 +0100 Subject: [PATCH 1/5] 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 2/5] 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 3/5] 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 4/5] 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 5/5] 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