fix JpegEncoder disco buffer handling

src/ImageSharp/Formats/Jpeg/Components/Encoder/YCbCrForwardConverter{TPixel}.cs

@@ -55,9 +55,9 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Encoder
         /// <summary>
         /// Converts a 8x8 image area inside 'pixels' at position (x,y) placing the result members of the structure (<see cref="Y"/>, <see cref="Cb"/>, <see cref="Cr"/>)
         /// </summary>
-        public void Convert(ImageFrame<TPixel> frame, int x, int y)
+        public void Convert(ImageFrame<TPixel> frame, int x, int y, in RowOctet<TPixel> currentRows)
         {
-            this.pixelBlock.LoadAndStretchEdges(frame, x, y);
+            this.pixelBlock.LoadAndStretchEdges(frame.PixelBuffer, x, y, currentRows);
 
             Span<Rgb24> rgbSpan = this.rgbBlock.AsSpanUnsafe();
             PixelOperations<TPixel>.Instance.ToRgb24(frame.GetConfiguration(), this.pixelBlock.AsSpanUnsafe(), rgbSpan);

src/ImageSharp/Formats/Jpeg/Components/GenericBlock8x8.cs

@@ -54,24 +54,24 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components
             set => this[(y * 8) + x] = value;
         }
 
-        public void LoadAndStretchEdges<TPixel>(IPixelSource<TPixel> source, int sourceX, int sourceY)
+        // public void LoadAndStretchEdges<TPixel>(IPixelSource<TPixel> source, int sourceX, RowOctet<TPixel> currentRows)
-            where TPixel : struct, IPixel<TPixel>
+        //     where TPixel : struct, IPixel<TPixel>
-        {
+        // {
-            if (source.PixelBuffer is Buffer2D<T> buffer)
+        //     if (source.PixelBuffer is Buffer2D<T> buffer)
-            {
+        //     {
-                this.LoadAndStretchEdges(buffer, sourceX, sourceY);
+        //         this.LoadAndStretchEdges(buffer, sourceX, sourceY);
-            }
+        //     }
-            else
+        //     else
-            {
+        //     {
-                throw new InvalidOperationException("LoadAndStretchEdges<TPixels>() is only valid for TPixel == T !");
+        //         throw new InvalidOperationException("LoadAndStretchEdges<TPixels>() is only valid for TPixel == T !");
-            }
+        //     }
-        }
+        // }
 
         /// <summary>
         /// Load a 8x8 region of an image into the block.
         /// The "outlying" area of the block will be stretched out with pixels on the right and bottom edge of the image.
         /// </summary>
-        public void LoadAndStretchEdges(Buffer2D<T> source, int sourceX, int sourceY)
+        public void LoadAndStretchEdges(Buffer2D<T> source, int sourceX, int sourceY, in RowOctet<T> currentRows)
         {
             int width = Math.Min(8, source.Width - sourceX);
             int height = Math.Min(8, source.Height - sourceY);
@@ -85,15 +85,13 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components
             int remainderXCount = 8 - width;
 
             ref byte blockStart = ref Unsafe.As<GenericBlock8x8<T>, byte>(ref this);
-            ref byte imageStart = ref Unsafe.As<T, byte>(
-                                      ref Unsafe.Add(ref MemoryMarshal.GetReference(source.GetRowSpan(sourceY)), sourceX));
-
             int blockRowSizeInBytes = 8 * Unsafe.SizeOf<T>();
-            int imageRowSizeInBytes = source.Width * Unsafe.SizeOf<T>();
 
             for (int y = 0; y < height; y++)
             {
-                ref byte s = ref Unsafe.Add(ref imageStart, y * imageRowSizeInBytes);
+                Span<T> row = currentRows[y];
+
+                ref byte s = ref Unsafe.As<T, byte>(ref row[sourceX]);
                 ref byte d = ref Unsafe.Add(ref blockStart, y * blockRowSizeInBytes);
 
                 Unsafe.CopyBlock(ref d, ref s, byteWidth);
@@ -127,4 +125,4 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components
         /// </summary>
         public Span<T> AsSpanUnsafe() => new Span<T>(Unsafe.AsPointer(ref this), Size);
     }
-}
+}

src/ImageSharp/Formats/Jpeg/Components/RowOctet.cs

@@ -0,0 +1,60 @@
+// Copyright (c) Six Labors and contributors.
+// Licensed under the Apache License, Version 2.0.
+
+using System;
+using System.Runtime.InteropServices;
+using SixLabors.ImageSharp.Memory;
+
+namespace SixLabors.ImageSharp.Formats.Jpeg.Components
+{
+    /// <summary>
+    /// Cache 8 pixel rows on the stack, which may originate from different buffers of a <see cref="MemoryGroup{T}"/>.
+    /// </summary>
+    [StructLayout(LayoutKind.Sequential)]
+    internal readonly ref struct RowOctet<T>
+        where T : struct
+    {
+        private readonly Span<T> row0;
+        private readonly Span<T> row1;
+        private readonly Span<T> row2;
+        private readonly Span<T> row3;
+        private readonly Span<T> row4;
+        private readonly Span<T> row5;
+        private readonly Span<T> row6;
+        private readonly Span<T> row7;
+
+        public RowOctet(Buffer2D<T> buffer, int startY)
+        {
+            int y = startY;
+            int height = buffer.Height;
+            this.row0 = y < height ? buffer.GetRowSpan(y++) : default;
+            this.row1 = y < height ? buffer.GetRowSpan(y++) : default;
+            this.row2 = y < height ? buffer.GetRowSpan(y++) : default;
+            this.row3 = y < height ? buffer.GetRowSpan(y++) : default;
+            this.row4 = y < height ? buffer.GetRowSpan(y++) : default;
+            this.row5 = y < height ? buffer.GetRowSpan(y++) : default;
+            this.row6 = y < height ? buffer.GetRowSpan(y++) : default;
+            this.row7 = y < height ? buffer.GetRowSpan(y) : default;
+        }
+
+        public Span<T> this[int y]
+        {
+            get
+            {
+                // No unsafe tricks, since Span<T> can't be used as a generic argument
+                return y switch
+                {
+                    0 => this.row0,
+                    1 => this.row1,
+                    2 => this.row2,
+                    3 => this.row3,
+                    4 => this.row4,
+                    5 => this.row5,
+                    6 => this.row6,
+                    7 => this.row7,
+                    _ => throw new IndexOutOfRangeException()
+                };
+            }
+        }
+    }
+}

src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs

@@ -9,6 +9,7 @@ using SixLabors.ImageSharp.Common.Helpers;
 using SixLabors.ImageSharp.Formats.Jpeg.Components;
 using SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder;
 using SixLabors.ImageSharp.Formats.Jpeg.Components.Encoder;
+using SixLabors.ImageSharp.Memory;
 using SixLabors.ImageSharp.Metadata;
 using SixLabors.ImageSharp.Metadata.Profiles.Exif;
 using SixLabors.ImageSharp.Metadata.Profiles.Icc;
@@ -409,12 +410,16 @@ namespace SixLabors.ImageSharp.Formats.Jpeg
             int prevDCY = 0, prevDCCb = 0, prevDCCr = 0;
 
             var pixelConverter = YCbCrForwardConverter<TPixel>.Create();
+            ImageFrame<TPixel> frame = pixels.Frames.RootFrame;
+            Buffer2D<TPixel> pixelBuffer = frame.PixelBuffer;
 
             for (int y = 0; y < pixels.Height; y += 8)
             {
+                var currentRows = new RowOctet<TPixel>(pixelBuffer, y);
+
                 for (int x = 0; x < pixels.Width; x += 8)
                 {
-                    pixelConverter.Convert(pixels.Frames.RootFrame, x, y);
+                    pixelConverter.Convert(frame, x, y, currentRows);
 
                     prevDCY = this.WriteBlock(
                         QuantIndex.Luminance,
@@ -935,6 +940,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg
 
             // ReSharper disable once InconsistentNaming
             int prevDCY = 0, prevDCCb = 0, prevDCCr = 0;
+            ImageFrame<TPixel> frame = pixels.Frames.RootFrame;
+            Buffer2D<TPixel> pixelBuffer = frame.PixelBuffer;
 
             for (int y = 0; y < pixels.Height; y += 16)
             {
@@ -945,7 +952,10 @@ namespace SixLabors.ImageSharp.Formats.Jpeg
                         int xOff = (i & 1) * 8;
                         int yOff = (i & 2) * 4;
 
-                        pixelConverter.Convert(pixels.Frames.RootFrame, x + xOff, y + yOff);
+                        // TODO: Try pushing this to the outer loop!
+                        var currentRows = new RowOctet<TPixel>(pixelBuffer, y + yOff);
+
+                        pixelConverter.Convert(frame, x + xOff, y + yOff, currentRows);
 
                         cbPtr[i] = pixelConverter.Cb;
                         crPtr[i] = pixelConverter.Cr;

tests/ImageSharp.Tests/Formats/Jpg/GenericBlock8x8Tests.cs

@@ -41,7 +41,8 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg
             using (Image<TPixel> s = provider.GetImage())
             {
                 var d = default(GenericBlock8x8<TPixel>);
-                d.LoadAndStretchEdges(s.Frames.RootFrame, 0, 0);
+                var rowOctet = new RowOctet<TPixel>(s.GetRootFramePixelBuffer(), 0);
+                d.LoadAndStretchEdges(s.Frames.RootFrame.PixelBuffer, 0, 0, rowOctet);
 
                 TPixel a = s.Frames.RootFrame[0, 0];
                 TPixel b = d[0, 0];
@@ -65,7 +66,8 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg
             using (Image<TPixel> s = provider.GetImage())
             {
                 var d = default(GenericBlock8x8<TPixel>);
-                d.LoadAndStretchEdges(s.Frames.RootFrame, 6, 7);
+                var rowOctet = new RowOctet<TPixel>(s.GetRootFramePixelBuffer(), 7);
+                d.LoadAndStretchEdges(s.Frames.RootFrame.PixelBuffer, 6, 7, rowOctet);
 
                 Assert.Equal(s[6, 7], d[0, 0]);
                 Assert.Equal(s[6, 8], d[0, 1]);

tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs

@@ -3,6 +3,7 @@
 
 using System.IO;
 using SixLabors.ImageSharp.Formats.Jpeg;
+using SixLabors.ImageSharp.Memory;
 using SixLabors.ImageSharp.Metadata;
 using SixLabors.ImageSharp.PixelFormats;
 using SixLabors.ImageSharp.Processing;
@@ -81,9 +82,20 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg
             where TPixel : struct, IPixel<TPixel> => TestJpegEncoderCore(provider, subsample, quality);
 
         [Theory]
-        [WithTestPatternImages(nameof(BitsPerPixel_Quality), 600, 400, PixelTypes.Rgba32)]
+        [WithFile(TestImages.Png.CalliphoraPartial, PixelTypes.Rgba32, JpegSubsample.Ratio444)]
-        public void EncodeBaseline_WorksWithDiscontiguousBuffers<TPixel>(TestImageProvider<TPixel> provider, JpegSubsample subsample, int quality)
+        [WithTestPatternImages(587, 821, PixelTypes.Rgba32, JpegSubsample.Ratio444)]
-            where TPixel : struct, IPixel<TPixel> => TestJpegEncoderCore(provider, subsample, quality, true, ImageComparer.TolerantPercentage(0.1f));
+        [WithTestPatternImages(677, 683, PixelTypes.Bgra32, JpegSubsample.Ratio420)]
+        [WithSolidFilledImages(400, 400, "Red", PixelTypes.Bgr24, JpegSubsample.Ratio420)]
+        public void EncodeBaseline_WorksWithDiscontiguousBuffers<TPixel>(TestImageProvider<TPixel> provider, JpegSubsample subsample)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            ImageComparer comparer = subsample == JpegSubsample.Ratio444
+                ? ImageComparer.TolerantPercentage(0.1f)
+                : ImageComparer.TolerantPercentage(5f);
+
+            provider.LimitAllocatorBufferCapacity();
+            TestJpegEncoderCore(provider, subsample, 100, comparer);
+        }
 
         /// <summary>
         /// Anton's SUPER-SCIENTIFIC tolerance threshold calculation
@@ -112,15 +124,9 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg
             TestImageProvider<TPixel> provider,
             JpegSubsample subsample,
             int quality = 100,
-            bool enforceDiscontiguousBuffers = false,
             ImageComparer comparer = null)
             where TPixel : struct, IPixel<TPixel>
         {
-            if (enforceDiscontiguousBuffers)
-            {
-                provider.LimitAllocatorBufferCapacity();
-            }
-
             using Image<TPixel> image = provider.GetImage();
 
             // There is no alpha in Jpeg!
@@ -132,10 +138,6 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg
                 Quality = quality
             };
             string info = $"{subsample}-Q{quality}";
-            if (enforceDiscontiguousBuffers)
-            {
-                info += "-Disco";
-            }
 
             comparer ??= GetComparer(quality, subsample);
 

tests/ImageSharp.Tests/TestUtilities/ImageProviders/SolidProvider.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0.
 
 using SixLabors.ImageSharp.Advanced;
+using SixLabors.ImageSharp.Memory;
 using SixLabors.ImageSharp.PixelFormats;
 using Xunit.Abstractions;
 
@@ -53,7 +54,7 @@ namespace SixLabors.ImageSharp.Tests
                 Image<TPixel> image = base.GetImage();
                 Color color = new Rgba32(this.r, this.g, this.b, this.a);
 
-                image.GetPixelSpan().Fill(color.ToPixel<TPixel>());
+                image.GetRootFramePixelBuffer().MemoryGroup.Fill(color.ToPixel<TPixel>());
                 return image;
             }
 

tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs

@@ -3,12 +3,14 @@
 
 using System;
 using System.IO;
+using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 
 using ImageMagick;
 
 using SixLabors.ImageSharp.Advanced;
 using SixLabors.ImageSharp.Formats;
+using SixLabors.ImageSharp.Memory;
 using SixLabors.ImageSharp.PixelFormats;
 
 namespace SixLabors.ImageSharp.Tests.TestUtilities.ReferenceCodecs
@@ -17,45 +19,64 @@ namespace SixLabors.ImageSharp.Tests.TestUtilities.ReferenceCodecs
     {
         public static MagickReferenceDecoder Instance { get; } = new MagickReferenceDecoder();
 
+        private static void FromRgba32Bytes<TPixel>(Configuration configuration, Span<byte> rgbaBytes, IMemoryGroup<TPixel> destinationGroup)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            foreach (Memory<TPixel> m in destinationGroup)
+            {
+                Span<TPixel> destBuffer = m.Span;
+                PixelOperations<TPixel>.Instance.FromRgba32Bytes(
+                    configuration,
+                    rgbaBytes,
+                    destBuffer,
+                    destBuffer.Length);
+                rgbaBytes = rgbaBytes.Slice(destBuffer.Length * 4);
+            }
+        }
+
+        private static void FromRgba64Bytes<TPixel>(Configuration configuration, Span<byte> rgbaBytes, IMemoryGroup<TPixel> destinationGroup)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            foreach (Memory<TPixel> m in destinationGroup)
+            {
+                Span<TPixel> destBuffer = m.Span;
+                PixelOperations<TPixel>.Instance.FromRgba64Bytes(
+                    configuration,
+                    rgbaBytes,
+                    destBuffer,
+                    destBuffer.Length);
+                rgbaBytes = rgbaBytes.Slice(destBuffer.Length * 8);
+            }
+        }
+
         public Image<TPixel> Decode<TPixel>(Configuration configuration, Stream stream)
             where TPixel : struct, IPixel<TPixel>
         {
-            using (var magickImage = new MagickImage(stream))
+            using var magickImage = new MagickImage(stream);
+            var result = new Image<TPixel>(configuration, magickImage.Width, magickImage.Height);
+            MemoryGroup<TPixel> resultPixels = result.GetRootFramePixelBuffer().MemoryGroup;
+
+            using (IPixelCollection pixels = magickImage.GetPixelsUnsafe())
             {
-                var result = new Image<TPixel>(configuration, magickImage.Width, magickImage.Height);
+                if (magickImage.Depth == 8)
-                Span<TPixel> resultPixels = result.GetPixelSpan();
+                {
+                    byte[] data = pixels.ToByteArray(PixelMapping.RGBA);
 
-                using (IPixelCollection pixels = magickImage.GetPixelsUnsafe())
+                    FromRgba32Bytes(configuration, data, resultPixels);
+                }
+                else if (magickImage.Depth == 16)
                 {
-                    if (magickImage.Depth == 8)
+                    ushort[] data = pixels.ToShortArray(PixelMapping.RGBA);
-                    {
+                    Span<byte> bytes = MemoryMarshal.Cast<ushort, byte>(data.AsSpan());
-                        byte[] data = pixels.ToByteArray(PixelMapping.RGBA);
+                    FromRgba64Bytes(configuration, bytes, resultPixels);
-
+                }
-                        PixelOperations<TPixel>.Instance.FromRgba32Bytes(
+                else
-                            configuration,
+                {
-                            data,
+                    throw new InvalidOperationException();
-                            resultPixels,
-                            resultPixels.Length);
-                    }
-                    else if (magickImage.Depth == 16)
-                    {
-                        ushort[] data = pixels.ToShortArray(PixelMapping.RGBA);
-                        Span<byte> bytes = MemoryMarshal.Cast<ushort, byte>(data.AsSpan());
-
-                        PixelOperations<TPixel>.Instance.FromRgba64Bytes(
-                            configuration,
-                            bytes,
-                            resultPixels,
-                            resultPixels.Length);
-                    }
-                    else
-                    {
-                        throw new InvalidOperationException();
-                    }
                 }
-
-                return result;
             }
+
+            return result;
         }
 
         public Image Decode(Configuration configuration, Stream stream) => this.Decode<Rgba32>(configuration, stream);

tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs

@@ -657,12 +657,12 @@ namespace SixLabors.ImageSharp.Tests
                 testOutputDetails,
                 appendPixelTypeToFileName);
 
-            referenceDecoder = referenceDecoder ?? TestEnvironment.GetReferenceDecoder(actualOutputFile);
+            referenceDecoder ??= TestEnvironment.GetReferenceDecoder(actualOutputFile);
 
-            using (var actualImage = Image.Load<TPixel>(actualOutputFile, referenceDecoder))
+            using (var encodedImage = Image.Load<TPixel>(actualOutputFile, referenceDecoder))
             {
                 ImageComparer comparer = customComparer ?? ImageComparer.Exact;
-                comparer.VerifySimilarity(actualImage, image);
+                comparer.VerifySimilarity(encodedImage, image);
             }
         }