address review findings + some more cleanup

src/ImageSharp/Common/Helpers/SimdUtils.BasicIntrinsics256.cs

@@ -28,7 +28,7 @@ namespace SixLabors.ImageSharp
                 ref ReadOnlySpan<byte> source,
                 ref Span<float> dest)
             {
-                DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same size!");
+                DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same length!");
 
                 if (!IsAvailable)
                 {
@@ -57,7 +57,7 @@ namespace SixLabors.ImageSharp
                 ref ReadOnlySpan<float> source,
                 ref Span<byte> dest)
             {
-                DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same size!");
+                DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same length!");
 
                 if (!IsAvailable)
                 {
@@ -78,16 +78,15 @@ namespace SixLabors.ImageSharp
 
             /// <summary>
             /// SIMD optimized implementation for <see cref="SimdUtils.BulkConvertByteToNormalizedFloat"/>.
-            /// Works only with `dest.Length` divisible by 8.
+            /// Works only with span Length divisible by 8.
             /// Implementation adapted from:
             /// http://lolengine.net/blog/2011/3/20/understanding-fast-float-integer-conversions
             /// http://stackoverflow.com/a/536278
             /// </summary>
             internal static void BulkConvertByteToNormalizedFloat(ReadOnlySpan<byte> source, Span<float> dest)
             {
-                GuardAvx2(nameof(BulkConvertByteToNormalizedFloat));
+                VerifyIsAvx2Compatible(nameof(BulkConvertByteToNormalizedFloat));
-
+                VerifySpanInput(source, dest, 8);
-                DebugGuard.IsTrue(ImageMaths.Modulo8(dest.Length) == 0, nameof(source), "dest.Length should be divisable by 8!");
 
                 var bVec = new Vector<float>(256.0f / 255.0f);
                 var magicFloat = new Vector<float>(32768.0f);
@@ -128,9 +127,8 @@ namespace SixLabors.ImageSharp
             /// </summary>
             internal static void BulkConvertNormalizedFloatToByteClampOverflows(ReadOnlySpan<float> source, Span<byte> dest)
             {
-                GuardAvx2(nameof(BulkConvertNormalizedFloatToByteClampOverflows));
+                VerifyIsAvx2Compatible(nameof(BulkConvertNormalizedFloatToByteClampOverflows));
-
+                VerifySpanInput(source, dest, 8);
-                DebugGuard.IsTrue(ImageMaths.Modulo8(source.Length) == 0, nameof(source), "source.Length should be divisible by 8!");
 
                 if (source.Length == 0)
                 {
@@ -168,9 +166,9 @@ namespace SixLabors.ImageSharp
             }
 
             /// <summary>
-            /// Convert 'source.Length' <see cref="float"/> values normalized into [0..1] from 'source'
+            /// Convert all <see cref="float"/> values normalized into [0..1] from 'source'
             /// into 'dest' buffer of <see cref="byte"/>. The values are scaled up into [0-255] and rounded.
-            /// The implementation is SIMD optimized and works only with `source.Length` divisible by 8.
+            /// This implementation is SIMD optimized and works only when span Length is divisible by 8.
             /// Based on:
             /// <see>
             ///     <cref>http://lolengine.net/blog/2011/3/20/understanding-fast-float-integer-conversions</cref>
@@ -178,12 +176,8 @@ namespace SixLabors.ImageSharp
             /// </summary>
             internal static void BulkConvertNormalizedFloatToByte(ReadOnlySpan<float> source, Span<byte> dest)
             {
-                GuardAvx2(nameof(BulkConvertNormalizedFloatToByte));
+                VerifyIsAvx2Compatible(nameof(BulkConvertNormalizedFloatToByte));
-
+                VerifySpanInput(source, dest, 8);
-                DebugGuard.IsTrue(
-                    ImageMaths.Modulo8(source.Length) == 0,
-                    nameof(source),
-                    "source.Length should be divisible by 8!");
 
                 if (source.Length == 0)
                 {

src/ImageSharp/Common/Helpers/SimdUtils.ExtendedIntrinsics.cs

@@ -35,7 +35,7 @@ namespace SixLabors.ImageSharp
                 ref ReadOnlySpan<byte> source,
                 ref Span<float> dest)
             {
-                DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same size!");
+                DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same length!");
 
                 if (!IsAvailable)
                 {
@@ -62,7 +62,7 @@ namespace SixLabors.ImageSharp
                 ref ReadOnlySpan<float> source,
                 ref Span<byte> dest)
             {
-                DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same size!");
+                DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same length!");
 
                 if (!IsAvailable)
                 {
@@ -88,10 +88,7 @@ namespace SixLabors.ImageSharp
             /// </summary>
             internal static void BulkConvertByteToNormalizedFloat(ReadOnlySpan<byte> source, Span<float> dest)
             {
-                DebugGuard.IsTrue(
+                VerifySpanInput(source, dest, Vector<byte>.Count);
-                    ImageMaths.ModuloP2(dest.Length, Vector<byte>.Count) == 0,
-                    nameof(source),
-                    "dest.Length should be divisible by Vector<byte>.Count!");
 
                 int n = dest.Length / Vector<byte>.Count;
 
@@ -126,10 +123,7 @@ namespace SixLabors.ImageSharp
                 ReadOnlySpan<float> source,
                 Span<byte> dest)
             {
-                DebugGuard.IsTrue(
+                VerifySpanInput(source, dest, Vector<byte>.Count);
-                    ImageMaths.ModuloP2(dest.Length, Vector<byte>.Count) == 0,
-                    nameof(dest),
-                    "dest.Length should be divisible by Vector<byte>.Count!");
 
                 int n = dest.Length / Vector<byte>.Count;
 

src/ImageSharp/Common/Helpers/SimdUtils.FallbackIntrinsics128.cs

@@ -26,7 +26,7 @@ namespace SixLabors.ImageSharp
                 ref ReadOnlySpan<byte> source,
                 ref Span<float> dest)
             {
-                DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same size!");
+                DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same length!");
 
                 int remainder = ImageMaths.Modulo4(source.Length);
                 int adjustedCount = source.Length - remainder;
@@ -50,7 +50,7 @@ namespace SixLabors.ImageSharp
                 ref ReadOnlySpan<float> source,
                 ref Span<byte> dest)
             {
-                DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same size!");
+                DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same length!");
 
                 int remainder = ImageMaths.Modulo4(source.Length);
                 int adjustedCount = source.Length - remainder;
@@ -72,10 +72,7 @@ namespace SixLabors.ImageSharp
             [MethodImpl(InliningOptions.ColdPath)]
             internal static void BulkConvertByteToNormalizedFloat(ReadOnlySpan<byte> source, Span<float> dest)
             {
-                DebugGuard.IsTrue(
+                VerifySpanInput(source, dest, 4);
-                    ImageMaths.Modulo4(dest.Length) == 0,
-                    nameof(dest),
-                    "dest.Length should be divisible by 4!");
 
                 int count = dest.Length / 4;
                 if (count == 0)
@@ -109,10 +106,7 @@ namespace SixLabors.ImageSharp
                 ReadOnlySpan<float> source,
                 Span<byte> dest)
             {
-                DebugGuard.IsTrue(
+                VerifySpanInput(source, dest, 4);
-                    ImageMaths.Modulo4(source.Length) == 0,
-                    nameof(source),
-                    "source.Length should be divisible by 4!");
 
                 int count = source.Length / 4;
                 if (count == 0)

src/ImageSharp/Common/Helpers/SimdUtils.cs

@@ -154,12 +154,32 @@ namespace SixLabors.ImageSharp
         private static byte ConvertToByte(float f) => (byte)ImageMaths.Clamp((f * 255f) + 0.5f, 0, 255f);
 
         [Conditional("DEBUG")]
-        private static void GuardAvx2(string operation)
+        private static void VerifyIsAvx2Compatible(string operation)
         {
             if (!IsAvx2CompatibleArchitecture)
             {
                 throw new NotSupportedException($"{operation} is supported only on AVX2 CPU!");
             }
         }
+
+        [Conditional("DEBUG")]
+        private static void VerifySpanInput(ReadOnlySpan<byte> source, Span<float> dest, int shouldBeDivisibleBy)
+        {
+            DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same length!");
+            DebugGuard.IsTrue(
+                ImageMaths.ModuloP2(dest.Length, shouldBeDivisibleBy) == 0,
+                nameof(source),
+                $"length should be divisable by {shouldBeDivisibleBy}!");
+        }
+
+        [Conditional("DEBUG")]
+        private static void VerifySpanInput(ReadOnlySpan<float> source, Span<byte> dest, int shouldBeDivisibleBy)
+        {
+            DebugGuard.IsTrue(source.Length == dest.Length, nameof(source), "Input spans must be of same length!");
+            DebugGuard.IsTrue(
+                ImageMaths.ModuloP2(dest.Length, shouldBeDivisibleBy) == 0,
+                nameof(source),
+                $"length should be divisable by {shouldBeDivisibleBy}!");
+        }
     }
 }

src/ImageSharp/Common/Tuples/Octet.cs

@@ -3,8 +3,14 @@ using System.Runtime.InteropServices;
 
 namespace SixLabors.ImageSharp.Tuples
 {
+    /// <summary>
+    /// Contains 8 element value tuples of various types.
+    /// </summary>
     internal static class Octet
     {
+        /// <summary>
+        /// Value tuple of <see cref="uint"/>-s
+        /// </summary>
         [StructLayout(LayoutKind.Explicit, Size = 8 * sizeof(uint))]
         public struct OfUInt32
         {
@@ -34,7 +40,7 @@ namespace SixLabors.ImageSharp.Tuples
 
             public override string ToString()
             {
-                return $"[{this.V0},{this.V1},{this.V2},{this.V3},{this.V4},{this.V5},{this.V6},{this.V7}]";
+                return $"{nameof(Octet)}.{nameof(OfUInt32)}({this.V0},{this.V1},{this.V2},{this.V3},{this.V4},{this.V5},{this.V6},{this.V7})";
             }
 
             [MethodImpl(InliningOptions.ShortMethod)]
@@ -51,6 +57,9 @@ namespace SixLabors.ImageSharp.Tuples
             }
         }
 
+        /// <summary>
+        /// Value tuple of <see cref="byte"/>-s
+        /// </summary>
         [StructLayout(LayoutKind.Explicit, Size = 8)]
         public struct OfByte
         {
@@ -80,7 +89,7 @@ namespace SixLabors.ImageSharp.Tuples
 
             public override string ToString()
             {
-                return $"[{this.V0},{this.V1},{this.V2},{this.V3},{this.V4},{this.V5},{this.V6},{this.V7}]";
+                return $"{nameof(Octet)}.{nameof(OfByte)}({this.V0},{this.V1},{this.V2},{this.V3},{this.V4},{this.V5},{this.V6},{this.V7})";
             }
 
             [MethodImpl(InliningOptions.ShortMethod)]

src/ImageSharp/Common/Tuples/Vector4Pair.cs

@@ -7,6 +7,7 @@ namespace SixLabors.ImageSharp.Tuples
     /// <summary>
     /// Its faster to process multiple Vector4-s together, so let's pair them!
     /// On AVX2 this pair should be convertible to <see cref="Vector{T}"/> of <see cref="float"/>!
+    /// TODO: Investigate defining this as union with an Octet.OfSingle type.
     /// </summary>
     [StructLayout(LayoutKind.Sequential)]
     internal struct Vector4Pair
@@ -15,8 +16,6 @@ namespace SixLabors.ImageSharp.Tuples
 
         public Vector4 B;
 
-        private static readonly Vector4 Scale = new Vector4(1 / 255f);
-
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public void MultiplyInplace(float value)
         {
@@ -52,8 +51,9 @@ namespace SixLabors.ImageSharp.Tuples
             b = b.FastRound();
 
             // Downscale by 1/255
-            this.A *= Scale;
+            var scale = new Vector4(1 / 255f);
-            this.B *= Scale;
+            this.A *= scale;
+            this.B *= scale;
         }
 
         /// <summary>
@@ -74,7 +74,7 @@ namespace SixLabors.ImageSharp.Tuples
 
         public override string ToString()
         {
-            return $"{this.A}, {this.B}";
+            return $"{nameof(Vector4Pair)}({this.A}, {this.B})";
         }
     }
 }

src/ImageSharp/Formats/Jpeg/Components/Decoder/ColorConverters/JpegColorConverter.FromYCbCrSimd.cs

@@ -109,7 +109,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder.ColorConverters
 
                     // Collect (r0,r1...r8) (g0,g1...g8) (b0,b1...b8) vector values in the expected (r0,g0,g1,1), (r1,g1,g2,1) ... order:
                     ref Vector4Octet destination = ref Unsafe.Add(ref resultBase, i);
-                    destination.Collect(ref r, ref g, ref b);
+                    destination.Pack(ref r, ref g, ref b);
                 }
             }
         }

src/ImageSharp/Formats/Jpeg/Components/Decoder/ColorConverters/JpegColorConverter.FromYCbCrSimdAvx2.cs

@@ -102,7 +102,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder.ColorConverters
 
                     // Collect (r0,r1...r8) (g0,g1...g8) (b0,b1...b8) vector values in the expected (r0,g0,g1,1), (r1,g1,g2,1) ... order:
                     ref Vector4Octet destination = ref Unsafe.Add(ref resultBase, i);
-                    destination.Collect(ref rr, ref gg, ref bb);
+                    destination.Pack(ref rr, ref gg, ref bb);
                 }
             }
         }

src/ImageSharp/Formats/Jpeg/Components/Decoder/ColorConverters/JpegColorConverter.cs

@@ -157,9 +157,9 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder.ColorConverters
             public Vector4 V0, V1, V2, V3, V4, V5, V6, V7;
 
             /// <summary>
-            /// Collect (r0,r1...r8) (g0,g1...g8) (b0,b1...b8) vector values in the expected (r0,g0,g1,1), (r1,g1,g2,1) ... order.
+            /// Pack (r0,r1...r7) (g0,g1...g7) (b0,b1...b7) vector values as (r0,g0,b0,1), (r1,g1,b1,1) ...
             /// </summary>
-            public void Collect(ref Vector4Pair r, ref Vector4Pair g, ref Vector4Pair b)
+            public void Pack(ref Vector4Pair r, ref Vector4Pair g, ref Vector4Pair b)
             {
                 this.V0.X = r.A.X;
                 this.V0.Y = g.A.X;

src/ImageSharp/PixelFormats/PixelOperations{TPixel}.cs

@@ -30,11 +30,10 @@ namespace SixLabors.ImageSharp.PixelFormats
         internal virtual void PackFromVector4(ReadOnlySpan<Vector4> sourceVectors, Span<TPixel> destinationColors, int count)
         {
             ReadOnlySpan<Vector4> sourceVectors1 = sourceVectors;
-            Span<TPixel> destinationColors1 = destinationColors;
+            GuardSpans(sourceVectors1, nameof(sourceVectors1), destinationColors, nameof(destinationColors), count);
-            GuardSpans(sourceVectors1, nameof(sourceVectors1), destinationColors1, nameof(destinationColors1), count);
 
             ref Vector4 sourceRef = ref MemoryMarshal.GetReference(sourceVectors1);
-            ref TPixel destRef = ref MemoryMarshal.GetReference(destinationColors1);
+            ref TPixel destRef = ref MemoryMarshal.GetReference(destinationColors);
 
             for (int i = 0; i < count; i++)
             {
@@ -53,11 +52,10 @@ namespace SixLabors.ImageSharp.PixelFormats
         internal virtual void ToVector4(ReadOnlySpan<TPixel> sourceColors, Span<Vector4> destinationVectors, int count)
         {
             ReadOnlySpan<TPixel> sourceColors1 = sourceColors;
-            Span<Vector4> destinationVectors1 = destinationVectors;
+            GuardSpans(sourceColors1, nameof(sourceColors1), destinationVectors, nameof(destinationVectors), count);
-            GuardSpans(sourceColors1, nameof(sourceColors1), destinationVectors1, nameof(destinationVectors1), count);
 
             ref TPixel sourceRef = ref MemoryMarshal.GetReference(sourceColors1);
-            ref Vector4 destRef = ref MemoryMarshal.GetReference(destinationVectors1);
+            ref Vector4 destRef = ref MemoryMarshal.GetReference(destinationVectors);
 
             for (int i = 0; i < count; i++)
             {