Introduce Numerics class and Refactor Clamp Utils #1436
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1436 +/- ##
==========================================
+ Coverage 83.63% 83.68% +0.05%
==========================================
Files 733 732 -1
Lines 31922 31984 +62
Branches 3590 3605 +15
==========================================
+ Hits 26697 26766 +69
+ Misses 4511 4505 -6
+ Partials 714 713 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
LGTM with suggestion:
We may want to move all Color/Pixel related numeric operations from ImageMath and Vector4Utils to a separate ColorNumerics class and probably get rid of Vector4Utils as a result. Cohesion around the Vector4 type itself is no good cohesion here.
src/ImageSharp/Color/Color.cs
Outdated
| ImageMaths.UpscaleFrom8BitTo16Bit(g), | ||
| ImageMaths.UpscaleFrom8BitTo16Bit(b), | ||
| ImageMaths.UpscaleFrom8BitTo16Bit(a)); | ||
| ImageMath.UpscaleFrom8BitTo16Bit(r), |
There was a problem hiding this comment.
I would move this to Numerics too. Or to a separate ColorNumerics (or similarly named) class.
| @@ -59,7 +59,7 @@ internal static class DenseMatrixUtils | |||
| ref Vector4 target = ref Unsafe.Add(ref targetRowRef, column); | |||
| vector.W = target.W; | |||
|
|
|||
| Vector4Utilities.UnPremultiply(ref vector); | |||
| Vector4Utils.UnPremultiply(ref vector); | |||
There was a problem hiding this comment.
Numerics or ColorNumerics ?
There was a problem hiding this comment.
I'll move both to Numerics.
| @@ -30,7 +30,7 @@ public SkewProcessor(float degreesX, float degreesY, Size sourceSize) | |||
| /// <param name="sourceSize">The source image size</param> | |||
| public SkewProcessor(float degreesX, float degreesY, IResampler sampler, Size sourceSize) | |||
| : this( | |||
| TransformUtilities.CreateSkewMatrixDegrees(degreesX, degreesY, sourceSize), | |||
| TransformUtils.CreateSkewMatrixDegrees(degreesX, degreesY, sourceSize), | |||
There was a problem hiding this comment.
So we are back to the Utils postfix convention? Agreed, shorter is better here.
There was a problem hiding this comment.
Yeah less typing. Unfortunately there's a public GeometryUtilities class I couldn't change.
There was a problem hiding this comment.
We probably shouldn't have left that public for the 2 methods it has.
There was a problem hiding this comment.
Yeah... We shouldn't. Annoyed at myself for not spotting it.
Co-authored-by: Anton Firszov <antonfir@gmail.com>
Introduce Numerics class and Refactor Clamp Utils
Prerequisites
Description
There's a lot of cosmetic changes here but very little functional. I've been wanting to do a major cleanup for a while.
Numericsutility method to consolidate non-image specific maths magritings several fromImageMaths.Clampextension methods with staticNumericsimplVector4FastClamptoNumerics.Clamp(Span, min, max)methods (As yet unused but have plans).