From 664d838291b5fc0aa3b975c7749023c69ebc8258 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Sat, 20 Oct 2018 19:47:08 +0200 Subject: [PATCH] fix accuracy issues --- .../Helpers/SimdUtils.ExtendedIntrinsics.cs | 20 +++--- .../PixelFormats/Rgba32.PixelOperations.cs | 57 +++++++++++---- .../ImageSharp.Tests/Common/SimdUtilsTests.cs | 70 +++++++++---------- .../TestUtilities/TestDataGenerator.cs | 9 +-- 4 files changed, 91 insertions(+), 65 deletions(-) diff --git a/src/ImageSharp/Common/Helpers/SimdUtils.ExtendedIntrinsics.cs b/src/ImageSharp/Common/Helpers/SimdUtils.ExtendedIntrinsics.cs index 3131f1873..ec91e5098 100644 --- a/src/ImageSharp/Common/Helpers/SimdUtils.ExtendedIntrinsics.cs +++ b/src/ImageSharp/Common/Helpers/SimdUtils.ExtendedIntrinsics.cs @@ -85,7 +85,7 @@ namespace SixLabors.ImageSharp { Guard.IsTrue( dest.Length % Vector.Count == 0, - nameof(source), + nameof(dest), "dest.Length should be divisable by Vector.Count!"); int n = dest.Length / Vector.Count; @@ -93,8 +93,6 @@ namespace SixLabors.ImageSharp ref Vector sourceBase = ref Unsafe.As>(ref MemoryMarshal.GetReference(source)); ref Vector destBase = ref Unsafe.As>(ref MemoryMarshal.GetReference(dest)); - Vector scale = new Vector(255); - for (int i = 0; i < n; i++) { ref Vector s = ref Unsafe.Add(ref sourceBase, i * 4); @@ -104,10 +102,10 @@ namespace SixLabors.ImageSharp Vector f2 = Unsafe.Add(ref s, 2); Vector f3 = Unsafe.Add(ref s, 3); - Vector w0 = ConvertToUInt32(f0, scale); - Vector w1 = ConvertToUInt32(f1, scale); - Vector w2 = ConvertToUInt32(f2, scale); - Vector w3 = ConvertToUInt32(f3, scale); + Vector w0 = ConvertToUInt32(f0); + Vector w1 = ConvertToUInt32(f1); + Vector w2 = ConvertToUInt32(f2); + Vector w3 = ConvertToUInt32(f3); Vector u0 = Vector.Narrow(w0, w1); Vector u1 = Vector.Narrow(w2, w3); @@ -119,10 +117,12 @@ namespace SixLabors.ImageSharp } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static Vector ConvertToUInt32(Vector vf, Vector scale) + private static Vector ConvertToUInt32(Vector vf) { - vf = Vector.Min(Vector.Max(vf, Vector.Zero), Vector.One); - vf *= scale; + Vector maxBytes = new Vector(255f); + vf *= maxBytes; + vf += new Vector(0.5f); + vf = Vector.Min(Vector.Max(vf, Vector.Zero), maxBytes); Vector vi = Vector.ConvertToInt32(vf); return Vector.AsVectorUInt32(vi); } diff --git a/src/ImageSharp/PixelFormats/Rgba32.PixelOperations.cs b/src/ImageSharp/PixelFormats/Rgba32.PixelOperations.cs index 0b96a599b..bfef60c60 100644 --- a/src/ImageSharp/PixelFormats/Rgba32.PixelOperations.cs +++ b/src/ImageSharp/PixelFormats/Rgba32.PixelOperations.cs @@ -52,22 +52,13 @@ namespace SixLabors.ImageSharp.PixelFormats return; } - int remainder = count % 2; - int alignedCount = count - remainder; - - if (alignedCount > 0) + if (SimdUtils.ExtendedIntrinsics.IsAvailable) { - ReadOnlySpan rawSrc = MemoryMarshal.Cast(sourceVectors.Slice(0, alignedCount)); - Span rawDest = MemoryMarshal.Cast(destinationColors); - - SimdUtils.BulkConvertNormalizedFloatToByteClampOverflows(rawSrc, rawDest); + ConvertFromVector4ExtendedIntrinsics(sourceVectors, destinationColors, count); } - - if (remainder > 0) + else { - // actually: remainder == 1 - int lastIdx = count - 1; - destinationColors[lastIdx].PackFromVector4(sourceVectors[lastIdx]); + ConvertFromVector4StandardIntrinsics(sourceVectors, destinationColors, count); } } @@ -144,6 +135,46 @@ namespace SixLabors.ImageSharp.PixelFormats destinationVectors[lastIdx] = sourceColors[lastIdx].ToVector4(); } } + + private static void ConvertFromVector4ExtendedIntrinsics(ReadOnlySpan sourceVectors, Span destinationColors, int count) + { + int remainder = count % 8; + int alignedCount = count - remainder; + + if (alignedCount > 0) + { + ReadOnlySpan rawSrc = MemoryMarshal.Cast(sourceVectors); + Span rawDest = MemoryMarshal.Cast(destinationColors.Slice(0, alignedCount)); + + SimdUtils.ExtendedIntrinsics.BulkConvertNormalizedFloatToByteClampOverflows(rawSrc, rawDest); + } + + if (remainder > 0) + { + PackFromVector4Common(sourceVectors.Slice(alignedCount), destinationColors.Slice(alignedCount), remainder); + } + } + + private static void ConvertFromVector4StandardIntrinsics(ReadOnlySpan sourceVectors, Span destinationColors, int count) + { + int remainder = count % 2; + int alignedCount = count - remainder; + + if (alignedCount > 0) + { + ReadOnlySpan rawSrc = MemoryMarshal.Cast(sourceVectors.Slice(0, alignedCount)); + Span rawDest = MemoryMarshal.Cast(destinationColors); + + SimdUtils.BulkConvertNormalizedFloatToByteClampOverflows(rawSrc, rawDest); + } + + if (remainder > 0) + { + // actually: remainder == 1 + int lastIdx = count - 1; + destinationColors[lastIdx].PackFromVector4(sourceVectors[lastIdx]); + } + } } } } \ No newline at end of file diff --git a/tests/ImageSharp.Tests/Common/SimdUtilsTests.cs b/tests/ImageSharp.Tests/Common/SimdUtilsTests.cs index 7ed18ef86..4e1717bda 100644 --- a/tests/ImageSharp.Tests/Common/SimdUtilsTests.cs +++ b/tests/ImageSharp.Tests/Common/SimdUtilsTests.cs @@ -160,31 +160,6 @@ namespace SixLabors.ImageSharp.Tests.Common Assert.Equal(expected, dest); } - private static float Clamp255(float x) => Math.Min(255f, Math.Max(0f, x)); - - [Theory] - [InlineData(1, 0)] - [InlineData(1, 8)] - [InlineData(2, 16)] - [InlineData(3, 128)] - public void BulkConvertNormalizedFloatToByteClampOverflows(int seed, int count) - { - if (this.SkipOnNonAvx2()) - { - return; - } - - float[] orig = new Random(seed).GenerateRandomRoundedFloatArray(count, -50, 444); - float[] normalized = orig.Select(f => f / 255f).ToArray(); - - byte[] dest = new byte[count]; - - SimdUtils.BulkConvertNormalizedFloatToByteClampOverflows(normalized, dest); - - byte[] expected = orig.Select(f => (byte)Clamp255(f)).ToArray(); - - Assert.Equal(expected, dest); - } [Theory] [InlineData(1, 0)] @@ -222,23 +197,44 @@ namespace SixLabors.ImageSharp.Tests.Common Assert.Equal(expected, result, new ApproximateFloatComparer(1e-5f)); } + + public static readonly TheoryData BulkConvertNormalizedFloatToByteClampOverflows_Data = + new TheoryData + { + 0, 64, 1024 + }; + [Theory] - [InlineData(1, 0)] - [InlineData(2, 32)] - [InlineData(3, 128)] - public void ExtendedIntrinsics_BulkConvertNormalizedFloatToByteClampOverflows(int seed, int count) + [MemberData(nameof(BulkConvertNormalizedFloatToByteClampOverflows_Data))] + public void BulkConvertNormalizedFloatToByteClampOverflows(int count) { - float[] orig = new Random(seed).GenerateRandomRoundedFloatArray(count, -50, 444); - float[] normalized = orig.Select(f => f / 255f).ToArray(); + if (this.SkipOnNonAvx2()) + { + return; + } - byte[] dest = new byte[count]; + float[] source = new Random(count).GenerateRandomFloatArray(count, -0.1f, 1.2f); + byte[] expected = source.Select(NormalizedFloatToByte).ToArray(); + byte[] actual = new byte[count]; - SimdUtils.ExtendedIntrinsics.BulkConvertNormalizedFloatToByteClampOverflows(normalized, dest); + SimdUtils.BulkConvertNormalizedFloatToByteClampOverflows(source, actual); - byte[] expected = orig.Select(f => (byte)Clamp255(f)).ToArray(); + Assert.Equal(expected, actual); + } - Assert.Equal(expected, dest); + [Theory] + [MemberData(nameof(BulkConvertNormalizedFloatToByteClampOverflows_Data))] + public void ExtendedIntrinsics_BulkConvertNormalizedFloatToByteClampOverflows(int count) + { + float[] source = new Random(count).GenerateRandomFloatArray(count, -0.1f, 1.2f); + byte[] expected = source.Select(NormalizedFloatToByte).ToArray(); + byte[] actual = new byte[count]; + + SimdUtils.ExtendedIntrinsics.BulkConvertNormalizedFloatToByteClampOverflows(source, actual); + + Assert.Equal(expected, actual); } + private static byte NormalizedFloatToByte(float f) => (byte)Math.Min(255f, Math.Max(0f, f * 255f + 0.5f)); [Theory] [InlineData(0)] @@ -265,7 +261,7 @@ namespace SixLabors.ImageSharp.Tests.Common float[] source = { 0, 7, 42, 255, 0.5f, 1.1f, 2.6f, 16f }; - var expected = source.Select(f => (byte)Math.Round(f)).ToArray(); + byte[] expected = source.Select(f => (byte)Math.Round(f)).ToArray(); source = source.Select(f => f / 255f).ToArray(); @@ -299,8 +295,6 @@ namespace SixLabors.ImageSharp.Tests.Common iiRef = x; - //Tuple8.OfUInt32 ii = Unsafe.As, Tuple8.OfUInt32>(ref x); - ref Tuple8.OfByte d = ref MemoryMarshal.Cast(dest)[0]; d.LoadFrom(ref ii); diff --git a/tests/ImageSharp.Tests/TestUtilities/TestDataGenerator.cs b/tests/ImageSharp.Tests/TestUtilities/TestDataGenerator.cs index 6f3b18e1f..912b86e34 100644 --- a/tests/ImageSharp.Tests/TestUtilities/TestDataGenerator.cs +++ b/tests/ImageSharp.Tests/TestUtilities/TestDataGenerator.cs @@ -33,19 +33,20 @@ namespace SixLabors.ImageSharp.Tests return values; } - public static float[] GenerateRandomRoundedFloatArray(this Random rnd, int length, int minVal, int maxValExclusive) + public static float[] GenerateRandomRoundedFloatArray(this Random rnd, int length, float minVal, float maxVal) { float[] values = new float[length]; for (int i = 0; i < length; i++) { - int val = rnd.Next(minVal, maxValExclusive); - values[i] = (float)val; + values[i] = (float) Math.Round(rnd.GetRandomFloat(minVal, maxVal)); } return values; } + + public static byte[] GenerateRandomByteArray(this Random rnd, int length) { byte[] values = new byte[length]; @@ -53,7 +54,7 @@ namespace SixLabors.ImageSharp.Tests return values; } - private static float GetRandomFloat(Random rnd, float minVal, float maxVal) + private static float GetRandomFloat(this Random rnd, float minVal, float maxVal) { return (float)rnd.NextDouble() * (maxVal - minVal) + minVal; }