diff --git a/src/ImageSharp/Formats/Webp/Lossy/QuantEnc.cs b/src/ImageSharp/Formats/Webp/Lossy/QuantEnc.cs index e726b04b82..704b55f52e 100644 --- a/src/ImageSharp/Formats/Webp/Lossy/QuantEnc.cs +++ b/src/ImageSharp/Formats/Webp/Lossy/QuantEnc.cs @@ -540,8 +540,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) @@ -607,10 +611,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 f92446f28b..88b77e2337 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.Heif; @@ -589,9 +588,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() { @@ -617,9 +616,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() {