From 7fd7f18a76aab3e830f628a6ce162bcd9da9550b Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 23 Jan 2018 12:23:16 +1100 Subject: [PATCH 1/8] Use premultiplication for convolution #428 --- .../Common/Extensions/Vector4Extensions.cs | 26 +++++++++++++++++++ .../Convolution/Convolution2DProcessor.cs | 4 +-- .../Convolution/Convolution2PassProcessor.cs | 4 +-- .../Convolution/ConvolutionProcessor.cs | 4 +-- 4 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp/Common/Extensions/Vector4Extensions.cs b/src/ImageSharp/Common/Extensions/Vector4Extensions.cs index 7cb193e828..64ebeb1f33 100644 --- a/src/ImageSharp/Common/Extensions/Vector4Extensions.cs +++ b/src/ImageSharp/Common/Extensions/Vector4Extensions.cs @@ -12,6 +12,32 @@ namespace SixLabors.ImageSharp /// internal static class Vector4Extensions { + /// + /// Premultiplies the "x", "y", "z" components of a vector by its "w" component leaving the "w" component intact. + /// + /// The to premultiply + /// The + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static Vector4 Premultiply(this Vector4 source) + { + float w = source.W; + Vector4 premultiplied = source * w; + return new Vector4(premultiplied.X, premultiplied.Y, premultiplied.Z, w); + } + + /// + /// Reverses the result of premultiplying a vector via . + /// + /// The to premultiply + /// The + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static Vector4 UnPremultiply(this Vector4 source) + { + float w = source.W; + Vector4 unpremultiplied = source / w; + return new Vector4(unpremultiplied.X, unpremultiplied.Y, unpremultiplied.Z, w); + } + /// /// Compresses a linear color signal to its sRGB equivalent. /// diff --git a/src/ImageSharp/Processing/Processors/Convolution/Convolution2DProcessor.cs b/src/ImageSharp/Processing/Processors/Convolution/Convolution2DProcessor.cs index b85432ac54..cf9b7be9c8 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/Convolution2DProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/Convolution2DProcessor.cs @@ -93,7 +93,7 @@ namespace SixLabors.ImageSharp.Processing.Processors int offsetX = x + fxr; offsetX = offsetX.Clamp(0, maxX); - var currentColor = sourceOffsetRow[offsetX].ToVector4(); + Vector4 currentColor = sourceOffsetRow[offsetX].ToVector4().Premultiply(); if (fy < kernelXHeight) { @@ -118,7 +118,7 @@ namespace SixLabors.ImageSharp.Processing.Processors float blue = MathF.Sqrt((bX * bX) + (bY * bY)); ref TPixel pixel = ref targetRow[x]; - pixel.PackFromVector4(new Vector4(red, green, blue, sourceRow[x].ToVector4().W)); + pixel.PackFromVector4(new Vector4(red, green, blue, sourceRow[x].ToVector4().W).UnPremultiply()); } }); diff --git a/src/ImageSharp/Processing/Processors/Convolution/Convolution2PassProcessor.cs b/src/ImageSharp/Processing/Processors/Convolution/Convolution2PassProcessor.cs index 362fa5c508..57f434ee08 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/Convolution2PassProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/Convolution2PassProcessor.cs @@ -113,13 +113,13 @@ namespace SixLabors.ImageSharp.Processing.Processors offsetX = offsetX.Clamp(0, maxX); - var currentColor = row[offsetX].ToVector4(); + Vector4 currentColor = row[offsetX].ToVector4().Premultiply(); destination += kernel[fy, fx] * currentColor; } } ref TPixel pixel = ref targetRow[x]; - pixel.PackFromVector4(destination); + pixel.PackFromVector4(destination.UnPremultiply()); } }); } diff --git a/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessor.cs b/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessor.cs index c0d3fdcfec..96db9a4ce0 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessor.cs @@ -80,7 +80,7 @@ namespace SixLabors.ImageSharp.Processing.Processors offsetX = offsetX.Clamp(0, maxX); - var currentColor = sourceOffsetRow[offsetX].ToVector4(); + Vector4 currentColor = sourceOffsetRow[offsetX].ToVector4().Premultiply(); currentColor *= this.KernelXY[fy, fx]; red += currentColor.X; @@ -90,7 +90,7 @@ namespace SixLabors.ImageSharp.Processing.Processors } ref TPixel pixel = ref targetRow[x]; - pixel.PackFromVector4(new Vector4(red, green, blue, sourceRow[x].ToVector4().W)); + pixel.PackFromVector4(new Vector4(red, green, blue, sourceRow[x].ToVector4().W).UnPremultiply()); } }); From 534b426ad1518d6605e805199cf89ea4a4638be9 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 23 Jan 2018 16:31:28 +1100 Subject: [PATCH 2/8] Use premultiply for resize. Fix #428 --- .../Processors/Transforms/AffineTransformProcessor.cs | 4 ++-- .../Transforms/ProjectiveTransformProcessor.cs | 4 ++-- .../Processing/Processors/Transforms/WeightsWindow.cs | 8 ++++---- .../Processors/Convolution/DetectEdgesTest.cs | 11 +++-------- .../Processing/Processors/Transforms/ResizeTests.cs | 2 +- 5 files changed, 12 insertions(+), 17 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Transforms/AffineTransformProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/AffineTransformProcessor.cs index 07c247d734..9d7056b67d 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/AffineTransformProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/AffineTransformProcessor.cs @@ -204,7 +204,7 @@ namespace SixLabors.ImageSharp.Processing.Processors var vector = source[i, j].ToVector4(); // Values are first premultiplied to prevent darkening of edge pixels - var mupltiplied = new Vector4(new Vector3(vector.X, vector.Y, vector.Z) * vector.W, vector.W); + Vector4 mupltiplied = vector.Premultiply(); sum += mupltiplied * xWeight * yWeight; } } @@ -212,7 +212,7 @@ namespace SixLabors.ImageSharp.Processing.Processors ref TPixel dest = ref destRow[x]; // Reverse the premultiplication - dest.PackFromVector4(new Vector4(new Vector3(sum.X, sum.Y, sum.Z) / sum.W, sum.W)); + dest.PackFromVector4(sum.UnPremultiply()); } }); } diff --git a/src/ImageSharp/Processing/Processors/Transforms/ProjectiveTransformProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/ProjectiveTransformProcessor.cs index f53585093b..463d3717d0 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/ProjectiveTransformProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/ProjectiveTransformProcessor.cs @@ -199,7 +199,7 @@ namespace SixLabors.ImageSharp.Processing.Processors var vector = source[i, j].ToVector4(); // Values are first premultiplied to prevent darkening of edge pixels - var mupltiplied = new Vector4(new Vector3(vector.X, vector.Y, vector.Z) * vector.W, vector.W); + Vector4 mupltiplied = vector.Premultiply(); sum += mupltiplied * xWeight * yWeight; } } @@ -207,7 +207,7 @@ namespace SixLabors.ImageSharp.Processing.Processors ref TPixel dest = ref destRow[x]; // Reverse the premultiplication - dest.PackFromVector4(new Vector4(new Vector3(sum.X, sum.Y, sum.Z) / sum.W, sum.W)); + dest.PackFromVector4(sum.UnPremultiply()); } }); } diff --git a/src/ImageSharp/Processing/Processors/Transforms/WeightsWindow.cs b/src/ImageSharp/Processing/Processors/Transforms/WeightsWindow.cs index d23ee5a060..b0a530514e 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/WeightsWindow.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/WeightsWindow.cs @@ -87,7 +87,7 @@ namespace SixLabors.ImageSharp.Processing.Processors { float weight = Unsafe.Add(ref horizontalValues, i); Vector4 v = Unsafe.Add(ref vecPtr, i); - result += v * weight; + result += v.Premultiply() * weight; } return result; @@ -114,10 +114,10 @@ namespace SixLabors.ImageSharp.Processing.Processors { float weight = Unsafe.Add(ref horizontalValues, i); Vector4 v = Unsafe.Add(ref vecPtr, i); - result += v.Expand() * weight; + result += v.Premultiply().Expand() * weight; } - return result; + return result.UnPremultiply(); } /// @@ -144,7 +144,7 @@ namespace SixLabors.ImageSharp.Processing.Processors result += firstPassPixels[x, index] * yw; } - return result; + return result.UnPremultiply(); } } } \ No newline at end of file diff --git a/tests/ImageSharp.Tests/Processing/Processors/Convolution/DetectEdgesTest.cs b/tests/ImageSharp.Tests/Processing/Processors/Convolution/DetectEdgesTest.cs index c2d8916384..1b678f20bc 100644 --- a/tests/ImageSharp.Tests/Processing/Processors/Convolution/DetectEdgesTest.cs +++ b/tests/ImageSharp.Tests/Processing/Processors/Convolution/DetectEdgesTest.cs @@ -14,7 +14,7 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Convolution public class DetectEdgesTest : FileTestBase { public static readonly string[] CommonTestImages = { TestImages.Png.Bike }; - + public static readonly TheoryData DetectEdgesFilters = new TheoryData { EdgeDetection.Kayyali, @@ -54,9 +54,7 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Convolution { image.Mutate(x => x.DetectEdges()); image.DebugSave(provider); - - // TODO: Enable once we have updated the images - // image.CompareToReferenceOutput(provider); + image.CompareToReferenceOutput(provider); } } @@ -85,10 +83,7 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Convolution image.DebugSave(provider); // TODO: Enable once we have updated the images - //image.CompareToReferenceOutput(provider); - - // TODO: We don't need this any longer after switching to ReferenceImages - //ImageComparer.Tolerant().EnsureProcessorChangesAreConstrained(source, image, bounds); + image.CompareToReferenceOutput(provider); } } } diff --git a/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeTests.cs b/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeTests.cs index 92f1af58ed..72febea340 100644 --- a/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeTests.cs +++ b/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeTests.cs @@ -89,7 +89,7 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Transforms { using (Image image = provider.GetImage()) { - image.Mutate(x => x.Resize(image.Width / 2, image.Height / 2, true)); + image.Mutate(x => x.Resize(image.Width / 2, image.Height / 2, KnownResamplers.NearestNeighbor)); // Comparer fights decoder with gif-s. Could not use CompareToReferenceOutput here :( image.DebugSave(provider, extension: Extensions.Gif); From 9865e3d2daa0497ec1a28e2c093b04d12dce6b11 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 23 Jan 2018 16:35:56 +1100 Subject: [PATCH 3/8] Update reference images --- tests/Images/External | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Images/External b/tests/Images/External index ddc4045926..757411f91f 160000 --- a/tests/Images/External +++ b/tests/Images/External @@ -1 +1 @@ -Subproject commit ddc4045926bb0331be8c1785d9adf5f76dc4e52e +Subproject commit 757411f91f1164e41a300874655a77ef3b390067 From b02a47dbe5bca85d4a4dd426ec69175dc41694cf Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 23 Jan 2018 16:41:47 +1100 Subject: [PATCH 4/8] Re-enable the reference output for Detect Edges --- .../Processing/Processors/Convolution/DetectEdgesTest.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/tests/ImageSharp.Tests/Processing/Processors/Convolution/DetectEdgesTest.cs b/tests/ImageSharp.Tests/Processing/Processors/Convolution/DetectEdgesTest.cs index 1b678f20bc..323842556e 100644 --- a/tests/ImageSharp.Tests/Processing/Processors/Convolution/DetectEdgesTest.cs +++ b/tests/ImageSharp.Tests/Processing/Processors/Convolution/DetectEdgesTest.cs @@ -3,7 +3,6 @@ using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Processing; -using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; using SixLabors.Primitives; using Xunit; @@ -39,9 +38,7 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Convolution { image.Mutate(x => x.DetectEdges(detector)); image.DebugSave(provider, detector.ToString()); - - // TODO: Enable once we have updated the images - // image.CompareToReferenceOutput(provider, detector.ToString()); + image.CompareToReferenceOutput(provider, detector.ToString()); } } @@ -81,8 +78,6 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Convolution image.Mutate(x => x.DetectEdges(bounds)); image.DebugSave(provider); - - // TODO: Enable once we have updated the images image.CompareToReferenceOutput(provider); } } From 8b8a9c2cb0d535ba05f1c9edbff3e89cafd3dd0b Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 23 Jan 2018 16:47:40 +1100 Subject: [PATCH 5/8] Add kaboom image for future testing --- tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/Input/Png/kaboom.png | 3 +++ 2 files changed, 4 insertions(+) create mode 100644 tests/Images/Input/Png/kaboom.png diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index c542fa8808..864c963327 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -35,6 +35,7 @@ namespace SixLabors.ImageSharp.Tests public const string Rgb48BppInterlaced = "Png/rgb-48bpp-interlaced.png"; public const string SnakeGame = "Png/SnakeGame.png"; public const string Icon = "Png/icon.png"; + public const string Kaboom = "Png/kaboom.png"; // Filtered test images from http://www.schaik.com/pngsuite/pngsuite_fil_png.html public const string Filter0 = "Png/filter0.png"; diff --git a/tests/Images/Input/Png/kaboom.png b/tests/Images/Input/Png/kaboom.png new file mode 100644 index 0000000000..29f2bbfc15 --- /dev/null +++ b/tests/Images/Input/Png/kaboom.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:b8d493f5472e58b3184d1c6fe3795a731b6cb6de765ae0d27726d69aeff06d6b +size 573925 From 531ebd6bf62cee4c144e6ecb03118c9fac79cb1c Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 23 Jan 2018 17:19:28 +1100 Subject: [PATCH 6/8] Add new test for kaboom --- .../Processing/Processors/Transforms/ResizeTests.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeTests.cs b/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeTests.cs index 72febea340..8370a802cd 100644 --- a/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeTests.cs +++ b/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeTests.cs @@ -82,6 +82,19 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Transforms } } + [Theory] + [WithFile(TestImages.Png.Kaboom, DefaultPixelType)] + public void Resize_DoesNotBleedAlphaPixels(TestImageProvider provider) + where TPixel : struct, IPixel + { + using (Image image = provider.GetImage()) + { + image.Mutate(x => x.Resize(image.Width / 2, image.Height / 2)); + image.DebugSave(provider); + image.CompareToReferenceOutput(provider); + } + } + [Theory] [WithFile(TestImages.Gif.Giphy, DefaultPixelType)] public void Resize_IsAppliedToAllFrames(TestImageProvider provider) From 4a220ee05eb3a42dd644d37a3f02e97ede435550 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 23 Jan 2018 17:20:09 +1100 Subject: [PATCH 7/8] Update reference images --- tests/Images/External | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Images/External b/tests/Images/External index 757411f91f..376605e05b 160000 --- a/tests/Images/External +++ b/tests/Images/External @@ -1 +1 @@ -Subproject commit 757411f91f1164e41a300874655a77ef3b390067 +Subproject commit 376605e05bb704d425b2d17bf5b310f5376da22e From 05775a3ed141a81dabbb6a33c10d83f8d26542a2 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 24 Jan 2018 12:06:43 +1100 Subject: [PATCH 8/8] Drop an unnecessary constructor invocation for perf --- src/ImageSharp/Common/Extensions/Vector4Extensions.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/ImageSharp/Common/Extensions/Vector4Extensions.cs b/src/ImageSharp/Common/Extensions/Vector4Extensions.cs index 64ebeb1f33..8133ebb38e 100644 --- a/src/ImageSharp/Common/Extensions/Vector4Extensions.cs +++ b/src/ImageSharp/Common/Extensions/Vector4Extensions.cs @@ -13,7 +13,7 @@ namespace SixLabors.ImageSharp internal static class Vector4Extensions { /// - /// Premultiplies the "x", "y", "z" components of a vector by its "w" component leaving the "w" component intact. + /// Pre-multiplies the "x", "y", "z" components of a vector by its "w" component leaving the "w" component intact. /// /// The to premultiply /// The @@ -22,7 +22,8 @@ namespace SixLabors.ImageSharp { float w = source.W; Vector4 premultiplied = source * w; - return new Vector4(premultiplied.X, premultiplied.Y, premultiplied.Z, w); + premultiplied.W = w; + return premultiplied; } /// @@ -35,7 +36,8 @@ namespace SixLabors.ImageSharp { float w = source.W; Vector4 unpremultiplied = source / w; - return new Vector4(unpremultiplied.X, unpremultiplied.Y, unpremultiplied.Z, w); + unpremultiplied.W = w; + return unpremultiplied; } ///