From 6c5eb7ffe09fad0e436857f212f896d858180340 Mon Sep 17 00:00:00 2001 From: Jason Finch Date: Sun, 18 Oct 2026 09:26:15 +1000 Subject: [PATCH 1/2] Apply the luma AC sharpening in the vectorized WebP quantizer The AVX2 and SSE4.1 paths of QuantEnc.QuantizeBlock computed abs(in) + sharpen but discarded the sum, so they quantized without the luma AC sharpening bias that the scalar path applies. Lossy output therefore depended on the CPU: x64 machines with SSE4.1 produced a different bitstream from the scalar path used on ARM64 or with hardware intrinsics disabled. Assign the sums back so every path quantizes the same value. Lossy output is now byte-identical with AVX2, with SSE4.1 only and without hardware intrinsics, and the lossy-with-alpha size bounds no longer need a separate value for ARM64. --- src/ImageSharp/Formats/Webp/Lossy/QuantEnc.cs | 11 +++++-- .../Formats/WebP/QuantEncTests.cs | 33 +++++++++++++++++++ .../Formats/WebP/WebpEncoderTests.cs | 13 ++++---- 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/src/ImageSharp/Formats/Webp/Lossy/QuantEnc.cs b/src/ImageSharp/Formats/Webp/Lossy/QuantEnc.cs index 7548d5d7ea..efcd3f956c 100644 --- a/src/ImageSharp/Formats/Webp/Lossy/QuantEnc.cs +++ b/src/ImageSharp/Formats/Webp/Lossy/QuantEnc.cs @@ -523,8 +523,12 @@ public static int QuantizeBlock(Span input, Span output, ref Vp8Ma Vector256 coeff0 = Avx2.Abs(input0); // coeff = abs(in) + sharpen + // The sum must be assigned back: the scalar path quantizes abs(in) + sharpen, so the vector path + // has to as well or it skips the luma AC sharpening and picks different levels. Transform outputs + // stay below 2^14 and sharpen is at most (90 * 284) >> 11 = 12, so the 16-bit lane sum cannot + // wrap and matches the scalar uint value. Vector256 sharpen0 = Unsafe.As>(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) @@ -590,10 +594,11 @@ public static int QuantizeBlock(Span input, Span output, ref Vp8Ma Vector128 coeff8 = Ssse3.Abs(input8); // coeff = abs(in) + sharpen + // As in the AVX2 path, the sums must be assigned back so sharpening matches the scalar path. Vector128 sharpen0 = Unsafe.As>(ref mtx.Sharpen[0]); Vector128 sharpen8 = Unsafe.As>(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) diff --git a/tests/ImageSharp.Tests/Formats/WebP/QuantEncTests.cs b/tests/ImageSharp.Tests/Formats/WebP/QuantEncTests.cs index 3808ba6d03..b036cd9837 100644 --- a/tests/ImageSharp.Tests/Formats/WebP/QuantEncTests.cs +++ b/tests/ImageSharp.Tests/Formats/WebP/QuantEncTests.cs @@ -38,6 +38,30 @@ private static unsafe void RunQuantizeBlockTest() Assert.Equal(expectedResult, actualResult); } + private static void RunQuantizeBlockWithSharpeningTest() + { + // arrange + // A luma AC matrix (type 0) gets non-zero sharpening for every AC coefficient. With Q = 200 the + // sharpen values are 2, 5 or 8, and the inputs sit just below a level boundary, so several levels + // are only reached when the sharpening is added before quantizing. + 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[] expectedInput = [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(expectedInput, input); + Assert.Equal(1, actualResult); + } + [Fact] public void QuantizeBlock_Works() => RunQuantizeBlockTest(); @@ -49,4 +73,13 @@ private static unsafe void RunQuantizeBlockTest() [Fact] public void QuantizeBlock_WithoutAVX2_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunQuantizeBlockTest, HwIntrinsics.DisableAVX2); + + [Fact] + public void QuantizeBlock_WithSharpening_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunQuantizeBlockWithSharpeningTest, HwIntrinsics.AllowAll); + + [Fact] + public void QuantizeBlock_WithSharpening_WithoutSSE2_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunQuantizeBlockWithSharpeningTest, HwIntrinsics.DisableHWIntrinsic); + + [Fact] + public void QuantizeBlock_WithSharpening_WithoutAVX2_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunQuantizeBlockWithSharpeningTest, HwIntrinsics.DisableAVX2); } diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpEncoderTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpEncoderTests.cs index 73bb1457da..33917df0e8 100644 --- a/tests/ImageSharp.Tests/Formats/WebP/WebpEncoderTests.cs +++ b/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(TestImageP public void Encode_Lossy_WithAlpha_Works(TestImageProvider provider) where TPixel : unmanaged, IPixel { - // Floating point differences result in minor pixel differences affecting compression. - // Output have been manually verified. - int expectedFileSize = TestEnvironment.OSArchitecture == Architecture.Arm64 ? 64060 : 64020; + // The lossy encoder produces the same bitstream with and without hardware intrinsics, + // so the expected size is the same on every architecture. + const int expectedFileSize = 64060; WebpEncoder encoder = new() { @@ -577,9 +576,9 @@ public void Encode_Lossy_WithAlpha_Works(TestImageProvider provi public void Encode_Lossy_WithAlphaUsingCompression_Works(TestImageProvider provider) where TPixel : unmanaged, IPixel { - // Floating point differences result in minor pixel differences affecting compression. - // Output have been manually verified. - int expectedFileSize = TestEnvironment.OSArchitecture == Architecture.Arm64 ? 16240 : 16200; + // The lossy encoder produces the same bitstream with and without hardware intrinsics, + // so the expected size is the same on every architecture. + const int expectedFileSize = 16240; WebpEncoder encoder = new() { From 56c7aa7563a7a1534c9035bf418706bc9be8d060 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 27 Oct 2026 16:42:17 +1000 Subject: [PATCH 2/2] Clarify WebP sharpening comments and native regression expectations --- src/ImageSharp/Formats/Webp/Lossy/QuantEnc.cs | 15 ++++----- .../Formats/WebP/QuantEncTests.cs | 31 ++++++++++++++----- .../Formats/WebP/WebpEncoderTests.cs | 8 ++--- 3 files changed, 35 insertions(+), 19 deletions(-) diff --git a/src/ImageSharp/Formats/Webp/Lossy/QuantEnc.cs b/src/ImageSharp/Formats/Webp/Lossy/QuantEnc.cs index efcd3f956c..1fda9bf977 100644 --- a/src/ImageSharp/Formats/Webp/Lossy/QuantEnc.cs +++ b/src/ImageSharp/Formats/Webp/Lossy/QuantEnc.cs @@ -522,11 +522,10 @@ public static int QuantizeBlock(Span input, Span output, ref Vp8Ma // coeff = abs(in) Vector256 coeff0 = Avx2.Abs(input0); - // coeff = abs(in) + sharpen - // The sum must be assigned back: the scalar path quantizes abs(in) + sharpen, so the vector path - // has to as well or it skips the luma AC sharpening and picks different levels. Transform outputs - // stay below 2^14 and sharpen is at most (90 * 284) >> 11 = 12, so the 16-bit lane sum cannot - // wrap and matches the scalar uint value. + // 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 sharpen0 = Unsafe.As>(ref mtx.Sharpen[0]); coeff0 = Avx2.Add(coeff0.AsInt16(), sharpen0).AsUInt16(); @@ -593,8 +592,10 @@ public static int QuantizeBlock(Span input, Span output, ref Vp8Ma Vector128 coeff0 = Ssse3.Abs(input0); Vector128 coeff8 = Ssse3.Abs(input8); - // coeff = abs(in) + sharpen - // As in the AVX2 path, the sums must be assigned back so sharpening matches the scalar path. + // 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 sharpen0 = Unsafe.As>(ref mtx.Sharpen[0]); Vector128 sharpen8 = Unsafe.As>(ref mtx.Sharpen[8]); coeff0 = Sse2.Add(coeff0.AsInt16(), sharpen0).AsUInt16(); diff --git a/tests/ImageSharp.Tests/Formats/WebP/QuantEncTests.cs b/tests/ImageSharp.Tests/Formats/WebP/QuantEncTests.cs index b036cd9837..1ca07bc0ac 100644 --- a/tests/ImageSharp.Tests/Formats/WebP/QuantEncTests.cs +++ b/tests/ImageSharp.Tests/Formats/WebP/QuantEncTests.cs @@ -38,16 +38,20 @@ private static unsafe void RunQuantizeBlockTest() Assert.Equal(expectedResult, actualResult); } + /// + /// Checks luma sharpening at quantization boundaries against fixed results from local libwebp. + /// private static void RunQuantizeBlockWithSharpeningTest() { - // arrange - // A luma AC matrix (type 0) gets non-zero sharpening for every AC coefficient. With Q = 200 the - // sharpen values are 2, 5 or 8, and the inputs sit just below a level boundary, so several levels - // are only reached when the sharpening is added before quantizing. + // 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[] expectedInput = [280, 0, 200, -400, 200, -400, 200, -200, 600, -200, 200, -400, 200, -200, 400, -200]; + 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; @@ -58,7 +62,7 @@ private static void RunQuantizeBlockWithSharpeningTest() // assert Assert.Equal(expectedOutput, output); - Assert.Equal(expectedInput, input); + Assert.Equal(expectedReconstructed, input); Assert.Equal(1, actualResult); } @@ -74,12 +78,23 @@ private static void RunQuantizeBlockWithSharpeningTest() [Fact] public void QuantizeBlock_WithoutAVX2_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunQuantizeBlockTest, HwIntrinsics.DisableAVX2); + /// + /// Runs the sharpening regression with the available hardware intrinsics. + /// [Fact] public void QuantizeBlock_WithSharpening_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunQuantizeBlockWithSharpeningTest, HwIntrinsics.AllowAll); + /// + /// Runs the sharpening regression through the scalar fallback. + /// [Fact] - public void QuantizeBlock_WithSharpening_WithoutSSE2_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunQuantizeBlockWithSharpeningTest, HwIntrinsics.DisableHWIntrinsic); + public void QuantizeBlock_WithSharpening_WithoutHardwareIntrinsics_Works() + => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunQuantizeBlockWithSharpeningTest, HwIntrinsics.DisableHWIntrinsic); + /// + /// Runs the sharpening regression without AVX2, including the SSE4.1 path on supported x86 hardware. + /// [Fact] - public void QuantizeBlock_WithSharpening_WithoutAVX2_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunQuantizeBlockWithSharpeningTest, HwIntrinsics.DisableAVX2); + public void QuantizeBlock_WithSharpening_WithoutAVX2_Works() + => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunQuantizeBlockWithSharpeningTest, HwIntrinsics.DisableAVX2); } diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpEncoderTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpEncoderTests.cs index 33917df0e8..10c3902e4e 100644 --- a/tests/ImageSharp.Tests/Formats/WebP/WebpEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/WebP/WebpEncoderTests.cs @@ -548,8 +548,8 @@ public void Encode_Lossy_WithDifferentMethodsAndQuality_Works(TestImageP public void Encode_Lossy_WithAlpha_Works(TestImageProvider provider) where TPixel : unmanaged, IPixel { - // The lossy encoder produces the same bitstream with and without hardware intrinsics, - // so the expected size is the same on every architecture. + // 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() @@ -576,8 +576,8 @@ public void Encode_Lossy_WithAlpha_Works(TestImageProvider provi public void Encode_Lossy_WithAlphaUsingCompression_Works(TestImageProvider provider) where TPixel : unmanaged, IPixel { - // The lossy encoder produces the same bitstream with and without hardware intrinsics, - // so the expected size is the same on every architecture. + // 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()