From 17699d1953b54ac29ade9d8fc876aa97fcb9027d Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 3 Mar 2026 17:48:55 +1000 Subject: [PATCH 1/3] Add row-stride overloads for memory APIs --- src/ImageSharp/Image.LoadPixelData.cs | 120 +++- src/ImageSharp/Image.WrapMemory.cs | 534 ++++++++++++------ src/ImageSharp/ImageFrame.LoadPixelData.cs | 63 ++- src/ImageSharp/ImageFrame.cs | 2 +- .../ImageFrameCollection{TPixel}.cs | 9 +- src/ImageSharp/ImageFrame{TPixel}.cs | 99 ++-- src/ImageSharp/Image{TPixel}.cs | 53 +- src/ImageSharp/Memory/Buffer2DExtensions.cs | 6 +- src/ImageSharp/Memory/Buffer2DRegion{T}.cs | 12 +- src/ImageSharp/Memory/Buffer2D{T}.cs | 229 +++++++- .../MemoryGroupExtensions.cs | 191 ++++--- .../Memory/MemoryAllocatorExtensions.cs | 6 + ...ransformItemsDelegate{TSource, TTarget}.cs | 8 - .../Memory/TransformItemsInplaceDelegate.cs | 6 - .../Transforms/CropProcessor{TPixel}.cs | 2 +- .../Linear/RotateProcessor{TPixel}.cs | 2 +- .../Resize/ResizeProcessor{TPixel}.cs | 2 +- .../ImageFrameCollectionTests.Generic.cs | 2 +- .../Image/ImageTests.LoadPixelData.cs | 74 +++ .../Image/ImageTests.WrapMemory.cs | 97 ++++ .../Memory/Buffer2DTests.CopyTo.cs | 43 ++ .../Memory/Buffer2DTests.SwapOrCopyContent.cs | 69 +++ .../Memory/Buffer2DTests.WrapMemory.cs | 91 +++ .../ImageSharp.Tests/Memory/Buffer2DTests.cs | 46 +- .../MemoryGroupTests.CopyTo.cs | 126 ++--- .../DiscontiguousBuffers/MemoryGroupTests.cs | 78 --- 26 files changed, 1438 insertions(+), 532 deletions(-) delete mode 100644 src/ImageSharp/Memory/TransformItemsDelegate{TSource, TTarget}.cs delete mode 100644 src/ImageSharp/Memory/TransformItemsInplaceDelegate.cs create mode 100644 tests/ImageSharp.Tests/Memory/Buffer2DTests.CopyTo.cs create mode 100644 tests/ImageSharp.Tests/Memory/Buffer2DTests.WrapMemory.cs diff --git a/src/ImageSharp/Image.LoadPixelData.cs b/src/ImageSharp/Image.LoadPixelData.cs index efe1b6e2fb..2847f0d57d 100644 --- a/src/ImageSharp/Image.LoadPixelData.cs +++ b/src/ImageSharp/Image.LoadPixelData.cs @@ -2,7 +2,6 @@ // Licensed under the Six Labors Split License. using System.Runtime.InteropServices; -using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.PixelFormats; namespace SixLabors.ImageSharp; @@ -25,6 +24,27 @@ public abstract partial class Image where TPixel : unmanaged, IPixel => LoadPixelData(Configuration.Default, data, width, height); + /// + /// Create a new instance of the class from raw data + /// using pixels between source row starts. + /// + /// The readonly span containing image data. + /// The width of the final image. + /// The height of the final image. + /// The number of pixels between row starts in . + /// The pixel format. + /// + /// or is not positive, + /// or is less than . + /// + /// + /// is smaller than ((height - 1) * rowStride) + width. + /// + /// A new . + public static Image LoadPixelData(ReadOnlySpan data, int width, int height, int rowStride) + where TPixel : unmanaged, IPixel + => LoadPixelData(Configuration.Default, data, width, height, rowStride); + /// /// Create a new instance of the class from the given readonly span of bytes in format. /// @@ -38,6 +58,28 @@ public abstract partial class Image where TPixel : unmanaged, IPixel => LoadPixelData(Configuration.Default, data, width, height); + /// + /// Create a new instance of the class from a readonly span of bytes in + /// format using bytes between source row starts. + /// + /// The readonly span containing image data. + /// The width of the final image. + /// The height of the final image. + /// The number of bytes between row starts in . + /// The pixel format. + /// + /// or is not positive, + /// or resolves to fewer than pixels. + /// + /// + /// is not divisible by the pixel size, + /// or is smaller than the required strided image length. + /// + /// A new . + public static Image LoadPixelData(ReadOnlySpan data, int width, int height, int rowStrideInBytes) + where TPixel : unmanaged, IPixel + => LoadPixelData(Configuration.Default, data, width, height, rowStrideInBytes); + /// /// Create a new instance of the class from the given readonly span of bytes in format. /// @@ -53,6 +95,40 @@ public abstract partial class Image where TPixel : unmanaged, IPixel => LoadPixelData(configuration, MemoryMarshal.Cast(data), width, height); + /// + /// Create a new instance of the class from a readonly span of bytes in + /// format using bytes between source row starts. + /// + /// The configuration for the decoder. + /// The readonly span containing image data. + /// The width of the final image. + /// The height of the final image. + /// The number of bytes between row starts in . + /// The pixel format. + /// The configuration is null. + /// + /// or is not positive, + /// or resolves to fewer than pixels. + /// + /// + /// is not divisible by the pixel size, + /// or is smaller than the required strided image length. + /// + /// A new . + public static Image LoadPixelData( + Configuration configuration, + ReadOnlySpan data, + int width, + int height, + int rowStrideInBytes) + where TPixel : unmanaged, IPixel + { + Guard.NotNull(configuration, nameof(configuration)); + + int rowStride = GetPixelRowStrideFromByteStride(width, rowStrideInBytes, nameof(rowStrideInBytes)); + return LoadPixelData(configuration, MemoryMarshal.Cast(data), width, height, rowStride); + } + /// /// Create a new instance of the class from the raw data. /// @@ -66,20 +142,42 @@ public abstract partial class Image /// A new . public static Image LoadPixelData(Configuration configuration, ReadOnlySpan data, int width, int height) where TPixel : unmanaged, IPixel + => LoadPixelData(configuration, data, width, height, width); + + /// + /// Create a new instance of the class from raw data + /// using pixels between source row starts. + /// + /// The configuration for the decoder. + /// The readonly span containing the image pixel data. + /// The width of the final image. + /// The height of the final image. + /// The number of pixels between row starts in . + /// The configuration is null. + /// + /// or is not positive, + /// or is less than . + /// + /// + /// is smaller than ((height - 1) * rowStride) + width. + /// + /// The pixel format. + /// A new . + public static Image LoadPixelData( + Configuration configuration, + ReadOnlySpan data, + int width, + int height, + int rowStride) + where TPixel : unmanaged, IPixel { Guard.NotNull(configuration, nameof(configuration)); - - if (data.IsEmpty) - { - throw new ArgumentException("Pixel data cannot be empty.", nameof(data)); - } - - int count = width * height; - Guard.MustBeGreaterThanOrEqualTo(data.Length, count, nameof(data)); + ValidateWrapMemoryStride(width, height, rowStride, nameof(rowStride)); + long requiredLength = GetRequiredLength(width, height, rowStride); + Guard.MustBeGreaterThanOrEqualTo(data.Length, requiredLength, nameof(data)); Image image = new(configuration, width, height); - data = data[..count]; - data.CopyTo(image.Frames.RootFrame.PixelBuffer.FastMemoryGroup); + image.Frames.RootFrame.PixelBuffer.CopyFrom(data, rowStride); return image; } diff --git a/src/ImageSharp/Image.WrapMemory.cs b/src/ImageSharp/Image.WrapMemory.cs index 03bec8bc6a..e44af5d0d1 100644 --- a/src/ImageSharp/Image.WrapMemory.cs +++ b/src/ImageSharp/Image.WrapMemory.cs @@ -2,6 +2,7 @@ // Licensed under the Six Labors Split License. using System.Buffers; +using System.Runtime.CompilerServices; using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.PixelFormats; @@ -58,29 +59,55 @@ public abstract partial class Image /// /// - /// Wraps an existing contiguous memory area of at least 'width' x 'height' pixels allowing viewing/manipulation as - /// an instance. - /// - /// - /// Please note: using this method does not transfer the ownership of the underlying buffer of the input - /// to the new instance. This means that consumers of this method must ensure that the input buffer - /// is either self-contained, (for example, a instance wrapping a new array that was - /// created), or that the owning object is not disposed until the returned is disposed. + /// Wraps an existing memory area allowing viewing/manipulation as an + /// with pixels between row starts. /// /// - /// If the input instance is one retrieved from an instance - /// rented from a memory pool (such as ), and that owning instance is disposed while the image is still - /// in use, this will lead to undefined behavior and possibly runtime crashes (as the same buffer might then be modified by other - /// consumers while the returned image is still working on it). Make sure to control the lifetime of the input buffers appropriately. + /// Please note: using this method does not transfer the ownership of the underlying buffer of the input + /// to the new instance. Consumers must ensure that + /// the input buffer remains valid for the full lifetime of the returned image. /// /// - /// The pixel type - /// The - /// The pixel memory. - /// The width of the memory image. - /// The height of the memory image. + /// The pixel type. + /// The . + /// The source pixel memory. + /// The width of the memory image in pixels. + /// The height of the memory image in pixels. + /// The number of pixels between row starts in . + /// The . /// The configuration is null. + /// The metadata is null. + /// + /// or is not positive, + /// or is less than . + /// + /// + /// The length of is less than + /// ((height - 1) * rowStride) + width. + /// /// An instance. + public static Image WrapMemory( + Configuration configuration, + Memory pixelMemory, + int width, + int height, + int rowStride, + ImageMetadata metadata) + where TPixel : unmanaged, IPixel + { + Guard.NotNull(configuration, nameof(configuration)); + Guard.NotNull(metadata, nameof(metadata)); + + ValidateWrapMemoryStride(width, height, rowStride, nameof(rowStride)); + + long requiredLength = GetRequiredLength(width, height, rowStride); + Guard.IsTrue(pixelMemory.Length >= requiredLength, nameof(pixelMemory), "The length of the input memory is less than the specified image size"); + + MemoryGroup memorySource = MemoryGroup.Wrap(pixelMemory); + return new Image(configuration, memorySource, width, height, rowStride, metadata); + } + + /// public static Image WrapMemory( Configuration configuration, Memory pixelMemory, @@ -89,29 +116,17 @@ public abstract partial class Image where TPixel : unmanaged, IPixel => WrapMemory(configuration, pixelMemory, width, height, new ImageMetadata()); - /// - /// - /// Wraps an existing contiguous memory area of at least 'width' x 'height' pixels allowing viewing/manipulation as - /// an instance. - /// - /// - /// Please note: using this method does not transfer the ownership of the underlying buffer of the input - /// to the new instance. This means that consumers of this method must ensure that the input buffer - /// is either self-contained, (for example, a instance wrapping a new array that was - /// created), or that the owning object is not disposed until the returned is disposed. - /// - /// - /// If the input instance is one retrieved from an instance - /// rented from a memory pool (such as ), and that owning instance is disposed while the image is still - /// in use, this will lead to undefined behavior and possibly runtime crashes (as the same buffer might then be modified by other - /// consumers while the returned image is still working on it). Make sure to control the lifetime of the input buffers appropriately. - /// - /// - /// The pixel type. - /// The pixel memory. - /// The width of the memory image. - /// The height of the memory image. - /// An instance. + /// + public static Image WrapMemory( + Configuration configuration, + Memory pixelMemory, + int width, + int height, + int rowStride) + where TPixel : unmanaged, IPixel + => WrapMemory(configuration, pixelMemory, width, height, rowStride, new ImageMetadata()); + + /// public static Image WrapMemory( Memory pixelMemory, int width, @@ -119,6 +134,15 @@ public abstract partial class Image where TPixel : unmanaged, IPixel => WrapMemory(Configuration.Default, pixelMemory, width, height); + /// + public static Image WrapMemory( + Memory pixelMemory, + int width, + int height, + int rowStride) + where TPixel : unmanaged, IPixel + => WrapMemory(Configuration.Default, pixelMemory, width, height, rowStride); + /// /// Wraps an existing contiguous memory area of at least 'width' x 'height' pixels, /// allowing to view/manipulate it as an instance. @@ -152,19 +176,55 @@ public abstract partial class Image } /// - /// Wraps an existing contiguous memory area of at least 'width' x 'height' pixels, - /// allowing to view/manipulate it as an instance. - /// The ownership of the is being transferred to the new instance, - /// meaning that the caller is not allowed to dispose . - /// It will be disposed together with the result image. + /// + /// Wraps an existing memory owner allowing viewing/manipulation as an + /// with pixels between row starts. + /// + /// + /// Ownership of is transferred to the returned image. The caller + /// must not dispose the owner manually. + /// /// /// The pixel type. - /// The - /// The that is being transferred to the image. - /// The width of the memory image. - /// The height of the memory image. + /// The . + /// The pixel memory owner transferred to the image. + /// The width of the memory image in pixels. + /// The height of the memory image in pixels. + /// The number of pixels between row starts in the source memory. + /// The . /// The configuration is null. - /// An instance + /// The metadata is null. + /// + /// or is not positive, + /// or is less than . + /// + /// + /// The length of is less than + /// ((height - 1) * rowStride) + width. + /// + /// An instance. + public static Image WrapMemory( + Configuration configuration, + IMemoryOwner pixelMemoryOwner, + int width, + int height, + int rowStride, + ImageMetadata metadata) + where TPixel : unmanaged, IPixel + { + Guard.NotNull(configuration, nameof(configuration)); + Guard.NotNull(metadata, nameof(metadata)); + + ValidateWrapMemoryStride(width, height, rowStride, nameof(rowStride)); + + long requiredLength = GetRequiredLength(width, height, rowStride); + Guard.IsTrue(pixelMemoryOwner.Memory.Length >= requiredLength, nameof(pixelMemoryOwner), "The length of the input memory is less than the specified image size"); + + MemoryGroup memorySource = MemoryGroup.Wrap(pixelMemoryOwner); + return new Image(configuration, memorySource, width, height, rowStride, metadata); + } + + /// public static Image WrapMemory( Configuration configuration, IMemoryOwner pixelMemoryOwner, @@ -173,18 +233,17 @@ public abstract partial class Image where TPixel : unmanaged, IPixel => WrapMemory(configuration, pixelMemoryOwner, width, height, new ImageMetadata()); - /// - /// Wraps an existing contiguous memory area of at least 'width' x 'height' pixels, - /// allowing to view/manipulate it as an instance. - /// The ownership of the is being transferred to the new instance, - /// meaning that the caller is not allowed to dispose . - /// It will be disposed together with the result image. - /// - /// The pixel type - /// The that is being transferred to the image. - /// The width of the memory image. - /// The height of the memory image. - /// An instance. + /// + public static Image WrapMemory( + Configuration configuration, + IMemoryOwner pixelMemoryOwner, + int width, + int height, + int rowStride) + where TPixel : unmanaged, IPixel + => WrapMemory(configuration, pixelMemoryOwner, width, height, rowStride, new ImageMetadata()); + + /// public static Image WrapMemory( IMemoryOwner pixelMemoryOwner, int width, @@ -192,6 +251,15 @@ public abstract partial class Image where TPixel : unmanaged, IPixel => WrapMemory(Configuration.Default, pixelMemoryOwner, width, height); + /// + public static Image WrapMemory( + IMemoryOwner pixelMemoryOwner, + int width, + int height, + int rowStride) + where TPixel : unmanaged, IPixel + => WrapMemory(Configuration.Default, pixelMemoryOwner, width, height, rowStride); + /// /// /// Wraps an existing contiguous memory area of at least 'width' x 'height' pixels allowing viewing/manipulation as @@ -240,29 +308,56 @@ public abstract partial class Image /// /// - /// Wraps an existing contiguous memory area of at least 'width' x 'height' pixels allowing viewing/manipulation as - /// an instance. + /// Wraps an existing byte memory area allowing viewing/manipulation as an + /// with bytes between row starts. /// /// - /// Please note: using this method does not transfer the ownership of the underlying buffer of the input - /// to the new instance. This means that consumers of this method must ensure that the input buffer - /// is either self-contained, (for example, a instance wrapping a new array that was - /// created), or that the owning object is not disposed until the returned is disposed. - /// - /// - /// If the input instance is one retrieved from an instance - /// rented from a memory pool (such as ), and that owning instance is disposed while the image is still - /// in use, this will lead to undefined behavior and possibly runtime crashes (as the same buffer might then be modified by other - /// consumers while the returned image is still working on it). Make sure to control the lifetime of the input buffers appropriately. + /// Please note: using this method does not transfer the ownership of the underlying buffer of the input + /// to the new instance. Consumers must ensure that + /// the input buffer remains valid for the full lifetime of the returned image. /// /// - /// The pixel type - /// The - /// The byte memory representing the pixel data. - /// The width of the memory image. - /// The height of the memory image. + /// The pixel type. + /// The . + /// The source byte memory. + /// The width of the memory image in pixels. + /// The height of the memory image in pixels. + /// The number of bytes between row starts in . + /// The . /// The configuration is null. + /// The metadata is null. + /// + /// or is not positive, + /// or resolves to less than pixels. + /// + /// + /// is not divisible by the size of , + /// or is smaller than the required strided image length. + /// /// An instance. + public static Image WrapMemory( + Configuration configuration, + Memory byteMemory, + int width, + int height, + int rowStrideInBytes, + ImageMetadata metadata) + where TPixel : unmanaged, IPixel + { + Guard.NotNull(configuration, nameof(configuration)); + Guard.NotNull(metadata, nameof(metadata)); + + int rowStride = GetPixelRowStrideFromByteStride(width, rowStrideInBytes, nameof(rowStrideInBytes)); + long requiredLength = GetRequiredLength(width, height, rowStride); + + ByteMemoryManager memoryManager = new(byteMemory); + Guard.IsTrue(memoryManager.Memory.Length >= requiredLength, nameof(byteMemory), "The length of the input memory is less than the specified image size"); + + MemoryGroup memorySource = MemoryGroup.Wrap(memoryManager.Memory); + return new Image(configuration, memorySource, width, height, rowStride, metadata); + } + + /// public static Image WrapMemory( Configuration configuration, Memory byteMemory, @@ -271,29 +366,17 @@ public abstract partial class Image where TPixel : unmanaged, IPixel => WrapMemory(configuration, byteMemory, width, height, new ImageMetadata()); - /// - /// - /// Wraps an existing contiguous memory area of at least 'width' x 'height' pixels allowing viewing/manipulation as - /// an instance. - /// - /// - /// Please note: using this method does not transfer the ownership of the underlying buffer of the input - /// to the new instance. This means that consumers of this method must ensure that the input buffer - /// is either self-contained, (for example, a instance wrapping a new array that was - /// created), or that the owning object is not disposed until the returned is disposed. - /// - /// - /// If the input instance is one retrieved from an instance - /// rented from a memory pool (such as ), and that owning instance is disposed while the image is still - /// in use, this will lead to undefined behavior and possibly runtime crashes (as the same buffer might then be modified by other - /// consumers while the returned image is still working on it). Make sure to control the lifetime of the input buffers appropriately. - /// - /// - /// The pixel type. - /// The byte memory representing the pixel data. - /// The width of the memory image. - /// The height of the memory image. - /// An instance. + /// + public static Image WrapMemory( + Configuration configuration, + Memory byteMemory, + int width, + int height, + int rowStrideInBytes) + where TPixel : unmanaged, IPixel + => WrapMemory(configuration, byteMemory, width, height, rowStrideInBytes, new ImageMetadata()); + + /// public static Image WrapMemory( Memory byteMemory, int width, @@ -301,6 +384,15 @@ public abstract partial class Image where TPixel : unmanaged, IPixel => WrapMemory(Configuration.Default, byteMemory, width, height); + /// + public static Image WrapMemory( + Memory byteMemory, + int width, + int height, + int rowStrideInBytes) + where TPixel : unmanaged, IPixel + => WrapMemory(Configuration.Default, byteMemory, width, height, rowStrideInBytes); + /// /// Wraps an existing contiguous memory area of at least 'width' x 'height' pixels, /// allowing to view/manipulate it as an instance. @@ -337,19 +429,56 @@ public abstract partial class Image } /// - /// Wraps an existing contiguous memory area of at least 'width' x 'height' pixels, - /// allowing to view/manipulate it as an instance. - /// The ownership of the is being transferred to the new instance, - /// meaning that the caller is not allowed to dispose . - /// It will be disposed together with the result image. + /// + /// Wraps an existing byte memory owner allowing viewing/manipulation as an + /// with bytes between row starts. + /// + /// + /// Ownership of is transferred to the returned image. The caller + /// must not dispose the owner manually. + /// /// /// The pixel type. - /// The - /// The that is being transferred to the image. - /// The width of the memory image. - /// The height of the memory image. + /// The . + /// The byte memory owner transferred to the image. + /// The width of the memory image in pixels. + /// The height of the memory image in pixels. + /// The number of bytes between row starts in the source memory. + /// The . /// The configuration is null. - /// An instance + /// The metadata is null. + /// + /// or is not positive, + /// or resolves to less than pixels. + /// + /// + /// is not divisible by the size of , + /// or is smaller than the required strided image length. + /// + /// An instance. + public static Image WrapMemory( + Configuration configuration, + IMemoryOwner byteMemoryOwner, + int width, + int height, + int rowStrideInBytes, + ImageMetadata metadata) + where TPixel : unmanaged, IPixel + { + Guard.NotNull(configuration, nameof(configuration)); + Guard.NotNull(metadata, nameof(metadata)); + + int rowStride = GetPixelRowStrideFromByteStride(width, rowStrideInBytes, nameof(rowStrideInBytes)); + + ByteMemoryOwner pixelMemoryOwner = new(byteMemoryOwner); + long requiredLength = GetRequiredLength(width, height, rowStride); + Guard.IsTrue(pixelMemoryOwner.Memory.Length >= requiredLength, nameof(byteMemoryOwner), "The length of the input memory is less than the specified image size"); + + MemoryGroup memorySource = MemoryGroup.Wrap(pixelMemoryOwner); + return new Image(configuration, memorySource, width, height, rowStride, metadata); + } + + /// public static Image WrapMemory( Configuration configuration, IMemoryOwner byteMemoryOwner, @@ -358,18 +487,17 @@ public abstract partial class Image where TPixel : unmanaged, IPixel => WrapMemory(configuration, byteMemoryOwner, width, height, new ImageMetadata()); - /// - /// Wraps an existing contiguous memory area of at least 'width' x 'height' pixels, - /// allowing to view/manipulate it as an instance. - /// The ownership of the is being transferred to the new instance, - /// meaning that the caller is not allowed to dispose . - /// It will be disposed together with the result image. - /// - /// The pixel type - /// The that is being transferred to the image. - /// The width of the memory image. - /// The height of the memory image. - /// An instance. + /// + public static Image WrapMemory( + Configuration configuration, + IMemoryOwner byteMemoryOwner, + int width, + int height, + int rowStrideInBytes) + where TPixel : unmanaged, IPixel + => WrapMemory(configuration, byteMemoryOwner, width, height, rowStrideInBytes, new ImageMetadata()); + + /// public static Image WrapMemory( IMemoryOwner byteMemoryOwner, int width, @@ -377,6 +505,15 @@ public abstract partial class Image where TPixel : unmanaged, IPixel => WrapMemory(Configuration.Default, byteMemoryOwner, width, height); + /// + public static Image WrapMemory( + IMemoryOwner byteMemoryOwner, + int width, + int height, + int rowStrideInBytes) + where TPixel : unmanaged, IPixel + => WrapMemory(Configuration.Default, byteMemoryOwner, width, height, rowStrideInBytes); + /// /// /// Wraps an existing contiguous memory area of at least 'width' x 'height' pixels allowing viewing/manipulation as @@ -434,35 +571,60 @@ public abstract partial class Image /// /// - /// Wraps an existing contiguous memory area of at least 'width' x 'height' pixels allowing viewing/manipulation as - /// an instance. - /// - /// - /// Please note: this method relies on callers to carefully manage the target memory area being referenced by the - /// pointer and that the lifetime of such a memory area is at least equal to that of the returned - /// instance. For example, if the input pointer references an unmanaged memory area, - /// callers must ensure that the memory area is not freed as long as the returned is - /// in use and not disposed. The same applies if the input memory area points to a pinned managed object, as callers - /// must ensure that objects will remain pinned as long as the instance is in use. - /// Failing to do so constitutes undefined behavior and will likely lead to memory corruption and runtime crashes. + /// Wraps an unmanaged memory area allowing viewing/manipulation as an + /// with bytes between row starts. /// /// - /// Note also that if you have a or an array (which can be cast to ) of - /// either or values, it is highly recommended to use one of the other - /// available overloads of this method instead (such as - /// or , to make the resulting code less error - /// prone and avoid having to pin the underlying memory buffer in use. This method is primarily meant to be used when - /// doing interop or working with buffers that are located in unmanaged memory. + /// Callers must ensure the memory referenced by remains valid for the full + /// lifetime of the returned image. /// /// - /// The pixel type - /// The - /// The pointer to the target memory buffer to wrap. - /// The byte length of the memory allocated. - /// The width of the memory image. - /// The height of the memory image. + /// The pixel type. + /// The . + /// The pointer to the source memory. + /// The byte length of the source memory. + /// The width of the memory image in pixels. + /// The height of the memory image in pixels. + /// The number of bytes between row starts in the source memory. + /// The . /// The configuration is null. + /// The metadata is null. + /// + /// is null, + /// is not divisible by the size of , + /// or is smaller than the required strided image length. + /// + /// + /// or is not positive, + /// or resolves to less than pixels. + /// /// An instance. + public static unsafe Image WrapMemory( + Configuration configuration, + void* pointer, + int bufferSizeInBytes, + int width, + int height, + int rowStrideInBytes, + ImageMetadata metadata) + where TPixel : unmanaged, IPixel + { + Guard.IsFalse(pointer == null, nameof(pointer), "Pointer must be not null"); + Guard.NotNull(configuration, nameof(configuration)); + Guard.NotNull(metadata, nameof(metadata)); + + int rowStride = GetPixelRowStrideFromByteStride(width, rowStrideInBytes, nameof(rowStrideInBytes)); + long requiredLength = GetRequiredLength(width, height, rowStride); + + Guard.MustBeLessThanOrEqualTo(requiredLength, int.MaxValue, nameof(height)); + Guard.MustBeGreaterThanOrEqualTo(bufferSizeInBytes / Unsafe.SizeOf(), requiredLength, nameof(bufferSizeInBytes)); + + UnmanagedMemoryManager memoryManager = new(pointer, (int)requiredLength); + MemoryGroup memorySource = MemoryGroup.Wrap(memoryManager.Memory); + return new Image(configuration, memorySource, width, height, rowStride, metadata); + } + + /// public static unsafe Image WrapMemory( Configuration configuration, void* pointer, @@ -472,35 +634,18 @@ public abstract partial class Image where TPixel : unmanaged, IPixel => WrapMemory(configuration, pointer, bufferSizeInBytes, width, height, new ImageMetadata()); - /// - /// - /// Wraps an existing contiguous memory area of at least 'width' x 'height' pixels allowing viewing/manipulation as - /// an instance. - /// - /// - /// Please note: this method relies on callers to carefully manage the target memory area being referenced by the - /// pointer and that the lifetime of such a memory area is at least equal to that of the returned - /// instance. For example, if the input pointer references an unmanaged memory area, - /// callers must ensure that the memory area is not freed as long as the returned is - /// in use and not disposed. The same applies if the input memory area points to a pinned managed object, as callers - /// must ensure that objects will remain pinned as long as the instance is in use. - /// Failing to do so constitutes undefined behavior and will likely lead to memory corruption and runtime crashes. - /// - /// - /// Note also that if you have a or an array (which can be cast to ) of - /// either or values, it is highly recommended to use one of the other - /// available overloads of this method instead (such as - /// or , to make the resulting code less error - /// prone and avoid having to pin the underlying memory buffer in use. This method is primarily meant to be used when - /// doing interop or working with buffers that are located in unmanaged memory. - /// - /// - /// The pixel type. - /// The pointer to the target memory buffer to wrap. - /// The byte length of the memory allocated. - /// The width of the memory image. - /// The height of the memory image. - /// An instance. + /// + public static unsafe Image WrapMemory( + Configuration configuration, + void* pointer, + int bufferSizeInBytes, + int width, + int height, + int rowStrideInBytes) + where TPixel : unmanaged, IPixel + => WrapMemory(configuration, pointer, bufferSizeInBytes, width, height, rowStrideInBytes, new ImageMetadata()); + + /// public static unsafe Image WrapMemory( void* pointer, int bufferSizeInBytes, @@ -508,4 +653,41 @@ public abstract partial class Image int height) where TPixel : unmanaged, IPixel => WrapMemory(Configuration.Default, pointer, bufferSizeInBytes, width, height); + + /// + public static unsafe Image WrapMemory( + void* pointer, + int bufferSizeInBytes, + int width, + int height, + int rowStrideInBytes) + where TPixel : unmanaged, IPixel + => WrapMemory(Configuration.Default, pointer, bufferSizeInBytes, width, height, rowStrideInBytes); + + private static void ValidateWrapMemoryStride(int width, int height, int rowStride, string rowStrideParamName) + { + Guard.MustBeGreaterThan(width, 0, nameof(width)); + Guard.MustBeGreaterThan(height, 0, nameof(height)); + Guard.MustBeGreaterThanOrEqualTo(rowStride, width, rowStrideParamName); + } + + private static int GetPixelRowStrideFromByteStride(int width, int rowStrideInBytes, string rowStrideParamName) + where TPixel : unmanaged, IPixel + { + int pixelSizeInBytes = Unsafe.SizeOf(); + + Guard.MustBeGreaterThan(width, 0, nameof(width)); + Guard.MustBeGreaterThan(rowStrideInBytes, 0, rowStrideParamName); + Guard.IsTrue( + rowStrideInBytes % pixelSizeInBytes == 0, + rowStrideParamName, + "The row stride in bytes must be divisible by the pixel size."); + + int rowStride = rowStrideInBytes / pixelSizeInBytes; + Guard.MustBeGreaterThanOrEqualTo(rowStride, width, rowStrideParamName); + return rowStride; + } + + private static long GetRequiredLength(int width, int height, int rowStride) + => checked(((long)(height - 1) * rowStride) + width); } diff --git a/src/ImageSharp/ImageFrame.LoadPixelData.cs b/src/ImageSharp/ImageFrame.LoadPixelData.cs index 003a0349ae..6e2658b6e4 100644 --- a/src/ImageSharp/ImageFrame.LoadPixelData.cs +++ b/src/ImageSharp/ImageFrame.LoadPixelData.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.PixelFormats; @@ -25,6 +26,36 @@ public partial class ImageFrame where TPixel : unmanaged, IPixel => LoadPixelData(configuration, MemoryMarshal.Cast(data), width, height); + /// + /// Create a new instance of the class from the given byte array in format. + /// + /// The configuration which allows altering default behaviour or extending the library. + /// The byte array containing image data. + /// The width of the final image. + /// The height of the final image. + /// The number of bytes between row starts in . + /// The pixel format. + /// A new . + internal static ImageFrame LoadPixelData( + Configuration configuration, + ReadOnlySpan data, + int width, + int height, + int rowStrideInBytes) + where TPixel : unmanaged, IPixel + { + int pixelSizeInBytes = Unsafe.SizeOf(); + Guard.MustBeGreaterThan(width, 0, nameof(width)); + Guard.MustBeGreaterThan(rowStrideInBytes, 0, nameof(rowStrideInBytes)); + Guard.IsTrue( + rowStrideInBytes % pixelSizeInBytes == 0, + nameof(rowStrideInBytes), + "The row stride in bytes must be divisible by the pixel size."); + + int rowStride = rowStrideInBytes / pixelSizeInBytes; + return LoadPixelData(configuration, MemoryMarshal.Cast(data), width, height, rowStride); + } + /// /// Create a new instance of the class from the raw data. /// @@ -36,14 +67,36 @@ public partial class ImageFrame /// A new . internal static ImageFrame LoadPixelData(Configuration configuration, ReadOnlySpan data, int width, int height) where TPixel : unmanaged, IPixel + => LoadPixelData(configuration, data, width, height, width); + + /// + /// Create a new instance of the class from raw data + /// using pixels between source row starts. + /// + /// The configuration which allows altering default behaviour or extending the library. + /// The span containing the image pixel data. + /// The width of the final image. + /// The height of the final image. + /// The number of pixels between row starts in . + /// The pixel format. + /// A new . + internal static ImageFrame LoadPixelData( + Configuration configuration, + ReadOnlySpan data, + int width, + int height, + int rowStride) + where TPixel : unmanaged, IPixel { - int count = width * height; - Guard.MustBeGreaterThanOrEqualTo(data.Length, count, nameof(data)); + Guard.MustBeGreaterThan(width, 0, nameof(width)); + Guard.MustBeGreaterThan(height, 0, nameof(height)); + Guard.MustBeGreaterThanOrEqualTo(rowStride, width, nameof(rowStride)); - ImageFrame image = new(configuration, width, height); + long requiredLength = checked(((long)(height - 1) * rowStride) + width); + Guard.MustBeGreaterThanOrEqualTo(data.Length, requiredLength, nameof(data)); - data = data[..count]; - data.CopyTo(image.PixelBuffer.FastMemoryGroup); + ImageFrame image = new(configuration, width, height); + image.PixelBuffer.CopyFrom(data, rowStride); return image; } diff --git a/src/ImageSharp/ImageFrame.cs b/src/ImageSharp/ImageFrame.cs index 3d4b63fad5..cd6fa6d98a 100644 --- a/src/ImageSharp/ImageFrame.cs +++ b/src/ImageSharp/ImageFrame.cs @@ -71,7 +71,7 @@ public abstract partial class ImageFrame : IConfigurationProvider, IDisposable /// Whether to dispose of managed and unmanaged objects. protected abstract void Dispose(bool disposing); - internal abstract void CopyPixelsTo(MemoryGroup destination) + internal abstract void CopyPixelsTo(Buffer2D destination) where TDestinationPixel : unmanaged, IPixel; /// diff --git a/src/ImageSharp/ImageFrameCollection{TPixel}.cs b/src/ImageSharp/ImageFrameCollection{TPixel}.cs index ad7d719744..892adb7aae 100644 --- a/src/ImageSharp/ImageFrameCollection{TPixel}.cs +++ b/src/ImageSharp/ImageFrameCollection{TPixel}.cs @@ -27,11 +27,16 @@ public sealed class ImageFrameCollection : ImageFrameCollection, IEnumer } internal ImageFrameCollection(Image parent, int width, int height, MemoryGroup memorySource) + : this(parent, width, height, width, memorySource) + { + } + + internal ImageFrameCollection(Image parent, int width, int height, int rowStride, MemoryGroup memorySource) { this.parent = parent ?? throw new ArgumentNullException(nameof(parent)); // Frames are already cloned within the caller - this.frames.Add(new ImageFrame(parent.Configuration, width, height, memorySource)); + this.frames.Add(new ImageFrame(parent.Configuration, width, height, rowStride, memorySource)); } internal ImageFrameCollection(Image parent, IEnumerable> frames) @@ -416,7 +421,7 @@ public sealed class ImageFrameCollection : ImageFrameCollection, IEnumer this.parent.Configuration, source.Size, source.Metadata.DeepClone()); - source.CopyPixelsTo(result.PixelBuffer.FastMemoryGroup); + source.CopyPixelsTo(result.PixelBuffer); return result; } } diff --git a/src/ImageSharp/ImageFrame{TPixel}.cs b/src/ImageSharp/ImageFrame{TPixel}.cs index de71e77ca0..4e25c4e581 100644 --- a/src/ImageSharp/ImageFrame{TPixel}.cs +++ b/src/ImageSharp/ImageFrame{TPixel}.cs @@ -114,7 +114,20 @@ public sealed class ImageFrame : ImageFrame, IPixelSource /// The height of the image in pixels. /// The memory source. internal ImageFrame(Configuration configuration, int width, int height, MemoryGroup memorySource) - : this(configuration, width, height, memorySource, new ImageFrameMetadata()) + : this(configuration, width, height, width, memorySource, new ImageFrameMetadata()) + { + } + + /// + /// Initializes a new instance of the class wrapping an existing buffer. + /// + /// The configuration providing initialization code which allows extending the library. + /// The width of the image in pixels. + /// The height of the image in pixels. + /// The number of elements between row starts. + /// The memory source. + internal ImageFrame(Configuration configuration, int width, int height, int rowStride, MemoryGroup memorySource) + : this(configuration, width, height, rowStride, memorySource, new ImageFrameMetadata()) { } @@ -127,12 +140,26 @@ public sealed class ImageFrame : ImageFrame, IPixelSource /// The memory source. /// The metadata. internal ImageFrame(Configuration configuration, int width, int height, MemoryGroup memorySource, ImageFrameMetadata metadata) + : this(configuration, width, height, width, memorySource, metadata) + { + } + + /// + /// Initializes a new instance of the class wrapping an existing buffer. + /// + /// The configuration providing initialization code which allows extending the library. + /// The width of the image in pixels. + /// The height of the image in pixels. + /// The number of elements between row starts. + /// The memory source. + /// The metadata. + internal ImageFrame(Configuration configuration, int width, int height, int rowStride, MemoryGroup memorySource, ImageFrameMetadata metadata) : base(configuration, width, height, metadata) { Guard.MustBeGreaterThan(width, 0, nameof(width)); Guard.MustBeGreaterThan(height, 0, nameof(height)); - this.PixelBuffer = new Buffer2D(memorySource, width, height); + this.PixelBuffer = new Buffer2D(memorySource, width, height, rowStride); } /// @@ -150,7 +177,7 @@ public sealed class ImageFrame : ImageFrame, IPixelSource source.PixelBuffer.Width, source.PixelBuffer.Height, configuration.PreferContiguousImageBuffers); - source.PixelBuffer.FastMemoryGroup.CopyTo(this.PixelBuffer.FastMemoryGroup); + source.PixelBuffer.CopyTo(this.PixelBuffer); } /// @@ -270,16 +297,23 @@ public sealed class ImageFrame : ImageFrame, IPixelSource } /// - /// Copy image pixels to . + /// Copy image pixels to using the backing row layout. /// + /// + /// Destination length must be at least ((Height - 1) * PixelBuffer.RowStride) + Width. + /// /// The to copy image pixels to. - public void CopyPixelDataTo(Span destination) => this.GetPixelMemoryGroup().CopyTo(destination); + public void CopyPixelDataTo(Span destination) => this.PixelBuffer.CopyTo(destination); /// - /// Copy image pixels to . + /// Copy image pixels to using the backing row layout. /// + /// + /// Destination length must be at least + /// (((Height - 1) * PixelBuffer.RowStride) + Width) * sizeof(TPixel) bytes. + /// /// The of to copy image pixels to. - public void CopyPixelDataTo(Span destination) => this.GetPixelMemoryGroup().CopyTo(MemoryMarshal.Cast(destination)); + public void CopyPixelDataTo(Span destination) => this.PixelBuffer.CopyTo(MemoryMarshal.Cast(destination)); /// /// Gets the representation of the pixels as a in the source image's pixel format @@ -294,17 +328,7 @@ public sealed class ImageFrame : ImageFrame, IPixelSource /// The referencing the image buffer. /// The indicating the success. public bool DangerousTryGetSinglePixelMemory(out Memory memory) - { - IMemoryGroup mg = this.GetPixelMemoryGroup(); - if (mg.Count > 1) - { - memory = default; - return false; - } - - memory = mg.Single(); - return true; - } + => this.PixelBuffer.DangerousTryGetSingleMemory(out memory); /// /// Gets a reference to the pixel at the specified position. @@ -327,7 +351,7 @@ public sealed class ImageFrame : ImageFrame, IPixelSource throw new ArgumentException("ImageFrame.CopyTo(): target must be of the same size!", nameof(target)); } - this.PixelBuffer.FastMemoryGroup.CopyTo(target.FastMemoryGroup); + this.PixelBuffer.CopyTo(target); } /// @@ -370,20 +394,32 @@ public sealed class ImageFrame : ImageFrame, IPixelSource this.isDisposed = true; } - internal override void CopyPixelsTo(MemoryGroup destination) + internal override void CopyPixelsTo(Buffer2D destination) { + Guard.NotNull(destination, nameof(destination)); + Guard.IsTrue( + destination.Width == this.Width && destination.Height == this.Height, + nameof(destination), + "Destination buffer must have the same dimensions as the source frame."); + if (typeof(TPixel) == typeof(TDestinationPixel)) { - this.PixelBuffer.FastMemoryGroup.TransformTo(destination, (s, d) => + for (int y = 0; y < this.Height; y++) { - Span d1 = MemoryMarshal.Cast(d); - s.CopyTo(d1); - }); + Span sourceRow = this.PixelBuffer.DangerousGetRowSpan(y); + Span destinationRow = destination.DangerousGetRowSpan(y); + sourceRow.CopyTo(MemoryMarshal.Cast(destinationRow)); + } + return; } - this.PixelBuffer.FastMemoryGroup.TransformTo(destination, (s, d) - => PixelOperations.Instance.To(this.Configuration, s, d)); + for (int y = 0; y < this.Height; y++) + { + Span sourceRow = this.PixelBuffer.DangerousGetRowSpan(y); + Span destinationRow = destination.DangerousGetRowSpan(y); + PixelOperations.Instance.To(this.Configuration, sourceRow, destinationRow); + } } /// @@ -441,16 +477,7 @@ public sealed class ImageFrame : ImageFrame, IPixelSource /// The value to initialize the bitmap with. internal void Clear(TPixel value) { - MemoryGroup group = this.PixelBuffer.FastMemoryGroup; - - if (value.Equals(default)) - { - group.Clear(); - } - else - { - group.Fill(value); - } + this.PixelBuffer.Clear(value); } [MethodImpl(InliningOptions.ShortMethod)] diff --git a/src/ImageSharp/Image{TPixel}.cs b/src/ImageSharp/Image{TPixel}.cs index dff8f577fd..18db343563 100644 --- a/src/ImageSharp/Image{TPixel}.cs +++ b/src/ImageSharp/Image{TPixel}.cs @@ -91,7 +91,7 @@ public sealed class Image : Image Configuration configuration, Buffer2D pixelBuffer, ImageMetadata metadata) - : this(configuration, pixelBuffer.FastMemoryGroup, pixelBuffer.Width, pixelBuffer.Height, metadata) + : this(configuration, pixelBuffer.FastMemoryGroup, pixelBuffer.Width, pixelBuffer.Height, pixelBuffer.RowStride, metadata) { } @@ -110,8 +110,29 @@ public sealed class Image : Image int width, int height, ImageMetadata metadata) + : this(configuration, memoryGroup, width, height, width, metadata) + { + } + + /// + /// Initializes a new instance of the class + /// wrapping an external . + /// + /// The configuration providing initialization code which allows extending the library. + /// The memory source. + /// The width of the image in pixels. + /// The height of the image in pixels. + /// The number of elements between row starts. + /// The images metadata. + internal Image( + Configuration configuration, + MemoryGroup memoryGroup, + int width, + int height, + int rowStride, + ImageMetadata metadata) : base(configuration, TPixel.GetPixelTypeInfo(), metadata, width, height) - => this.frames = new ImageFrameCollection(this, width, height, memoryGroup); + => this.frames = new ImageFrameCollection(this, width, height, rowStride, memoryGroup); /// /// Initializes a new instance of the class @@ -287,16 +308,24 @@ public sealed class Image : Image } /// - /// Copy image pixels to . + /// Copy image pixels to using the root frame backing row layout. /// + /// + /// Destination length must be at least + /// ((Height - 1) * Frames.RootFrame.PixelBuffer.RowStride) + Width. + /// /// The to copy image pixels to. - public void CopyPixelDataTo(Span destination) => this.GetPixelMemoryGroup().CopyTo(destination); + public void CopyPixelDataTo(Span destination) => this.Frames.RootFrame.CopyPixelDataTo(destination); /// - /// Copy image pixels to . + /// Copy image pixels to using the root frame backing row layout. /// + /// + /// Destination length must be at least + /// (((Height - 1) * Frames.RootFrame.PixelBuffer.RowStride) + Width) * sizeof(TPixel) bytes. + /// /// The of to copy image pixels to. - public void CopyPixelDataTo(Span destination) => this.GetPixelMemoryGroup().CopyTo(MemoryMarshal.Cast(destination)); + public void CopyPixelDataTo(Span destination) => this.Frames.RootFrame.CopyPixelDataTo(destination); /// /// Gets the representation of the pixels as a in the source image's pixel format @@ -311,17 +340,7 @@ public sealed class Image : Image /// The referencing the image buffer. /// The indicating the success. public bool DangerousTryGetSinglePixelMemory(out Memory memory) - { - IMemoryGroup mg = this.GetPixelMemoryGroup(); - if (mg.Count > 1) - { - memory = default; - return false; - } - - memory = mg.Single(); - return true; - } + => this.Frames.RootFrame.DangerousTryGetSinglePixelMemory(out memory); /// /// Clones the current image. diff --git a/src/ImageSharp/Memory/Buffer2DExtensions.cs b/src/ImageSharp/Memory/Buffer2DExtensions.cs index f0fa1438dd..96b9bc172f 100644 --- a/src/ImageSharp/Memory/Buffer2DExtensions.cs +++ b/src/ImageSharp/Memory/Buffer2DExtensions.cs @@ -45,7 +45,7 @@ public static class Buffer2DExtensions Buffer2DRegion sourceRegion = source.GetRegion(rectangle); if (sourceRegion.IsFullBufferArea) { - sourceRegion.Buffer.FastMemoryGroup.CopyTo(buffer.FastMemoryGroup); + sourceRegion.Buffer.CopyTo(buffer); } else { @@ -81,7 +81,7 @@ public static class Buffer2DExtensions CheckColumnRegionsDoNotOverlap(buffer, sourceIndex, destinationIndex, columnCount); int elementSize = Unsafe.SizeOf(); - int width = buffer.Width * elementSize; + int rowByteStride = buffer.RowStride * elementSize; int sOffset = sourceIndex * elementSize; int dOffset = destinationIndex * elementSize; long count = columnCount * elementSize; @@ -98,7 +98,7 @@ public static class Buffer2DExtensions Buffer.MemoryCopy(sPtr, dPtr, count, count); - basePtr += width; + basePtr += rowByteStride; } } } diff --git a/src/ImageSharp/Memory/Buffer2DRegion{T}.cs b/src/ImageSharp/Memory/Buffer2DRegion{T}.cs index f4b257b587..7bfb6f5731 100644 --- a/src/ImageSharp/Memory/Buffer2DRegion{T}.cs +++ b/src/ImageSharp/Memory/Buffer2DRegion{T}.cs @@ -59,9 +59,9 @@ public readonly struct Buffer2DRegion public int Height => this.Rectangle.Height; /// - /// Gets the pixel stride which is equal to the width of . + /// Gets the number of elements between row starts in . /// - public int Stride => this.Buffer.Width; + public int Stride => this.Buffer.RowStride; /// /// Gets the size of the area. @@ -146,9 +146,9 @@ public readonly struct Buffer2DRegion internal void Clear() { // Optimization for when the size of the area is the same as the buffer size. - if (this.IsFullBufferArea) + if (this.IsFullBufferArea && this.Buffer.RowStride == this.Buffer.Width) { - this.Buffer.FastMemoryGroup.Clear(); + this.Buffer.Clear(default); return; } @@ -166,9 +166,9 @@ public readonly struct Buffer2DRegion internal void Fill(T value) { // Optimization for when the size of the area is the same as the buffer size. - if (this.IsFullBufferArea) + if (this.IsFullBufferArea && this.Buffer.RowStride == this.Buffer.Width) { - this.Buffer.FastMemoryGroup.Fill(value); + this.Buffer.Clear(value); return; } diff --git a/src/ImageSharp/Memory/Buffer2D{T}.cs b/src/ImageSharp/Memory/Buffer2D{T}.cs index 39c6e62e15..3194aebae7 100644 --- a/src/ImageSharp/Memory/Buffer2D{T}.cs +++ b/src/ImageSharp/Memory/Buffer2D{T}.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using System.Buffers; using System.Runtime.CompilerServices; namespace SixLabors.ImageSharp.Memory; @@ -20,10 +21,27 @@ public sealed class Buffer2D : IDisposable /// The number of elements in a row. /// The number of rows. internal Buffer2D(MemoryGroup memoryGroup, int width, int height) + : this(memoryGroup, width, height, width) { + } + + /// + /// Initializes a new instance of the class. + /// + /// The to wrap. + /// The number of elements in a row. + /// The number of rows. + /// The number of elements between row starts. + internal Buffer2D(MemoryGroup memoryGroup, int width, int height, int rowStride) + { + Guard.MustBeGreaterThan(width, 0, nameof(width)); + Guard.MustBeGreaterThan(height, 0, nameof(height)); + Guard.MustBeGreaterThanOrEqualTo(rowStride, width, nameof(rowStride)); + this.FastMemoryGroup = memoryGroup; this.Width = width; this.Height = height; + this.RowStride = rowStride; } /// @@ -36,6 +54,11 @@ public sealed class Buffer2D : IDisposable /// public int Height { get; private set; } + /// + /// Gets the number of elements between row starts in the backing memory. + /// + public int RowStride { get; private set; } + /// /// Gets the backing . /// @@ -75,6 +98,168 @@ public sealed class Buffer2D : IDisposable } } + /// + /// Wraps an existing memory area as a with tightly packed rows. + /// + /// + /// This method does not transfer ownership of to the returned . + /// The caller is responsible for ensuring that the memory remains valid for the entire lifetime of the returned buffer. + /// If originates from an (for example from ), + /// do not dispose that owner while the returned buffer is still in use. + /// + /// The source memory. + /// The number of elements in each row. + /// The number of rows. + /// The wrapped instance. + /// Thrown when or is not positive. + /// Thrown when is shorter than width * height. +#pragma warning disable CA1000 // Do not declare static members on generic types + public static Buffer2D WrapMemory(Memory memory, int width, int height) +#pragma warning restore CA1000 // Do not declare static members on generic types + => WrapMemory(memory, width, height, width); + + /// + /// Wraps an existing memory area as a using the specified row stride. + /// + /// + /// This method does not transfer ownership of to the returned . + /// The caller is responsible for ensuring that the memory remains valid for the entire lifetime of the returned buffer. + /// If originates from an (for example from ), + /// do not dispose that owner while the returned buffer is still in use. + /// The minimum required length is ((height - 1) * stride) + width elements. + /// + /// The source memory. + /// The number of elements in each row. + /// The number of rows. + /// The number of elements between row starts in the source memory. + /// The wrapped instance. + /// + /// Thrown when or is not positive, + /// or when is less than . + /// + /// Thrown when is shorter than the required buffer size. +#pragma warning disable CA1000 // Do not declare static members on generic types + public static Buffer2D WrapMemory(Memory memory, int width, int height, int stride) +#pragma warning restore CA1000 // Do not declare static members on generic types + { + Guard.MustBeGreaterThan(width, 0, nameof(width)); + Guard.MustBeGreaterThan(height, 0, nameof(height)); + Guard.MustBeGreaterThanOrEqualTo(stride, width, nameof(stride)); + + long requiredLength = checked(((long)(height - 1) * stride) + width); + Guard.IsTrue(memory.Length >= requiredLength, nameof(memory), "The length of the input memory is less than the specified buffer size"); + + MemoryGroup memorySource = MemoryGroup.Wrap(memory); + return new Buffer2D(memorySource, width, height, stride); + } + + /// + /// Gets the representation of the values as a single contiguous + /// when the backing group is a single tightly packed segment. + /// + /// The referencing the buffer. + /// + /// when the buffer can be copied as one contiguous block + /// without per-row handling; otherwise . + /// + public bool DangerousTryGetSingleMemory(out Memory memory) + { + if (this.MemoryGroup.Count > 1 || this.RowStride != this.Width) + { + memory = default; + return false; + } + + int logicalLength = checked((int)((long)this.Width * this.Height)); + memory = this.MemoryGroup[0][..logicalLength]; + return true; + } + + /// + /// Copies this buffer into using the source logical row layout. + /// + /// + /// When dimensions are equal, destination stride is respected. + /// When dimensions differ, source stride is used to copy the source logical layout into destination memory. + /// + /// The destination buffer. + internal void CopyTo(Buffer2D destination) + { + Guard.NotNull(destination, nameof(destination)); + + bool sameDimensions = this.Width == destination.Width && this.Height == destination.Height; + int destinationStride = sameDimensions ? destination.RowStride : this.RowStride; + + // Different dimensions use source logical layout. This supports SwapOrCopyContent, + // where metadata is swapped after data copy. + this.FastMemoryGroup.CopyTo( + this.RowStride, + destination.FastMemoryGroup, + destinationStride, + this.Width, + this.Height); + } + + /// + /// Copies this buffer into using the source row stride as destination layout. + /// + /// The destination span. + internal void CopyTo(Span destination) + { + long requiredLength = checked(((long)(this.Height - 1) * this.RowStride) + this.Width); + Guard.MustBeGreaterThanOrEqualTo(destination.Length, requiredLength, nameof(destination)); + + this.FastMemoryGroup.CopyTo( + this.RowStride, + destination, + this.RowStride, + this.Width, + this.Height); + } + + /// + /// Copies tightly packed row-major data from into this buffer. + /// + /// The source data. + internal void CopyFrom(ReadOnlySpan source) => this.CopyFrom(source, this.Width); + + /// + /// Copies row-major data from into this buffer using + /// elements between source row starts. + /// + /// The source data. + /// The number of elements between source row starts. + internal void CopyFrom(ReadOnlySpan source, int sourceStride) + { + Guard.MustBeGreaterThanOrEqualTo(sourceStride, this.Width, nameof(sourceStride)); + + long requiredLength = checked(((long)(this.Height - 1) * sourceStride) + this.Width); + Guard.MustBeGreaterThanOrEqualTo(source.Length, requiredLength, nameof(source)); + + // Copy row by row so padded source rows map correctly into the destination logical rows. + int sourceOffset = 0; + for (int y = 0; y < this.Height; y++) + { + source.Slice(sourceOffset, this.Width).CopyTo(this.DangerousGetRowSpan(y)); + sourceOffset += sourceStride; + } + } + + /// + /// Clears this buffer when is default; otherwise fills it with . + /// + /// The fill value. + internal void Clear(T value) + { + if (value.Equals(default)) + { + this.FastMemoryGroup.Clear(); + return; + } + + this.FastMemoryGroup.Fill(value); + } + /// /// Disposes the instance /// @@ -102,7 +287,13 @@ public sealed class Buffer2D : IDisposable this.ThrowYOutOfRangeException(y); } - return this.FastMemoryGroup.GetRowSpanCoreUnsafe(y, this.Width); + if (this.RowStride == this.Width) + { + return this.FastMemoryGroup.GetRowSpanCoreUnsafe(y, this.Width); + } + + int rowStart = checked(y * this.RowStride); + return this.FastMemoryGroup[0].Span.Slice(rowStart, this.Width); } internal bool DangerousTryGetPaddedRowSpan(int y, int padding, out Span paddedSpan) @@ -111,8 +302,10 @@ public sealed class Buffer2D : IDisposable DebugGuard.MustBeLessThan(y, this.Height, nameof(y)); int stride = this.Width + padding; - - Span slice = this.FastMemoryGroup.GetRemainingSliceOfBuffer(y * (long)this.Width); + long rowStart = y * (long)this.RowStride; + Span slice = this.RowStride == this.Width + ? this.FastMemoryGroup.GetRemainingSliceOfBuffer(rowStart) + : this.FastMemoryGroup[0].Span[checked((int)rowStart)..]; if (slice.Length < stride) { @@ -127,7 +320,10 @@ public sealed class Buffer2D : IDisposable [MethodImpl(InliningOptions.ShortMethod)] internal ref T GetElementUnsafe(int x, int y) { - Span span = this.FastMemoryGroup.GetRowSpanCoreUnsafe(y, this.Width); + Span span = this.RowStride == this.Width + ? this.FastMemoryGroup.GetRowSpanCoreUnsafe(y, this.Width) + : this.FastMemoryGroup[0].Span.Slice(checked(y * this.RowStride), this.Width); + return ref span[x]; } @@ -141,6 +337,13 @@ public sealed class Buffer2D : IDisposable { DebugGuard.MustBeGreaterThanOrEqualTo(y, 0, nameof(y)); DebugGuard.MustBeLessThan(y, this.Height, nameof(y)); + + if (this.RowStride != this.Width) + { + int rowStart = checked(y * this.RowStride); + return this.FastMemoryGroup[0].Slice(rowStart, this.Width); + } + return this.FastMemoryGroup.View.GetBoundedMemorySlice(y * (long)this.Width, this.Width); } @@ -153,7 +356,7 @@ public sealed class Buffer2D : IDisposable /// Thrown when the backing group is discontiguous. /// [MethodImpl(InliningOptions.ShortMethod)] - internal Span DangerousGetSingleSpan() => this.FastMemoryGroup.Single().Span; + internal Span DangerousGetSingleSpan() => this.FastMemoryGroup[0].Span; /// /// Gets a to the backing data of if the backing group consists of a single contiguous memory buffer. @@ -164,7 +367,7 @@ public sealed class Buffer2D : IDisposable /// Thrown when the backing group is discontiguous. /// [MethodImpl(InliningOptions.ShortMethod)] - internal Memory DangerousGetSingleMemory() => this.FastMemoryGroup.Single(); + internal Memory DangerousGetSingleMemory() => this.FastMemoryGroup[0]; /// /// Swaps the contents of 'destination' with 'source' if the buffers are owned (1), @@ -185,21 +388,31 @@ public sealed class Buffer2D : IDisposable } else { - if (destination.FastMemoryGroup.TotalLength != source.FastMemoryGroup.TotalLength) + long sourceLayoutLength = GetRequiredLength(source.Width, source.Height, source.RowStride); + long destinationLayoutLength = GetRequiredLength(destination.Width, destination.Height, destination.RowStride); + + bool destinationCanRepresentSource = destination.FastMemoryGroup.TotalLength >= sourceLayoutLength; + bool sourceCanRepresentDestination = source.FastMemoryGroup.TotalLength >= destinationLayoutLength; + if (!destinationCanRepresentSource || !sourceCanRepresentDestination) { throw new InvalidMemoryOperationException( "Trying to copy/swap incompatible buffers. This is most likely caused by applying an unsupported processor to wrapped-memory images."); } - source.FastMemoryGroup.CopyTo(destination.MemoryGroup); + source.CopyTo(destination); } (destination.Width, source.Width) = (source.Width, destination.Width); (destination.Height, source.Height) = (source.Height, destination.Height); + (destination.RowStride, source.RowStride) = (source.RowStride, destination.RowStride); return swapped; } [MethodImpl(InliningOptions.ColdPath)] private void ThrowYOutOfRangeException(int y) => throw new ArgumentOutOfRangeException($"DangerousGetRowSpan({y}). Y was out of range. Height={this.Height}"); + + [MethodImpl(InliningOptions.ShortMethod)] + private static long GetRequiredLength(int width, int height, int stride) + => ((long)(height - 1) * stride) + width; } diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupExtensions.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupExtensions.cs index 148b5f6bfb..b399d3d700 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupExtensions.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupExtensions.cs @@ -71,128 +71,159 @@ internal static class MemoryGroupExtensions return memory.Slice(bufferStart, length); } - internal static void CopyTo(this IMemoryGroup source, Span target) + /// + /// Copies a 2D logical region from into + /// using the provided source and target strides. + /// + /// The element type. + /// The source memory group. + /// Elements between source row starts. + /// The destination span. + /// Elements between destination row starts. + /// The logical row width to copy. + /// The number of rows to copy. + internal static void CopyTo( + this IMemoryGroup source, + int sourceStride, + Span target, + int targetStride, + int width, + int height) where T : struct { Guard.NotNull(source, nameof(source)); - Guard.MustBeGreaterThanOrEqualTo(target.Length, source.TotalLength, nameof(target)); + Guard.MustBeGreaterThanOrEqualTo(width, 0, nameof(width)); + Guard.MustBeGreaterThanOrEqualTo(height, 0, nameof(height)); + Guard.MustBeGreaterThanOrEqualTo(sourceStride, width, nameof(sourceStride)); + Guard.MustBeGreaterThanOrEqualTo(targetStride, width, nameof(targetStride)); - MemoryGroupCursor cur = new(source); - long position = 0; - while (position < source.TotalLength) - { - int fwd = Math.Min(cur.LookAhead(), target.Length); - cur.GetSpan(fwd).CopyTo(target); + long sourceRequired = height == 0 ? 0 : checked(((long)(height - 1) * sourceStride) + width); + long targetRequired = height == 0 ? 0 : checked(((long)(height - 1) * targetStride) + width); + Guard.MustBeGreaterThanOrEqualTo(source.TotalLength, sourceRequired, nameof(source)); + Guard.MustBeGreaterThanOrEqualTo(target.Length, targetRequired, nameof(target)); - cur.Forward(fwd); - target = target[fwd..]; - position += fwd; + if (width == 0 || height == 0) + { + return; } - } - internal static void CopyTo(this Span source, IMemoryGroup target) - where T : struct - => CopyTo((ReadOnlySpan)source, target); + MemoryGroupCursor sourceCursor = new(source); + int sourceSkip = sourceStride - width; - internal static void CopyTo(this ReadOnlySpan source, IMemoryGroup target) - where T : struct - { - Guard.NotNull(target, nameof(target)); - Guard.MustBeGreaterThanOrEqualTo(target.TotalLength, source.Length, nameof(target)); - - MemoryGroupCursor cur = new(target); - - while (!source.IsEmpty) + for (int y = 0; y < height; y++) { - int fwd = Math.Min(cur.LookAhead(), source.Length); - source[..fwd].CopyTo(cur.GetSpan(fwd)); - cur.Forward(fwd); - source = source[fwd..]; + int rowStart = checked(y * targetStride); + Span destinationRow = target.Slice(rowStart, width); + CopyFromCursorToSpan(ref sourceCursor, destinationRow); + + // Trailing padding after the last row is optional, so only skip between rows. + if (y < height - 1) + { + ForwardCursor(ref sourceCursor, sourceSkip); + } } } - internal static void CopyTo(this IMemoryGroup? source, IMemoryGroup? target) + /// + /// Copies a 2D logical region from into + /// using the provided source and target strides. + /// + /// The element type. + /// The source memory group. + /// Elements between source row starts. + /// The destination memory group. + /// Elements between destination row starts. + /// The logical row width to copy. + /// The number of rows to copy. + internal static void CopyTo( + this IMemoryGroup source, + int sourceStride, + IMemoryGroup target, + int targetStride, + int width, + int height) where T : struct { Guard.NotNull(source, nameof(source)); Guard.NotNull(target, nameof(target)); Guard.IsTrue(source.IsValid, nameof(source), "Source group must be valid."); Guard.IsTrue(target.IsValid, nameof(target), "Target group must be valid."); - Guard.MustBeLessThanOrEqualTo(source.TotalLength, target.TotalLength, "Destination buffer too short!"); + Guard.MustBeGreaterThanOrEqualTo(width, 0, nameof(width)); + Guard.MustBeGreaterThanOrEqualTo(height, 0, nameof(height)); + Guard.MustBeGreaterThanOrEqualTo(sourceStride, width, nameof(sourceStride)); + Guard.MustBeGreaterThanOrEqualTo(targetStride, width, nameof(targetStride)); - if (source.IsEmpty()) + long sourceRequired = height == 0 ? 0 : checked(((long)(height - 1) * sourceStride) + width); + long targetRequired = height == 0 ? 0 : checked(((long)(height - 1) * targetStride) + width); + Guard.MustBeGreaterThanOrEqualTo(source.TotalLength, sourceRequired, nameof(source)); + Guard.MustBeGreaterThanOrEqualTo(target.TotalLength, targetRequired, nameof(target)); + + if (width == 0 || height == 0) { return; } - long position = 0; - MemoryGroupCursor srcCur = new(source); - MemoryGroupCursor trgCur = new(target); + MemoryGroupCursor sourceCursor = new(source); + MemoryGroupCursor targetCursor = new(target); + int sourceSkip = sourceStride - width; + int targetSkip = targetStride - width; - while (position < source.TotalLength) + for (int y = 0; y < height; y++) { - int fwd = Math.Min(srcCur.LookAhead(), trgCur.LookAhead()); - Span srcSpan = srcCur.GetSpan(fwd); - Span trgSpan = trgCur.GetSpan(fwd); - srcSpan.CopyTo(trgSpan); - - srcCur.Forward(fwd); - trgCur.Forward(fwd); - position += fwd; + CopyFromCursorToCursor(ref sourceCursor, ref targetCursor, width); + + // Trailing padding after the last row is optional, so only skip between rows. + if (y < height - 1) + { + ForwardCursor(ref sourceCursor, sourceSkip); + ForwardCursor(ref targetCursor, targetSkip); + } } } - internal static void TransformTo( - this IMemoryGroup source, - IMemoryGroup target, - TransformItemsDelegate transform) - where TSource : struct - where TTarget : struct + private static void CopyFromCursorToCursor( + ref MemoryGroupCursor source, + ref MemoryGroupCursor target, + int count) + where T : struct { - Guard.NotNull(source, nameof(source)); - Guard.NotNull(target, nameof(target)); - Guard.NotNull(transform, nameof(transform)); - Guard.IsTrue(source.IsValid, nameof(source), "Source group must be valid."); - Guard.IsTrue(target.IsValid, nameof(target), "Target group must be valid."); - Guard.MustBeLessThanOrEqualTo(source.TotalLength, target.TotalLength, "Destination buffer too short!"); - - if (source.IsEmpty()) + int remaining = count; + while (remaining > 0) { - return; + int fwd = Math.Min(remaining, Math.Min(source.LookAhead(), target.LookAhead())); + source.GetSpan(fwd).CopyTo(target.GetSpan(fwd)); + source.Forward(fwd); + target.Forward(fwd); + remaining -= fwd; } + } - long position = 0; - MemoryGroupCursor srcCur = new(source); - MemoryGroupCursor trgCur = new(target); - - while (position < source.TotalLength) + private static void CopyFromCursorToSpan(ref MemoryGroupCursor source, Span target) + where T : struct + { + int remaining = target.Length; + while (remaining > 0) { - int fwd = Math.Min(srcCur.LookAhead(), trgCur.LookAhead()); - Span srcSpan = srcCur.GetSpan(fwd); - Span trgSpan = trgCur.GetSpan(fwd); - transform(srcSpan, trgSpan); - - srcCur.Forward(fwd); - trgCur.Forward(fwd); - position += fwd; + int copied = target.Length - remaining; + int fwd = Math.Min(remaining, source.LookAhead()); + source.GetSpan(fwd).CopyTo(target[copied..]); + source.Forward(fwd); + remaining -= fwd; } } - internal static void TransformInplace( - this IMemoryGroup memoryGroup, - TransformItemsInplaceDelegate transform) + private static void ForwardCursor(ref MemoryGroupCursor cursor, int steps) where T : struct { - foreach (Memory memory in memoryGroup) + int remaining = steps; + while (remaining > 0) { - transform(memory.Span); + int fwd = Math.Min(remaining, cursor.LookAhead()); + cursor.Forward(fwd); + remaining -= fwd; } } - internal static bool IsEmpty(this IMemoryGroup group) - where T : struct - => group.Count == 0; - private struct MemoryGroupCursor where T : struct { diff --git a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs index ff306e1e45..57ebcab768 100644 --- a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs +++ b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs @@ -29,6 +29,9 @@ public static class MemoryAllocatorExtensions AllocationOptions options = AllocationOptions.None) where T : struct { + Guard.MustBeGreaterThan(width, 0, nameof(width)); + Guard.MustBeGreaterThan(height, 0, nameof(height)); + long groupLength = (long)width * height; MemoryGroup memoryGroup; if (preferContiguosImageBuffers && groupLength < int.MaxValue) @@ -104,6 +107,9 @@ public static class MemoryAllocatorExtensions AllocationOptions options = AllocationOptions.None) where T : struct { + Guard.MustBeGreaterThan(width, 0, nameof(width)); + Guard.MustBeGreaterThan(height, 0, nameof(height)); + long groupLength = (long)width * height; MemoryGroup memoryGroup = memoryAllocator.AllocateGroup( groupLength, diff --git a/src/ImageSharp/Memory/TransformItemsDelegate{TSource, TTarget}.cs b/src/ImageSharp/Memory/TransformItemsDelegate{TSource, TTarget}.cs deleted file mode 100644 index bc3d17f8f0..0000000000 --- a/src/ImageSharp/Memory/TransformItemsDelegate{TSource, TTarget}.cs +++ /dev/null @@ -1,8 +0,0 @@ -// Copyright (c) Six Labors. -// Licensed under the Six Labors Split License. - -namespace SixLabors.ImageSharp.Memory; - -#pragma warning disable SA1649 // File name should match first type name -internal delegate void TransformItemsDelegate(ReadOnlySpan source, Span target); -#pragma warning restore SA1649 // File name should match first type name diff --git a/src/ImageSharp/Memory/TransformItemsInplaceDelegate.cs b/src/ImageSharp/Memory/TransformItemsInplaceDelegate.cs deleted file mode 100644 index d1ef51fb85..0000000000 --- a/src/ImageSharp/Memory/TransformItemsInplaceDelegate.cs +++ /dev/null @@ -1,6 +0,0 @@ -// Copyright (c) Six Labors. -// Licensed under the Six Labors Split License. - -namespace SixLabors.ImageSharp.Memory; - -internal delegate void TransformItemsInplaceDelegate(Span data); diff --git a/src/ImageSharp/Processing/Processors/Transforms/CropProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Transforms/CropProcessor{TPixel}.cs index 96c350c6c8..5ef2422a36 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/CropProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/CropProcessor{TPixel}.cs @@ -53,7 +53,7 @@ internal class CropProcessor : TransformProcessor && this.SourceRectangle == this.cropRectangle) { // the cloned will be blank here copy all the pixel data over - source.GetPixelMemoryGroup().CopyTo(destination.GetPixelMemoryGroup()); + source.PixelBuffer.CopyTo(destination.PixelBuffer); return; } diff --git a/src/ImageSharp/Processing/Processors/Transforms/Linear/RotateProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Transforms/Linear/RotateProcessor{TPixel}.cs index 8d3ded09f9..e9d0ecf57d 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Linear/RotateProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Linear/RotateProcessor{TPixel}.cs @@ -97,7 +97,7 @@ internal class RotateProcessor : AffineTransformProcessor if (MathF.Abs(degrees) < Constants.Epsilon) { // The destination will be blank here so copy all the pixel data over - source.GetPixelMemoryGroup().CopyTo(destination.GetPixelMemoryGroup()); + source.PixelBuffer.CopyTo(destination.PixelBuffer); return true; } diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs index 49439545b3..48f4a746f0 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs @@ -91,7 +91,7 @@ internal class ResizeProcessor : TransformProcessor, IResampling ImageFrame destinationFrame = destination.Frames[i]; // The cloned will be blank here copy all the pixel data over - sourceFrame.GetPixelMemoryGroup().CopyTo(destinationFrame.GetPixelMemoryGroup()); + sourceFrame.PixelBuffer.CopyTo(destinationFrame.PixelBuffer); } return; diff --git a/tests/ImageSharp.Tests/Image/ImageFrameCollectionTests.Generic.cs b/tests/ImageSharp.Tests/Image/ImageFrameCollectionTests.Generic.cs index d7f2c7d28e..417ee18fbe 100644 --- a/tests/ImageSharp.Tests/Image/ImageFrameCollectionTests.Generic.cs +++ b/tests/ImageSharp.Tests/Image/ImageFrameCollectionTests.Generic.cs @@ -64,7 +64,7 @@ public abstract partial class ImageFrameCollectionTests using ImageFrame addedFrame = this.Collection.AddFrame(Array.Empty()); }); - Assert.StartsWith($"Parameter \"data\" ({typeof(int)}) must be greater than or equal to {100}, was {0}", ex.Message); + Assert.StartsWith($"Parameter \"data\" ({typeof(long)}) must be greater than or equal to {100}, was {0}", ex.Message); } [Fact] diff --git a/tests/ImageSharp.Tests/Image/ImageTests.LoadPixelData.cs b/tests/ImageSharp.Tests/Image/ImageTests.LoadPixelData.cs index 47ae0daf72..171aa6d4c5 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.LoadPixelData.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.LoadPixelData.cs @@ -52,5 +52,79 @@ public partial class ImageTests Assert.Equal(Color.White, Color.FromPixel(img[1, 0])); Assert.Equal(Color.Black, Color.FromPixel(img[1, 1])); } + + [Fact] + public void FromPixels_WithRowStride() + { + Rgba32[] data = + [ + Color.Black.ToPixel(), + Color.White.ToPixel(), + Color.Red.ToPixel(), // padding + Color.White.ToPixel(), + Color.Black.ToPixel(), + Color.Red.ToPixel() // padding + ]; + + using Image img = Image.LoadPixelData(data.AsSpan(), width: 2, height: 2, rowStride: 3); + Assert.NotNull(img); + Assert.Equal(Color.Black, Color.FromPixel(img[0, 0])); + Assert.Equal(Color.White, Color.FromPixel(img[0, 1])); + Assert.Equal(Color.White, Color.FromPixel(img[1, 0])); + Assert.Equal(Color.Black, Color.FromPixel(img[1, 1])); + } + + [Fact] + public void FromBytes_WithRowStrideInBytes() + { + byte[] data = + [ + 0, 0, 0, 255, // 0,0 + 255, 255, 255, 255, // 0,1 + 255, 0, 0, 255, // padding + 255, 255, 255, 255, // 1,0 + 0, 0, 0, 255, // 1,1 + 255, 0, 0, 255 // padding + ]; + + using Image img = Image.LoadPixelData(data.AsSpan(), width: 2, height: 2, rowStrideInBytes: 12); + Assert.NotNull(img); + Assert.Equal(Color.Black, Color.FromPixel(img[0, 0])); + Assert.Equal(Color.White, Color.FromPixel(img[0, 1])); + Assert.Equal(Color.White, Color.FromPixel(img[1, 0])); + Assert.Equal(Color.Black, Color.FromPixel(img[1, 1])); + } + + [Fact] + public void FromPixels_WithRowStride_InvalidStride_Throws() + { + Rgba32[] data = new Rgba32[6]; + Assert.ThrowsAny( + () => Image.LoadPixelData(data.AsSpan(), width: 2, height: 2, rowStride: 1)); + } + + [Fact] + public void FromPixels_WithRowStride_InvalidLength_Throws() + { + Rgba32[] data = new Rgba32[4]; + Assert.ThrowsAny( + () => Image.LoadPixelData(data.AsSpan(), width: 2, height: 2, rowStride: 3)); + } + + [Fact] + public void FromBytes_WithRowStrideInBytes_InvalidStride_Throws() + { + byte[] data = new byte[24]; + Assert.ThrowsAny( + () => Image.LoadPixelData(data.AsSpan(), width: 2, height: 2, rowStrideInBytes: 10)); + } + + [Fact] + public void FromBytes_WithRowStrideInBytes_InvalidLength_Throws() + { + byte[] data = new byte[19]; + Assert.ThrowsAny( + () => Image.LoadPixelData(data.AsSpan(), width: 2, height: 2, rowStrideInBytes: 12)); + } } } diff --git a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs index 6322e65aaa..ef98fc43a7 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs @@ -141,6 +141,103 @@ public partial class ImageTests } } + [Fact] + public void WrapMemory_MemoryOfT_Strided_CreatedImageIsCorrect() + { + Rgba32[] source = + [ + new Rgba32(1, 1, 1, 255), + new Rgba32(2, 2, 2, 255), + new Rgba32(3, 3, 3, 255), + new Rgba32(90, 90, 90, 255), + new Rgba32(4, 4, 4, 255), + new Rgba32(5, 5, 5, 255), + new Rgba32(6, 6, 6, 255), + new Rgba32(91, 91, 91, 255) + ]; + + using Image image = Image.WrapMemory(source.AsMemory(), width: 3, height: 2, rowStride: 4); + + Assert.Equal(4, image.Frames.RootFrame.PixelBuffer.RowStride); + Assert.False(image.DangerousTryGetSinglePixelMemory(out Memory _)); + Assert.Equal(source[0], image[0, 0]); + Assert.Equal(source[2], image[2, 0]); + Assert.Equal(source[4], image[0, 1]); + Assert.Equal(source[6], image[2, 1]); + } + + [Fact] + public void WrapMemory_MemoryOfT_Strided_CopyPixelDataTo_UsesRowStrideLayout() + { + Rgba32[] source = + [ + new Rgba32(1, 1, 1, 255), + new Rgba32(2, 2, 2, 255), + new Rgba32(3, 3, 3, 255), + new Rgba32(90, 90, 90, 255), + new Rgba32(4, 4, 4, 255), + new Rgba32(5, 5, 5, 255), + new Rgba32(6, 6, 6, 255), + new Rgba32(91, 91, 91, 255) + ]; + + using Image image = Image.WrapMemory(source.AsMemory(), width: 3, height: 2, rowStride: 4); + + Rgba32 sentinel = new(250, 1, 1, 255); + Rgba32[] destination = [sentinel, sentinel, sentinel, sentinel, sentinel, sentinel, sentinel]; + image.CopyPixelDataTo(destination); + + Assert.Equal(source[0], destination[0]); + Assert.Equal(source[1], destination[1]); + Assert.Equal(source[2], destination[2]); + Assert.Equal(sentinel, destination[3]); + Assert.Equal(source[4], destination[4]); + Assert.Equal(source[5], destination[5]); + Assert.Equal(source[6], destination[6]); + Assert.ThrowsAny(() => image.CopyPixelDataTo(new Rgba32[6])); + } + + [Fact] + public void WrapMemory_MemoryOfByte_Strided_CreatedImageIsCorrect() + { + int pixelSize = Unsafe.SizeOf(); + byte[] sourceBytes = new byte[8 * pixelSize]; + Span source = MemoryMarshal.Cast(sourceBytes); + + source[0] = new Rgba32(1, 1, 1, 255); + source[1] = new Rgba32(2, 2, 2, 255); + source[2] = new Rgba32(3, 3, 3, 255); + source[4] = new Rgba32(4, 4, 4, 255); + source[5] = new Rgba32(5, 5, 5, 255); + source[6] = new Rgba32(6, 6, 6, 255); + + using Image image = Image.WrapMemory( + sourceBytes.AsMemory(), + width: 3, + height: 2, + rowStrideInBytes: 4 * pixelSize); + + Assert.Equal(4, image.Frames.RootFrame.PixelBuffer.RowStride); + Assert.False(image.DangerousTryGetSinglePixelMemory(out Memory _)); + Assert.Equal(source[0], image[0, 0]); + Assert.Equal(source[2], image[2, 0]); + Assert.Equal(source[4], image[0, 1]); + Assert.Equal(source[6], image[2, 1]); + } + + [Fact] + public void WrapMemory_Strided_InvalidStride_Throws() + { + Rgba32[] pixelSource = new Rgba32[8]; + byte[] byteSource = new byte[8 * Unsafe.SizeOf()]; + + Assert.ThrowsAny(() => Image.WrapMemory(pixelSource.AsMemory(), width: 3, height: 2, rowStride: 2)); + Assert.ThrowsAny(() => Image.WrapMemory(pixelSource.AsMemory(0, 6), width: 3, height: 2, rowStride: 4)); + + Assert.ThrowsAny(() => Image.WrapMemory(byteSource.AsMemory(), width: 3, height: 2, rowStrideInBytes: (4 * Unsafe.SizeOf()) - 1)); + Assert.ThrowsAny(() => Image.WrapMemory(byteSource.AsMemory(0, 6 * Unsafe.SizeOf()), width: 3, height: 2, rowStrideInBytes: 4 * Unsafe.SizeOf())); + } + [Fact] public void WrapSystemDrawingBitmap_WhenObserved() { diff --git a/tests/ImageSharp.Tests/Memory/Buffer2DTests.CopyTo.cs b/tests/ImageSharp.Tests/Memory/Buffer2DTests.CopyTo.cs new file mode 100644 index 0000000000..b19095be9e --- /dev/null +++ b/tests/ImageSharp.Tests/Memory/Buffer2DTests.CopyTo.cs @@ -0,0 +1,43 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using SixLabors.ImageSharp.Memory; + +namespace SixLabors.ImageSharp.Tests.Memory; + +public partial class Buffer2DTests +{ + [Fact] + public void CopyToSpan_StridedSource_WritesDestinationUsingSourceStride() + { + int[] source = [1, 2, 3, 777, 4, 5, 6, 888]; + int[] destination = [-1, -1, -1, -1, -1, -1, -1]; + + using Buffer2D buffer = Buffer2D.WrapMemory(source.AsMemory(), width: 3, height: 2, stride: 4); + buffer.CopyTo(destination); + + Assert.Equal(new[] { 1, 2, 3, -1, 4, 5, 6 }, destination); + } + + [Fact] + public void CopyToSpan_PackedSource_WritesPackedDestination() + { + int[] source = [1, 2, 3, 4, 5, 6]; + int[] destination = new int[6]; + + using Buffer2D buffer = Buffer2D.WrapMemory(source.AsMemory(), width: 3, height: 2); + buffer.CopyTo(destination); + + Assert.Equal(new[] { 1, 2, 3, 4, 5, 6 }, destination); + } + + [Fact] + public void CopyToSpan_StridedSource_ThrowsWhenDestinationTooShort() + { + int[] source = [1, 2, 3, 777, 4, 5, 6, 888]; + int[] destination = new int[6]; + + using Buffer2D buffer = Buffer2D.WrapMemory(source.AsMemory(), width: 3, height: 2, stride: 4); + Assert.Throws(() => buffer.CopyTo(destination)); + } +} diff --git a/tests/ImageSharp.Tests/Memory/Buffer2DTests.SwapOrCopyContent.cs b/tests/ImageSharp.Tests/Memory/Buffer2DTests.SwapOrCopyContent.cs index e17c8c46f7..caf935cc47 100644 --- a/tests/ImageSharp.Tests/Memory/Buffer2DTests.SwapOrCopyContent.cs +++ b/tests/ImageSharp.Tests/Memory/Buffer2DTests.SwapOrCopyContent.cs @@ -152,5 +152,74 @@ public partial class Buffer2DTests Assert.Equal(color, source.MemoryGroup[0].Span[10]); Assert.NotEqual(color, dest.MemoryGroup[0].Span[10]); } + + [Fact] + public void WhenDestIsNotMemoryOwner_DifferentSizeSameTotal_PackedLayout_ShouldCopy() + { + int[] data = new int[6]; + using TestMemoryManager destOwner = new(data); + using Buffer2D dest = new(MemoryGroup.Wrap(destOwner.Memory), 2, 3); + using Buffer2D source = this.memoryAllocator.Allocate2D(3, 2, AllocationOptions.Clean); + + source[0, 0] = 1; + source[1, 0] = 2; + source[2, 0] = 3; + source[0, 1] = 4; + source[1, 1] = 5; + source[2, 1] = 6; + + bool swap = Buffer2D.SwapOrCopyContent(dest, source); + + Assert.False(swap); + Assert.Equal(new Size(3, 2), dest.Size()); + Assert.Equal(6, dest[2, 1]); + } + + [Fact] + public void WhenDestIsNotMemoryOwner_DifferentSizeSameTotal_StridedLayout_ShouldCopy() + { + int[] data = new int[5]; + using Buffer2D dest = Buffer2D.WrapMemory(data.AsMemory(), width: 2, height: 2, stride: 3); + using Buffer2D source = this.memoryAllocator.Allocate2D(1, 5, AllocationOptions.Clean); + + source[0, 0] = 1; + source[0, 1] = 2; + source[0, 2] = 3; + source[0, 3] = 4; + source[0, 4] = 5; + + bool swap = Buffer2D.SwapOrCopyContent(dest, source); + + Assert.False(swap); + Assert.Equal(new Size(1, 5), dest.Size()); + Assert.Equal(1, dest[0, 0]); + Assert.Equal(2, dest[0, 1]); + Assert.Equal(3, dest[0, 2]); + Assert.Equal(4, dest[0, 3]); + Assert.Equal(5, dest[0, 4]); + } + + [Fact] + public void WhenDestIsNotMemoryOwner_DifferentSizeDifferentTotal_ButBothLayoutsFit_ShouldCopy() + { + int[] data = new int[5]; + using TestMemoryManager destOwner = new(data); + using Buffer2D dest = new(MemoryGroup.Wrap(destOwner.Memory), 1, 3); + using Buffer2D source = this.memoryAllocator.Allocate2D(2, 2, AllocationOptions.Clean); + + source[0, 0] = 1; + source[1, 0] = 2; + source[0, 1] = 3; + source[1, 1] = 4; + + bool swap = Buffer2D.SwapOrCopyContent(dest, source); + + Assert.False(swap); + Assert.Equal(new Size(2, 2), dest.Size()); + Assert.Equal(1, dest[0, 0]); + Assert.Equal(2, dest[1, 0]); + Assert.Equal(3, dest[0, 1]); + Assert.Equal(4, dest[1, 1]); + } } } diff --git a/tests/ImageSharp.Tests/Memory/Buffer2DTests.WrapMemory.cs b/tests/ImageSharp.Tests/Memory/Buffer2DTests.WrapMemory.cs new file mode 100644 index 0000000000..f65f193eae --- /dev/null +++ b/tests/ImageSharp.Tests/Memory/Buffer2DTests.WrapMemory.cs @@ -0,0 +1,91 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using System.Runtime.CompilerServices; +using SixLabors.ImageSharp.Memory; + +namespace SixLabors.ImageSharp.Tests.Memory; + +public partial class Buffer2DTests +{ + [Fact] + public void WrapMemory_Packed_DangerousTryGetSinglePixelMemory_ReturnsTrue() + { + int[] data = [1, 2, 3, 4, 5, 6]; + + using Buffer2D buffer = Buffer2D.WrapMemory(data.AsMemory(), width: 3, height: 2, stride: 3); + + Assert.True(buffer.DangerousTryGetSingleMemory(out Memory memory)); + Assert.Equal(3, buffer.RowStride); + Assert.Equal(6, memory.Length); + Assert.True(Unsafe.AreSame(ref data[0], ref memory.Span[0])); + Assert.SpanPointsTo(buffer.DangerousGetRowSpan(1), data.AsMemory(), bufferOffset: 3); + } + + [Fact] + public void WrapMemory_Strided_DangerousTryGetSinglePixelMemory_ReturnsFalse() + { + int[] data = [1, 2, 3, 777, 4, 5, 6, 888]; + + using Buffer2D buffer = Buffer2D.WrapMemory(data.AsMemory(), width: 3, height: 2, stride: 4); + + Assert.False(buffer.DangerousTryGetSingleMemory(out Memory _)); + Assert.Equal(4, buffer.RowStride); + Assert.Equal(4, new Buffer2DRegion(buffer).Stride); + + Span row0 = buffer.DangerousGetRowSpan(0); + Span row1 = buffer.DangerousGetRowSpan(1); + + Assert.Equal(3, row0.Length); + Assert.Equal(3, row1.Length); + Assert.Equal(1, row0[0]); + Assert.Equal(3, row0[2]); + Assert.Equal(4, row1[0]); + Assert.Equal(6, row1[2]); + + Assert.SpanPointsTo(row0, data.AsMemory(), bufferOffset: 0); + Assert.SpanPointsTo(row1, data.AsMemory(), bufferOffset: 4); + } + + [Fact] + public void WrapMemory_Packed_WithTrailingData_DangerousTryGetSinglePixelMemory_IsLogicalSize() + { + int[] data = [1, 2, 3, 4, 5, 6, 777, 888]; + + using Buffer2D buffer = Buffer2D.WrapMemory(data.AsMemory(), width: 3, height: 2, stride: 3); + + Assert.True(buffer.DangerousTryGetSingleMemory(out Memory memory)); + Assert.Equal(6, memory.Length); + Assert.True(Unsafe.AreSame(ref data[0], ref memory.Span[0])); + } + + [Fact] + public void WrapMemory_Strided_ThrowsWhenStrideIsLessThanWidth() + { + int[] data = new int[10]; + + Assert.Throws(() => Buffer2D.WrapMemory(data.AsMemory(), width: 3, height: 2, stride: 2)); + } + + [Fact] + public void WrapMemory_Strided_ThrowsWhenInputMemoryTooSmall() + { + int[] data = new int[6]; + + Assert.Throws(() => Buffer2D.WrapMemory(data.AsMemory(), width: 3, height: 2, stride: 4)); + } + + [Theory] + [InlineData(0, 1)] + [InlineData(1, 0)] + [InlineData(0, 0)] + [InlineData(-1, 1)] + [InlineData(1, -1)] + public void WrapMemory_ThrowsWhenWidthOrHeightIsNotPositive(int width, int height) + { + int[] data = new int[16]; + + Assert.Throws(() => Buffer2D.WrapMemory(data.AsMemory(), width, height)); + Assert.Throws(() => Buffer2D.WrapMemory(data.AsMemory(), width, height, stride: 4)); + } +} diff --git a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs index 6dfdd294c7..8e3f3d286a 100644 --- a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs +++ b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs @@ -50,17 +50,11 @@ public partial class Buffer2DTests [InlineData(Big, 1, 0)] [InlineData(60, 42, 0)] [InlineData(3, 0, 0)] - public unsafe void Construct_Empty(int bufferCapacity, int width, int height) + public unsafe void Construct_Empty_Throws(int bufferCapacity, int width, int height) { this.MemoryAllocator.BufferCapacityInBytes = sizeof(TestStructs.Foo) * bufferCapacity; - using (Buffer2D buffer = this.MemoryAllocator.Allocate2D(width, height)) - { - Assert.Equal(width, buffer.Width); - Assert.Equal(height, buffer.Height); - Assert.Equal(0, buffer.FastMemoryGroup.TotalLength); - Assert.Equal(0, buffer.DangerousGetSingleSpan().Length); - } + Assert.Throws(() => this.MemoryAllocator.Allocate2D(width, height)); } [Theory] @@ -339,25 +333,47 @@ public partial class Buffer2DTests Assert.NotSame(mgBefore, buffer1.MemoryGroup); } - public static TheoryData InvalidLengths { get; set; } = new() + public static TheoryData InvalidDimensions { get; set; } = new() { { new Size(-1, -1) }, + { new Size(0, 1) }, + { new Size(1, 0) }, + { new Size(0, 0) } + }; + + public static TheoryData OverflowDimensions { get; set; } = new() + { { new Size(32768, 32769) }, { new Size(32769, 32768) } }; [Theory] - [MemberData(nameof(InvalidLengths))] - public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException(Size size) + [MemberData(nameof(InvalidDimensions))] + public void Allocate_InvalidDimensions_ThrowsArgumentOutOfRangeException(Size size) + => Assert.Throws(() => this.MemoryAllocator.Allocate2D(size.Width, size.Height)); + + [Theory] + [MemberData(nameof(InvalidDimensions))] + public void Allocate_InvalidDimensions_SizeOverload_ThrowsArgumentOutOfRangeException(Size size) + => Assert.Throws(() => this.MemoryAllocator.Allocate2D(new Size(size))); + + [Theory] + [MemberData(nameof(InvalidDimensions))] + public void Allocate_InvalidDimensions_OverAligned_ThrowsArgumentOutOfRangeException(Size size) + => Assert.Throws(() => this.MemoryAllocator.Allocate2DOveraligned(size.Width, size.Height, 1)); + + [Theory] + [MemberData(nameof(OverflowDimensions))] + public void Allocate_OverflowDimensions_ThrowsInvalidMemoryOperationException(Size size) => Assert.Throws(() => this.MemoryAllocator.Allocate2D(size.Width, size.Height)); [Theory] - [MemberData(nameof(InvalidLengths))] - public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException_Size(Size size) + [MemberData(nameof(OverflowDimensions))] + public void Allocate_OverflowDimensions_SizeOverload_ThrowsInvalidMemoryOperationException(Size size) => Assert.Throws(() => this.MemoryAllocator.Allocate2D(new Size(size))); [Theory] - [MemberData(nameof(InvalidLengths))] - public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException_OverAligned(Size size) + [MemberData(nameof(OverflowDimensions))] + public void Allocate_OverflowDimensions_OverAligned_ThrowsInvalidMemoryOperationException(Size size) => Assert.Throws(() => this.MemoryAllocator.Allocate2DOveraligned(size.Width, size.Height, 1)); } diff --git a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.CopyTo.cs b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.CopyTo.cs index c140571fcc..ee5cd9cd05 100644 --- a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.CopyTo.cs +++ b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.CopyTo.cs @@ -9,100 +9,74 @@ public partial class MemoryGroupTests { public class CopyTo : MemoryGroupTestsBase { - public static readonly TheoryData WhenSourceBufferIsShorterOrEqual_Data = - CopyAndTransformData; - - [Theory] - [MemberData(nameof(WhenSourceBufferIsShorterOrEqual_Data))] - public void WhenSourceBufferIsShorterOrEqual(int srcTotal, int srcBufLen, int trgTotal, int trgBufLen) + [Fact] + public void GroupToSpan_StridedSource_DoesNotRequireTrailingPadding() { - using MemoryGroup src = this.CreateTestGroup(srcTotal, srcBufLen, true); - using MemoryGroup trg = this.CreateTestGroup(trgTotal, trgBufLen, false); - - src.CopyTo(trg); + using MemoryGroup src = this.CreateTestGroup(totalLength: 7, bufferLength: 4, fillSequence: true); + int[] trg = new int[6]; - int pos = 0; - MemoryGroupIndex i = src.MinIndex(); - MemoryGroupIndex j = trg.MinIndex(); - for (; i < src.MaxIndex(); i += 1, j += 1, pos++) - { - int a = src.GetElementAt(i); - int b = trg.GetElementAt(j); + src.CopyTo( + sourceStride: 4, + trg, + targetStride: 3, + width: 3, + height: 2); - Assert.True(a == b, $"Mismatch @ {pos} Expected: {a} Actual: {b}"); - } + Assert.Equal(new[] { 1, 2, 3, 5, 6, 7 }, trg); } [Fact] - public void WhenTargetBufferTooShort_Throws() + public void GroupToGroup_StridedCopy_DoesNotRequireTrailingPadding() { - using MemoryGroup src = this.CreateTestGroup(10, 20, true); - using MemoryGroup trg = this.CreateTestGroup(5, 20, false); - - Assert.Throws(() => src.CopyTo(trg)); + using MemoryGroup src = this.CreateTestGroup(totalLength: 11, bufferLength: 5, fillSequence: true); + using MemoryGroup trg = this.CreateTestGroup(totalLength: 13, bufferLength: 6, fillSequence: false); + + src.CopyTo( + sourceStride: 4, + trg, + targetStride: 5, + width: 3, + height: 3); + + Assert.Equal(1, GetElementAtLinearIndex(trg, 0)); + Assert.Equal(2, GetElementAtLinearIndex(trg, 1)); + Assert.Equal(3, GetElementAtLinearIndex(trg, 2)); + Assert.Equal(5, GetElementAtLinearIndex(trg, 5)); + Assert.Equal(6, GetElementAtLinearIndex(trg, 6)); + Assert.Equal(7, GetElementAtLinearIndex(trg, 7)); + Assert.Equal(9, GetElementAtLinearIndex(trg, 10)); + Assert.Equal(10, GetElementAtLinearIndex(trg, 11)); + Assert.Equal(11, GetElementAtLinearIndex(trg, 12)); } - [Theory] - [InlineData(30, 10, 40)] - [InlineData(42, 23, 42)] - [InlineData(1, 3, 10)] - [InlineData(0, 4, 0)] - public void GroupToSpan_Success(long totalLength, int bufferLength, int spanLength) + [Fact] + public void GroupToSpan_StridedSource_HeightOne() { - using MemoryGroup src = this.CreateTestGroup(totalLength, bufferLength, true); - int[] trg = new int[spanLength]; - src.CopyTo(trg); + using MemoryGroup src = this.CreateTestGroup(totalLength: 3, bufferLength: 2, fillSequence: true); + int[] trg = new int[3]; - int expected = 1; - foreach (int val in trg.AsSpan().Slice(0, (int)totalLength)) - { - Assert.Equal(expected, val); - expected++; - } - } + src.CopyTo( + sourceStride: 8, + trg, + targetStride: 3, + width: 3, + height: 1); - [Theory] - [InlineData(20, 7, 19)] - [InlineData(2, 1, 1)] - public void GroupToSpan_OutOfRange(long totalLength, int bufferLength, int spanLength) - { - using MemoryGroup src = this.CreateTestGroup(totalLength, bufferLength, true); - int[] trg = new int[spanLength]; - Assert.ThrowsAny(() => src.CopyTo(trg)); + Assert.Equal(new[] { 1, 2, 3 }, trg); } - [Theory] - [InlineData(30, 35, 10)] - [InlineData(42, 23, 42)] - [InlineData(10, 3, 1)] - [InlineData(0, 3, 0)] - public void SpanToGroup_Success(long totalLength, int bufferLength, int spanLength) + private static int GetElementAtLinearIndex(MemoryGroup group, int index) { - int[] src = new int[spanLength]; - for (int i = 0; i < src.Length; i++) - { - src[i] = i + 1; - } - - using MemoryGroup trg = this.CreateTestGroup(totalLength, bufferLength); - src.AsSpan().CopyTo(trg); - - int position = 0; - for (MemoryGroupIndex i = trg.MinIndex(); position < spanLength; i += 1, position++) + int pos = 0; + for (MemoryGroupIndex i = group.MinIndex(); i < group.MaxIndex(); i += 1, pos++) { - int expected = position + 1; - Assert.Equal(expected, trg.GetElementAt(i)); + if (pos == index) + { + return group.GetElementAt(i); + } } - } - [Theory] - [InlineData(10, 3, 11)] - [InlineData(0, 3, 1)] - public void SpanToGroup_OutOfRange(long totalLength, int bufferLength, int spanLength) - { - int[] src = new int[spanLength]; - using MemoryGroup trg = this.CreateTestGroup(totalLength, bufferLength, true); - Assert.ThrowsAny(() => src.AsSpan().CopyTo(trg)); + throw new ArgumentOutOfRangeException(nameof(index)); } } } diff --git a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.cs b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.cs index fe1867f20c..b82b20120a 100644 --- a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.cs +++ b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.cs @@ -26,76 +26,6 @@ public partial class MemoryGroupTests : MemoryGroupTestsBase Assert.False(g.IsValid); } -#pragma warning disable SA1509 - private static readonly TheoryData CopyAndTransformData = - new() - { - { 20, 10, 20, 10 }, - { 20, 5, 20, 4 }, - { 20, 4, 20, 5 }, - { 18, 6, 20, 5 }, - { 19, 10, 20, 10 }, - { 21, 10, 22, 2 }, - { 1, 5, 5, 4 }, - - { 30, 12, 40, 5 }, - { 30, 5, 40, 12 }, - }; - - public class TransformTo : MemoryGroupTestsBase - { - public static readonly TheoryData WhenSourceBufferIsShorterOrEqual_Data = - CopyAndTransformData; - - [Theory] - [MemberData(nameof(WhenSourceBufferIsShorterOrEqual_Data))] - public void WhenSourceBufferIsShorterOrEqual(int srcTotal, int srcBufLen, int trgTotal, int trgBufLen) - { - using MemoryGroup src = this.CreateTestGroup(srcTotal, srcBufLen, true); - using MemoryGroup trg = this.CreateTestGroup(trgTotal, trgBufLen, false); - - src.TransformTo(trg, MultiplyAllBy2); - - int pos = 0; - MemoryGroupIndex i = src.MinIndex(); - MemoryGroupIndex j = trg.MinIndex(); - for (; i < src.MaxIndex(); i += 1, j += 1, pos++) - { - int a = src.GetElementAt(i); - int b = trg.GetElementAt(j); - - Assert.True(b == 2 * a, $"Mismatch @ {pos} Expected: {a} Actual: {b}"); - } - } - - [Fact] - public void WhenTargetBufferTooShort_Throws() - { - using MemoryGroup src = this.CreateTestGroup(10, 20, true); - using MemoryGroup trg = this.CreateTestGroup(5, 20, false); - - Assert.Throws(() => src.TransformTo(trg, MultiplyAllBy2)); - } - } - - [Theory] - [InlineData(100, 5)] - [InlineData(100, 101)] - public void TransformInplace(int totalLength, int bufferLength) - { - using MemoryGroup src = this.CreateTestGroup(10, 20, true); - - src.TransformInplace(s => MultiplyAllBy2(s, s)); - - int cnt = 1; - for (MemoryGroupIndex i = src.MinIndex(); i < src.MaxIndex(); i += 1) - { - int val = src.GetElementAt(i); - Assert.Equal(expected: cnt * 2, val); - cnt++; - } - } - [Fact] public void Wrap() { @@ -224,12 +154,4 @@ public partial class MemoryGroupTests : MemoryGroupTestsBase } } - private static void MultiplyAllBy2(ReadOnlySpan source, Span target) - { - Assert.Equal(source.Length, target.Length); - for (int k = 0; k < source.Length; k++) - { - target[k] = source[k] * 2; - } - } } From 6a4d10291ba6bf6f98a16e30ee1e7b78862fc0dd Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 3 Mar 2026 18:12:04 +1000 Subject: [PATCH 2/3] Update ImageTests.WrapMemory.cs --- tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs index ef98fc43a7..65eeb124f6 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs @@ -202,7 +202,7 @@ public partial class ImageTests { int pixelSize = Unsafe.SizeOf(); byte[] sourceBytes = new byte[8 * pixelSize]; - Span source = MemoryMarshal.Cast(sourceBytes); + Span source = MemoryMarshal.Cast(sourceBytes.AsSpan()); source[0] = new Rgba32(1, 1, 1, 255); source[1] = new Rgba32(2, 2, 2, 255); From d557bb29e0a4d4dd022d07dbd1caff4791b91b23 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 3 Mar 2026 20:17:24 +1000 Subject: [PATCH 3/3] Fix review feedback --- src/ImageSharp/Image.WrapMemory.cs | 4 ++-- src/ImageSharp/Memory/Buffer2D{T}.cs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ImageSharp/Image.WrapMemory.cs b/src/ImageSharp/Image.WrapMemory.cs index e44af5d0d1..eac202a41f 100644 --- a/src/ImageSharp/Image.WrapMemory.cs +++ b/src/ImageSharp/Image.WrapMemory.cs @@ -609,14 +609,14 @@ public abstract partial class Image ImageMetadata metadata) where TPixel : unmanaged, IPixel { - Guard.IsFalse(pointer == null, nameof(pointer), "Pointer must be not null"); + Guard.IsFalse(pointer == null, nameof(pointer), "Pointer must not be null"); Guard.NotNull(configuration, nameof(configuration)); Guard.NotNull(metadata, nameof(metadata)); int rowStride = GetPixelRowStrideFromByteStride(width, rowStrideInBytes, nameof(rowStrideInBytes)); long requiredLength = GetRequiredLength(width, height, rowStride); - Guard.MustBeLessThanOrEqualTo(requiredLength, int.MaxValue, nameof(height)); + Guard.MustBeLessThanOrEqualTo(requiredLength, int.MaxValue, nameof(requiredLength)); Guard.MustBeGreaterThanOrEqualTo(bufferSizeInBytes / Unsafe.SizeOf(), requiredLength, nameof(bufferSizeInBytes)); UnmanagedMemoryManager memoryManager = new(pointer, (int)requiredLength); diff --git a/src/ImageSharp/Memory/Buffer2D{T}.cs b/src/ImageSharp/Memory/Buffer2D{T}.cs index 3194aebae7..fe997a4fbf 100644 --- a/src/ImageSharp/Memory/Buffer2D{T}.cs +++ b/src/ImageSharp/Memory/Buffer2D{T}.cs @@ -356,7 +356,7 @@ public sealed class Buffer2D : IDisposable /// Thrown when the backing group is discontiguous. /// [MethodImpl(InliningOptions.ShortMethod)] - internal Span DangerousGetSingleSpan() => this.FastMemoryGroup[0].Span; + internal Span DangerousGetSingleSpan() => this.FastMemoryGroup.Single().Span; /// /// Gets a to the backing data of if the backing group consists of a single contiguous memory buffer. @@ -367,7 +367,7 @@ public sealed class Buffer2D : IDisposable /// Thrown when the backing group is discontiguous. /// [MethodImpl(InliningOptions.ShortMethod)] - internal Memory DangerousGetSingleMemory() => this.FastMemoryGroup[0]; + internal Memory DangerousGetSingleMemory() => this.FastMemoryGroup.Single(); /// /// Swaps the contents of 'destination' with 'source' if the buffers are owned (1), @@ -414,5 +414,5 @@ public sealed class Buffer2D : IDisposable [MethodImpl(InliningOptions.ShortMethod)] private static long GetRequiredLength(int width, int height, int stride) - => ((long)(height - 1) * stride) + width; + => checked(((long)(height - 1) * stride) + width); }