fix rounding in YCbCr conversion, started refactoring dequantiziation logic

src/ImageSharp/Formats/Jpeg/Common/Block8x8F.cs

@@ -421,7 +421,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Common
         /// </summary>
         /// <param name="scaleVec">Vector to multiply by</param>
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        public void MultiplyAllInplace(float scaleVec)
+        public void MultiplyInplace(float scaleVec)
         {
             this.V0L *= scaleVec;
             this.V0R *= scaleVec;
@@ -441,6 +441,27 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Common
             this.V7R *= scaleVec;
         }
 
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public void MultiplyInplace(ref Block8x8F other)
+        {
+            this.V0L *= other.V0L;
+            this.V0R *= other.V0R;
+            this.V1L *= other.V1L;
+            this.V1R *= other.V1R;
+            this.V2L *= other.V2L;
+            this.V2R *= other.V2R;
+            this.V3L *= other.V3L;
+            this.V3R *= other.V3R;
+            this.V4L *= other.V4L;
+            this.V4R *= other.V4R;
+            this.V5L *= other.V5L;
+            this.V5R *= other.V5R;
+            this.V6L *= other.V6L;
+            this.V6R *= other.V6R;
+            this.V7L *= other.V7L;
+            this.V7R *= other.V7R;
+        }
+
         /// <summary>
         /// Adds a vector to all elements of the block.
         /// </summary>
@@ -472,16 +493,18 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Common
         /// <param name="blockPtr">Block pointer</param>
         /// <param name="qtPtr">Qt pointer</param>
         /// <param name="unzigPtr">Unzig pointer</param>
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        // [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static unsafe void DequantizeBlock(Block8x8F* blockPtr, Block8x8F* qtPtr, int* unzigPtr)
         {
             float* b = (float*)blockPtr;
             float* qtp = (float*)qtPtr;
-            for (int zig = 0; zig < Size; zig++)
+            for (int qtIndex = 0; qtIndex < Size; qtIndex++)
             {
-                float* unzigPos = b + unzigPtr[zig];
+                int blockIndex = unzigPtr[qtIndex];
+                float* unzigPos = b + blockIndex;
+
                 float val = *unzigPos;
-                val *= qtp[zig];
+                val *= qtp[qtIndex];
                 *unzigPos = val;
             }
         }

src/ImageSharp/Formats/Jpeg/Common/Decoder/JpegBlockPostProcessor.cs

@@ -23,21 +23,24 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Common.Decoder
         /// </summary>
         private DataPointers pointers;
 
+        private Size subSamplingDivisors;
+
         /// <summary>
         /// Initialize the <see cref="JpegBlockPostProcessor"/> instance on the stack.
         /// </summary>
-        /// <param name="postProcessor">The <see cref="JpegBlockPostProcessor"/> instance</param>
-        public static void Init(JpegBlockPostProcessor* postProcessor)
+        public static void Init(JpegBlockPostProcessor* postProcessor, IRawJpegData decoder, IJpegComponent component)
         {
             postProcessor->data = ComputationData.Create();
             postProcessor->pointers = new DataPointers(&postProcessor->data);
+
+            int qtIndex = component.QuantizationTableIndex;
+            postProcessor->data.QuantiazationTable = decoder.QuantizationTables[qtIndex];
+            postProcessor->subSamplingDivisors = component.SubSamplingDivisors;
         }
 
-        public void QuantizeAndTransform(IRawJpegData decoder, IJpegComponent component, ref Block8x8 sourceBlock)
+        public void QuantizeAndTransform(ref Block8x8 sourceBlock)
         {
             this.data.SourceBlock = sourceBlock.AsFloatBlock();
-            int qtIndex = component.QuantizationTableIndex;
-            this.data.QuantiazationTable = decoder.QuantizationTables[qtIndex];
 
             Block8x8F* b = this.pointers.SourceBlock;
 
@@ -47,12 +50,10 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Common.Decoder
         }
 
         public void ProcessBlockColorsInto(
-            IRawJpegData decoder,
-            IJpegComponent component,
             ref Block8x8 sourceBlock,
             BufferArea<float> destArea)
         {
-            this.QuantizeAndTransform(decoder, component, ref sourceBlock);
+            this.QuantizeAndTransform(ref sourceBlock);
 
             this.data.WorkspaceBlock1.NormalizeColorsInplace();
 
@@ -61,8 +62,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Common.Decoder
             // Unfortunately, we need to emulate this to be "more accurate" :(
             this.data.WorkspaceBlock1.RoundInplace();
 
-            Size divs = component.SubSamplingDivisors;
-            this.data.WorkspaceBlock1.CopyTo(destArea, divs.Width, divs.Height);
+            this.data.WorkspaceBlock1.CopyTo(destArea, this.subSamplingDivisors.Width, this.subSamplingDivisors.Height);
         }
 
         /// <summary>

src/ImageSharp/Formats/Jpeg/Common/Decoder/JpegColorConverter.FromYCbCr.cs

@@ -121,9 +121,25 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Common.Decoder
                     tmp.MultiplyInplace(1.772F);
                     b.AddInplace(ref tmp);
 
-                    r.RoundAndDownscale();
-                    g.RoundAndDownscale();
-                    b.RoundAndDownscale();
+                    if (Vector<float>.Count == 4)
+                    {
+                        // TODO: Find a way to properly run & test this path on modern AVX2 PC-s! (Have I already mentioned that Vector<T> is terrible?)
+                        r.RoundAndDownscaleBasic();
+                        g.RoundAndDownscaleBasic();
+                        b.RoundAndDownscaleBasic();
+                    }
+                    else if (Vector<float>.Count == 8)
+                    {
+                        r.RoundAndDownscaleAvx2();
+                        g.RoundAndDownscaleAvx2();
+                        b.RoundAndDownscaleAvx2();
+                    }
+                    else
+                    {
+                        // TODO: Run fallback scalar code here
+                        // However, no issues expected before someone implements this: https://github.com/dotnet/coreclr/issues/12007
+                        throw new NotImplementedException("Your CPU architecture is too modern!");
+                    }
 
                     // Collect (r0,r1...r8) (g0,g1...g8) (b0,b1...b8) vector values in the expected (r0,g0,g1,1), (r1,g1,g2,1) ... order:
                     ref Vector4Octet destination = ref Unsafe.Add(ref resultBase, i);
@@ -145,17 +161,31 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Common.Decoder
                 private static readonly Vector4 Half = new Vector4(0.5f);
 
                 [MethodImpl(MethodImplOptions.AggressiveInlining)]
-                public void RoundAndDownscale()
+                public void RoundAndDownscaleBasic()
                 {
-                    // Emulate rounding:
-                    this.A += Half;
-                    this.B += Half;
+                    ref Vector<float> a = ref Unsafe.As<Vector4, Vector<float>>(ref this.A);
+                    a = a.FastRound();
+
+                    ref Vector<float> b = ref Unsafe.As<Vector4, Vector<float>>(ref this.B);
+                    b = b.FastRound();
 
                     // Downscale by 1/255
                     this.A *= Scale;
                     this.B *= Scale;
                 }
 
+                [MethodImpl(MethodImplOptions.AggressiveInlining)]
+                public void RoundAndDownscaleAvx2()
+                {
+                    ref Vector<float> self = ref Unsafe.As<Vector4Pair, Vector<float>>(ref this);
+                    Vector<float> v = self;
+                    v = v.FastRound();
+
+                    // Downscale by 1/255
+                    v *= new Vector<float>(1 / 255f);
+                    self = v;
+                }
+
                 [MethodImpl(MethodImplOptions.AggressiveInlining)]
                 public void MultiplyInplace(float value)
                 {

src/ImageSharp/Formats/Jpeg/Common/Decoder/JpegComponentPostProcessor.cs

@@ -69,7 +69,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Common.Decoder
         public unsafe void CopyBlocksToColorBuffer()
         {
             var blockPp = default(JpegBlockPostProcessor);
-            JpegBlockPostProcessor.Init(&blockPp);
+            JpegBlockPostProcessor.Init(&blockPp, this.ImagePostProcessor.RawJpeg, this.Component);
 
             for (int y = 0; y < this.BlockRowsPerStep; y++)
             {
@@ -95,7 +95,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Common.Decoder
                         this.blockAreaSize.Width,
                         this.blockAreaSize.Height);
 
-                    blockPp.ProcessBlockColorsInto(this.ImagePostProcessor.RawJpeg, this.Component, ref block, destArea);
+                    blockPp.ProcessBlockColorsInto(ref block, destArea);
                 }
             }
 

src/ImageSharp/Formats/Jpeg/Common/FastFloatingPointDCT.cs

@@ -60,7 +60,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Common
             IDCT8x4_RightPart(ref temp, ref dest);
 
             // TODO: What if we leave the blocks in a scaled-by-x8 state until final color packing?
-            dest.MultiplyAllInplace(C_0_125);
+            dest.MultiplyInplace(C_0_125);
         }
 
         /// <summary>
@@ -334,7 +334,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Common
             FDCT8x4_LeftPart(ref temp, ref dest);
             FDCT8x4_RightPart(ref temp, ref dest);
 
-            dest.MultiplyAllInplace(C_0_125);
+            dest.MultiplyInplace(C_0_125);
         }
     }
 }

src/ImageSharp/Formats/Jpeg/Common/UnzigData.cs

@@ -6,7 +6,6 @@ using System.Runtime.InteropServices;
 namespace SixLabors.ImageSharp.Formats.Jpeg.Common
 {
     /// <summary>
-    /// TODO: This should be simply just a <see cref="Block8x8"/>!
     /// Holds the Jpeg UnZig array in a value/stack type.
     /// Unzig maps from the zigzag ordering to the natural ordering. For example,
     /// unzig[3] is the column and row of the fourth element in zigzag order. The

tests/ImageSharp.Tests/Formats/Jpg/Block8x8FTests.cs

@@ -228,7 +228,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg
             float[] data = Create8x8FloatData();
             var block = new Block8x8F();
             block.LoadFrom(data);
-            block.MultiplyAllInplace(5);
+            block.MultiplyInplace(5);
 
             int stride = 256;
             int height = 42;
@@ -370,5 +370,41 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg
                 Assert.Equal(expected, actual);
             }
         }
+
+        [Fact]
+        public void MultiplyInplace_ByOtherBlock()
+        {
+            Block8x8F original = CreateRandomFloatBlock(-500, 500, 42);
+            Block8x8F m = CreateRandomFloatBlock(-500, 500, 42);
+
+            Block8x8F actual = original;
+            
+            actual.MultiplyInplace(ref m);
+
+            for (int i = 0; i < Block8x8F.Size; i++)
+            {
+                Assert.Equal(original[i]*m[i], actual[i]);
+            }
+        }
+
+        [Theory]
+        [InlineData(1)]
+        [InlineData(2)]
+        [InlineData(3)]
+        public unsafe void DequantizeBlock(int seed)
+        {
+            Block8x8F original = CreateRandomFloatBlock(-500, 500, seed);
+            Block8x8F qt = CreateRandomFloatBlock(0, 10, seed + 42);
+
+            var unzig = UnzigData.Create();
+
+            Block8x8F expected = original;
+            Block8x8F actual = original;
+
+            ReferenceImplementations.DequantizeBlock(&expected, &qt, unzig.Data);
+            Block8x8F.DequantizeBlock(&actual, &qt, unzig.Data);
+
+            this.CompareBlocks(expected, actual, 0);
+        }
     }
 }

tests/ImageSharp.Tests/Formats/Jpg/Utils/ReferenceImplementations.cs

@@ -19,6 +19,21 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg.Utils
     /// </summary>
     internal static partial class ReferenceImplementations
     {
+        public static unsafe void DequantizeBlock(Block8x8F* blockPtr, Block8x8F* qtPtr, int* unzigPtr)
+        {
+            float* b = (float*)blockPtr;
+            float* qtp = (float*)qtPtr;
+            for (int qtIndex = 0; qtIndex < Block8x8F.Size; qtIndex++)
+            {
+                int i = unzigPtr[qtIndex];
+                float* unzigPos = b + i;
+
+                float val = *unzigPos;
+                val *= qtp[qtIndex];
+                *unzigPos = val;
+            }
+        }
+
         /// <summary>
         /// Transpose 8x8 block stored linearly in a <see cref="Span{T}"/> (inplace)
         /// </summary>

tests/ImageSharp.Tests/TestUtilities/ApproximateFloatComparer.cs

@@ -20,7 +20,7 @@ namespace SixLabors.ImageSharp.Tests
         {
             float d = x - y;
 
-            return d > -this.Eps && d < this.Eps;
+            return d >= -this.Eps && d <= this.Eps;
         }
 
         public int GetHashCode(float obj)

tests/ImageSharp.Tests/TestUtilities/ImageComparison/ImageSimilarityReport.cs

@@ -26,9 +26,24 @@
         // TODO: This should not be a nullable value!
         public float? TotalNormalizedDifference { get; }
 
-        public string DifferencePercentageString => this.TotalNormalizedDifference.HasValue
-                                                  ? $"{this.TotalNormalizedDifference.Value * 100:0.0000}%"
-                                                  : "?";
+        public string DifferencePercentageString
+        {
+            get
+            {
+                if (!this.TotalNormalizedDifference.HasValue)
+                {
+                    return "?";
+                }
+                else if (this.TotalNormalizedDifference == 0)
+                {
+                    return "0%";
+                }
+                else
+                {
+                    return $"{this.TotalNormalizedDifference.Value * 100:0.0000}%";
+                }
+            }
+        }
 
         public PixelDifference[] Differences { get; }
 

tests/ImageSharp.Tests/TestUtilities/ImageProviders/FileProvider.cs

@@ -109,11 +109,10 @@ namespace SixLabors.ImageSharp.Tests
             {
                 this.FilePath = filePath;
             }
-
-            public FileProvider()
-            {
-            }
-
+            
+            /// <summary>
+            /// Gets the file path relative to the "~/tests/images" folder
+            /// </summary>
             public string FilePath { get; private set; }
 
             public override string SourceFileOrDescription => this.FilePath;