Merge pull request #1109 from SixLabors/af/disco-buffers

Implement discontiguous buffer handling

Author: JimBobSquarePants

shared-infrastructure

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0.
 
 using System;
+using System.Linq;
 using System.Runtime.InteropServices;
 
 using SixLabors.ImageSharp.Memory;
@@ -40,23 +41,66 @@ public static Configuration GetConfiguration(this ImageFrame source)
             => GetConfiguration((IConfigurationProvider)source);
 
         /// <summary>
-        /// Gets the configuration .
+        /// Gets the configuration.
         /// </summary>
         /// <param name="source">The source image</param>
         /// <returns>Returns the bounds of the image</returns>
         private static Configuration GetConfiguration(IConfigurationProvider source)
             => source?.Configuration ?? Configuration.Default;
 
         /// <summary>
-        /// Gets the representation of the pixels as a <see cref="Span{T}"/> of contiguous memory in the source image's pixel format
-        /// stored in row major order.
+        /// Gets the representation of the pixels as a <see cref="IMemoryGroup{T}"/> containing the backing pixel data of the image
+        /// stored in row major order, as a list of contiguous <see cref="Memory{T}"/> blocks in the source image's pixel format.
         /// </summary>
+        /// <param name="source">The source image.</param>
         /// <typeparam name="TPixel">The type of the pixel.</typeparam>
-        /// <param name="source">The source.</param>
+        /// <returns>The <see cref="IMemoryGroup{T}"/>.</returns>
+        /// <remarks>
+        /// Certain Image Processors may invalidate the returned <see cref="IMemoryGroup{T}"/> and all it's buffers,
+        /// therefore it's not recommended to mutate the image while holding a reference to it's <see cref="IMemoryGroup{T}"/>.
+        /// </remarks>
+        public static IMemoryGroup<TPixel> GetPixelMemoryGroup<TPixel>(this ImageFrame<TPixel> source)
+            where TPixel : struct, IPixel<TPixel>
+            => source?.PixelBuffer.FastMemoryGroup.View ?? throw new ArgumentNullException(nameof(source));
+
+        /// <summary>
+        /// Gets the representation of the pixels as a <see cref="IMemoryGroup{T}"/> containing the backing pixel data of the image
+        /// stored in row major order, as a list of contiguous <see cref="Memory{T}"/> blocks in the source image's pixel format.
+        /// </summary>
+        /// <param name="source">The source image.</param>
+        /// <typeparam name="TPixel">The type of the pixel.</typeparam>
+        /// <returns>The <see cref="IMemoryGroup{T}"/>.</returns>
+        /// <remarks>
+        /// Certain Image Processors may invalidate the returned <see cref="IMemoryGroup{T}"/> and all it's buffers,
+        /// therefore it's not recommended to mutate the image while holding a reference to it's <see cref="IMemoryGroup{T}"/>.
+        /// </remarks>
+        public static IMemoryGroup<TPixel> GetPixelMemoryGroup<TPixel>(this Image<TPixel> source)
+            where TPixel : struct, IPixel<TPixel>
+            => source?.Frames.RootFrame.GetPixelMemoryGroup() ?? throw new ArgumentNullException(nameof(source));
+
+        /// <summary>
+        /// Gets the representation of the pixels as a <see cref="Span{T}"/> in the source image's pixel format
+        /// stored in row major order, if the backing buffer is contiguous.
+        /// </summary>
+        /// <typeparam name="TPixel">The type of the pixel.</typeparam>
+        /// <param name="source">The source image.</param>
         /// <returns>The <see cref="Span{TPixel}"/></returns>
+        /// <exception cref="InvalidOperationException">Thrown when the backing buffer is discontiguous.</exception>
+        [Obsolete(
+            @"GetPixelSpan might fail, because the backing buffer could be discontiguous for large images. Use GetPixelMemoryGroup or GetPixelRowSpan instead!")]
         public static Span<TPixel> GetPixelSpan<TPixel>(this ImageFrame<TPixel> source)
             where TPixel : struct, IPixel<TPixel>
-            => source.GetPixelMemory().Span;
+        {
+            Guard.NotNull(source, nameof(source));
+
+            IMemoryGroup<TPixel> mg = source.GetPixelMemoryGroup();
+            if (mg.Count > 1)
+            {
+                throw new InvalidOperationException($"GetPixelSpan is invalid, since the backing buffer of this {source.Width}x{source.Height} sized image is discontiguous!");
+            }
+
+            return mg.Single().Span;
+        }
 
         /// <summary>
         /// Gets the representation of the pixels as a <see cref="Span{T}"/> of contiguous memory in the source image's pixel format
@@ -65,9 +109,16 @@ public static Span<TPixel> GetPixelSpan<TPixel>(this ImageFrame<TPixel> source)
         /// <typeparam name="TPixel">The type of the pixel.</typeparam>
         /// <param name="source">The source.</param>
         /// <returns>The <see cref="Span{TPixel}"/></returns>
+        /// <exception cref="InvalidOperationException">Thrown when the backing buffer is discontiguous.</exception>
+        [Obsolete(
+            @"GetPixelSpan might fail, because the backing buffer could be discontiguous for large images. Use GetPixelMemoryGroup or GetPixelRowSpan instead!")]
         public static Span<TPixel> GetPixelSpan<TPixel>(this Image<TPixel> source)
             where TPixel : struct, IPixel<TPixel>
-            => source.Frames.RootFrame.GetPixelSpan();
+        {
+            Guard.NotNull(source, nameof(source));
+
+            return source.Frames.RootFrame.GetPixelSpan();
+        }
 
         /// <summary>
         /// Gets the representation of the pixels as a <see cref="Span{T}"/> of contiguous memory
@@ -79,7 +130,13 @@ public static Span<TPixel> GetPixelSpan<TPixel>(this Image<TPixel> source)
         /// <returns>The <see cref="Span{TPixel}"/></returns>
         public static Span<TPixel> GetPixelRowSpan<TPixel>(this ImageFrame<TPixel> source, int rowIndex)
             where TPixel : struct, IPixel<TPixel>
-            => source.PixelBuffer.GetRowSpan(rowIndex);
+        {
+            Guard.NotNull(source, nameof(source));
+            Guard.MustBeGreaterThanOrEqualTo(rowIndex, 0, nameof(rowIndex));
+            Guard.MustBeLessThan(rowIndex, source.Height, nameof(rowIndex));
+
+            return source.PixelBuffer.GetRowSpan(rowIndex);
+        }
 
         /// <summary>
         /// Gets the representation of the pixels as <see cref="Span{T}"/> of of contiguous memory
@@ -91,58 +148,12 @@ public static Span<TPixel> GetPixelRowSpan<TPixel>(this ImageFrame<TPixel> sourc
         /// <returns>The <see cref="Span{TPixel}"/></returns>
         public static Span<TPixel> GetPixelRowSpan<TPixel>(this Image<TPixel> source, int rowIndex)
             where TPixel : struct, IPixel<TPixel>
-            => source.Frames.RootFrame.GetPixelRowSpan(rowIndex);
-
-        /// <summary>
-        /// Returns a reference to the 0th element of the Pixel buffer,
-        /// allowing direct manipulation of pixel data through unsafe operations.
-        /// The pixel buffer is a contiguous memory area containing Width*Height TPixel elements laid out in row-major order.
-        /// </summary>
-        /// <typeparam name="TPixel">The Pixel format.</typeparam>
-        /// <param name="source">The source image frame</param>
-        /// <returns>A pinnable reference the first root of the pixel buffer.</returns>
-        [Obsolete("This method will be removed in our next release! Please use MemoryMarshal.GetReference(source.GetPixelSpan())!")]
-        public static ref TPixel DangerousGetPinnableReferenceToPixelBuffer<TPixel>(this ImageFrame<TPixel> source)
-            where TPixel : struct, IPixel<TPixel>
-            => ref DangerousGetPinnableReferenceToPixelBuffer((IPixelSource<TPixel>)source);
-
-        /// <summary>
-        /// Returns a reference to the 0th element of the Pixel buffer,
-        /// allowing direct manipulation of pixel data through unsafe operations.
-        /// The pixel buffer is a contiguous memory area containing Width*Height TPixel elements laid out in row-major order.
-        /// </summary>
-        /// <typeparam name="TPixel">The Pixel format.</typeparam>
-        /// <param name="source">The source image</param>
-        /// <returns>A pinnable reference the first root of the pixel buffer.</returns>
-        [Obsolete("This method will be removed in our next release! Please use MemoryMarshal.GetReference(source.GetPixelSpan())!")]
-        public static ref TPixel DangerousGetPinnableReferenceToPixelBuffer<TPixel>(this Image<TPixel> source)
-            where TPixel : struct, IPixel<TPixel>
-            => ref source.Frames.RootFrame.DangerousGetPinnableReferenceToPixelBuffer();
-
-        /// <summary>
-        /// Gets the representation of the pixels as a <see cref="Memory{T}"/> of contiguous memory in the source image's pixel format
-        /// stored in row major order.
-        /// </summary>
-        /// <typeparam name="TPixel">The Pixel format.</typeparam>
-        /// <param name="source">The source <see cref="ImageFrame{TPixel}"/></param>
-        /// <returns>The <see cref="Memory{T}"/></returns>
-        internal static Memory<TPixel> GetPixelMemory<TPixel>(this ImageFrame<TPixel> source)
-            where TPixel : struct, IPixel<TPixel>
         {
-            return source.PixelBuffer.MemorySource.Memory;
-        }
+            Guard.NotNull(source, nameof(source));
+            Guard.MustBeGreaterThanOrEqualTo(rowIndex, 0, nameof(rowIndex));
+            Guard.MustBeLessThan(rowIndex, source.Height, nameof(rowIndex));
 
-        /// <summary>
-        /// Gets the representation of the pixels as a <see cref="Memory{T}"/> of contiguous memory in the source image's pixel format
-        /// stored in row major order.
-        /// </summary>
-        /// <typeparam name="TPixel">The Pixel format.</typeparam>
-        /// <param name="source">The source <see cref="Image{TPixel}"/></param>
-        /// <returns>The <see cref="Memory{T}"/></returns>
-        internal static Memory<TPixel> GetPixelMemory<TPixel>(this Image<TPixel> source)
-            where TPixel : struct, IPixel<TPixel>
-        {
-            return source.Frames.RootFrame.GetPixelMemory();
+            return source.Frames.RootFrame.PixelBuffer.GetRowSpan(rowIndex);
         }
 
         /// <summary>
@@ -153,9 +164,15 @@ internal static Memory<TPixel> GetPixelMemory<TPixel>(this Image<TPixel> source)
         /// <param name="source">The source.</param>
         /// <param name="rowIndex">The row.</param>
         /// <returns>The <see cref="Span{TPixel}"/></returns>
-        internal static Memory<TPixel> GetPixelRowMemory<TPixel>(this ImageFrame<TPixel> source, int rowIndex)
+        public static Memory<TPixel> GetPixelRowMemory<TPixel>(this ImageFrame<TPixel> source, int rowIndex)
             where TPixel : struct, IPixel<TPixel>
-            => source.PixelBuffer.GetRowMemory(rowIndex);
+        {
+            Guard.NotNull(source, nameof(source));
+            Guard.MustBeGreaterThanOrEqualTo(rowIndex, 0, nameof(rowIndex));
+            Guard.MustBeLessThan(rowIndex, source.Height, nameof(rowIndex));
+
+            return source.PixelBuffer.GetSafeRowMemory(rowIndex);
+        }
 
         /// <summary>
         /// Gets the representation of the pixels as <see cref="Span{T}"/> of of contiguous memory
@@ -165,9 +182,15 @@ internal static Memory<TPixel> GetPixelRowMemory<TPixel>(this ImageFrame<TPixel>
         /// <param name="source">The source.</param>
         /// <param name="rowIndex">The row.</param>
         /// <returns>The <see cref="Span{TPixel}"/></returns>
-        internal static Memory<TPixel> GetPixelRowMemory<TPixel>(this Image<TPixel> source, int rowIndex)
+        public static Memory<TPixel> GetPixelRowMemory<TPixel>(this Image<TPixel> source, int rowIndex)
             where TPixel : struct, IPixel<TPixel>
-            => source.Frames.RootFrame.GetPixelRowMemory(rowIndex);
+        {
+            Guard.NotNull(source, nameof(source));
+            Guard.MustBeGreaterThanOrEqualTo(rowIndex, 0, nameof(rowIndex));
+            Guard.MustBeLessThan(rowIndex, source.Height, nameof(rowIndex));
+
+            return source.Frames.RootFrame.PixelBuffer.GetSafeRowMemory(rowIndex);
+        }
 
         /// <summary>
         /// Gets the <see cref="MemoryAllocator"/> assigned to 'source'.
@@ -176,15 +199,5 @@ internal static Memory<TPixel> GetPixelRowMemory<TPixel>(this Image<TPixel> sour
         /// <returns>Returns the configuration.</returns>
         internal static MemoryAllocator GetMemoryAllocator(this IConfigurationProvider source)
             => GetConfiguration(source).MemoryAllocator;
-
-        /// <summary>
-        /// Returns a reference to the 0th element of the Pixel buffer.
-        /// Such a reference can be used for pinning but must never be dereferenced.
-        /// </summary>
-        /// <param name="source">The source image frame</param>
-        /// <returns>A reference to the element.</returns>
-        private static ref TPixel DangerousGetPinnableReferenceToPixelBuffer<TPixel>(IPixelSource<TPixel> source)
-            where TPixel : struct, IPixel<TPixel>
-            => ref MemoryMarshal.GetReference(source.PixelBuffer.GetSpan());
     }
 }

src/ImageSharp/Advanced/AdvancedImageExtensions.cs

@@ -7,7 +7,7 @@ namespace SixLabors.ImageSharp
 {
     /// <summary>
     /// The exception that is thrown when the library tries to load
-    /// an image, which has an invalid format.
+    /// an image, which has format or content that is invalid or unsupported by ImageSharp.
     /// </summary>
     public class ImageFormatException : Exception
     {

src/ImageSharp/Common/Exceptions/ImageFormatException.cs

@@ -108,7 +108,8 @@ public int MaxDegreeOfParallelism
         /// The default value is 1MB.
         /// </summary>
         /// <remarks>
-        /// Currently only used by Resize.
+        /// Currently only used by Resize. If the working buffer is expected to be discontiguous,
+        /// min(WorkingBufferSizeHintInBytes, BufferCapacityInBytes) should be used.
         /// </remarks>
         internal int WorkingBufferSizeHintInBytes { get; set; } = 1 * 1024 * 1024;
 

src/ImageSharp/Configuration.cs

@@ -1,7 +1,8 @@
-// Copyright (c) Six Labors and contributors.
+// Copyright (c) Six Labors and contributors.
 // Licensed under the Apache License, Version 2.0.
 
 using System.IO;
+using SixLabors.ImageSharp.Memory;
 using SixLabors.ImageSharp.PixelFormats;
 
 namespace SixLabors.ImageSharp.Formats.Bmp
@@ -32,7 +33,20 @@ public Image<TPixel> Decode<TPixel>(Configuration configuration, Stream stream)
         {
             Guard.NotNull(stream, nameof(stream));
 
-            return new BmpDecoderCore(configuration, this).Decode<TPixel>(stream);
+            var decoder = new BmpDecoderCore(configuration, this);
+
+            try
+            {
+                return decoder.Decode<TPixel>(stream);
+            }
+            catch (InvalidMemoryOperationException ex)
+            {
+                Size dims = decoder.Dimensions;
+
+                // TODO: use InvalidImageContentException here, if we decide to define it
+                // https://github.com/SixLabors/ImageSharp/issues/1110
+                throw new ImageFormatException($"Can not decode image. Failed to allocate buffers for possibly degenerate dimensions: {dims.Width}x{dims.Height}. This error can happen for very large RLE bitmaps, which are not supported.", ex);
+            }
         }
 
         /// <inheritdoc />