fixed jpeg quality issue pointed out in conversation under #75

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

@@ -333,18 +333,19 @@ namespace ImageSharp.Formats.Jpg
         /// <param name="dest">Destination block</param>
         /// <param name="qt">Quantization table</param>
         /// <param name="unzigPtr">Pointer to <see cref="UnzigData"/> elements</param>
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        // [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        public static unsafe void UnZigDivRound(Block8x8F* src, Block8x8F* dest, Block8x8F* qt, int* unzigPtr)
+        public static unsafe void UnZigDivRound(Block8x8F* src, int* dest, Block8x8F* qt, int* unzigPtr)
         {
             float* s = (float*)src;
-            float* d = (float*)dest;
             float* q = (float*)qt;
 
             for (int zig = 0; zig < ScalarCount; zig++)
             {
-                float val = s[unzigPtr[zig]] / q[zig];
+                int a = (int)s[unzigPtr[zig]];
-                val = (int)val;
+                int b = (int)q[zig];
-                d[zig] = val;
+
+                int val = DivideRound(a, b);
+                dest[zig] = val;
             }
         }
 
@@ -373,5 +374,22 @@ namespace ImageSharp.Formats.Jpg
                 }
             }
         }
+
+        /// <summary>
+        /// Performs division and rounding of a rational number represented by a dividend and a divisior into an integer.
+        /// </summary>
+        /// <param name="dividend">The dividend</param>
+        /// <param name="divisor">The divisor</param>
+        /// <returns>The result integer</returns>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private static int DivideRound(int dividend, int divisor)
+        {
+            if (dividend >= 0)
+            {
+                return (dividend + (divisor >> 1)) / divisor;
+            }
+
+            return -((-dividend + (divisor >> 1)) / divisor);
+        }
     }
 }

src/ImageSharp.Formats.Jpeg/JpegEncoderCore.cs

@@ -434,7 +434,9 @@ namespace ImageSharp.Formats
             UnzigData unzig = UnzigData.Create();
 
             // ReSharper disable once InconsistentNaming
-            float prevDCY = 0, prevDCCb = 0, prevDCCr = 0;
+            int prevDCY = 0, prevDCCb = 0, prevDCCr = 0;
+
+            int* unzigDest = stackalloc int[Block8x8F.ScalarCount];
 
             using (PixelArea<TColor> rgbBytes = new PixelArea<TColor>(8, 8, ComponentOrder.Xyz))
             {
@@ -449,6 +451,7 @@ namespace ImageSharp.Formats
                             prevDCY,
                             &b,
                             &temp1,
+                            unzigDest,
                             &temp2,
                             &onStackLuminanceQuantTable,
                             unzig.Data);
@@ -457,6 +460,7 @@ namespace ImageSharp.Formats
                             prevDCCb,
                             &cb,
                             &temp1,
+                            unzigDest,
                             &temp2,
                             &onStackChrominanceQuantTable,
                             unzig.Data);
@@ -465,6 +469,7 @@ namespace ImageSharp.Formats
                             prevDCCr,
                             &cr,
                             &temp1,
+                            unzigDest,
                             &temp2,
                             &onStackChrominanceQuantTable,
                             unzig.Data);
@@ -522,31 +527,31 @@ namespace ImageSharp.Formats
         /// <param name="prevDC">The previous DC value.</param>
         /// <param name="src">Source block</param>
         /// <param name="tempDest">Temporal block to be used as FDCT Destination</param>
-        /// <param name="temp2">Temporal block 2</param>
+        /// <param name="d">Working buffer for unzigged stuff</param>
+        /// <param name="tempWorker">Temporal block 2</param>
         /// <param name="quant">Quantization table</param>
         /// <param name="unzigPtr">The 8x8 Unzig block pointer</param>
         /// <returns>
         /// The <see cref="int"/>
         /// </returns>
-        private float WriteBlock(
+        private int WriteBlock(
             QuantIndex index,
-            float prevDC,
+            int prevDC,
             Block8x8F* src,
             Block8x8F* tempDest,
-            Block8x8F* temp2,
+            int* d,
+            Block8x8F* tempWorker,
             Block8x8F* quant,
             int* unzigPtr)
         {
-            DCT.TransformFDCT(ref *src, ref *tempDest, ref *temp2);
+            DCT.TransformFDCT(ref *src, ref *tempDest, ref *tempWorker);
-
-            Block8x8F.UnZigDivRound(tempDest, temp2, quant, unzigPtr);
 
-            float* d = (float*)temp2;
+            Block8x8F.UnZigDivRound(tempDest, d, quant, unzigPtr);
 
             // Emit the DC delta.
-            float dc = d[0];
+            int dc = d[0];
 
-            this.EmitHuffRLE((HuffIndex)((2 * (int)index) + 0), 0, (int)(dc - prevDC));
+            this.EmitHuffRLE((HuffIndex)((2 * (int)index) + 0), 0, dc - prevDC);
 
             // Emit the AC components.
             HuffIndex h = (HuffIndex)((2 * (int)index) + 1);
@@ -554,7 +559,7 @@ namespace ImageSharp.Formats
 
             for (int zig = 1; zig < Block8x8F.ScalarCount; zig++)
             {
-                float ac = d[zig];
+                int ac = d[zig];
 
                 if (ac == 0)
                 {
@@ -568,7 +573,7 @@ namespace ImageSharp.Formats
                         runLength -= 16;
                     }
 
-                    this.EmitHuffRLE(h, runLength, (int)ac);
+                    this.EmitHuffRLE(h, runLength, ac);
                     runLength = 0;
                 }
             }
@@ -802,8 +807,10 @@ namespace ImageSharp.Formats
 
             UnzigData unzig = UnzigData.Create();
 
+            int* unzigDest = stackalloc int[Block8x8F.ScalarCount];
+
             // ReSharper disable once InconsistentNaming
-            float prevDCY = 0, prevDCCb = 0, prevDCCr = 0;
+            int prevDCY = 0, prevDCCb = 0, prevDCCr = 0;
 
             using (PixelArea<TColor> rgbBytes = new PixelArea<TColor>(8, 8, ComponentOrder.Xyz))
             {
@@ -823,6 +830,7 @@ namespace ImageSharp.Formats
                                 prevDCY,
                                 &b,
                                 &temp1,
+                                unzigDest,
                                 &temp2,
                                 &onStackLuminanceQuantTable,
                                 unzig.Data);
@@ -834,6 +842,7 @@ namespace ImageSharp.Formats
                             prevDCCb,
                             &b,
                             &temp1,
+                            unzigDest,
                             &temp2,
                             &onStackChrominanceQuantTable,
                             unzig.Data);
@@ -844,6 +853,7 @@ namespace ImageSharp.Formats
                             prevDCCr,
                             &b,
                             &temp1,
+                            unzigDest,
                             &temp2,
                             &onStackChrominanceQuantTable,
                             unzig.Data);

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

@@ -17,6 +17,7 @@ namespace ImageSharp.Tests
     using System.Numerics;
 
     using ImageSharp.Formats.Jpg;
+    using ImageSharp.Processing;
 
     public class JpegTests : MeasureFixture
     {
@@ -25,6 +26,63 @@ namespace ImageSharp.Tests
         {
         }
 
+        [Theory]
+        [WithFile(TestImages.Jpeg.Snake, PixelTypes.StandardImageClass, 75, JpegSubsample.Ratio420)]
+        [WithFile(TestImages.Jpeg.Lake, PixelTypes.StandardImageClass, 75, JpegSubsample.Ratio420)]
+        [WithFile(TestImages.Jpeg.Snake, PixelTypes.StandardImageClass, 75, JpegSubsample.Ratio444)]
+        [WithFile(TestImages.Jpeg.Lake, PixelTypes.StandardImageClass, 75, JpegSubsample.Ratio444)]
+        public void LoadResizeSave<TColor>(TestImageProvider<TColor> provider, int quality, JpegSubsample subsample)
+            where TColor : struct, IPackedPixel, IEquatable<TColor>
+        {
+            var image = provider.GetImage()
+                .Resize(new ResizeOptions
+                {
+                    Size = new Size(150, 150),
+                    Mode = ResizeMode.Max
+                });
+            image.Quality = 75;
+            JpegEncoder encoder = new JpegEncoder { Subsample = subsample, Quality = quality };
+
+            provider.Utility.TestName += $"{subsample}_Q{quality}";
+            provider.Utility.SaveTestOutputFile(image, "png");
+            provider.Utility.SaveTestOutputFile(image, "jpg", encoder);
+        }
+
+        // Benchmark, enable manually!
+        // [Theory]
+        [InlineData(1, 75, JpegSubsample.Ratio420)]
+        [InlineData(30, 75, JpegSubsample.Ratio420)]
+        [InlineData(30, 75, JpegSubsample.Ratio444)]
+        [InlineData(30, 100, JpegSubsample.Ratio444)]
+        public void Encoder_Benchmark(int executionCount, int quality, JpegSubsample subsample)
+        {
+            string[] testFiles = TestImages.Bmp.All
+                .Concat(new[] { TestImages.Jpeg.Calliphora, TestImages.Jpeg.Cmyk })
+                .ToArray();
+
+            var testImages =
+                testFiles.Select(
+                        tf => TestImageProvider<Color>.File(tf, pixelTypeOverride: PixelTypes.StandardImageClass).GetImage())
+                    .ToArray();
+
+            using (MemoryStream ms = new MemoryStream())
+            {
+                this.Measure(executionCount,
+                    () =>
+                        {
+                            foreach (Image<Color> img in testImages)
+                            {
+                                JpegEncoder encoder = new JpegEncoder() { Quality = quality, Subsample = subsample };
+                                img.Save(ms, encoder);
+                                ms.Seek(0, SeekOrigin.Begin);
+                            }
+                        },
+                    // ReSharper disable once ExplicitCallerInfoArgument
+                    $@"Encode {testFiles.Length} images"
+                    );
+            }
+        }
+
         public static IEnumerable<string> AllJpegFiles => TestImages.Jpeg.All;
 
         [Theory]

tests/ImageSharp.Tests/TestImages.cs

@@ -51,6 +51,9 @@ namespace ImageSharp.Tests
             public const string Festzug = "Jpg/Festzug.jpg";
             public const string Hiyamugi = "Jpg/Hiyamugi.jpg";
 
+            public const string Snake = "Jpg/Snake.jpg";
+            public const string Lake = "Jpg/Lake.jpg";
+
             public const string Jpeg400 = "Jpg/baseline/jpeg400jfif.jpg";
             public const string Jpeg420 = "Jpg/baseline/jpeg420exif.jpg";
             public const string Jpeg422 = "Jpg/baseline/jpeg422jfif.jpg";

tests/ImageSharp.Tests/TestUtilities/ImagingTestCaseUtility.cs

@@ -74,16 +74,19 @@ namespace ImageSharp.Tests
         /// <typeparam name="TColor">The pixel format of the image</typeparam>
         /// <param name="image">The image instance</param>
         /// <param name="extension">The requested extension</param>
-        public void SaveTestOutputFile<TColor>(Image<TColor> image, string extension = null)
+        /// <param name="encoder">Optional encoder</param>
+        public void SaveTestOutputFile<TColor>(Image<TColor> image, string extension = null, IImageEncoder encoder = null)
             where TColor : struct, IPackedPixel, IEquatable<TColor>
         {
             string path = this.GetTestOutputFileName(extension);
 
             var format = GetImageFormatByExtension(extension);
 
+            encoder = encoder ?? format.Encoder;
+
             using (var stream = File.OpenWrite(path))
             {
-                image.Save(stream, format);
+                image.Save(stream, encoder);
             }
         }