From 5f575ca7e2c61e02f534631e6c6b966467f66db0 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 22 Apr 2020 15:21:14 +0100 Subject: [PATCH 1/6] Capture and throw degenerate matrices. --- .../Exceptions/ImageProcessingException.cs | 13 ++++-- .../Processing/AffineTransformBuilder.cs | 9 +++- .../DegenerateTransformException.cs | 42 +++++++++++++++++++ .../Transforms/TransformUtilities.cs | 29 +++++++++++++ .../Processing/ProjectiveTransformBuilder.cs | 9 +++- .../Transforms/AffineTransformBuilderTests.cs | 7 ++-- .../ProjectiveTransformBuilderTests.cs | 23 ++++++---- .../Transforms/TransformBuilderTestBase.cs | 34 +++++++++------ 8 files changed, 136 insertions(+), 30 deletions(-) create mode 100644 src/ImageSharp/Processing/Processors/Transforms/DegenerateTransformException.cs diff --git a/src/ImageSharp/Common/Exceptions/ImageProcessingException.cs b/src/ImageSharp/Common/Exceptions/ImageProcessingException.cs index 3c75a6418a..c254eaeb3f 100644 --- a/src/ImageSharp/Common/Exceptions/ImageProcessingException.cs +++ b/src/ImageSharp/Common/Exceptions/ImageProcessingException.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System; @@ -8,8 +8,15 @@ namespace SixLabors.ImageSharp /// /// The exception that is thrown when an error occurs when applying a process to an image. /// - public sealed class ImageProcessingException : Exception + public class ImageProcessingException : Exception { + /// + /// Initializes a new instance of the class. + /// + public ImageProcessingException() + { + } + /// /// Initializes a new instance of the class with the name of the /// parameter that causes this exception. @@ -32,4 +39,4 @@ namespace SixLabors.ImageSharp { } } -} \ No newline at end of file +} diff --git a/src/ImageSharp/Processing/AffineTransformBuilder.cs b/src/ImageSharp/Processing/AffineTransformBuilder.cs index dde7beb3e9..51a110dfcd 100644 --- a/src/ImageSharp/Processing/AffineTransformBuilder.cs +++ b/src/ImageSharp/Processing/AffineTransformBuilder.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System; @@ -284,6 +284,11 @@ namespace SixLabors.ImageSharp.Processing matrix *= factory(size); } + if (TransformUtilities.IsNaN(matrix)) + { + throw new DegenerateTransformException("Matrix is NaN. Check input values."); + } + return matrix; } @@ -299,4 +304,4 @@ namespace SixLabors.ImageSharp.Processing return this; } } -} \ No newline at end of file +} diff --git a/src/ImageSharp/Processing/Processors/Transforms/DegenerateTransformException.cs b/src/ImageSharp/Processing/Processors/Transforms/DegenerateTransformException.cs new file mode 100644 index 0000000000..970a044dc6 --- /dev/null +++ b/src/ImageSharp/Processing/Processors/Transforms/DegenerateTransformException.cs @@ -0,0 +1,42 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + +using System; + +namespace SixLabors.ImageSharp.Processing.Processors.Transforms +{ + /// + /// Represents an error that occurs during a transform operation. + /// + public sealed class DegenerateTransformException : ImageProcessingException + { + /// + /// Initializes a new instance of the class. + /// + public DegenerateTransformException() + { + } + + /// + /// Initializes a new instance of the class + /// with a specified error message. + /// + /// The message that describes the error. + public DegenerateTransformException(string message) + : base(message) + { + } + + /// + /// Initializes a new instance of the class + /// with a specified error message and a reference to the inner exception that is + /// the cause of this exception. + /// + /// The error message that explains the reason for the exception. + /// The exception that is the cause of the current exception, or a null reference ( in Visual Basic) if no inner exception is specified. + public DegenerateTransformException(string message, Exception innerException) + : base(message, innerException) + { + } + } +} diff --git a/src/ImageSharp/Processing/Processors/Transforms/TransformUtilities.cs b/src/ImageSharp/Processing/Processors/Transforms/TransformUtilities.cs index 0760d2e3e7..329d57f3d0 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/TransformUtilities.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/TransformUtilities.cs @@ -12,6 +12,35 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// internal static class TransformUtilities { + /// + /// Returns a value that indicates whether the specified matrix contains any values + /// that are not a number . + /// + /// The transform matrix. + /// The . + [MethodImpl(InliningOptions.ShortMethod)] + public static bool IsNaN(Matrix3x2 matrix) + { + return float.IsNaN(matrix.M11) || float.IsNaN(matrix.M12) + || float.IsNaN(matrix.M21) || float.IsNaN(matrix.M22) + || float.IsNaN(matrix.M31) || float.IsNaN(matrix.M32); + } + + /// + /// Returns a value that indicates whether the specified matrix contains any values + /// that are not a number . + /// + /// The transform matrix. + /// The . + [MethodImpl(InliningOptions.ShortMethod)] + public static bool IsNaN(Matrix4x4 matrix) + { + return float.IsNaN(matrix.M11) || float.IsNaN(matrix.M12) || float.IsNaN(matrix.M13) || float.IsNaN(matrix.M14) + || float.IsNaN(matrix.M21) || float.IsNaN(matrix.M22) || float.IsNaN(matrix.M23) || float.IsNaN(matrix.M24) + || float.IsNaN(matrix.M31) || float.IsNaN(matrix.M32) || float.IsNaN(matrix.M33) || float.IsNaN(matrix.M34) + || float.IsNaN(matrix.M41) || float.IsNaN(matrix.M42) || float.IsNaN(matrix.M43) || float.IsNaN(matrix.M44); + } + /// /// Applies the projective transform against the given coordinates flattened into the 2D space. /// diff --git a/src/ImageSharp/Processing/ProjectiveTransformBuilder.cs b/src/ImageSharp/Processing/ProjectiveTransformBuilder.cs index ef44dc16d0..caebc34a36 100644 --- a/src/ImageSharp/Processing/ProjectiveTransformBuilder.cs +++ b/src/ImageSharp/Processing/ProjectiveTransformBuilder.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System; @@ -300,6 +300,11 @@ namespace SixLabors.ImageSharp.Processing matrix *= factory(size); } + if (TransformUtilities.IsNaN(matrix)) + { + throw new DegenerateTransformException("Matrix is NaN. Check input values."); + } + return matrix; } @@ -315,4 +320,4 @@ namespace SixLabors.ImageSharp.Processing return this; } } -} \ No newline at end of file +} diff --git a/tests/ImageSharp.Tests/Processing/Transforms/AffineTransformBuilderTests.cs b/tests/ImageSharp.Tests/Processing/Transforms/AffineTransformBuilderTests.cs index 42017f3afb..70b5be73e5 100644 --- a/tests/ImageSharp.Tests/Processing/Transforms/AffineTransformBuilderTests.cs +++ b/tests/ImageSharp.Tests/Processing/Transforms/AffineTransformBuilderTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System.Numerics; @@ -8,7 +8,8 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms { public class AffineTransformBuilderTests : TransformBuilderTestBase { - protected override AffineTransformBuilder CreateBuilder(Rectangle rectangle) => new AffineTransformBuilder(); + protected override AffineTransformBuilder CreateBuilder() + => new AffineTransformBuilder(); protected override void AppendRotationDegrees(AffineTransformBuilder builder, float degrees) => builder.AppendRotationDegrees(degrees); @@ -67,4 +68,4 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms return Vector2.Transform(sourcePoint, matrix); } } -} \ No newline at end of file +} diff --git a/tests/ImageSharp.Tests/Processing/Transforms/ProjectiveTransformBuilderTests.cs b/tests/ImageSharp.Tests/Processing/Transforms/ProjectiveTransformBuilderTests.cs index 309a73fb4b..22388a0ac1 100644 --- a/tests/ImageSharp.Tests/Processing/Transforms/ProjectiveTransformBuilderTests.cs +++ b/tests/ImageSharp.Tests/Processing/Transforms/ProjectiveTransformBuilderTests.cs @@ -1,6 +1,7 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. +using System; using System.Numerics; using SixLabors.ImageSharp.Processing; @@ -8,20 +9,25 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms { public class ProjectiveTransformBuilderTests : TransformBuilderTestBase { - protected override ProjectiveTransformBuilder CreateBuilder(Rectangle rectangle) => new ProjectiveTransformBuilder(); + protected override ProjectiveTransformBuilder CreateBuilder() + => new ProjectiveTransformBuilder(); - protected override void AppendRotationDegrees(ProjectiveTransformBuilder builder, float degrees) => builder.AppendRotationDegrees(degrees); + protected override void AppendRotationDegrees(ProjectiveTransformBuilder builder, float degrees) + => builder.AppendRotationDegrees(degrees); - protected override void AppendRotationDegrees(ProjectiveTransformBuilder builder, float degrees, Vector2 origin) => builder.AppendRotationDegrees(degrees, origin); + protected override void AppendRotationDegrees(ProjectiveTransformBuilder builder, float degrees, Vector2 origin) + => builder.AppendRotationDegrees(degrees, origin); - protected override void AppendRotationRadians(ProjectiveTransformBuilder builder, float radians) => builder.AppendRotationRadians(radians); + protected override void AppendRotationRadians(ProjectiveTransformBuilder builder, float radians) + => builder.AppendRotationRadians(radians); - protected override void AppendRotationRadians(ProjectiveTransformBuilder builder, float radians, Vector2 origin) => builder.AppendRotationRadians(radians, origin); + protected override void AppendRotationRadians(ProjectiveTransformBuilder builder, float radians, Vector2 origin) + => builder.AppendRotationRadians(radians, origin); protected override void AppendScale(ProjectiveTransformBuilder builder, SizeF scale) => builder.AppendScale(scale); protected override void AppendSkewDegrees(ProjectiveTransformBuilder builder, float degreesX, float degreesY) - => builder.AppendSkewDegrees(degreesX, degreesY); + => builder.AppendSkewDegrees(degreesX, degreesY); protected override void AppendSkewDegrees(ProjectiveTransformBuilder builder, float degreesX, float degreesY, Vector2 origin) => builder.AppendSkewDegrees(degreesX, degreesY, origin); @@ -44,7 +50,8 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms protected override void PrependSkewRadians(ProjectiveTransformBuilder builder, float radiansX, float radiansY, Vector2 origin) => builder.PrependSkewRadians(radiansX, radiansY, origin); - protected override void PrependTranslation(ProjectiveTransformBuilder builder, PointF translate) => builder.PrependTranslation(translate); + protected override void PrependTranslation(ProjectiveTransformBuilder builder, PointF translate) + => builder.PrependTranslation(translate); protected override void PrependRotationRadians(ProjectiveTransformBuilder builder, float radians, Vector2 origin) => builder.PrependRotationRadians(radians, origin); diff --git a/tests/ImageSharp.Tests/Processing/Transforms/TransformBuilderTestBase.cs b/tests/ImageSharp.Tests/Processing/Transforms/TransformBuilderTestBase.cs index 21359799e7..8c75cea7fd 100644 --- a/tests/ImageSharp.Tests/Processing/Transforms/TransformBuilderTestBase.cs +++ b/tests/ImageSharp.Tests/Processing/Transforms/TransformBuilderTestBase.cs @@ -31,7 +31,7 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms { // These operations should be size-agnostic: var size = new Size(123, 321); - TBuilder builder = this.CreateBuilder(size); + TBuilder builder = this.CreateBuilder(); this.AppendScale(builder, new SizeF(scale)); this.AppendTranslation(builder, translate); @@ -57,7 +57,7 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms { // Translate ans scale are size-agnostic: var size = new Size(456, 432); - TBuilder builder = this.CreateBuilder(size); + TBuilder builder = this.CreateBuilder(); this.AppendTranslation(builder, translate); this.AppendScale(builder, new SizeF(scale)); @@ -72,7 +72,7 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms public void LocationOffsetIsPrepended(int locationX, int locationY) { var rectangle = new Rectangle(locationX, locationY, 10, 10); - TBuilder builder = this.CreateBuilder(rectangle); + TBuilder builder = this.CreateBuilder(); this.AppendScale(builder, new SizeF(2, 2)); @@ -94,7 +94,7 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms float y) { var size = new Size(width, height); - TBuilder builder = this.CreateBuilder(size); + TBuilder builder = this.CreateBuilder(); this.AppendRotationDegrees(builder, degrees); @@ -122,7 +122,7 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms float y) { var size = new Size(width, height); - TBuilder builder = this.CreateBuilder(size); + TBuilder builder = this.CreateBuilder(); var centerPoint = new Vector2(cx, cy); this.AppendRotationDegrees(builder, degrees, centerPoint); @@ -149,7 +149,7 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms float y) { var size = new Size(width, height); - TBuilder builder = this.CreateBuilder(size); + TBuilder builder = this.CreateBuilder(); this.AppendSkewDegrees(builder, degreesX, degreesY); @@ -176,7 +176,7 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms float y) { var size = new Size(width, height); - TBuilder builder = this.CreateBuilder(size); + TBuilder builder = this.CreateBuilder(); var centerPoint = new Vector2(cx, cy); this.AppendSkewDegrees(builder, degreesX, degreesY, centerPoint); @@ -194,8 +194,8 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms public void AppendPrependOpposite() { var rectangle = new Rectangle(-1, -1, 3, 3); - TBuilder b1 = this.CreateBuilder(rectangle); - TBuilder b2 = this.CreateBuilder(rectangle); + TBuilder b1 = this.CreateBuilder(); + TBuilder b2 = this.CreateBuilder(); const float pi = (float)Math.PI; @@ -232,14 +232,24 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms Assert.ThrowsAny( () => { - TBuilder builder = this.CreateBuilder(size); + TBuilder builder = this.CreateBuilder(); this.Execute(builder, new Rectangle(Point.Empty, size), Vector2.Zero); }); } - protected TBuilder CreateBuilder(Size size) => this.CreateBuilder(new Rectangle(Point.Empty, size)); + [Fact] + public void ThrowsForInvalidMatrix() + { + Assert.ThrowsAny( + () => + { + TBuilder builder = this.CreateBuilder(); + this.AppendSkewDegrees(builder, 45, 45); + this.Execute(builder, new Rectangle(0, 0, 150, 150), Vector2.Zero); + }); + } - protected abstract TBuilder CreateBuilder(Rectangle rectangle); + protected abstract TBuilder CreateBuilder(); protected abstract void AppendRotationDegrees(TBuilder builder, float degrees); From 8db87e4991d10e807126e8d92ef09e237f44718a Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 22 Apr 2020 15:25:09 +0100 Subject: [PATCH 2/6] Add exception docs. --- src/ImageSharp/Processing/AffineTransformBuilder.cs | 3 +++ src/ImageSharp/Processing/ProjectiveTransformBuilder.cs | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/ImageSharp/Processing/AffineTransformBuilder.cs b/src/ImageSharp/Processing/AffineTransformBuilder.cs index 51a110dfcd..c7007679fc 100644 --- a/src/ImageSharp/Processing/AffineTransformBuilder.cs +++ b/src/ImageSharp/Processing/AffineTransformBuilder.cs @@ -268,6 +268,9 @@ namespace SixLabors.ImageSharp.Processing /// Returns the combined matrix for a given source rectangle. /// /// The rectangle in the source image. + /// + /// The resultant matrix contains one or more values equivalent to . + /// /// The . public Matrix3x2 BuildMatrix(Rectangle sourceRectangle) { diff --git a/src/ImageSharp/Processing/ProjectiveTransformBuilder.cs b/src/ImageSharp/Processing/ProjectiveTransformBuilder.cs index caebc34a36..a407074aa2 100644 --- a/src/ImageSharp/Processing/ProjectiveTransformBuilder.cs +++ b/src/ImageSharp/Processing/ProjectiveTransformBuilder.cs @@ -284,6 +284,9 @@ namespace SixLabors.ImageSharp.Processing /// Returns the combined matrix for a given source rectangle. /// /// The rectangle in the source image. + /// + /// The resultant matrix contains one or more values equivalent to . + /// /// The . public Matrix4x4 BuildMatrix(Rectangle sourceRectangle) { From 18a2554d85361c6559465952bd4d653580126a70 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 22 Apr 2020 22:57:56 +0100 Subject: [PATCH 3/6] Check determinant and update error message. --- src/ImageSharp/Processing/AffineTransformBuilder.cs | 4 ++-- src/ImageSharp/Processing/ProjectiveTransformBuilder.cs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Processing/AffineTransformBuilder.cs b/src/ImageSharp/Processing/AffineTransformBuilder.cs index c7007679fc..d793c67a2b 100644 --- a/src/ImageSharp/Processing/AffineTransformBuilder.cs +++ b/src/ImageSharp/Processing/AffineTransformBuilder.cs @@ -287,9 +287,9 @@ namespace SixLabors.ImageSharp.Processing matrix *= factory(size); } - if (TransformUtilities.IsNaN(matrix)) + if (TransformUtilities.IsNaN(matrix) || matrix.GetDeterminant() == 0) { - throw new DegenerateTransformException("Matrix is NaN. Check input values."); + throw new DegenerateTransformException("Matrix is degenerate. Check input values."); } return matrix; diff --git a/src/ImageSharp/Processing/ProjectiveTransformBuilder.cs b/src/ImageSharp/Processing/ProjectiveTransformBuilder.cs index a407074aa2..0535eaeb1c 100644 --- a/src/ImageSharp/Processing/ProjectiveTransformBuilder.cs +++ b/src/ImageSharp/Processing/ProjectiveTransformBuilder.cs @@ -303,9 +303,9 @@ namespace SixLabors.ImageSharp.Processing matrix *= factory(size); } - if (TransformUtilities.IsNaN(matrix)) + if (TransformUtilities.IsNaN(matrix) || matrix.GetDeterminant() == 0) { - throw new DegenerateTransformException("Matrix is NaN. Check input values."); + throw new DegenerateTransformException("Matrix is degenerate. Check input values."); } return matrix; From f14ddb3a7365d1568325975395ae0765a2e182cc Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 22 Apr 2020 23:55:26 +0100 Subject: [PATCH 4/6] Remove inheritance --- src/ImageSharp/Common/Exceptions/ImageProcessingException.cs | 2 +- .../Processors/Transforms/DegenerateTransformException.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Common/Exceptions/ImageProcessingException.cs b/src/ImageSharp/Common/Exceptions/ImageProcessingException.cs index c254eaeb3f..ccd0b71e54 100644 --- a/src/ImageSharp/Common/Exceptions/ImageProcessingException.cs +++ b/src/ImageSharp/Common/Exceptions/ImageProcessingException.cs @@ -8,7 +8,7 @@ namespace SixLabors.ImageSharp /// /// The exception that is thrown when an error occurs when applying a process to an image. /// - public class ImageProcessingException : Exception + public sealed class ImageProcessingException : Exception { /// /// Initializes a new instance of the class. diff --git a/src/ImageSharp/Processing/Processors/Transforms/DegenerateTransformException.cs b/src/ImageSharp/Processing/Processors/Transforms/DegenerateTransformException.cs index 970a044dc6..4d46540bcc 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/DegenerateTransformException.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/DegenerateTransformException.cs @@ -8,7 +8,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// /// Represents an error that occurs during a transform operation. /// - public sealed class DegenerateTransformException : ImageProcessingException + public sealed class DegenerateTransformException : Exception { /// /// Initializes a new instance of the class. From 95c40e04febbce242bdc52b2cdf8874e2d78f21f Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 22 Apr 2020 23:56:26 +0100 Subject: [PATCH 5/6] Add determinant check and update check location/message --- .../Processing/AffineTransformBuilder.cs | 37 ++++++++++++++--- .../Transforms/TransformUtilities.cs | 22 ++++++++++ .../Processing/ProjectiveTransformBuilder.cs | 41 +++++++++++++++---- 3 files changed, 87 insertions(+), 13 deletions(-) diff --git a/src/ImageSharp/Processing/AffineTransformBuilder.cs b/src/ImageSharp/Processing/AffineTransformBuilder.cs index d793c67a2b..1478d2951b 100644 --- a/src/ImageSharp/Processing/AffineTransformBuilder.cs +++ b/src/ImageSharp/Processing/AffineTransformBuilder.cs @@ -247,15 +247,33 @@ namespace SixLabors.ImageSharp.Processing /// Prepends a raw matrix. /// /// The matrix to prepend. + /// + /// The resultant matrix is degenerate containing one or more values equivalent + /// to or a zero determinant and therefore cannot be used + /// for linear transforms. + /// /// The . - public AffineTransformBuilder PrependMatrix(Matrix3x2 matrix) => this.Prepend(_ => matrix); + public AffineTransformBuilder PrependMatrix(Matrix3x2 matrix) + { + CheckDegenerate(matrix); + return this.Prepend(_ => matrix); + } /// /// Appends a raw matrix. /// /// The matrix to append. + /// + /// The resultant matrix is degenerate containing one or more values equivalent + /// to or a zero determinant and therefore cannot be used + /// for linear transforms. + /// /// The . - public AffineTransformBuilder AppendMatrix(Matrix3x2 matrix) => this.Append(_ => matrix); + public AffineTransformBuilder AppendMatrix(Matrix3x2 matrix) + { + CheckDegenerate(matrix); + return this.Append(_ => matrix); + } /// /// Returns the combined matrix for a given source size. @@ -269,7 +287,9 @@ namespace SixLabors.ImageSharp.Processing /// /// The rectangle in the source image. /// - /// The resultant matrix contains one or more values equivalent to . + /// The resultant matrix is degenerate containing one or more values equivalent + /// to or a zero determinant and therefore cannot be used + /// for linear transforms. /// /// The . public Matrix3x2 BuildMatrix(Rectangle sourceRectangle) @@ -287,12 +307,17 @@ namespace SixLabors.ImageSharp.Processing matrix *= factory(size); } - if (TransformUtilities.IsNaN(matrix) || matrix.GetDeterminant() == 0) + CheckDegenerate(matrix); + + return matrix; + } + + private static void CheckDegenerate(Matrix3x2 matrix) + { + if (TransformUtilities.IsDegenerate(matrix)) { throw new DegenerateTransformException("Matrix is degenerate. Check input values."); } - - return matrix; } private AffineTransformBuilder Prepend(Func factory) diff --git a/src/ImageSharp/Processing/Processors/Transforms/TransformUtilities.cs b/src/ImageSharp/Processing/Processors/Transforms/TransformUtilities.cs index 329d57f3d0..b474b43712 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/TransformUtilities.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/TransformUtilities.cs @@ -12,6 +12,28 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// internal static class TransformUtilities { + /// + /// Returns a value that indicates whether the specified matrix is degenerate + /// containing one or more values equivalent to or a + /// zero determinant and therefore cannot be used for linear transforms. + /// + /// The transform matrix. + public static bool IsDegenerate(Matrix3x2 matrix) + => IsNaN(matrix) || IsZero(matrix.GetDeterminant()); + + /// + /// Returns a value that indicates whether the specified matrix is degenerate + /// containing one or more values equivalent to or a + /// zero determinant and therefore cannot be used for linear transforms. + /// + /// The transform matrix. + public static bool IsDegenerate(Matrix4x4 matrix) + => IsNaN(matrix) || IsZero(matrix.GetDeterminant()); + + [MethodImpl(InliningOptions.ShortMethod)] + private static bool IsZero(float a) + => a > -Constants.EpsilonSquared && a < Constants.EpsilonSquared; + /// /// Returns a value that indicates whether the specified matrix contains any values /// that are not a number . diff --git a/src/ImageSharp/Processing/ProjectiveTransformBuilder.cs b/src/ImageSharp/Processing/ProjectiveTransformBuilder.cs index 0535eaeb1c..b7e65b4cc0 100644 --- a/src/ImageSharp/Processing/ProjectiveTransformBuilder.cs +++ b/src/ImageSharp/Processing/ProjectiveTransformBuilder.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Numerics; +using System.Runtime.CompilerServices; using SixLabors.ImageSharp.Processing.Processors.Transforms; namespace SixLabors.ImageSharp.Processing @@ -263,29 +264,50 @@ namespace SixLabors.ImageSharp.Processing /// Prepends a raw matrix. /// /// The matrix to prepend. + /// + /// The resultant matrix is degenerate containing one or more values equivalent + /// to or a zero determinant and therefore cannot be used + /// for linear transforms. + /// /// The . - public ProjectiveTransformBuilder PrependMatrix(Matrix4x4 matrix) => this.Prepend(_ => matrix); + public ProjectiveTransformBuilder PrependMatrix(Matrix4x4 matrix) + { + CheckDegenerate(matrix); + return this.Prepend(_ => matrix); + } /// /// Appends a raw matrix. /// /// The matrix to append. + /// + /// The resultant matrix is degenerate containing one or more values equivalent + /// to or a zero determinant and therefore cannot be used + /// for linear transforms. + /// /// The . - public ProjectiveTransformBuilder AppendMatrix(Matrix4x4 matrix) => this.Append(_ => matrix); + public ProjectiveTransformBuilder AppendMatrix(Matrix4x4 matrix) + { + CheckDegenerate(matrix); + return this.Append(_ => matrix); + } /// /// Returns the combined matrix for a given source size. /// /// The source image size. /// The . - public Matrix4x4 BuildMatrix(Size sourceSize) => this.BuildMatrix(new Rectangle(Point.Empty, sourceSize)); + public Matrix4x4 BuildMatrix(Size sourceSize) + => this.BuildMatrix(new Rectangle(Point.Empty, sourceSize)); /// /// Returns the combined matrix for a given source rectangle. /// /// The rectangle in the source image. /// - /// The resultant matrix contains one or more values equivalent to . + /// The resultant matrix is degenerate containing one or more values equivalent + /// to or a zero determinant and therefore cannot be used + /// for linear transforms. /// /// The . public Matrix4x4 BuildMatrix(Rectangle sourceRectangle) @@ -303,12 +325,17 @@ namespace SixLabors.ImageSharp.Processing matrix *= factory(size); } - if (TransformUtilities.IsNaN(matrix) || matrix.GetDeterminant() == 0) + CheckDegenerate(matrix); + + return matrix; + } + + private static void CheckDegenerate(Matrix4x4 matrix) + { + if (TransformUtilities.IsDegenerate(matrix)) { throw new DegenerateTransformException("Matrix is degenerate. Check input values."); } - - return matrix; } private ProjectiveTransformBuilder Prepend(Func factory) From a446546af070c6625b10905bd45da947199daf52 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Fri, 24 Apr 2020 12:08:49 +0200 Subject: [PATCH 6/6] Fix warning --- .../Drawing/DrawImageExtensionsTests.cs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/tests/ImageSharp.Tests/Drawing/DrawImageExtensionsTests.cs b/tests/ImageSharp.Tests/Drawing/DrawImageExtensionsTests.cs index 3f90412ae9..0aff95d994 100644 --- a/tests/ImageSharp.Tests/Drawing/DrawImageExtensionsTests.cs +++ b/tests/ImageSharp.Tests/Drawing/DrawImageExtensionsTests.cs @@ -1,17 +1,10 @@ // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. -using System; -using System.Linq; -using Moq; -using SixLabors.ImageSharp; -using SixLabors.ImageSharp.Advanced; -using SixLabors.ImageSharp.Formats.Png; using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Processing; using SixLabors.ImageSharp.Processing.Processors.Drawing; using SixLabors.ImageSharp.Tests.Processing; -using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; using Xunit; @@ -19,7 +12,6 @@ namespace SixLabors.ImageSharp.Tests.Drawing { public class DrawImageExtensionsTests : BaseImageOperationsExtensionTest { - [Fact] public void DrawImage_OpacityOnly_VerifyGraphicOptionsTakenFromContext() { @@ -28,7 +20,7 @@ namespace SixLabors.ImageSharp.Tests.Drawing this.options.ColorBlendingMode = PixelColorBlendingMode.Screen; this.operations.DrawImage(null, 0.5f); - var dip = this.Verify(); + DrawImageProcessor dip = this.Verify(); Assert.Equal(0.5, dip.Opacity); Assert.Equal(this.options.AlphaCompositionMode, dip.AlphaCompositionMode); @@ -43,7 +35,7 @@ namespace SixLabors.ImageSharp.Tests.Drawing this.options.ColorBlendingMode = PixelColorBlendingMode.Screen; this.operations.DrawImage(null, PixelColorBlendingMode.Multiply, 0.5f); - var dip = this.Verify(); + DrawImageProcessor dip = this.Verify(); Assert.Equal(0.5, dip.Opacity); Assert.Equal(this.options.AlphaCompositionMode, dip.AlphaCompositionMode); @@ -58,7 +50,7 @@ namespace SixLabors.ImageSharp.Tests.Drawing this.options.ColorBlendingMode = PixelColorBlendingMode.Screen; this.operations.DrawImage(null, Point.Empty, 0.5f); - var dip = this.Verify(); + DrawImageProcessor dip = this.Verify(); Assert.Equal(0.5, dip.Opacity); Assert.Equal(this.options.AlphaCompositionMode, dip.AlphaCompositionMode); @@ -73,7 +65,7 @@ namespace SixLabors.ImageSharp.Tests.Drawing this.options.ColorBlendingMode = PixelColorBlendingMode.Screen; this.operations.DrawImage(null, Point.Empty, PixelColorBlendingMode.Multiply, 0.5f); - var dip = this.Verify(); + DrawImageProcessor dip = this.Verify(); Assert.Equal(0.5, dip.Opacity); Assert.Equal(this.options.AlphaCompositionMode, dip.AlphaCompositionMode);