Merge pull request #3257 from SixLabors/fix/webp-quantizer-sharpening-v4

[V4] Apply luma AC sharpening in the SIMD WebP quantizer

Author: JimBobSquarePants

src/ImageSharp/Formats/Webp/Lossy/QuantEnc.cs

@@ -522,9 +522,12 @@ public static int QuantizeBlock(Span<short> input, Span<short> output, ref Vp8Ma
             // coeff = abs(in)
             Vector256<ushort> coeff0 = Avx2.Abs(input0);
 
-            // coeff = abs(in) + sharpen
+            // Each lane stores the magnitude of one input coefficient.
+            // Add its sharpening bias before quantization, as the scalar path does.
+            // A magnitude is at most 32768. Vp8Matrix.Expand limits each bias to 12.
+            // The sum fits in an unsigned 16-bit lane.
             Vector256<short> sharpen0 = Unsafe.As<short, Vector256<short>>(ref mtx.Sharpen[0]);
-            Avx2.Add(coeff0.AsInt16(), sharpen0);
+            coeff0 = Avx2.Add(coeff0.AsInt16(), sharpen0).AsUInt16();
 
             // out = (coeff * iQ + B) >> QFIX
             // doing calculations with 32b precision (QFIX=17)
@@ -589,11 +592,14 @@ public static int QuantizeBlock(Span<short> input, Span<short> output, ref Vp8Ma
             Vector128<ushort> coeff0 = Ssse3.Abs(input0);
             Vector128<ushort> coeff8 = Ssse3.Abs(input8);
 
-            // coeff = abs(in) + sharpen
+            // coeff0 holds magnitudes for coefficients 0 through 7.
+            // coeff8 holds magnitudes for coefficients 8 through 15.
+            // Add each coefficient's sharpening bias before quantization.
+            // As in AVX2, each sum fits in an unsigned 16-bit lane.
             Vector128<short> sharpen0 = Unsafe.As<short, Vector128<short>>(ref mtx.Sharpen[0]);
             Vector128<short> sharpen8 = Unsafe.As<short, Vector128<short>>(ref mtx.Sharpen[8]);
-            Sse2.Add(coeff0.AsInt16(), sharpen0);
-            Sse2.Add(coeff8.AsInt16(), sharpen8);
+            coeff0 = Sse2.Add(coeff0.AsInt16(), sharpen0).AsUInt16();
+            coeff8 = Sse2.Add(coeff8.AsInt16(), sharpen8).AsUInt16();
 
             // out = (coeff * iQ + B) >> QFIX
             // doing calculations with 32b precision (QFIX=17)

tests/ImageSharp.Tests/Formats/WebP/QuantEncTests.cs

@@ -38,6 +38,34 @@ private static unsafe void RunQuantizeBlockTest()
         Assert.Equal(expectedResult, actualResult);
     }
 
+    /// <summary>
+    /// Checks luma sharpening at quantization boundaries against fixed results from local libwebp.
+    /// </summary>
+    private static void RunQuantizeBlockWithSharpeningTest()
+    {
+        // The AC quantizer step is 200. Its sharpening biases are 2, 5 or 8.
+        // Several magnitudes require that bias to reach the next level.
+        // The input includes positive and negative coefficients.
+        // The expected values come from libwebp's scalar quantizer.
+        // Levels use VP8 zigzag order. Reconstructed coefficients use the input's 4 by 4 order.
+        short[] input = [300, -110, 112, -310, 113, -500, 290, -115, 700, -108, 295, -490, 111, -298, 505, -299];
+        short[] output = new short[16];
+        short[] expectedOutput = [7, 0, 1, 3, -2, 1, -2, 1, -1, 1, -1, 1, -1, -2, 2, -1];
+        short[] expectedReconstructed = [280, 0, 200, -400, 200, -400, 200, -200, 600, -200, 200, -400, 200, -200, 400, -200];
+        Vp8Matrix vp8Matrix = default;
+        vp8Matrix.Q[0] = 40;
+        vp8Matrix.Q[1] = 200;
+        vp8Matrix.Expand(0);
+
+        // act
+        int actualResult = QuantEnc.QuantizeBlock(input, output, ref vp8Matrix);
+
+        // assert
+        Assert.Equal(expectedOutput, output);
+        Assert.Equal(expectedReconstructed, input);
+        Assert.Equal(1, actualResult);
+    }
+
     [Fact]
     public void QuantizeBlock_Works() => RunQuantizeBlockTest();
 
@@ -49,4 +77,24 @@ private static unsafe void RunQuantizeBlockTest()
 
     [Fact]
     public void QuantizeBlock_WithoutAVX2_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunQuantizeBlockTest, HwIntrinsics.DisableAVX2);
+
+    /// <summary>
+    /// Runs the sharpening regression with the available hardware intrinsics.
+    /// </summary>
+    [Fact]
+    public void QuantizeBlock_WithSharpening_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunQuantizeBlockWithSharpeningTest, HwIntrinsics.AllowAll);
+
+    /// <summary>
+    /// Runs the sharpening regression through the scalar fallback.
+    /// </summary>
+    [Fact]
+    public void QuantizeBlock_WithSharpening_WithoutHardwareIntrinsics_Works()
+        => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunQuantizeBlockWithSharpeningTest, HwIntrinsics.DisableHWIntrinsic);
+
+    /// <summary>
+    /// Runs the sharpening regression without AVX2, including the SSE4.1 path on supported x86 hardware.
+    /// </summary>
+    [Fact]
+    public void QuantizeBlock_WithSharpening_WithoutAVX2_Works()
+        => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunQuantizeBlockWithSharpeningTest, HwIntrinsics.DisableAVX2);
 }

tests/ImageSharp.Tests/Formats/WebP/WebpEncoderTests.cs

@@ -1,7 +1,6 @@
 // Copyright (c) Six Labors.
 // Licensed under the Six Labors Split License.
 
-using System.Runtime.InteropServices;
 using SixLabors.ImageSharp.Formats;
 using SixLabors.ImageSharp.Formats.Gif;
 using SixLabors.ImageSharp.Formats.Jpeg;
@@ -549,9 +548,9 @@ public void Encode_Lossy_WithDifferentMethodsAndQuality_Works<TPixel>(TestImageP
     public void Encode_Lossy_WithAlpha_Works<TPixel>(TestImageProvider<TPixel> provider)
         where TPixel : unmanaged, IPixel<TPixel>
     {
-        // Floating point differences result in minor pixel differences affecting compression.
-        // Output have been manually verified.
-        int expectedFileSize = TestEnvironment.OSArchitecture == Architecture.Arm64 ? 64060 : 64020;
+        // All quantizer paths apply the same luma AC sharpening. This stored fixture therefore uses
+        // one size bound for the scalar, SSE4.1 and AVX2 paths.
+        const int expectedFileSize = 64060;
 
         WebpEncoder encoder = new()
         {
@@ -577,9 +576,9 @@ public void Encode_Lossy_WithAlpha_Works<TPixel>(TestImageProvider<TPixel> provi
     public void Encode_Lossy_WithAlphaUsingCompression_Works<TPixel>(TestImageProvider<TPixel> provider)
         where TPixel : unmanaged, IPixel<TPixel>
     {
-        // Floating point differences result in minor pixel differences affecting compression.
-        // Output have been manually verified.
-        int expectedFileSize = TestEnvironment.OSArchitecture == Architecture.Arm64 ? 16240 : 16200;
+        // All quantizer paths apply the same luma AC sharpening. This stored fixture therefore uses
+        // one size bound for the scalar, SSE4.1 and AVX2 paths.
+        const int expectedFileSize = 16240;
 
         WebpEncoder encoder = new()
         {