From 038f047a1c6839383acb47ed1f4b8f242d53812b Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 8 May 2025 15:39:16 +1000 Subject: [PATCH] Initial fixes based on feedback --- .../Jpeg/Components/Block8x8F.Vector128.cs | 3 ++- .../Jpeg/Components/Block8x8F.Vector256.cs | 2 +- .../Formats/Jpeg/Components/Block8x8F.cs | 24 ++++++++++++++----- .../Formats/Jpg/Block8x8FTests.cs | 2 +- 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Block8x8F.Vector128.cs b/src/ImageSharp/Formats/Jpeg/Components/Block8x8F.Vector128.cs index ffd405714d..3daa476939 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Block8x8F.Vector128.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Block8x8F.Vector128.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using System.Numerics; using System.Runtime.CompilerServices; using System.Runtime.Intrinsics; using SixLabors.ImageSharp.Common.Helpers; @@ -19,8 +20,8 @@ internal partial struct Block8x8F [MethodImpl(InliningOptions.ShortMethod)] public void NormalizeColorsAndRoundInPlaceVector128(float maximum) { - Vector128 off = Vector128.Create(MathF.Ceiling(maximum * 0.5F)); Vector128 max = Vector128.Create(maximum); + Vector128 off = Vector128.Ceiling(max * .5F); this.V0L = NormalizeAndRoundVector128(this.V0L.AsVector128(), off, max).AsVector4(); this.V0R = NormalizeAndRoundVector128(this.V0R.AsVector128(), off, max).AsVector4(); diff --git a/src/ImageSharp/Formats/Jpeg/Components/Block8x8F.Vector256.cs b/src/ImageSharp/Formats/Jpeg/Components/Block8x8F.Vector256.cs index 3aab547e0f..4e4133496a 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Block8x8F.Vector256.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Block8x8F.Vector256.cs @@ -46,8 +46,8 @@ internal partial struct Block8x8F [MethodImpl(InliningOptions.ShortMethod)] public void NormalizeColorsAndRoundInPlaceVector256(float maximum) { - Vector256 off = Vector256.Create(MathF.Ceiling(maximum * 0.5F)); Vector256 max = Vector256.Create(maximum); + Vector256 off = Vector256.Ceiling(max * .5F); this.V256_0 = NormalizeAndRoundVector256(this.V256_0, off, max); this.V256_1 = NormalizeAndRoundVector256(this.V256_1, off, max); diff --git a/src/ImageSharp/Formats/Jpeg/Components/Block8x8F.cs b/src/ImageSharp/Formats/Jpeg/Components/Block8x8F.cs index f7ef44384a..6f9b4fd166 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Block8x8F.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Block8x8F.cs @@ -488,18 +488,30 @@ internal partial struct Block8x8F : IEquatable /// Value to compare to. public bool EqualsToScalar(int value) { - // TODO: Can we provide a Vector128 implementation for this? - if (Avx2.IsSupported) + if (Vector256.IsHardwareAccelerated) { - const int equalityMask = unchecked((int)0b1111_1111_1111_1111_1111_1111_1111_1111); - Vector256 targetVector = Vector256.Create(value); ref Vector256 blockStride = ref this.V256_0; for (nuint i = 0; i < RowCount; i++) { - Vector256 areEqual = Avx2.CompareEqual(Avx.ConvertToVector256Int32WithTruncation(Unsafe.Add(ref this.V256_0, i)), targetVector); - if (Avx2.MoveMask(areEqual.AsByte()) != equalityMask) + if (!Vector256.EqualsAll(Vector256.ConvertToInt32(Unsafe.Add(ref this.V256_0, i)), targetVector)) + { + return false; + } + } + + return true; + } + + if (Vector128.IsHardwareAccelerated) + { + Vector128 targetVector = Vector128.Create(value); + ref Vector4 blockStride = ref this.V0L; + + for (nuint i = 0; i < RowCount * 2; i++) + { + if (!Vector128.EqualsAll(Vector128.ConvertToInt32(Unsafe.Add(ref this.V0L, i).AsVector128()), targetVector)) { return false; } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/Block8x8FTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/Block8x8FTests.cs index 1c5d15dc2e..d1ade761cf 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/Block8x8FTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/Block8x8FTests.cs @@ -462,7 +462,7 @@ public partial class Block8x8FTests : JpegFixture // 3. DisableAvx2 - call fallback code of float implementation FeatureTestRunner.RunWithHwIntrinsicsFeature( RunTest, - HwIntrinsics.AllowAll | HwIntrinsics.DisableAVX2); + HwIntrinsics.AllowAll | HwIntrinsics.DisableAVX | HwIntrinsics.DisableSSE); } [Theory]