Decode components in correct order + cleanup + optimizations.

ColorSpaceGenerator/ColorSpaceGenerator.csproj

@@ -0,0 +1 @@
+  <ItemGroup>

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 <see cref="FastACTables"/> class.
         /// </summary>
         /// <param name="memoryAllocator">The memory allocator used to allocate memory for image processing operations.</param>
-        public FastACTables(MemoryAllocator memoryAllocator)
+        public FastACTables(MemoryAllocator memoryAllocator) => this.tables = memoryAllocator.Allocate2D<short>(512, 4, AllocationOptions.Clean);
-        {
-            this.tables = memoryAllocator.Allocate2D<short>(512, 4, AllocationOptions.Clean);
-        }
 
         /// <summary>
         /// Gets the <see cref="Span{Int16}"/> representing the table at the index in the collection.
         /// </summary>
         /// <param name="index">The table index.</param>
         /// <returns><see cref="Span{Int16}"/></returns>
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        [MethodImpl(InliningOptions.ShortMethod)]
-        public ReadOnlySpan<short> GetTableSpan(int index)
+        public ReadOnlySpan<short> GetTableSpan(int index) => this.tables.GetRowSpan(index);
-        {
-            return this.tables.GetRowSpan(index);
-        }
 
         /// <summary>
-        /// Gets a reference to the first element of the AC table indexed by <see cref="JpegComponent.ACHuffmanTableId"/>       /// </summary>
+        /// Gets a reference to the first element of the AC table indexed by <see cref="JpegComponent.ACHuffmanTableId"/>
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        /// </summary>
-        public ref short GetAcTableReference(JpegComponent component)
+        /// <param name="component">The frame component.</param>
-        {
+        [MethodImpl(InliningOptions.ShortMethod)]
-            return ref this.tables.GetRowSpan(component.ACHuffmanTableId)[0];
+        public ref short GetAcTableReference(JpegComponent component) => ref this.tables.GetRowSpan(component.ACHuffmanTableId)[0];
-        }
 
         /// <summary>
         /// 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));
                         }
                     }
                 }

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;
+                            sizesRef[x++] = i;
-                            x++;
                         }
                     }
 

src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs

@@ -45,6 +45,12 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder
         /// </summary>
         public byte[] ComponentIds { get; set; }
 
+        /// <summary>
+        /// Gets or sets the order in which to process the components
+        /// in interleaved mode.
+        /// </summary>
+        public byte[] ComponentOrder { get; set; }
+
         /// <summary>
         /// Gets or sets the frame component collection
         /// </summary>

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
         /// <param name="dcHuffmanTables">The DC Huffman tables.</param>
         /// <param name="acHuffmanTables">The AC Huffman tables.</param>
         /// <param name="fastACTables">The fast AC decoding tables.</param>
-        /// <param name="componentIndex">The component index within the array.</param>
         /// <param name="componentsLength">The length of the components. Different to the array length.</param>
         /// <param name="restartInterval">The reset interval.</param>
         /// <param name="spectralStart">The spectral selection start.</param>
@@ -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
         }
 
         /// <summary>
-        /// Non-interleaved data, we just need to process one block at a ti
+        /// Non-interleaved data, we just need to process one block at a time in trivial scanline order
-        /// in trivial scanline order
+        /// number of blocks to do just depends on how many actual "pixels" each component has,
-        /// number of blocks to do just depends on how many actual "pixels"
+        /// independent of interleaved MCU blocking and such.
-        /// component has, independent of interleaved MCU blocking and such
         /// </summary>
         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
         /// </summary>
         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];
+            uint mask = Bmask[n];
-            k &= 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])
                 {

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.ComponentIds = new byte[this.ComponentCount];
-                this.Frame.Components = new JpegComponent[this.Frame.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,

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
             };
 
         /// <summary>

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();

tests/Images/External

@@ -1 +1 @@
-Subproject commit 5f3cbd839fbbffae615d294d1dabafdcabc64cf9
+Subproject commit c0627f384c1d3d2f8d914c9578ae31354c35fd2c