From c1ab6b1885fa48aea7d95bcee2145e5a86164ef2 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Thu, 15 Aug 2019 19:02:14 +1000 Subject: [PATCH 01/11] remove patternVector in patternBrush --- src/ImageSharp.Drawing/Processing/PatternBrush.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/ImageSharp.Drawing/Processing/PatternBrush.cs b/src/ImageSharp.Drawing/Processing/PatternBrush.cs index a7a6785b92..1999af8a39 100644 --- a/src/ImageSharp.Drawing/Processing/PatternBrush.cs +++ b/src/ImageSharp.Drawing/Processing/PatternBrush.cs @@ -99,7 +99,6 @@ namespace SixLabors.ImageSharp.Processing new PatternBrushApplicator( source, this.pattern.ToPixelMatrix(source.Configuration), - this.patternVector, options); /// @@ -112,20 +111,17 @@ namespace SixLabors.ImageSharp.Processing /// The pattern. /// private readonly DenseMatrix pattern; - private readonly DenseMatrix patternVector; /// /// Initializes a new instance of the class. /// /// The source image. /// The pattern. - /// The patternVector. /// The options - public PatternBrushApplicator(ImageFrame source, in DenseMatrix pattern, DenseMatrix patternVector, GraphicsOptions options) + public PatternBrushApplicator(ImageFrame source, in DenseMatrix pattern, GraphicsOptions options) : base(source, options) { this.pattern = pattern; - this.patternVector = patternVector; } /// From ccd23cfadd14452a9c5c4e6f63d396dda1e9e0ed Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Thu, 15 Aug 2019 19:39:25 +1000 Subject: [PATCH 02/11] use SUPPORTS_EXTENDED_INTRINSICS to filter out some BasicIntrinsics256 methods --- src/ImageSharp/Common/Helpers/SimdUtils.BasicIntrinsics256.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ImageSharp/Common/Helpers/SimdUtils.BasicIntrinsics256.cs b/src/ImageSharp/Common/Helpers/SimdUtils.BasicIntrinsics256.cs index 5aa0b21ec1..bc07fbf317 100644 --- a/src/ImageSharp/Common/Helpers/SimdUtils.BasicIntrinsics256.cs +++ b/src/ImageSharp/Common/Helpers/SimdUtils.BasicIntrinsics256.cs @@ -19,6 +19,7 @@ namespace SixLabors.ImageSharp { public static bool IsAvailable { get; } = IsAvx2CompatibleArchitecture; +#if !SUPPORTS_EXTENDED_INTRINSICS /// /// as many elements as possible, slicing them down (keeping the remainder). /// @@ -74,6 +75,7 @@ namespace SixLabors.ImageSharp dest = dest.Slice(adjustedCount); } } +#endif /// /// SIMD optimized implementation for . From 7b6d5862ef6fa90d9ca4ba4e566a2466d7bfd1c3 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 1 Sep 2019 20:43:49 +1000 Subject: [PATCH 03/11] Removed submodule --- .gitmodules | 3 --- standards | 1 - 2 files changed, 4 deletions(-) delete mode 160000 standards diff --git a/.gitmodules b/.gitmodules index 37ef701cdf..e7972649f4 100644 --- a/.gitmodules +++ b/.gitmodules @@ -2,6 +2,3 @@ path = tests/Images/External url = https://github.com/SixLabors/Imagesharp.Tests.Images.git branch = master -[submodule "standards"] - path = standards - url = https://github.com/SixLabors/Standards diff --git a/standards b/standards deleted file mode 160000 index 8b085c0ec4..0000000000 --- a/standards +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 8b085c0ec4fb64797b9965741f7138f8f66a6b44 From f24169110edcb3bdd05ba09090266ce7f42967b8 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 1 Sep 2019 23:21:54 +1000 Subject: [PATCH 04/11] Update dependencies and submodule --- .editorconfig | 13 +- .gitmodules | 3 + Directory.Build.props | 2 +- Directory.Build.targets | 8 +- ImageSharp.sln | 1 - shared-infrastructure | 1 + src/Directory.Build.props | 4 +- .../ImageSharp.Drawing.csproj | 11 +- src/ImageSharp/Common/Helpers/DebugGuard.cs | 178 +---------- src/ImageSharp/Common/Helpers/Guard.cs | 294 ------------------ src/ImageSharp/ImageSharp.csproj | 13 + src/ImageSharp/Memory/BufferArea{T}.cs | 14 +- stylecop.json | 16 - 13 files changed, 52 insertions(+), 506 deletions(-) create mode 160000 shared-infrastructure delete mode 100644 src/ImageSharp/Common/Helpers/Guard.cs delete mode 100644 stylecop.json diff --git a/.editorconfig b/.editorconfig index 8f0e28eec6..b0d0662bf8 100644 --- a/.editorconfig +++ b/.editorconfig @@ -79,6 +79,7 @@ dotnet_naming_rule.private_and_internal_fields_should_be_camel_case.symbols = pr dotnet_naming_rule.public_and_protected_declarations_should_be_pascal_case.severity = suggestion dotnet_naming_rule.public_and_protected_declarations_should_be_pascal_case.style = pascal_case dotnet_naming_rule.public_and_protected_declarations_should_be_pascal_case.symbols = public_and_protected_declarations +dotnet_naming_symbols.public_and_protected_declarations.applicable_kinds = method, field, event, property dotnet_naming_rule.static_readonly_declarations_should_be_pascal_case.severity = suggestion dotnet_naming_rule.static_readonly_declarations_should_be_pascal_case.style = pascal_case @@ -322,11 +323,11 @@ csharp_space_between_square_brackets = false # suggest conditional delegate calls, # suggest deconstructed variable declarations, # generate expression-bodied accessors, -# don't generate expression-bodied constructors, +# generate expression-bodied constructors, # generate expression-bodied indexers, # generate expression-bodied lambdas, -# don't generate expression-bodied methods, -# don't generate expression-bodied operators, +# generate expression-bodied methods, +# generate expression-bodied operators, # generate expression-bodied properties, # suggest inlined variable declarations, # suggest local over anonymous functions, @@ -348,11 +349,11 @@ csharp_style_conditional_delegate_call = true:suggestion csharp_style_deconstructed_variable_declaration = true:suggestion csharp_style_expression_bodied_accessors = true:silent -csharp_style_expression_bodied_constructors = false:silent +csharp_style_expression_bodied_constructors = true:silent csharp_style_expression_bodied_indexers = true:silent csharp_style_expression_bodied_lambdas = true:silent -csharp_style_expression_bodied_methods = false:silent -csharp_style_expression_bodied_operators = false:silent +csharp_style_expression_bodied_methods = true:silent +csharp_style_expression_bodied_operators = true:silent csharp_style_expression_bodied_properties = true:silent csharp_style_inlined_variable_declaration = true:suggestion diff --git a/.gitmodules b/.gitmodules index e7972649f4..55389121f2 100644 --- a/.gitmodules +++ b/.gitmodules @@ -2,3 +2,6 @@ path = tests/Images/External url = https://github.com/SixLabors/Imagesharp.Tests.Images.git branch = master +[submodule "shared-infrastructure"] + path = shared-infrastructure + url = https://github.com/SixLabors/SharedInfrastructure diff --git a/Directory.Build.props b/Directory.Build.props index bf004921ea..efe4cc9665 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -52,7 +52,7 @@ - $(MSBuildThisFileDirectory)standards/SixLabors.snk + $(MSBuildThisFileDirectory)shared-infrastructure/SixLabors.snk Copyright © Six Labors and Contributors strict;IOperation true diff --git a/Directory.Build.targets b/Directory.Build.targets index d1183e5d46..1551a29d8c 100644 --- a/Directory.Build.targets +++ b/Directory.Build.targets @@ -28,10 +28,10 @@ - - - - + + + + diff --git a/ImageSharp.sln b/ImageSharp.sln index 1fd5e2d8b4..d4a0419eed 100644 --- a/ImageSharp.sln +++ b/ImageSharp.sln @@ -21,7 +21,6 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "SolutionItems", "SolutionIt LICENSE = LICENSE README.md = README.md run-tests.ps1 = run-tests.ps1 - stylecop.json = stylecop.json EndProjectSection EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = ".github", ".github", "{1799C43E-5C54-4A8F-8D64-B1475241DB0D}" diff --git a/shared-infrastructure b/shared-infrastructure new file mode 160000 index 0000000000..faf84e44ec --- /dev/null +++ b/shared-infrastructure @@ -0,0 +1 @@ +Subproject commit faf84e44ec90e8a42a7271bcd04fea76279efb08 diff --git a/src/Directory.Build.props b/src/Directory.Build.props index cd3d5e8cb3..6fbbb7c916 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -18,12 +18,12 @@ - $(MSBuildThisFileDirectory)..\standards\SixLabors.ruleset + $(MSBuildThisFileDirectory)..\shared-infrastructure\SixLabors.ruleset true - + diff --git a/src/ImageSharp.Drawing/ImageSharp.Drawing.csproj b/src/ImageSharp.Drawing/ImageSharp.Drawing.csproj index a092e3604b..5a53d3e78b 100644 --- a/src/ImageSharp.Drawing/ImageSharp.Drawing.csproj +++ b/src/ImageSharp.Drawing/ImageSharp.Drawing.csproj @@ -1,4 +1,4 @@ - + @@ -11,6 +11,15 @@ netcoreapp2.1;netstandard1.3;netstandard2.0 + + + $(DefineConstants);SUPPORTS_MATHF + + + + $(DefineConstants);SUPPORTS_HASHCODE + + diff --git a/src/ImageSharp/Common/Helpers/DebugGuard.cs b/src/ImageSharp/Common/Helpers/DebugGuard.cs index 43eebeac87..356dd419b5 100644 --- a/src/ImageSharp/Common/Helpers/DebugGuard.cs +++ b/src/ImageSharp/Common/Helpers/DebugGuard.cs @@ -1,168 +1,17 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System; using System.Diagnostics; // TODO: These should just call the guard equivalents -namespace SixLabors.ImageSharp +namespace SixLabors { /// /// Provides methods to protect against invalid parameters for a DEBUG build. /// - [DebuggerStepThrough] - internal static class DebugGuard + internal static partial class DebugGuard { - /// - /// Verifies, that the method parameter with specified object value is not null - /// and throws an exception if it is found to be so. - /// - /// The target object, which cannot be null. - /// The name of the parameter that is to be checked. - /// is null - [Conditional("DEBUG")] - public static void NotNull(T value, string parameterName) - where T : class - { - if (value is null) - { - throw new ArgumentNullException(parameterName); - } - } - - /// - /// Verifies that the specified value is less than a maximum value - /// and throws an exception if it is not. - /// - /// The target value, which should be validated. - /// The maximum value. - /// The name of the parameter that is to be checked. - /// The type of the value. - /// - /// is greater than the maximum value. - /// - [Conditional("DEBUG")] - public static void MustBeLessThan(TValue value, TValue max, string parameterName) - where TValue : IComparable - { - if (value.CompareTo(max) >= 0) - { - throw new ArgumentOutOfRangeException(parameterName, $"Value must be less than {max}."); - } - } - - /// - /// Verifies that the specified value is less than or equal to a maximum value - /// and throws an exception if it is not. - /// - /// The target value, which should be validated. - /// The maximum value. - /// The name of the parameter that is to be checked. - /// The type of the value. - /// - /// is greater than the maximum value. - /// - [Conditional("DEBUG")] - public static void MustBeLessThanOrEqualTo(TValue value, TValue max, string parameterName) - where TValue : IComparable - { - if (value.CompareTo(max) > 0) - { - throw new ArgumentOutOfRangeException(parameterName, $"Value must be less than or equal to {max}."); - } - } - - /// - /// Verifies that the specified value is greater than a minimum value - /// and throws an exception if it is not. - /// - /// The target value, which should be validated. - /// The minimum value. - /// The name of the parameter that is to be checked. - /// The type of the value. - /// - /// is less than the minimum value. - /// - [Conditional("DEBUG")] - public static void MustBeGreaterThan(TValue value, TValue min, string parameterName) - where TValue : IComparable - { - if (value.CompareTo(min) <= 0) - { - throw new ArgumentOutOfRangeException( - parameterName, - $"Value must be greater than {min}."); - } - } - - /// - /// Verifies that the specified value is greater than or equal to a minimum value - /// and throws an exception if it is not. - /// - /// The target value, which should be validated. - /// The minimum value. - /// The name of the parameter that is to be checked. - /// The type of the value. - /// - /// is less than the minimum value. - /// - [Conditional("DEBUG")] - public static void MustBeGreaterThanOrEqualTo(TValue value, TValue min, string parameterName) - where TValue : IComparable - { - if (value.CompareTo(min) < 0) - { - throw new ArgumentOutOfRangeException(parameterName, $"Value must be greater than or equal to {min}."); - } - } - - /// - /// Verifies that the specified value is greater than or equal to a minimum value and less than - /// or equal to a maximum value and throws an exception if it is not. - /// - /// The target value, which should be validated. - /// The minimum value. - /// The maximum value. - /// The name of the parameter that is to be checked. - /// The type of the value. - /// - /// is less than the minimum value of greater than the maximum value. - /// - [Conditional("DEBUG")] - public static void MustBeBetweenOrEqualTo(TValue value, TValue min, TValue max, string parameterName) - where TValue : IComparable - { - if (value.CompareTo(min) < 0 || value.CompareTo(max) > 0) - { - throw new ArgumentOutOfRangeException(parameterName, $"Value {value} must be greater than or equal to {min} and less than or equal to {max}."); - } - } - - /// - /// Verifies, that the method parameter with specified target value is true - /// and throws an exception if it is found to be so. - /// - /// - /// The target value, which cannot be false. - /// - /// - /// The name of the parameter that is to be checked. - /// - /// - /// The error message, if any to add to the exception. - /// - /// - /// is false - /// - [Conditional("DEBUG")] - public static void IsTrue(bool target, string parameterName, string message) - { - if (!target) - { - throw new ArgumentException(message, parameterName); - } - } - /// /// Verifies whether a specific condition is met, throwing an exception if it's false. /// @@ -177,25 +26,6 @@ namespace SixLabors.ImageSharp } } - /// - /// Verifies, that the method parameter with specified target value is false - /// and throws an exception if it is found to be so. - /// - /// The target value, which cannot be true. - /// The name of the parameter that is to be checked. - /// The error message, if any to add to the exception. - /// - /// is true - /// - [Conditional("DEBUG")] - public static void IsFalse(bool target, string parameterName, string message) - { - if (target) - { - throw new ArgumentException(message, parameterName); - } - } - /// /// Verifies, that the target span is of same size than the 'other' span. /// @@ -236,4 +66,4 @@ namespace SixLabors.ImageSharp } } } -} \ No newline at end of file +} diff --git a/src/ImageSharp/Common/Helpers/Guard.cs b/src/ImageSharp/Common/Helpers/Guard.cs deleted file mode 100644 index 7dc683c37f..0000000000 --- a/src/ImageSharp/Common/Helpers/Guard.cs +++ /dev/null @@ -1,294 +0,0 @@ -// Copyright (c) Six Labors and contributors. -// Licensed under the Apache License, Version 2.0. - -using System; -using System.Diagnostics; -using System.Runtime.CompilerServices; - -namespace SixLabors.ImageSharp -{ - /// - /// Provides methods to protect against invalid parameters. - /// - [DebuggerStepThrough] - internal static class Guard - { - /// - /// Ensures that the value is not null. - /// - /// The target object, which cannot be null. - /// The name of the parameter that is to be checked. - /// is null - [MethodImpl(InliningOptions.ShortMethod)] - public static void NotNull(T value, string parameterName) - where T : class - { - if (value is null) - { - ThrowArgumentNullException(parameterName); - } - } - - /// - /// Ensures that the target value is not null, empty, or whitespace. - /// - /// The target string, which should be checked against being null or empty. - /// Name of the parameter. - /// is null. - /// is empty or contains only blanks. - [MethodImpl(InliningOptions.ShortMethod)] - public static void NotNullOrWhiteSpace(string value, string parameterName) - { - if (value is null) - { - ThrowArgumentNullException(parameterName); - } - - if (string.IsNullOrWhiteSpace(value)) - { - ThrowArgumentException("Must not be empty or whitespace.", parameterName); - } - } - - /// - /// Ensures that the specified value is less than a maximum value. - /// - /// The target value, which should be validated. - /// The maximum value. - /// The name of the parameter that is to be checked. - /// The type of the value. - /// - /// is greater than the maximum value. - /// - [MethodImpl(InliningOptions.ShortMethod)] - public static void MustBeLessThan(TValue value, TValue max, string parameterName) - where TValue : IComparable - { - if (value.CompareTo(max) >= 0) - { - ThrowArgumentOutOfRangeException(parameterName, $"Value {value} must be less than {max}."); - } - } - - /// - /// Verifies that the specified value is less than or equal to a maximum value - /// and throws an exception if it is not. - /// - /// The target value, which should be validated. - /// The maximum value. - /// The name of the parameter that is to be checked. - /// The type of the value. - /// - /// is greater than the maximum value. - /// - [MethodImpl(InliningOptions.ShortMethod)] - public static void MustBeLessThanOrEqualTo(TValue value, TValue max, string parameterName) - where TValue : IComparable - { - if (value.CompareTo(max) > 0) - { - ThrowArgumentOutOfRangeException(parameterName, $"Value {value} must be less than or equal to {max}."); - } - } - - /// - /// Verifies that the specified value is greater than a minimum value - /// and throws an exception if it is not. - /// - /// The target value, which should be validated. - /// The minimum value. - /// The name of the parameter that is to be checked. - /// The type of the value. - /// - /// is less than the minimum value. - /// - [MethodImpl(InliningOptions.ShortMethod)] - public static void MustBeGreaterThan(TValue value, TValue min, string parameterName) - where TValue : IComparable - { - if (value.CompareTo(min) <= 0) - { - ThrowArgumentOutOfRangeException( - parameterName, - $"Value {value} must be greater than {min}."); - } - } - - /// - /// Verifies that the specified value is greater than or equal to a minimum value - /// and throws an exception if it is not. - /// - /// The target value, which should be validated. - /// The minimum value. - /// The name of the parameter that is to be checked. - /// The type of the value. - /// - /// is less than the minimum value. - /// - [MethodImpl(InliningOptions.ShortMethod)] - public static void MustBeGreaterThanOrEqualTo(TValue value, TValue min, string parameterName) - where TValue : IComparable - { - if (value.CompareTo(min) < 0) - { - ThrowArgumentOutOfRangeException(parameterName, $"Value {value} must be greater than or equal to {min}."); - } - } - - /// - /// Verifies that the specified value is greater than or equal to a minimum value and less than - /// or equal to a maximum value and throws an exception if it is not. - /// - /// The target value, which should be validated. - /// The minimum value. - /// The maximum value. - /// The name of the parameter that is to be checked. - /// The type of the value. - /// - /// is less than the minimum value of greater than the maximum value. - /// - [MethodImpl(InliningOptions.ShortMethod)] - public static void MustBeBetweenOrEqualTo(TValue value, TValue min, TValue max, string parameterName) - where TValue : IComparable - { - if (value.CompareTo(min) < 0 || value.CompareTo(max) > 0) - { - ThrowArgumentOutOfRangeException(parameterName, $"Value {value} must be greater than or equal to {min} and less than or equal to {max}."); - } - } - - /// - /// Verifies, that the method parameter with specified target value is true - /// and throws an exception if it is found to be so. - /// - /// The target value, which cannot be false. - /// The name of the parameter that is to be checked. - /// The error message, if any to add to the exception. - /// - /// is false - /// - [MethodImpl(InliningOptions.ShortMethod)] - public static void IsTrue(bool target, string parameterName, string message) - { - if (!target) - { - ThrowArgumentException(message, parameterName); - } - } - - /// - /// Verifies, that the method parameter with specified target value is false - /// and throws an exception if it is found to be so. - /// - /// The target value, which cannot be true. - /// The name of the parameter that is to be checked. - /// The error message, if any to add to the exception. - /// - /// is true - /// - [MethodImpl(InliningOptions.ShortMethod)] - public static void IsFalse(bool target, string parameterName, string message) - { - if (target) - { - ThrowArgumentException(message, parameterName); - } - } - - /// - /// Verifies, that the `source` span has the length of 'minLength', or longer. - /// - /// The element type of the spans - /// The source span. - /// The minimum length. - /// The name of the parameter that is to be checked. - /// - /// has less than items - /// - [MethodImpl(InliningOptions.ShortMethod)] - public static void MustBeSizedAtLeast(ReadOnlySpan source, int minLength, string parameterName) - { - if (source.Length < minLength) - { - ThrowArgumentException($"Span-s must be at least of length {minLength}!", parameterName); - } - } - - /// - /// Verifies that the 'destination' span is not shorter than 'source'. - /// - /// The source element type - /// The destination element type - /// The source span - /// The destination span - /// The name of the argument for 'destination' - [MethodImpl(InliningOptions.ShortMethod)] - public static void DestinationShouldNotBeTooShort( - ReadOnlySpan source, - Span destination, - string destinationParamName) - { - if (destination.Length < source.Length) - { - ThrowArgumentException("Destination span is too short!", destinationParamName); - } - } - - /// - /// Verifies that the 'destination' span is not shorter than 'source'. - /// - /// The source element type - /// The destination element type - /// The source span - /// The destination span - /// The name of the argument for 'destination' - [MethodImpl(InliningOptions.ShortMethod)] - public static void DestinationShouldNotBeTooShort( - Span source, - Span destination, - string destinationParamName) - { - if (destination.Length < source.Length) - { - ThrowArgumentException("Destination span is too short!", destinationParamName); - } - } - - /// - /// Verifies, that the `source` span has the length of 'minLength', or longer. - /// - /// The element type of the spans - /// The target span. - /// The minimum length. - /// The name of the parameter that is to be checked. - /// - /// has less than items - /// - [MethodImpl(InliningOptions.ShortMethod)] - public static void MustBeSizedAtLeast(Span source, int minLength, string parameterName) - { - if (source.Length < minLength) - { - ThrowArgumentException($"Span-s must be at least of length {minLength}!", parameterName); - } - } - - [MethodImpl(InliningOptions.ColdPath)] - private static void ThrowArgumentException(string message, string parameterName) - { - throw new ArgumentException(message, parameterName); - } - - [MethodImpl(InliningOptions.ColdPath)] - private static void ThrowArgumentOutOfRangeException(string parameterName, string message) - { - throw new ArgumentOutOfRangeException(parameterName, message); - } - - [MethodImpl(InliningOptions.ColdPath)] - private static void ThrowArgumentNullException(string parameterName) - { - throw new ArgumentNullException(parameterName); - } - } -} diff --git a/src/ImageSharp/ImageSharp.csproj b/src/ImageSharp/ImageSharp.csproj index 8dff3b9779..86b0848663 100644 --- a/src/ImageSharp/ImageSharp.csproj +++ b/src/ImageSharp/ImageSharp.csproj @@ -19,6 +19,15 @@ SixLabors.ImageSharp + + + $(DefineConstants);SUPPORTS_MATHF + + + + $(DefineConstants);SUPPORTS_HASHCODE + + $(DefineConstants);SUPPORTS_EXTENDED_INTRINSICS @@ -27,6 +36,10 @@ + + + + True diff --git a/src/ImageSharp/Memory/BufferArea{T}.cs b/src/ImageSharp/Memory/BufferArea{T}.cs index f71a281390..38f0b8129d 100644 --- a/src/ImageSharp/Memory/BufferArea{T}.cs +++ b/src/ImageSharp/Memory/BufferArea{T}.cs @@ -23,10 +23,10 @@ namespace SixLabors.ImageSharp.Memory [MethodImpl(MethodImplOptions.AggressiveInlining)] public BufferArea(Buffer2D destinationBuffer, Rectangle rectangle) { - ImageSharp.DebugGuard.MustBeGreaterThanOrEqualTo(rectangle.X, 0, nameof(rectangle)); - ImageSharp.DebugGuard.MustBeGreaterThanOrEqualTo(rectangle.Y, 0, nameof(rectangle)); - ImageSharp.DebugGuard.MustBeLessThanOrEqualTo(rectangle.Width, destinationBuffer.Width, nameof(rectangle)); - ImageSharp.DebugGuard.MustBeLessThanOrEqualTo(rectangle.Height, destinationBuffer.Height, nameof(rectangle)); + DebugGuard.MustBeGreaterThanOrEqualTo(rectangle.X, 0, nameof(rectangle)); + DebugGuard.MustBeGreaterThanOrEqualTo(rectangle.Y, 0, nameof(rectangle)); + DebugGuard.MustBeLessThanOrEqualTo(rectangle.Width, destinationBuffer.Width, nameof(rectangle)); + DebugGuard.MustBeLessThanOrEqualTo(rectangle.Height, destinationBuffer.Height, nameof(rectangle)); this.DestinationBuffer = destinationBuffer; this.Rectangle = rectangle; @@ -122,8 +122,8 @@ namespace SixLabors.ImageSharp.Memory [MethodImpl(MethodImplOptions.AggressiveInlining)] public BufferArea GetSubArea(Rectangle rectangle) { - ImageSharp.DebugGuard.MustBeLessThanOrEqualTo(rectangle.Width, this.Rectangle.Width, nameof(rectangle)); - ImageSharp.DebugGuard.MustBeLessThanOrEqualTo(rectangle.Height, this.Rectangle.Height, nameof(rectangle)); + DebugGuard.MustBeLessThanOrEqualTo(rectangle.Width, this.Rectangle.Width, nameof(rectangle)); + DebugGuard.MustBeLessThanOrEqualTo(rectangle.Height, this.Rectangle.Height, nameof(rectangle)); int x = this.Rectangle.X + rectangle.X; int y = this.Rectangle.Y + rectangle.Y; @@ -161,4 +161,4 @@ namespace SixLabors.ImageSharp.Memory } } } -} \ No newline at end of file +} diff --git a/stylecop.json b/stylecop.json deleted file mode 100644 index 485ab604a5..0000000000 --- a/stylecop.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "$schema": "https://raw.githubusercontent.com/DotNetAnalyzers/StyleCopAnalyzers/master/StyleCop.Analyzers/StyleCop.Analyzers/Settings/stylecop.schema.json", - "settings": { - "orderingRules": { - "usingDirectivesPlacement": "outsideNamespace", - "elementOrder": [ - "kind" - ] - }, - "documentationRules": { - "xmlHeader": false, - "documentInternalElements": false, - "copyrightText": "Copyright (c) Six Labors and contributors.\nLicensed under the Apache License, Version 2.0." - } - } -} \ No newline at end of file From cca018dd0cddc4a650e5ede2c2f33bf9be0d23dd Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Sun, 1 Sep 2019 23:11:12 +0200 Subject: [PATCH 05/11] delete GuardTests --- tests/ImageSharp.Tests/Helpers/GuardTests.cs | 274 ------------------- 1 file changed, 274 deletions(-) delete mode 100644 tests/ImageSharp.Tests/Helpers/GuardTests.cs diff --git a/tests/ImageSharp.Tests/Helpers/GuardTests.cs b/tests/ImageSharp.Tests/Helpers/GuardTests.cs deleted file mode 100644 index 6bccea2c3e..0000000000 --- a/tests/ImageSharp.Tests/Helpers/GuardTests.cs +++ /dev/null @@ -1,274 +0,0 @@ -// Copyright (c) Six Labors and contributors. -// Licensed under the Apache License, Version 2.0. - -using System; -using System.Diagnostics.CodeAnalysis; - -using Xunit; -// ReSharper disable InconsistentNaming - -namespace SixLabors.ImageSharp.Tests.Helpers -{ - /// - /// Tests the helper. - /// - public class GuardTests - { - class Test - { - } - - [Theory] - [InlineData(0, 0)] - [InlineData(0, 1)] - [InlineData(0, 42)] - [InlineData(1, 1)] - [InlineData(10, 42)] - [InlineData(42, 42)] - public void DestinationShouldNotBeTooShort_WhenOk(int sourceLength, int destLength) - { - ReadOnlySpan source = new int[sourceLength]; - Span dest = new float[destLength]; - - Guard.DestinationShouldNotBeTooShort(source, dest, nameof(dest)); - } - - [Theory] - [InlineData(1, 0)] - [InlineData(42, 41)] - public void DestinationShouldNotBeTooShort_WhenThrows(int sourceLength, int destLength) - { - Assert.ThrowsAny( - () => - { - ReadOnlySpan source = new int[sourceLength]; - Span dest = new float[destLength]; - Guard.DestinationShouldNotBeTooShort(source, dest, nameof(dest)); - }); - } - - /// - /// Tests that the method throws when the argument is null. - /// - [Fact] - public void NotNullThrowsWhenArgIsNull() - { - Assert.Throws(() => Guard.NotNull((Test)null, "foo")); - } - - /// - /// Tests that the method throws when the argument name is empty. - /// - [Fact] - public void NotNullThrowsWhenArgNameEmpty() - { - Assert.Throws(() => Guard.NotNull((Test)null, string.Empty)); - } - - /// - /// Tests that the method throws when the argument is empty. - /// - [Fact] - [SuppressMessage("StyleCop.CSharp.ReadabilityRules", "SA1122:UseStringEmptyForEmptyStrings", Justification = "Reviewed. Suppression is OK here.")] - public void NotEmptyOrWhiteSpaceThrowsWhenEmpty() - { - Assert.Throws(() => Guard.NotNullOrWhiteSpace("", string.Empty)); - } - - /// - /// Tests that the method throws when the argument is whitespace. - /// - [Fact] - public void NotEmptyOrWhiteSpaceThrowsOnWhitespace() - { - Assert.Throws(() => Guard.NotNullOrWhiteSpace(" ", string.Empty)); - } - - /// - /// Tests that the method throws when the argument name is null. - /// - [Fact] - public void NotEmptyOrWhiteSpaceThrowsWhenParameterNameNull() - { - Assert.Throws(() => Guard.NotNullOrWhiteSpace(null, null)); - } - - /// - /// Tests that the method throws when the argument is greater. - /// - [Fact] - public void LessThanThrowsWhenArgIsGreater() - { - Assert.Throws(() => Guard.MustBeLessThan(1, 0, "foo")); - } - - /// - /// Tests that the method throws when the argument is equal. - /// - [Fact] - public void LessThanThrowsWhenArgIsEqual() - { - Assert.Throws(() => Guard.MustBeLessThan(1, 1, "foo")); - } - - /// - /// Tests that the method throws when the argument is greater. - /// - [Fact] - public void LessThanOrEqualToThrowsWhenArgIsGreater() - { - Assert.Throws(() => Guard.MustBeLessThanOrEqualTo(1, 0, "foo")); - } - - /// - /// Tests that the method does not throw when the argument - /// is less. - /// - [Fact] - public void LessThanOrEqualToDoesNotThrowWhenArgIsLess() - { - Exception ex = Record.Exception(() => Guard.MustBeLessThanOrEqualTo(0, 1, "foo")); - Assert.Null(ex); - } - - /// - /// Tests that the method does not throw when the argument - /// is equal. - /// - [Fact] - public void LessThanOrEqualToDoesNotThrowWhenArgIsEqual() - { - Exception ex = Record.Exception(() => Guard.MustBeLessThanOrEqualTo(1, 1, "foo")); - Assert.Equal(1, 1); - Assert.Null(ex); - } - - /// - /// Tests that the method throws when the argument is greater. - /// - [Fact] - public void GreaterThanThrowsWhenArgIsLess() - { - Assert.Throws(() => Guard.MustBeGreaterThan(0, 1, "foo")); - } - - /// - /// Tests that the method throws when the argument is greater. - /// - [Fact] - public void GreaterThanThrowsWhenArgIsEqual() - { - Assert.Throws(() => Guard.MustBeGreaterThan(1, 1, "foo")); - } - - /// - /// Tests that the method throws when the argument name is greater. - /// - [Fact] - public void GreaterThanOrEqualToThrowsWhenArgIsLess() - { - Assert.Throws(() => Guard.MustBeGreaterThanOrEqualTo(0, 1, "foo")); - } - - /// - /// Tests that the method does not throw when the argument - /// is less. - /// - [Fact] - public void GreaterThanOrEqualToDoesNotThrowWhenArgIsGreater() - { - Exception ex = Record.Exception(() => Guard.MustBeGreaterThanOrEqualTo(1, 0, "foo")); - Assert.Null(ex); - } - - /// - /// Tests that the method does not throw when the argument - /// is equal. - /// - [Fact] - public void GreaterThanOrEqualToDoesNotThrowWhenArgIsEqual() - { - Exception ex = Record.Exception(() => Guard.MustBeGreaterThanOrEqualTo(1, 1, "foo")); - Assert.Equal(1, 1); - Assert.Null(ex); - } - - /// - /// Tests that the method throws when the argument is less. - /// - [Fact] - public void BetweenOrEqualToThrowsWhenArgIsLess() - { - Assert.Throws(() => Guard.MustBeBetweenOrEqualTo(-2, -1, 1, "foo")); - } - - /// - /// Tests that the method throws when the argument is greater. - /// - [Fact] - public void BetweenOrEqualToThrowsWhenArgIsGreater() - { - Assert.Throws(() => Guard.MustBeBetweenOrEqualTo(2, -1, 1, "foo")); - } - - /// - /// Tests that the method does not throw when the argument - /// is equal. - /// - [Fact] - public void BetweenOrEqualToDoesNotThrowWhenArgIsEqual() - { - Exception ex = Record.Exception(() => Guard.MustBeBetweenOrEqualTo(1, 1, 1, "foo")); - Assert.Null(ex); - } - - /// - /// Tests that the method does not throw when the argument - /// is equal. - /// - [Fact] - public void BetweenOrEqualToDoesNotThrowWhenArgIsBetween() - { - Exception ex = Record.Exception(() => Guard.MustBeBetweenOrEqualTo(0, -1, 1, "foo")); - Assert.Null(ex); - } - - /// - /// Tests that the method throws when the argument is false. - /// - [Fact] - public void IsTrueThrowsWhenArgIsFalse() - { - Assert.Throws(() => Guard.IsTrue(false, "foo", "message")); - } - - /// - /// Tests that the method does not throw when the argument is true. - /// - [Fact] - public void IsTrueDoesThrowsWhenArgIsTrue() - { - Exception ex = Record.Exception(() => Guard.IsTrue(true, "foo", "message")); - Assert.Null(ex); - } - - /// - /// Tests that the method throws when the argument is true. - /// - [Fact] - public void IsFalseThrowsWhenArgIsFalse() - { - Assert.Throws(() => Guard.IsFalse(true, "foo", "message")); - } - - /// - /// Tests that the method does not throw when the argument is false. - /// - [Fact] - public void IsFalseDoesThrowsWhenArgIsTrue() - { - Exception ex = Record.Exception(() => Guard.IsFalse(false, "foo", "message")); - Assert.Null(ex); - } - } -} \ No newline at end of file From 34c5b5a16c4424c7362ac26a68bdec7267629f0f Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 7 Sep 2019 12:04:51 +1000 Subject: [PATCH 06/11] Fix #997 --- .../Implementation/Converters/HslAndRgbConverter.cs | 4 ++-- .../Colorspaces/Conversion/RgbAndHslConversionTest.cs | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/ImageSharp/ColorSpaces/Conversion/Implementation/Converters/HslAndRgbConverter.cs b/src/ImageSharp/ColorSpaces/Conversion/Implementation/Converters/HslAndRgbConverter.cs index 761313b7e0..97465e526a 100644 --- a/src/ImageSharp/ColorSpaces/Conversion/Implementation/Converters/HslAndRgbConverter.cs +++ b/src/ImageSharp/ColorSpaces/Conversion/Implementation/Converters/HslAndRgbConverter.cs @@ -96,7 +96,7 @@ namespace SixLabors.ImageSharp.ColorSpaces.Conversion.Implementation } else { - s = chroma / (2F - chroma); + s = chroma / (2F - max - min); } return new Hsl(h, s, l); @@ -157,4 +157,4 @@ namespace SixLabors.ImageSharp.ColorSpaces.Conversion.Implementation return value; } } -} \ No newline at end of file +} diff --git a/tests/ImageSharp.Tests/Colorspaces/Conversion/RgbAndHslConversionTest.cs b/tests/ImageSharp.Tests/Colorspaces/Conversion/RgbAndHslConversionTest.cs index 502df84133..8b1fed84c2 100644 --- a/tests/ImageSharp.Tests/Colorspaces/Conversion/RgbAndHslConversionTest.cs +++ b/tests/ImageSharp.Tests/Colorspaces/Conversion/RgbAndHslConversionTest.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; @@ -65,6 +65,7 @@ namespace SixLabors.ImageSharp.Tests.Colorspaces.Conversion [InlineData(1, 0, 0, 0, 1, .5F)] [InlineData(0, 1, 0, 120, 1, .5F)] [InlineData(0, 0, 1, 240, 1, .5F)] + [InlineData(0.7, 0.8, 0.6, 90, 0.3333, 0.7F)] public void Convert_Rgb_To_Hsl(float r, float g, float b, float h, float s, float l) { // Arrange From 2a5dd6bc7644d21662549d9a910588b8537e6282 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 8 Sep 2019 13:49:47 +1000 Subject: [PATCH 07/11] Fix #999 and add tests --- .../Transforms/Resize/ResizeHelper.cs | 228 ++++++++---------- .../Transforms/Resize/ResizeProcessor.cs | 43 +--- .../Resize/ResizeProcessor{TPixel}.cs | 14 +- src/ImageSharp/Processing/ResizeOptions.cs | 8 +- .../Transforms/ResizeHelperTests.cs | 143 +++++++++-- .../Processing/Transforms/PadTest.cs | 4 +- .../Processing/Transforms/ResizeTests.cs | 20 +- tests/ImageSharp.Tests/xunit.runner.json | 9 +- 8 files changed, 268 insertions(+), 201 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeHelper.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeHelper.cs index ae7b112fc1..c9df1b2542 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeHelper.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeHelper.cs @@ -2,9 +2,7 @@ // Licensed under the Apache License, Version 2.0. using System; -using System.Linq; using System.Numerics; - using SixLabors.Primitives; namespace SixLabors.ImageSharp.Processing.Processors.Transforms @@ -30,17 +28,32 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// /// The source image size. /// The resize options. - /// The target width - /// The target height /// /// The tuple representing the location and the bounds /// - public static (Size, Rectangle) CalculateTargetLocationAndBounds( - Size sourceSize, - ResizeOptions options, - int width, - int height) + public static (Size, Rectangle) CalculateTargetLocationAndBounds(Size sourceSize, ResizeOptions options) { + int width = options.Size.Width; + int height = options.Size.Height; + + // Ensure target size is populated across both dimensions. + // These dimensions are used to calculate the final dimensions determined by the mode algorithm. + // If only one of the incoming dimensions is 0, it will be modified here to maintain aspect ratio. + // If it is not possible to keep aspect ratio, make sure at least the minimum is is kept. + const int Min = 1; + if (width == 0 && height > 0) + { + width = (int)MathF.Max(Min, MathF.Round(sourceSize.Width * height / (float)sourceSize.Height)); + } + + if (height == 0 && width > 0) + { + height = (int)MathF.Max(Min, MathF.Round(sourceSize.Height * width / (float)sourceSize.Width)); + } + + Guard.MustBeGreaterThan(width, 0, nameof(width)); + Guard.MustBeGreaterThan(height, 0, nameof(height)); + switch (options.Mode) { case ResizeMode.Crop: @@ -50,9 +63,9 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms case ResizeMode.BoxPad: return CalculateBoxPadRectangle(sourceSize, options, width, height); case ResizeMode.Max: - return CalculateMaxRectangle(sourceSize, options, width, height); + return CalculateMaxRectangle(sourceSize, width, height); case ResizeMode.Min: - return CalculateMinRectangle(sourceSize, options, width, height); + return CalculateMinRectangle(sourceSize, width, height); // Last case ResizeMode.Stretch: default: @@ -66,11 +79,6 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms int width, int height) { - if (width <= 0 || height <= 0) - { - return (new Size(source.Width, source.Height), new Rectangle(0, 0, source.Width, source.Height)); - } - int sourceWidth = source.Width; int sourceHeight = source.Height; @@ -84,55 +92,55 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms // Only calculate if upscaling. if (sourceWidth < boxPadWidth && sourceHeight < boxPadHeight) { - int destinationX; - int destinationY; - int destinationWidth = sourceWidth; - int destinationHeight = sourceHeight; + int targetX; + int targetY; + int targetWidth = sourceWidth; + int targetHeight = sourceHeight; width = boxPadWidth; height = boxPadHeight; switch (options.Position) { case AnchorPositionMode.Left: - destinationY = (height - sourceHeight) / 2; - destinationX = 0; + targetY = (height - sourceHeight) / 2; + targetX = 0; break; case AnchorPositionMode.Right: - destinationY = (height - sourceHeight) / 2; - destinationX = width - sourceWidth; + targetY = (height - sourceHeight) / 2; + targetX = width - sourceWidth; break; case AnchorPositionMode.TopRight: - destinationY = 0; - destinationX = width - sourceWidth; + targetY = 0; + targetX = width - sourceWidth; break; case AnchorPositionMode.Top: - destinationY = 0; - destinationX = (width - sourceWidth) / 2; + targetY = 0; + targetX = (width - sourceWidth) / 2; break; case AnchorPositionMode.TopLeft: - destinationY = 0; - destinationX = 0; + targetY = 0; + targetX = 0; break; case AnchorPositionMode.BottomRight: - destinationY = height - sourceHeight; - destinationX = width - sourceWidth; + targetY = height - sourceHeight; + targetX = width - sourceWidth; break; case AnchorPositionMode.Bottom: - destinationY = height - sourceHeight; - destinationX = (width - sourceWidth) / 2; + targetY = height - sourceHeight; + targetX = (width - sourceWidth) / 2; break; case AnchorPositionMode.BottomLeft: - destinationY = height - sourceHeight; - destinationX = 0; + targetY = height - sourceHeight; + targetX = 0; break; default: - destinationY = (height - sourceHeight) / 2; - destinationX = (width - sourceWidth) / 2; + targetY = (height - sourceHeight) / 2; + targetX = (width - sourceWidth) / 2; break; } - return (new Size(width, height), - new Rectangle(destinationX, destinationY, destinationWidth, destinationHeight)); + // Target image width and height can be different to the rectangle width and height. + return (new Size(width, height), new Rectangle(targetX, targetY, targetWidth, targetHeight)); } // Switch to pad mode to downscale and calculate from there. @@ -145,19 +153,14 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms int width, int height) { - if (width <= 0 || height <= 0) - { - return (new Size(source.Width, source.Height), new Rectangle(0, 0, source.Width, source.Height)); - } - float ratio; int sourceWidth = source.Width; int sourceHeight = source.Height; - int destinationX = 0; - int destinationY = 0; - int destinationWidth = width; - int destinationHeight = height; + int targetX = 0; + int targetY = 0; + int targetWidth = width; + int targetHeight = height; // Fractional variants for preserving aspect ratio. float percentHeight = MathF.Abs(height / (float)sourceHeight); @@ -167,19 +170,19 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms { ratio = percentWidth; - if (options.CenterCoordinates.Any()) + if (options.CenterCoordinates.HasValue) { - float center = -(ratio * sourceHeight) * options.CenterCoordinates.ToArray()[1]; - destinationY = (int)MathF.Round(center + (height / 2F)); + float center = -(ratio * sourceHeight) * options.CenterCoordinates.Value.Y; + targetY = (int)MathF.Round(center + (height / 2F)); - if (destinationY > 0) + if (targetY > 0) { - destinationY = 0; + targetY = 0; } - if (destinationY < (int)MathF.Round(height - (sourceHeight * ratio))) + if (targetY < (int)MathF.Round(height - (sourceHeight * ratio))) { - destinationY = (int)MathF.Round(height - (sourceHeight * ratio)); + targetY = (int)MathF.Round(height - (sourceHeight * ratio)); } } else @@ -189,38 +192,38 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms case AnchorPositionMode.Top: case AnchorPositionMode.TopLeft: case AnchorPositionMode.TopRight: - destinationY = 0; + targetY = 0; break; case AnchorPositionMode.Bottom: case AnchorPositionMode.BottomLeft: case AnchorPositionMode.BottomRight: - destinationY = (int)MathF.Round(height - (sourceHeight * ratio)); + targetY = (int)MathF.Round(height - (sourceHeight * ratio)); break; default: - destinationY = (int)MathF.Round((height - (sourceHeight * ratio)) / 2F); + targetY = (int)MathF.Round((height - (sourceHeight * ratio)) / 2F); break; } } - destinationHeight = (int)MathF.Ceiling(sourceHeight * percentWidth); + targetHeight = (int)MathF.Ceiling(sourceHeight * percentWidth); } else { ratio = percentHeight; - if (options.CenterCoordinates.Any()) + if (options.CenterCoordinates.HasValue) { - float center = -(ratio * sourceWidth) * options.CenterCoordinates.First(); - destinationX = (int)MathF.Round(center + (width / 2F)); + float center = -(ratio * sourceWidth) * options.CenterCoordinates.Value.X; + targetX = (int)MathF.Round(center + (width / 2F)); - if (destinationX > 0) + if (targetX > 0) { - destinationX = 0; + targetX = 0; } - if (destinationX < (int)MathF.Round(width - (sourceWidth * ratio))) + if (targetX < (int)MathF.Round(width - (sourceWidth * ratio))) { - destinationX = (int)MathF.Round(width - (sourceWidth * ratio)); + targetX = (int)MathF.Round(width - (sourceWidth * ratio)); } } else @@ -230,68 +233,64 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms case AnchorPositionMode.Left: case AnchorPositionMode.TopLeft: case AnchorPositionMode.BottomLeft: - destinationX = 0; + targetX = 0; break; case AnchorPositionMode.Right: case AnchorPositionMode.TopRight: case AnchorPositionMode.BottomRight: - destinationX = (int)MathF.Round(width - (sourceWidth * ratio)); + targetX = (int)MathF.Round(width - (sourceWidth * ratio)); break; default: - destinationX = (int)MathF.Round((width - (sourceWidth * ratio)) / 2F); + targetX = (int)MathF.Round((width - (sourceWidth * ratio)) / 2F); break; } } - destinationWidth = (int)MathF.Ceiling(sourceWidth * percentHeight); + targetWidth = (int)MathF.Ceiling(sourceWidth * percentHeight); } - return (new Size(width, height), - new Rectangle(destinationX, destinationY, destinationWidth, destinationHeight)); + // Target image width and height can be different to the rectangle width and height. + return (new Size(width, height), new Rectangle(targetX, targetY, targetWidth, targetHeight)); } private static (Size, Rectangle) CalculateMaxRectangle( Size source, - ResizeOptions options, int width, int height) { - int destinationWidth = width; - int destinationHeight = height; + int targetWidth = width; + int targetHeight = height; // Fractional variants for preserving aspect ratio. float percentHeight = MathF.Abs(height / (float)source.Height); float percentWidth = MathF.Abs(width / (float)source.Width); // Integers must be cast to floats to get needed precision - float ratio = options.Size.Height / (float)options.Size.Width; + float ratio = height / (float)width; float sourceRatio = source.Height / (float)source.Width; if (sourceRatio < ratio) { - destinationHeight = (int)MathF.Round(source.Height * percentWidth); - height = destinationHeight; + targetHeight = (int)MathF.Round(source.Height * percentWidth); } else { - destinationWidth = (int)MathF.Round(source.Width * percentHeight); - width = destinationWidth; + targetWidth = (int)MathF.Round(source.Width * percentHeight); } // Replace the size to match the rectangle. - return (new Size(width, height), new Rectangle(0, 0, destinationWidth, destinationHeight)); + return (new Size(targetWidth, targetHeight), new Rectangle(0, 0, targetWidth, targetHeight)); } private static (Size, Rectangle) CalculateMinRectangle( Size source, - ResizeOptions options, int width, int height) { int sourceWidth = source.Width; int sourceHeight = source.Height; - int destinationWidth; - int destinationHeight; + int targetWidth = width; + int targetHeight = height; // Don't upscale if (width > sourceWidth || height > sourceHeight) @@ -306,58 +305,45 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms if (widthDiff < heightDiff) { float sourceRatio = (float)sourceHeight / sourceWidth; - destinationHeight = (int)MathF.Round(width * sourceRatio); - height = destinationHeight; - destinationWidth = width; + targetHeight = (int)MathF.Round(width * sourceRatio); } else if (widthDiff > heightDiff) { float sourceRatioInverse = (float)sourceWidth / sourceHeight; - destinationWidth = (int)MathF.Round(height * sourceRatioInverse); - destinationHeight = height; - width = destinationWidth; + targetWidth = (int)MathF.Round(height * sourceRatioInverse); } else { if (height > width) { - destinationWidth = width; float percentWidth = MathF.Abs(width / (float)sourceWidth); - destinationHeight = (int)MathF.Round(sourceHeight * percentWidth); - height = destinationHeight; + targetHeight = (int)MathF.Round(sourceHeight * percentWidth); } else { - destinationHeight = height; float percentHeight = MathF.Abs(height / (float)sourceHeight); - destinationWidth = (int)MathF.Round(sourceWidth * percentHeight); - width = destinationWidth; + targetWidth = (int)MathF.Round(sourceWidth * percentHeight); } } // Replace the size to match the rectangle. - return (new Size(width, height), new Rectangle(0, 0, destinationWidth, destinationHeight)); + return (new Size(targetWidth, targetHeight), new Rectangle(0, 0, targetWidth, targetHeight)); } private static (Size, Rectangle) CalculatePadRectangle( - Size source, + Size sourceSize, ResizeOptions options, int width, int height) { - if (width <= 0 || height <= 0) - { - return (new Size(source.Width, source.Height), new Rectangle(0, 0, source.Width, source.Height)); - } - float ratio; - int sourceWidth = source.Width; - int sourceHeight = source.Height; + int sourceWidth = sourceSize.Width; + int sourceHeight = sourceSize.Height; - int destinationX = 0; - int destinationY = 0; - int destinationWidth = width; - int destinationHeight = height; + int targetX = 0; + int targetY = 0; + int targetWidth = width; + int targetHeight = height; // Fractional variants for preserving aspect ratio. float percentHeight = MathF.Abs(height / (float)sourceHeight); @@ -366,50 +352,50 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms if (percentHeight < percentWidth) { ratio = percentHeight; - destinationWidth = (int)MathF.Round(sourceWidth * percentHeight); + targetWidth = (int)MathF.Round(sourceWidth * percentHeight); switch (options.Position) { case AnchorPositionMode.Left: case AnchorPositionMode.TopLeft: case AnchorPositionMode.BottomLeft: - destinationX = 0; + targetX = 0; break; case AnchorPositionMode.Right: case AnchorPositionMode.TopRight: case AnchorPositionMode.BottomRight: - destinationX = (int)MathF.Round(width - (sourceWidth * ratio)); + targetX = (int)MathF.Round(width - (sourceWidth * ratio)); break; default: - destinationX = (int)MathF.Round((width - (sourceWidth * ratio)) / 2F); + targetX = (int)MathF.Round((width - (sourceWidth * ratio)) / 2F); break; } } else { ratio = percentWidth; - destinationHeight = (int)MathF.Round(sourceHeight * percentWidth); + targetHeight = (int)MathF.Round(sourceHeight * percentWidth); switch (options.Position) { case AnchorPositionMode.Top: case AnchorPositionMode.TopLeft: case AnchorPositionMode.TopRight: - destinationY = 0; + targetY = 0; break; case AnchorPositionMode.Bottom: case AnchorPositionMode.BottomLeft: case AnchorPositionMode.BottomRight: - destinationY = (int)MathF.Round(height - (sourceHeight * ratio)); + targetY = (int)MathF.Round(height - (sourceHeight * ratio)); break; default: - destinationY = (int)MathF.Round((height - (sourceHeight * ratio)) / 2F); + targetY = (int)MathF.Round((height - (sourceHeight * ratio)) / 2F); break; } } - return (new Size(width, height), - new Rectangle(destinationX, destinationY, destinationWidth, destinationHeight)); + // Target image width and height can be different to the rectangle width and height. + return (new Size(width, height), new Rectangle(targetX, targetY, targetWidth, targetHeight)); } } -} \ No newline at end of file +} diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs index cf27de5eb1..6f5f09e717 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs @@ -26,19 +26,19 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms { Guard.NotNull(sampler, nameof(sampler)); - // Ensure size is populated across both dimensions. + // Ensure target size is populated across both dimensions. // If only one of the incoming dimensions is 0, it will be modified here to maintain aspect ratio. // If it is not possible to keep aspect ratio, make sure at least the minimum is is kept. - const int min = 1; + const int Min = 1; if (width == 0 && height > 0) { - width = (int)MathF.Max(min, MathF.Round(sourceSize.Width * height / (float)sourceSize.Height)); + width = (int)MathF.Max(Min, MathF.Round(sourceSize.Width * height / (float)sourceSize.Height)); targetRectangle.Width = width; } if (height == 0 && width > 0) { - height = (int)MathF.Max(min, MathF.Round(sourceSize.Height * width / (float)sourceSize.Width)); + height = (int)MathF.Max(Min, MathF.Round(sourceSize.Height * width / (float)sourceSize.Width)); targetRectangle.Height = height; } @@ -46,8 +46,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms Guard.MustBeGreaterThan(height, 0, nameof(height)); this.Sampler = sampler; - this.Width = width; - this.Height = height; + this.TargetWidth = width; + this.TargetHeight = height; this.TargetRectangle = targetRectangle; this.Compand = compand; } @@ -62,32 +62,11 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms Guard.NotNull(options, nameof(options)); Guard.NotNull(options.Sampler, nameof(options.Sampler)); - int targetWidth = options.Size.Width; - int targetHeight = options.Size.Height; - - // Ensure size is populated across both dimensions. - // These dimensions are used to calculate the final dimensions determined by the mode algorithm. - // If only one of the incoming dimensions is 0, it will be modified here to maintain aspect ratio. - // If it is not possible to keep aspect ratio, make sure at least the minimum is is kept. - const int min = 1; - if (targetWidth == 0 && targetHeight > 0) - { - targetWidth = (int)MathF.Max(min, MathF.Round(sourceSize.Width * targetHeight / (float)sourceSize.Height)); - } - - if (targetHeight == 0 && targetWidth > 0) - { - targetHeight = (int)MathF.Max(min, MathF.Round(sourceSize.Height * targetWidth / (float)sourceSize.Width)); - } - - Guard.MustBeGreaterThan(targetWidth, 0, nameof(targetWidth)); - Guard.MustBeGreaterThan(targetHeight, 0, nameof(targetHeight)); - - (Size size, Rectangle rectangle) = ResizeHelper.CalculateTargetLocationAndBounds(sourceSize, options, targetWidth, targetHeight); + (Size size, Rectangle rectangle) = ResizeHelper.CalculateTargetLocationAndBounds(sourceSize, options); this.Sampler = options.Sampler; - this.Width = size.Width; - this.Height = size.Height; + this.TargetWidth = size.Width; + this.TargetHeight = size.Height; this.TargetRectangle = rectangle; this.Compand = options.Compand; } @@ -112,12 +91,12 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// /// Gets the target width. /// - public int Width { get; } + public int TargetWidth { get; } /// /// Gets the target height. /// - public int Height { get; } + public int TargetHeight { get; } /// /// Gets the resize rectangle. diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs index e16d4801ec..b85983a481 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs @@ -45,15 +45,15 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// /// Gets the target width. /// - public int Width => this.parameterSource.Width; + public int TargetWidth => this.parameterSource.TargetWidth; /// /// Gets the target height. /// - public int Height => this.parameterSource.Height; + public int TargetHeight => this.parameterSource.TargetHeight; /// - /// Gets the resize rectangle. + /// Gets the target resize rectangle. /// public Rectangle TargetRectangle => this.parameterSource.TargetRectangle; @@ -80,8 +80,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms IEnumerable> frames = source.Frames.Select, ImageFrame>( x => new ImageFrame( configuration, - this.Width, - this.Height, + this.TargetWidth, + this.TargetHeight, x.Metadata.DeepClone())); // Use the overload to prevent an extra frame being added @@ -128,8 +128,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms return; } - int width = this.Width; - int height = this.Height; + int width = this.TargetWidth; + int height = this.TargetHeight; int sourceX = sourceRectangle.X; int sourceY = sourceRectangle.Y; int startY = this.TargetRectangle.Y; diff --git a/src/ImageSharp/Processing/ResizeOptions.cs b/src/ImageSharp/Processing/ResizeOptions.cs index ee0dde6b27..96de1eee11 100644 --- a/src/ImageSharp/Processing/ResizeOptions.cs +++ b/src/ImageSharp/Processing/ResizeOptions.cs @@ -1,8 +1,6 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. -using System; -using System.Collections.Generic; using SixLabors.ImageSharp.Processing.Processors.Transforms; using SixLabors.Primitives; @@ -26,7 +24,7 @@ namespace SixLabors.ImageSharp.Processing /// /// Gets or sets the center coordinates. /// - public IEnumerable CenterCoordinates { get; set; } = Array.Empty(); + public PointF? CenterCoordinates { get; set; } /// /// Gets or sets the target size. @@ -44,4 +42,4 @@ namespace SixLabors.ImageSharp.Processing /// public bool Compand { get; set; } = false; } -} \ No newline at end of file +} diff --git a/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeHelperTests.cs b/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeHelperTests.cs index b5ed64f7ef..b351ec235f 100644 --- a/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeHelperTests.cs +++ b/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeHelperTests.cs @@ -11,19 +11,18 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Transforms { public class ResizeHelperTests { - [Theory] [InlineData(20, 100, 1, 2)] - [InlineData(20, 100, 20*100*16, 2)] - [InlineData(20, 100, 40*100*16, 2)] - [InlineData(20, 100, 59*100*16, 2)] - [InlineData(20, 100, 60*100*16, 3)] - [InlineData(17, 63, 5*17*63*16, 5)] - [InlineData(17, 63, 5*17*63*16+1, 5)] - [InlineData(17, 63, 6*17*63*16-1, 5)] - [InlineData(33, 400, 1*1024*1024, 4)] - [InlineData(33, 400, 8*1024*1024, 39)] - [InlineData(50, 300, 1*1024*1024, 4)] + [InlineData(20, 100, 20 * 100 * 16, 2)] + [InlineData(20, 100, 40 * 100 * 16, 2)] + [InlineData(20, 100, 59 * 100 * 16, 2)] + [InlineData(20, 100, 60 * 100 * 16, 3)] + [InlineData(17, 63, 5 * 17 * 63 * 16, 5)] + [InlineData(17, 63, (5 * 17 * 63 * 16) + 1, 5)] + [InlineData(17, 63, (6 * 17 * 63 * 16) - 1, 5)] + [InlineData(33, 400, 1 * 1024 * 1024, 4)] + [InlineData(33, 400, 8 * 1024 * 1024, 39)] + [InlineData(50, 300, 1 * 1024 * 1024, 4)] public void CalculateResizeWorkerHeightInWindowBands( int windowDiameter, int width, @@ -40,17 +39,121 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Transforms var sourceSize = new Size(200, 100); var target = new Size(400, 200); - var actual = ResizeHelper.CalculateTargetLocationAndBounds( + (Size size, Rectangle rectangle) = ResizeHelper.CalculateTargetLocationAndBounds( sourceSize, - new ResizeOptions{ + new ResizeOptions + { Mode = ResizeMode.Min, Size = target - }, - target.Width, - target.Height); - - Assert.Equal(sourceSize, actual.Item1); - Assert.Equal(new Rectangle(0, 0, sourceSize.Width, sourceSize.Height), actual.Item2); + }); + + Assert.Equal(sourceSize, size); + Assert.Equal(new Rectangle(0, 0, sourceSize.Width, sourceSize.Height), rectangle); + } + + [Fact] + public void MaxSizeAndRectangleAreCorrect() + { + var sourceSize = new Size(5072, 6761); + var target = new Size(0, 450); + + var expectedSize = new Size(338, 450); + var expectedRectangle = new Rectangle(Point.Empty, expectedSize); + + (Size size, Rectangle rectangle) = ResizeHelper.CalculateTargetLocationAndBounds( + sourceSize, + new ResizeOptions + { + Mode = ResizeMode.Max, + Size = target + }); + + Assert.Equal(expectedSize, size); + Assert.Equal(expectedRectangle, rectangle); + } + + [Fact] + public void CropSizeAndRectangleAreCorrect() + { + var sourceSize = new Size(100, 100); + var target = new Size(25, 50); + + var expectedSize = new Size(25, 50); + var expectedRectangle = new Rectangle(-12, 0, 50, 50); + + (Size size, Rectangle rectangle) = ResizeHelper.CalculateTargetLocationAndBounds( + sourceSize, + new ResizeOptions + { + Mode = ResizeMode.Crop, + Size = target + }); + + Assert.Equal(expectedSize, size); + Assert.Equal(expectedRectangle, rectangle); + } + + [Fact] + public void BoxPadSizeAndRectangleAreCorrect() + { + var sourceSize = new Size(100, 100); + var target = new Size(120, 110); + + var expectedSize = new Size(120, 110); + var expectedRectangle = new Rectangle(10, 5, 100, 100); + + (Size size, Rectangle rectangle) = ResizeHelper.CalculateTargetLocationAndBounds( + sourceSize, + new ResizeOptions + { + Mode = ResizeMode.BoxPad, + Size = target + }); + + Assert.Equal(expectedSize, size); + Assert.Equal(expectedRectangle, rectangle); + } + + [Fact] + public void PadSizeAndRectangleAreCorrect() + { + var sourceSize = new Size(100, 100); + var target = new Size(120, 110); + + var expectedSize = new Size(120, 110); + var expectedRectangle = new Rectangle(5, 0, 110, 110); + + (Size size, Rectangle rectangle) = ResizeHelper.CalculateTargetLocationAndBounds( + sourceSize, + new ResizeOptions + { + Mode = ResizeMode.Pad, + Size = target + }); + + Assert.Equal(expectedSize, size); + Assert.Equal(expectedRectangle, rectangle); + } + + [Fact] + public void StretchSizeAndRectangleAreCorrect() + { + var sourceSize = new Size(100, 100); + var target = new Size(57, 32); + + var expectedSize = new Size(57, 32); + var expectedRectangle = new Rectangle(Point.Empty, expectedSize); + + (Size size, Rectangle rectangle) = ResizeHelper.CalculateTargetLocationAndBounds( + sourceSize, + new ResizeOptions + { + Mode = ResizeMode.Stretch, + Size = target + }); + + Assert.Equal(expectedSize, size); + Assert.Equal(expectedRectangle, rectangle); } } -} \ No newline at end of file +} diff --git a/tests/ImageSharp.Tests/Processing/Transforms/PadTest.cs b/tests/ImageSharp.Tests/Processing/Transforms/PadTest.cs index b870ddd08a..33da33c717 100644 --- a/tests/ImageSharp.Tests/Processing/Transforms/PadTest.cs +++ b/tests/ImageSharp.Tests/Processing/Transforms/PadTest.cs @@ -20,8 +20,8 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms this.operations.Pad(width, height); ResizeProcessor resizeProcessor = this.Verify(); - Assert.Equal(width, resizeProcessor.Width); - Assert.Equal(height, resizeProcessor.Height); + Assert.Equal(width, resizeProcessor.TargetWidth); + Assert.Equal(height, resizeProcessor.TargetHeight); Assert.Equal(sampler, resizeProcessor.Sampler); } } diff --git a/tests/ImageSharp.Tests/Processing/Transforms/ResizeTests.cs b/tests/ImageSharp.Tests/Processing/Transforms/ResizeTests.cs index c7ebe65e8c..f268eda86c 100644 --- a/tests/ImageSharp.Tests/Processing/Transforms/ResizeTests.cs +++ b/tests/ImageSharp.Tests/Processing/Transforms/ResizeTests.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 SixLabors.ImageSharp.Processing; @@ -18,8 +18,8 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms this.operations.Resize(width, height); ResizeProcessor resizeProcessor = this.Verify(); - Assert.Equal(width, resizeProcessor.Width); - Assert.Equal(height, resizeProcessor.Height); + Assert.Equal(width, resizeProcessor.TargetWidth); + Assert.Equal(height, resizeProcessor.TargetHeight); } [Fact] @@ -31,8 +31,8 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms this.operations.Resize(width, height, sampler); ResizeProcessor resizeProcessor = this.Verify(); - Assert.Equal(width, resizeProcessor.Width); - Assert.Equal(height, resizeProcessor.Height); + Assert.Equal(width, resizeProcessor.TargetWidth); + Assert.Equal(height, resizeProcessor.TargetHeight); Assert.Equal(sampler, resizeProcessor.Sampler); } @@ -48,8 +48,8 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms this.operations.Resize(width, height, sampler, compand); ResizeProcessor resizeProcessor = this.Verify(); - Assert.Equal(width, resizeProcessor.Width); - Assert.Equal(height, resizeProcessor.Height); + Assert.Equal(width, resizeProcessor.TargetWidth); + Assert.Equal(height, resizeProcessor.TargetHeight); Assert.Equal(sampler, resizeProcessor.Sampler); Assert.Equal(compand, resizeProcessor.Compand); } @@ -74,8 +74,8 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms this.operations.Resize(resizeOptions); ResizeProcessor resizeProcessor = this.Verify(); - Assert.Equal(width, resizeProcessor.Width); - Assert.Equal(height, resizeProcessor.Height); + Assert.Equal(width, resizeProcessor.TargetWidth); + Assert.Equal(height, resizeProcessor.TargetHeight); Assert.Equal(sampler, resizeProcessor.Sampler); Assert.Equal(compand, resizeProcessor.Compand); @@ -87,4 +87,4 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms Assert.Equal(mode, resizeOptions.Mode); } } -} \ No newline at end of file +} diff --git a/tests/ImageSharp.Tests/xunit.runner.json b/tests/ImageSharp.Tests/xunit.runner.json index 5204242f03..d7b466d09d 100644 --- a/tests/ImageSharp.Tests/xunit.runner.json +++ b/tests/ImageSharp.Tests/xunit.runner.json @@ -1,5 +1,6 @@ { - "shadowCopy": false, - "methodDisplay": "method", - "diagnosticMessages": true -} \ No newline at end of file + "shadowCopy": false, + "methodDisplay": "method", + "diagnosticMessages": true, + "preEnumerateTheories": false +} From 54aea38d785cdcda571d3f631337360ff493df37 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 8 Sep 2019 17:20:45 +1000 Subject: [PATCH 08/11] revert preenumeration rule --- tests/ImageSharp.Tests/xunit.runner.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/ImageSharp.Tests/xunit.runner.json b/tests/ImageSharp.Tests/xunit.runner.json index d7b466d09d..749ece4387 100644 --- a/tests/ImageSharp.Tests/xunit.runner.json +++ b/tests/ImageSharp.Tests/xunit.runner.json @@ -1,6 +1,5 @@ { "shadowCopy": false, "methodDisplay": "method", - "diagnosticMessages": true, - "preEnumerateTheories": false + "diagnosticMessages": true } From cf0bb2540f4e8b20b7dfdd50953d5e3eaa036db5 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 8 Sep 2019 22:49:42 +1000 Subject: [PATCH 09/11] Fix #1004 --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 38 +++++++++++++++---- .../Formats/Png/Zlib/ZlibInflateStream.cs | 36 ++++++++++++++---- .../Formats/Png/PngDecoderTests.cs | 3 +- tests/ImageSharp.Tests/TestImages.cs | 1 + .../Images/Input/Png/zlib-ztxt-bad-header.png | 3 ++ 5 files changed, 64 insertions(+), 17 deletions(-) create mode 100644 tests/Images/Input/Png/zlib-ztxt-bad-header.png diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 9bc5a5079d..a9e588f6ee 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -175,11 +175,18 @@ namespace SixLabors.ImageSharp.Formats.Png this.InitializeImage(metadata, out image); } - using (var deframeStream = new ZlibInflateStream(this.currentStream, this.ReadNextDataChunk)) + var deframeStream = new ZlibInflateStream(this.currentStream, this.ReadNextDataChunk); + try { - deframeStream.AllocateNewBytes(chunk.Length); + deframeStream.AllocateNewBytes(chunk.Length, true); this.ReadScanlines(deframeStream.CompressedStream, image.Frames.RootFrame, pngMetadata); } + finally + { + // If an invalid Zlib stream is discovered the decoder will throw an exception + // due to the critical nature of the data chunk. + deframeStream.Dispose(); + } break; case PngChunkType.Palette: @@ -924,7 +931,11 @@ namespace SixLabors.ImageSharp.Formats.Png } ReadOnlySpan compressedData = data.Slice(zeroIndex + 2); - metadata.TextData.Add(new PngTextData(name, this.UncompressTextData(compressedData, PngConstants.Encoding), string.Empty, string.Empty)); + + if (this.TryUncompressTextData(compressedData, PngConstants.Encoding, out string uncompressed)) + { + metadata.TextData.Add(new PngTextData(name, uncompressed, string.Empty, string.Empty)); + } } /// @@ -987,7 +998,11 @@ namespace SixLabors.ImageSharp.Formats.Png if (compressionFlag == 1) { ReadOnlySpan compressedData = data.Slice(dataStartIdx); - metadata.TextData.Add(new PngTextData(keyword, this.UncompressTextData(compressedData, PngConstants.TranslatedEncoding), language, translatedKeyword)); + + if (this.TryUncompressTextData(compressedData, PngConstants.TranslatedEncoding, out string uncompressed)) + { + metadata.TextData.Add(new PngTextData(keyword, uncompressed, language, translatedKeyword)); + } } else { @@ -1001,13 +1016,19 @@ namespace SixLabors.ImageSharp.Formats.Png /// /// Compressed text data bytes. /// The string encoding to use. - /// A string. - private string UncompressTextData(ReadOnlySpan compressedData, Encoding encoding) + /// The uncompressed value. + /// The . + private bool TryUncompressTextData(ReadOnlySpan compressedData, Encoding encoding, out string value) { using (var memoryStream = new MemoryStream(compressedData.ToArray())) using (var inflateStream = new ZlibInflateStream(memoryStream, () => 0)) { - inflateStream.AllocateNewBytes(compressedData.Length); + if (!inflateStream.AllocateNewBytes(compressedData.Length, false)) + { + value = null; + return false; + } + var uncompressedBytes = new List(); // Note: this uses the a buffer which is only 4 bytes long to read the stream, maybe allocating a larger buffer makes sense here. @@ -1018,7 +1039,8 @@ namespace SixLabors.ImageSharp.Formats.Png bytesRead = inflateStream.CompressedStream.Read(this.buffer, 0, this.buffer.Length); } - return encoding.GetString(uncompressedBytes.ToArray()); + value = encoding.GetString(uncompressedBytes.ToArray()); + return true; } } diff --git a/src/ImageSharp/Formats/Png/Zlib/ZlibInflateStream.cs b/src/ImageSharp/Formats/Png/Zlib/ZlibInflateStream.cs index 405eeafeb9..df0e723322 100644 --- a/src/ImageSharp/Formats/Png/Zlib/ZlibInflateStream.cs +++ b/src/ImageSharp/Formats/Png/Zlib/ZlibInflateStream.cs @@ -87,13 +87,17 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib /// Adds new bytes from a frame found in the original stream /// /// blabla - public void AllocateNewBytes(int bytes) + /// Whether the chunk to be inflated is a critical chunk. + /// The . + public bool AllocateNewBytes(int bytes, bool isCriticalChunk) { this.currentDataRemaining = bytes; if (this.compressedStream is null) { - this.InitializeInflateStream(); + return this.InitializeInflateStream(isCriticalChunk); } + + return true; } /// @@ -197,7 +201,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib this.isDisposed = true; } - private void InitializeInflateStream() + private bool InitializeInflateStream(bool isCriticalChunk) { // Read the zlib header : http://tools.ietf.org/html/rfc1950 // CMF(Compression Method and flags) @@ -215,7 +219,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib this.currentDataRemaining -= 2; if (cmf == -1 || flag == -1) { - return; + return false; } if ((cmf & 0x0F) == 8) @@ -225,14 +229,28 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib if (cinfo > 7) { - // Values of CINFO above 7 are not allowed in RFC1950. - // CINFO is not defined in this specification for CM not equal to 8. - throw new ImageFormatException($"Invalid window size for ZLIB header: cinfo={cinfo}"); + if (isCriticalChunk) + { + // Values of CINFO above 7 are not allowed in RFC1950. + // CINFO is not defined in this specification for CM not equal to 8. + throw new ImageFormatException($"Invalid window size for ZLIB header: cinfo={cinfo}"); + } + else + { + return false; + } } } else { - throw new ImageFormatException($"Bad method for ZLIB header: cmf={cmf}"); + if (isCriticalChunk) + { + throw new ImageFormatException($"Bad method for ZLIB header: cmf={cmf}"); + } + else + { + return false; + } } // The preset dictionary. @@ -247,6 +265,8 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib // Initialize the deflate Stream. this.compressedStream = new DeflateStream(this, CompressionMode.Decompress, true); + + return true; } } } diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 2e9fd7481e..91b1ef2c17 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -40,7 +40,8 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png TestImages.Png.GrayAlpha8Bit, TestImages.Png.Gray1BitTrans, TestImages.Png.Bad.ZlibOverflow, - TestImages.Png.Bad.ZlibOverflow2 + TestImages.Png.Bad.ZlibOverflow2, + TestImages.Png.Bad.ZlibZtxtBadHeader, }; public static readonly string[] TestImages48Bpp = diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index e95ce09073..163d09bdde 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -90,6 +90,7 @@ namespace SixLabors.ImageSharp.Tests public const string CorruptedChunk = "Png/big-corrupted-chunk.png"; public const string ZlibOverflow = "Png/zlib-overflow.png"; public const string ZlibOverflow2 = "Png/zlib-overflow2.png"; + public const string ZlibZtxtBadHeader = "Png/zlib-ztxt-bad-header.png"; } public static readonly string[] All = diff --git a/tests/Images/Input/Png/zlib-ztxt-bad-header.png b/tests/Images/Input/Png/zlib-ztxt-bad-header.png new file mode 100644 index 0000000000..0eb37aab87 --- /dev/null +++ b/tests/Images/Input/Png/zlib-ztxt-bad-header.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:ce623255656921d491b5c389cd46931fbd6024575b87522c55d67a496dd761f0 +size 22781 From 855e75f674ddbb50b9d1bcfcd5ca08b65c787fb9 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 9 Sep 2019 23:41:51 +1000 Subject: [PATCH 10/11] Refactor helper to reduce code duplication --- .../Processing/Extensions/ResizeExtensions.cs | 50 +++++++++++++----- .../Transforms/Resize/ResizeHelper.cs | 35 +++++++++++-- .../Transforms/Resize/ResizeProcessor.cs | 51 ------------------- src/ImageSharp/Processing/ResizeMode.cs | 9 +++- src/ImageSharp/Processing/ResizeOptions.cs | 5 ++ 5 files changed, 79 insertions(+), 71 deletions(-) diff --git a/src/ImageSharp/Processing/Extensions/ResizeExtensions.cs b/src/ImageSharp/Processing/Extensions/ResizeExtensions.cs index 81b1c2c663..f494ed9094 100644 --- a/src/ImageSharp/Processing/Extensions/ResizeExtensions.cs +++ b/src/ImageSharp/Processing/Extensions/ResizeExtensions.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 SixLabors.ImageSharp.Processing.Processors.Transforms; @@ -12,16 +12,6 @@ namespace SixLabors.ImageSharp.Processing /// public static class ResizeExtensions { - /// - /// Resizes an image in accordance with the given . - /// - /// The image to resize. - /// The resize options. - /// The to allow chaining of operations. - /// Passing zero for one of height or width within the resize options will automatically preserve the aspect ratio of the original image or the nearest possible ratio. - public static IImageProcessingContext Resize(this IImageProcessingContext source, ResizeOptions options) - => source.ApplyProcessor(new ResizeProcessor(options, source.GetCurrentSize())); - /// /// Resizes an image to the given . /// @@ -128,7 +118,18 @@ namespace SixLabors.ImageSharp.Processing Rectangle sourceRectangle, Rectangle targetRectangle, bool compand) - => source.ApplyProcessor(new ResizeProcessor(sampler, width, height, source.GetCurrentSize(), targetRectangle, compand), sourceRectangle); + { + var options = new ResizeOptions + { + Size = new Size(width, height), + Mode = ResizeMode.Manual, + Sampler = sampler, + TargetRectangle = targetRectangle, + Compand = compand + }; + + return source.ApplyProcessor(new ResizeProcessor(options, source.GetCurrentSize()), sourceRectangle); + } /// /// Resizes an image to the given width and height with the given sampler and source rectangle. @@ -150,6 +151,27 @@ namespace SixLabors.ImageSharp.Processing IResampler sampler, Rectangle targetRectangle, bool compand) - => source.ApplyProcessor(new ResizeProcessor(sampler, width, height, source.GetCurrentSize(), targetRectangle, compand)); + { + var options = new ResizeOptions + { + Size = new Size(width, height), + Mode = ResizeMode.Manual, + Sampler = sampler, + TargetRectangle = targetRectangle, + Compand = compand + }; + + return Resize(source, options); + } + + /// + /// Resizes an image in accordance with the given . + /// + /// The image to resize. + /// The resize options. + /// The to allow chaining of operations. + /// Passing zero for one of height or width within the resize options will automatically preserve the aspect ratio of the original image or the nearest possible ratio. + public static IImageProcessingContext Resize(this IImageProcessingContext source, ResizeOptions options) + => source.ApplyProcessor(new ResizeProcessor(options, source.GetCurrentSize())); } -} \ No newline at end of file +} diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeHelper.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeHelper.cs index c9df1b2542..eacd3834f1 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeHelper.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeHelper.cs @@ -36,6 +36,11 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms int width = options.Size.Width; int height = options.Size.Height; + if (width <= 0 && height <= 0) + { + ThrowInvalid($"Target width {width} and height {height} must be greater than zero."); + } + // Ensure target size is populated across both dimensions. // These dimensions are used to calculate the final dimensions determined by the mode algorithm. // If only one of the incoming dimensions is 0, it will be modified here to maintain aspect ratio. @@ -51,9 +56,6 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms height = (int)MathF.Max(Min, MathF.Round(sourceSize.Height * width / (float)sourceSize.Width)); } - Guard.MustBeGreaterThan(width, 0, nameof(width)); - Guard.MustBeGreaterThan(height, 0, nameof(height)); - switch (options.Mode) { case ResizeMode.Crop: @@ -66,8 +68,10 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms return CalculateMaxRectangle(sourceSize, width, height); case ResizeMode.Min: return CalculateMinRectangle(sourceSize, width, height); + case ResizeMode.Manual: + return CalculateManualRectangle(options, width, height); - // Last case ResizeMode.Stretch: + // case ResizeMode.Stretch: default: return (new Size(width, height), new Rectangle(0, 0, width, height)); } @@ -397,5 +401,28 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms // Target image width and height can be different to the rectangle width and height. return (new Size(width, height), new Rectangle(targetX, targetY, targetWidth, targetHeight)); } + + private static (Size, Rectangle) CalculateManualRectangle( + ResizeOptions options, + int width, + int height) + { + if (!options.TargetRectangle.HasValue) + { + ThrowInvalid("Manual resizing requires a target location and size."); + } + + Rectangle targetRectangle = options.TargetRectangle.Value; + + int targetX = targetRectangle.X; + int targetY = targetRectangle.Y; + int targetWidth = targetRectangle.Width > 0 ? targetRectangle.Width : width; + int targetHeight = targetRectangle.Height > 0 ? targetRectangle.Height : height; + + // Target image width and height can be different to the rectangle width and height. + return (new Size(width, height), new Rectangle(targetX, targetY, targetWidth, targetHeight)); + } + + private static void ThrowInvalid(string message) => throw new InvalidOperationException(message); } } diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs index 6f5f09e717..35e22757c1 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs @@ -13,45 +13,6 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// public class ResizeProcessor : IImageProcessor { - /// - /// Initializes a new instance of the class. - /// - /// The . - /// The width. - /// The height. - /// The size of the source image. - /// The target rectangle to resize into. - /// A value indicating whether to apply RGBA companding. - public ResizeProcessor(IResampler sampler, int width, int height, Size sourceSize, Rectangle targetRectangle, bool compand) - { - Guard.NotNull(sampler, nameof(sampler)); - - // Ensure target size is populated across both dimensions. - // If only one of the incoming dimensions is 0, it will be modified here to maintain aspect ratio. - // If it is not possible to keep aspect ratio, make sure at least the minimum is is kept. - const int Min = 1; - if (width == 0 && height > 0) - { - width = (int)MathF.Max(Min, MathF.Round(sourceSize.Width * height / (float)sourceSize.Height)); - targetRectangle.Width = width; - } - - if (height == 0 && width > 0) - { - height = (int)MathF.Max(Min, MathF.Round(sourceSize.Height * width / (float)sourceSize.Width)); - targetRectangle.Height = height; - } - - Guard.MustBeGreaterThan(width, 0, nameof(width)); - Guard.MustBeGreaterThan(height, 0, nameof(height)); - - this.Sampler = sampler; - this.TargetWidth = width; - this.TargetHeight = height; - this.TargetRectangle = targetRectangle; - this.Compand = compand; - } - /// /// Initializes a new instance of the class. /// @@ -71,18 +32,6 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms this.Compand = options.Compand; } - /// - /// Initializes a new instance of the class. - /// - /// The sampler to perform the resize operation. - /// The target width. - /// The target height. - /// The source image size - public ResizeProcessor(IResampler sampler, int width, int height, Size sourceSize) - : this(sampler, width, height, sourceSize, new Rectangle(0, 0, width, height), false) - { - } - /// /// Gets the sampler to perform the resize operation. /// diff --git a/src/ImageSharp/Processing/ResizeMode.cs b/src/ImageSharp/Processing/ResizeMode.cs index 6adeac66da..142a926b30 100644 --- a/src/ImageSharp/Processing/ResizeMode.cs +++ b/src/ImageSharp/Processing/ResizeMode.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. namespace SixLabors.ImageSharp.Processing @@ -42,6 +42,11 @@ namespace SixLabors.ImageSharp.Processing /// /// Stretches the resized image to fit the bounds of its container. /// - Stretch + Stretch, + + /// + /// The target location and size of the resized image has been manually set. + /// + Manual } } diff --git a/src/ImageSharp/Processing/ResizeOptions.cs b/src/ImageSharp/Processing/ResizeOptions.cs index 96de1eee11..ef88dc35b3 100644 --- a/src/ImageSharp/Processing/ResizeOptions.cs +++ b/src/ImageSharp/Processing/ResizeOptions.cs @@ -41,5 +41,10 @@ namespace SixLabors.ImageSharp.Processing /// or expand individual pixel colors the value on processing. /// public bool Compand { get; set; } = false; + + /// + /// Gets or sets the target rectangle to resize into. + /// + public Rectangle? TargetRectangle { get; set; } } } From e89c1ef3d49693ffc48c75d1f3d7797fb6a0e9cc Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 10 Sep 2019 17:34:14 +1000 Subject: [PATCH 11/11] Clean up scanline decoding code. --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 38 ++++++++---------- .../Formats/Png/Zlib/ZlibInflateStream.cs | 40 +++++++++++-------- 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index a9e588f6ee..037f648f0a 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -5,6 +5,7 @@ using System; using System.Buffers.Binary; using System.Collections.Generic; using System.IO; +using System.IO.Compression; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Text; @@ -175,18 +176,7 @@ namespace SixLabors.ImageSharp.Formats.Png this.InitializeImage(metadata, out image); } - var deframeStream = new ZlibInflateStream(this.currentStream, this.ReadNextDataChunk); - try - { - deframeStream.AllocateNewBytes(chunk.Length, true); - this.ReadScanlines(deframeStream.CompressedStream, image.Frames.RootFrame, pngMetadata); - } - finally - { - // If an invalid Zlib stream is discovered the decoder will throw an exception - // due to the critical nature of the data chunk. - deframeStream.Dispose(); - } + this.ReadScanlines(chunk, image.Frames.RootFrame, pngMetadata); break; case PngChunkType.Palette: @@ -472,19 +462,25 @@ namespace SixLabors.ImageSharp.Formats.Png /// Reads the scanlines within the image. /// /// The pixel format. - /// The containing data. + /// The png chunk containing the compressed scanline data. /// The pixel data. /// The png metadata - private void ReadScanlines(Stream dataStream, ImageFrame image, PngMetadata pngMetadata) + private void ReadScanlines(PngChunk chunk, ImageFrame image, PngMetadata pngMetadata) where TPixel : struct, IPixel { - if (this.header.InterlaceMethod == PngInterlaceMode.Adam7) - { - this.DecodeInterlacedPixelData(dataStream, image, pngMetadata); - } - else + using (var deframeStream = new ZlibInflateStream(this.currentStream, this.ReadNextDataChunk)) { - this.DecodePixelData(dataStream, image, pngMetadata); + deframeStream.AllocateNewBytes(chunk.Length, true); + DeflateStream dataStream = deframeStream.CompressedStream; + + if (this.header.InterlaceMethod == PngInterlaceMode.Adam7) + { + this.DecodeInterlacedPixelData(dataStream, image, pngMetadata); + } + else + { + this.DecodePixelData(dataStream, image, pngMetadata); + } } } @@ -1021,7 +1017,7 @@ namespace SixLabors.ImageSharp.Formats.Png private bool TryUncompressTextData(ReadOnlySpan compressedData, Encoding encoding, out string value) { using (var memoryStream = new MemoryStream(compressedData.ToArray())) - using (var inflateStream = new ZlibInflateStream(memoryStream, () => 0)) + using (var inflateStream = new ZlibInflateStream(memoryStream)) { if (!inflateStream.AllocateNewBytes(compressedData.Length, false)) { diff --git a/src/ImageSharp/Formats/Png/Zlib/ZlibInflateStream.cs b/src/ImageSharp/Formats/Png/Zlib/ZlibInflateStream.cs index df0e723322..e4645c44ac 100644 --- a/src/ImageSharp/Formats/Png/Zlib/ZlibInflateStream.cs +++ b/src/ImageSharp/Formats/Png/Zlib/ZlibInflateStream.cs @@ -20,14 +20,14 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib private static readonly byte[] ChecksumBuffer = new byte[4]; /// - /// The inner raw memory stream + /// A default delegate to get more data from the inner stream. /// - private readonly Stream innerStream; + private static readonly Func GetDataNoOp = () => 0; /// - /// The compressed stream sitting over the top of the deframer + /// The inner raw memory stream /// - private DeflateStream compressedStream; + private readonly Stream innerStream; /// /// A value indicating whether this instance of the given entity has been disposed. @@ -55,8 +55,17 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib /// /// Initializes a new instance of the class. /// - /// The inner raw stream - /// A delegate to get more data from the inner stream + /// The inner raw stream. + public ZlibInflateStream(Stream innerStream) + : this(innerStream, GetDataNoOp) + { + } + + /// + /// Initializes a new instance of the class. + /// + /// The inner raw stream. + /// A delegate to get more data from the inner stream. public ZlibInflateStream(Stream innerStream, Func getData) { this.innerStream = innerStream; @@ -76,12 +85,12 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib public override long Length => throw new NotSupportedException(); /// - public override long Position { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } + public override long Position { get => throw new NotSupportedException(); set => throw new NotSupportedException(); } /// /// Gets the compressed stream over the deframed inner stream /// - public DeflateStream CompressedStream => this.compressedStream; + public DeflateStream CompressedStream { get; private set; } /// /// Adds new bytes from a frame found in the original stream @@ -92,7 +101,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib public bool AllocateNewBytes(int bytes, bool isCriticalChunk) { this.currentDataRemaining = bytes; - if (this.compressedStream is null) + if (this.CompressedStream is null) { return this.InitializeInflateStream(isCriticalChunk); } @@ -101,10 +110,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib } /// - public override void Flush() - { - throw new NotSupportedException(); - } + public override void Flush() => throw new NotSupportedException(); /// public override int ReadByte() @@ -186,10 +192,10 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib if (disposing) { // dispose managed resources - if (this.compressedStream != null) + if (this.CompressedStream != null) { - this.compressedStream.Dispose(); - this.compressedStream = null; + this.CompressedStream.Dispose(); + this.CompressedStream = null; } } @@ -264,7 +270,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib } // Initialize the deflate Stream. - this.compressedStream = new DeflateStream(this, CompressionMode.Decompress, true); + this.CompressedStream = new DeflateStream(this, CompressionMode.Decompress, true); return true; }