From dbd75c2d42493b7a5151823c376d63471f5b6d2f Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Sat, 28 Apr 2018 02:08:39 +0200 Subject: [PATCH] workaround Vector2 CLR bug + cover taper parameters with tests --- .../ProjectiveTransformProcessor.cs | 24 ++++++--- .../Transforms/ProjectiveTransformHelper.cs | 8 +-- .../Transforms/ProjectiveTransformTests.cs | 54 ++++++++++++++++++- 3 files changed, 74 insertions(+), 12 deletions(-) diff --git a/src/ImageSharp/Processing/Transforms/Processors/ProjectiveTransformProcessor.cs b/src/ImageSharp/Processing/Transforms/Processors/ProjectiveTransformProcessor.cs index 2ca1f2ef79..9f76540378 100644 --- a/src/ImageSharp/Processing/Transforms/Processors/ProjectiveTransformProcessor.cs +++ b/src/ImageSharp/Processing/Transforms/Processors/ProjectiveTransformProcessor.cs @@ -87,10 +87,14 @@ namespace SixLabors.ImageSharp.Processing.Transforms.Processors for (int x = 0; x < width; x++) { var v3 = Vector3.Transform(new Vector3(x, y, 1), matrix); - var point = Point.Round(new Vector2(v3.X, v3.Y) / MathF.Max(v3.Z, Epsilon)); - if (sourceBounds.Contains(point.X, point.Y)) + + float z = MathF.Max(v3.Z, Epsilon); + int px = (int)MathF.Round(v3.X / z); + int py = (int)MathF.Round(v3.Y / z); + + if (sourceBounds.Contains(px, py)) { - destRow[x] = source[point.X, point.Y]; + destRow[x] = source[px, py]; } } }); @@ -104,7 +108,10 @@ namespace SixLabors.ImageSharp.Processing.Transforms.Processors (float radius, float scale, float ratio) yRadiusScale = this.GetSamplingRadius(source.Height, destination.Height); float xScale = xRadiusScale.scale; float yScale = yRadiusScale.scale; - var radius = new Vector2(xRadiusScale.radius, yRadiusScale.radius); + + // Using Vector4 with dummy 0-s, because Vector2 SIMD implementation is not reliable: + var radius = new Vector4(xRadiusScale.radius, yRadiusScale.radius, 0, 0); + IResampler sampler = this.Sampler; var maxSource = new Vector4(maxSourceX, maxSourceY, maxSourceX, maxSourceY); int xLength = (int)MathF.Ceiling((radius.X * 2) + 2); @@ -130,11 +137,14 @@ namespace SixLabors.ImageSharp.Processing.Transforms.Processors // Use the single precision position to calculate correct bounding pixels // otherwise we get rogue pixels outside of the bounds. var v3 = Vector3.Transform(new Vector3(x, y, 1), matrix); - Vector2 point = new Vector2(v3.X, v3.Y) / MathF.Max(v3.Z, Epsilon); + float z = MathF.Max(v3.Z, Epsilon); + + // Using Vector4 with dummy 0-s, because Vector2 SIMD implementation is not reliable: + Vector4 point = new Vector4(v3.X, v3.Y, 0, 0) / z; // Clamp sampling pixel radial extents to the source image edges - Vector2 maxXY = point + radius; - Vector2 minXY = point - radius; + Vector4 maxXY = point + radius; + Vector4 minXY = point - radius; // max, maxY, minX, minY var extents = new Vector4( diff --git a/src/ImageSharp/Processing/Transforms/ProjectiveTransformHelper.cs b/src/ImageSharp/Processing/Transforms/ProjectiveTransformHelper.cs index dfdfe5f559..7c79776d9d 100644 --- a/src/ImageSharp/Processing/Transforms/ProjectiveTransformHelper.cs +++ b/src/ImageSharp/Processing/Transforms/ProjectiveTransformHelper.cs @@ -89,7 +89,7 @@ namespace SixLabors.ImageSharp.Processing.Transforms break; case TaperCorner.Both: - matrix.M12 = (size.Height / 2) * matrix.M13; + matrix.M12 = (size.Height * 0.5f) * matrix.M13; matrix.M32 = size.Height * (1 - taperFraction) / 2; break; } @@ -112,7 +112,7 @@ namespace SixLabors.ImageSharp.Processing.Transforms break; case TaperCorner.Both: - matrix.M21 = (size.Width / 2) * matrix.M23; + matrix.M21 = (size.Width * 0.5f) * matrix.M23; matrix.M31 = size.Width * (1 - taperFraction) / 2; break; } @@ -133,7 +133,7 @@ namespace SixLabors.ImageSharp.Processing.Transforms break; case TaperCorner.Both: - matrix.M12 = (size.Height / 2) * matrix.M13; + matrix.M12 = (size.Height * 0.5f) * matrix.M13; break; } @@ -153,7 +153,7 @@ namespace SixLabors.ImageSharp.Processing.Transforms break; case TaperCorner.Both: - matrix.M21 = (size.Width / 2) * matrix.M23; + matrix.M21 = (size.Width * 0.5f) * matrix.M23; break; } diff --git a/tests/ImageSharp.Tests/Processing/Transforms/ProjectiveTransformTests.cs b/tests/ImageSharp.Tests/Processing/Transforms/ProjectiveTransformTests.cs index c7b49fb926..5cb9b80937 100644 --- a/tests/ImageSharp.Tests/Processing/Transforms/ProjectiveTransformTests.cs +++ b/tests/ImageSharp.Tests/Processing/Transforms/ProjectiveTransformTests.cs @@ -13,9 +13,13 @@ using Xunit; namespace SixLabors.ImageSharp.Tests.Processing.Transforms { + using Xunit.Abstractions; + public class ProjectiveTransformTests { - private static readonly ImageComparer ValidatorComparer = ImageComparer.TolerantPercentage(0.005f, 3); + private static readonly ImageComparer ValidatorComparer = ImageComparer.TolerantPercentage(0.05f, 3); + + private ITestOutputHelper Output { get; } public static readonly TheoryData ResamplerNames = new TheoryData { @@ -36,6 +40,32 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms nameof(KnownResamplers.Welch), }; + public static readonly TheoryData TaperMatrixData = + new TheoryData + { + { TaperSide.Bottom, TaperCorner.Both }, + { TaperSide.Bottom, TaperCorner.LeftOrTop }, + { TaperSide.Bottom, TaperCorner.RightOrBottom }, + + { TaperSide.Top, TaperCorner.Both }, + { TaperSide.Top, TaperCorner.LeftOrTop }, + { TaperSide.Top, TaperCorner.RightOrBottom }, + + { TaperSide.Left, TaperCorner.Both }, + { TaperSide.Left, TaperCorner.LeftOrTop }, + { TaperSide.Left, TaperCorner.RightOrBottom }, + + { TaperSide.Right, TaperCorner.Both }, + { TaperSide.Right, TaperCorner.LeftOrTop }, + { TaperSide.Right, TaperCorner.RightOrBottom }, + + }; + + public ProjectiveTransformTests(ITestOutputHelper output) + { + this.Output = output; + } + [Theory] [WithTestPatternImages(nameof(ResamplerNames), 150, 150, PixelTypes.Rgba32)] public void Transform_WithSampler(TestImageProvider provider, string resamplerName) @@ -53,11 +83,33 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms } } + [Theory] + [WithSolidFilledImages(nameof(TaperMatrixData), 30, 30, nameof(Rgba32.Red), PixelTypes.Rgba32)] + public void Transform_WithTaperMatrix(TestImageProvider provider, TaperSide taperSide, TaperCorner taperCorner) + where TPixel : struct, IPixel + { + var taperMatrixComparer = ImageComparer.TolerantPercentage(0.2f); + using (Image image = provider.GetImage()) + { + Matrix4x4 m = ProjectiveTransformHelper.CreateTaperMatrix(image.Size(), taperSide, taperCorner, .5F); + image.Mutate(i => { i.Transform(m); }); + + string testOutputDetails = $"{taperSide}-{taperCorner}"; + image.DebugSave(provider, testOutputDetails); + + // TODO: Review ProjectiveTransformHelper API before adding assertion + // image.CompareFirstFrameToReferenceOutput(taperMatrixComparer, provider, testOutputDetails); + } + } + [Theory] [WithSolidFilledImages(100, 100, 0, 0, 255, PixelTypes.Rgba32)] public void RawTransformMatchesDocumentedExample(TestImageProvider provider) where TPixel : struct, IPixel { + // Printing some extra output to help investigating roundoff errors: + this.Output.WriteLine($"Vector.IsHardwareAccelerated: {Vector.IsHardwareAccelerated}"); + // This test matches the output described in the example at // https://docs.microsoft.com/en-us/xamarin/xamarin-forms/user-interface/graphics/skiasharp/transforms/non-affine using (Image image = provider.GetImage())