diff --git a/src/ImageSharp/Advanced/AdvancedImageExtensions.cs b/src/ImageSharp/Advanced/AdvancedImageExtensions.cs index 665d0e28b6..fc6ba10b0d 100644 --- a/src/ImageSharp/Advanced/AdvancedImageExtensions.cs +++ b/src/ImageSharp/Advanced/AdvancedImageExtensions.cs @@ -87,7 +87,7 @@ namespace SixLabors.ImageSharp.Advanced /// The /// Thrown when the backing buffer is discontiguous. [Obsolete( - @"GetPixelSpan might fail, because the backing buffer allowed to be discontiguous for large images. Use GetPixelMemoryGroup or GetPixelRowSpan instead!")] + @"GetPixelSpan might fail, because the backing buffer could be discontiguous for large images. Use GetPixelMemoryGroup or GetPixelRowSpan instead!")] public static Span GetPixelSpan(this ImageFrame source) where TPixel : struct, IPixel { @@ -109,7 +109,7 @@ namespace SixLabors.ImageSharp.Advanced /// The /// Thrown when the backing buffer is discontiguous. [Obsolete( - @"GetPixelSpan might fail, because the backing buffer allowed to be discontiguous for large images. Use GetPixelMemoryGroup or GetPixelRowSpan instead!")] + @"GetPixelSpan might fail, because the backing buffer could be discontiguous for large images. Use GetPixelMemoryGroup or GetPixelRowSpan instead!")] public static Span GetPixelSpan(this Image source) where TPixel : struct, IPixel => source.Frames.RootFrame.GetPixelSpan(); @@ -146,9 +146,9 @@ namespace SixLabors.ImageSharp.Advanced /// The source. /// The row. /// The - internal static Memory GetPixelRowMemory(this ImageFrame source, int rowIndex) + public static Memory GetPixelRowMemory(this ImageFrame source, int rowIndex) where TPixel : struct, IPixel - => source.PixelBuffer.GetRowMemory(rowIndex); + => source.PixelBuffer.GetRowMemorySafe(rowIndex); /// /// Gets the representation of the pixels as of of contiguous memory @@ -158,7 +158,7 @@ namespace SixLabors.ImageSharp.Advanced /// The source. /// The row. /// The - internal static Memory GetPixelRowMemory(this Image source, int rowIndex) + public static Memory GetPixelRowMemory(this Image source, int rowIndex) where TPixel : struct, IPixel => source.Frames.RootFrame.GetPixelRowMemory(rowIndex); @@ -169,15 +169,5 @@ namespace SixLabors.ImageSharp.Advanced /// Returns the configuration. internal static MemoryAllocator GetMemoryAllocator(this IConfigurationProvider source) => GetConfiguration(source).MemoryAllocator; - - /// - /// Returns a reference to the 0th element of the Pixel buffer. - /// Such a reference can be used for pinning but must never be dereferenced. - /// - /// The source image frame - /// A reference to the element. - private static ref TPixel DangerousGetPinnableReferenceToPixelBuffer(IPixelSource source) - where TPixel : struct, IPixel - => ref MemoryMarshal.GetReference(source.PixelBuffer.GetSingleSpan()); } } diff --git a/src/ImageSharp/Memory/Buffer2D{T}.cs b/src/ImageSharp/Memory/Buffer2D{T}.cs index ea2568efde..bb00b48cdb 100644 --- a/src/ImageSharp/Memory/Buffer2D{T}.cs +++ b/src/ImageSharp/Memory/Buffer2D{T}.cs @@ -83,13 +83,24 @@ namespace SixLabors.ImageSharp.Memory : this.GetRowMemorySlow(y).Span; } + /// + /// Disposes the instance + /// + public void Dispose() + { + this.MemoryGroup.Dispose(); + this.cachedMemory = default; + } + /// /// Gets a to the row 'y' beginning from the pixel at the first pixel on that row. + /// This method is intended for internal use only, since it does not use the indirection provided by + /// . /// /// The y (row) coordinate. /// The . [MethodImpl(MethodImplOptions.AggressiveInlining)] - public Memory GetRowMemory(int y) + internal Memory GetRowMemoryFast(int y) { return this.cachedMemory.Length > 0 ? this.cachedMemory.Slice(y * this.Width, this.Width) @@ -97,13 +108,11 @@ namespace SixLabors.ImageSharp.Memory } /// - /// Disposes the instance + /// Gets a to the row 'y' beginning from the pixel at the first pixel on that row. /// - public void Dispose() - { - this.MemoryGroup.Dispose(); - this.cachedMemory = default; - } + /// The y (row) coordinate. + /// The . + internal Memory GetRowMemorySafe(int y) => this.MemoryGroup.View.GetBoundedSlice(y * this.Width, this.Width); /// /// Swaps the contents of 'destination' with 'source' if the buffers are owned (1), diff --git a/tests/ImageSharp.Tests/Advanced/AdvancedImageExtensionsTests.cs b/tests/ImageSharp.Tests/Advanced/AdvancedImageExtensionsTests.cs index 548caa488d..de69d7207b 100644 --- a/tests/ImageSharp.Tests/Advanced/AdvancedImageExtensionsTests.cs +++ b/tests/ImageSharp.Tests/Advanced/AdvancedImageExtensionsTests.cs @@ -2,10 +2,13 @@ // Licensed under the Apache License, Version 2.0. using System; +using System.Linq; using System.Runtime.CompilerServices; using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Processing; +using SixLabors.ImageSharp.Tests.Memory.DiscontiguousBuffers; using Xunit; // ReSharper disable InconsistentNaming @@ -13,113 +16,145 @@ namespace SixLabors.ImageSharp.Tests.Advanced { public class AdvancedImageExtensionsTests { - public class GetPixelMemory + public class GetPixelMemoryGroup { [Theory] - [WithSolidFilledImages(1, 1, "Red", PixelTypes.Rgba32)] - [WithTestPatternImages(131, 127, PixelTypes.Rgba32 | PixelTypes.Bgr24)] - public void WhenMemoryIsOwned(TestImageProvider provider) + [WithBasicTestPatternImages(1, 1, PixelTypes.Rgba32)] + [WithBasicTestPatternImages(131, 127, PixelTypes.Rgba32)] + [WithBasicTestPatternImages(333, 555, PixelTypes.Bgr24)] + public void OwnedMemory_PixelDataIsCorrect(TestImageProvider provider) where TPixel : struct, IPixel { - using (Image image0 = provider.GetImage()) - { - var targetBuffer = new TPixel[image0.Width * image0.Height]; + provider.LimitAllocatorBufferCapacity(); - // Act: - Memory memory = image0.GetRootFramePixelBuffer().GetSingleMemory(); + using Image image = provider.GetImage(); - // Assert: - Assert.Equal(image0.Width * image0.Height, memory.Length); - memory.Span.CopyTo(targetBuffer); + // Act: + IMemoryGroup memoryGroup = image.GetPixelMemoryGroup(); - using (Image image1 = provider.GetImage()) - { - // We are using a copy of the original image for assertion - image1.ComparePixelBufferTo(targetBuffer); - } - } + // Assert: + VerifyMemoryGroupDataMatchesTestPattern(provider, memoryGroup, image.Size()); } [Theory] - [WithSolidFilledImages(1, 1, "Red", PixelTypes.Rgba32 | PixelTypes.Bgr24)] - [WithTestPatternImages(131, 127, PixelTypes.Rgba32 | PixelTypes.Bgr24)] - public void WhenMemoryIsConsumed(TestImageProvider provider) + [WithBlankImages(16, 16, PixelTypes.Rgba32)] + public void OwnedMemory_DestructiveMutate_ShouldInvalidateMemoryGroup(TestImageProvider provider) where TPixel : struct, IPixel { - using (Image image0 = provider.GetImage()) - { - var targetBuffer = new TPixel[image0.Width * image0.Height]; - image0.GetPixelSpan().CopyTo(targetBuffer); + using Image image = provider.GetImage(); + + IMemoryGroup memoryGroup = image.GetPixelMemoryGroup(); + Memory memory = memoryGroup.Single(); - var managerOfExternalMemory = new TestMemoryManager(targetBuffer); + image.Mutate(c => c.Resize(8, 8)); - Memory externalMemory = managerOfExternalMemory.Memory; + Assert.False(memoryGroup.IsValid); + Assert.ThrowsAny(() => _ = memoryGroup.First()); + Assert.ThrowsAny(() => _ = memory.Span); + } + + [Theory] + [WithBasicTestPatternImages(1, 1, PixelTypes.Rgba32)] + [WithBasicTestPatternImages(131, 127, PixelTypes.Bgr24)] + public void ConsumedMemory_PixelDataIsCorrect(TestImageProvider provider) + where TPixel : struct, IPixel + { + using Image image0 = provider.GetImage(); + var targetBuffer = new TPixel[image0.Width * image0.Height]; + image0.GetPixelSpan().CopyTo(targetBuffer); - using (var image1 = Image.WrapMemory(externalMemory, image0.Width, image0.Height)) - { - Memory internalMemory = image1.GetRootFramePixelBuffer().GetSingleMemory(); - Assert.Equal(targetBuffer.Length, internalMemory.Length); - Assert.True(Unsafe.AreSame(ref targetBuffer[0], ref internalMemory.Span[0])); + var managerOfExternalMemory = new TestMemoryManager(targetBuffer); - image0.ComparePixelBufferTo(internalMemory.Span); - } + Memory externalMemory = managerOfExternalMemory.Memory; - // Make sure externalMemory works after destruction: - image0.ComparePixelBufferTo(externalMemory.Span); + using (var image1 = Image.WrapMemory(externalMemory, image0.Width, image0.Height)) + { + VerifyMemoryGroupDataMatchesTestPattern(provider, image1.GetPixelMemoryGroup(), image1.Size()); } + + // Make sure externalMemory works after destruction: + VerifyMemoryGroupDataMatchesTestPattern(provider, image0.GetPixelMemoryGroup(), image0.Size()); } - } - [Theory] - [WithSolidFilledImages(1, 1, "Red", PixelTypes.Rgba32)] - [WithTestPatternImages(131, 127, PixelTypes.Rgba32 | PixelTypes.Bgr24)] - public void GetPixelRowMemory(TestImageProvider provider) - where TPixel : struct, IPixel - { - using (Image image = provider.GetImage()) + private static void VerifyMemoryGroupDataMatchesTestPattern( + TestImageProvider provider, + IMemoryGroup memoryGroup, + Size size) + where TPixel : struct, IPixel { - var targetBuffer = new TPixel[image.Width * image.Height]; + Assert.True(memoryGroup.IsValid); + Assert.Equal(size.Width * size.Height, memoryGroup.TotalLength); + Assert.True(memoryGroup.BufferLength % size.Width == 0); - // Act: - for (int y = 0; y < image.Height; y++) + int cnt = 0; + for (MemoryGroupIndex i = memoryGroup.MaxIndex(); i < memoryGroup.MaxIndex(); i += 1, cnt++) { - Memory rowMemory = image.GetPixelRowMemory(y); - rowMemory.Span.CopyTo(targetBuffer.AsSpan(image.Width * y)); - } + int y = cnt / size.Width; + int x = cnt % size.Width; - // Assert: - using (Image image1 = provider.GetImage()) - { - // We are using a copy of the original image for assertion - image1.ComparePixelBufferTo(targetBuffer); + TPixel expected = provider.GetExpectedBasicTestPatternPixelAt(x, y); + TPixel actual = memoryGroup.GetElementAt(i); + Assert.Equal(expected, actual); } } } [Theory] - [WithSolidFilledImages(1, 1, "Red", PixelTypes.Rgba32)] - [WithTestPatternImages(131, 127, PixelTypes.Rgba32 | PixelTypes.Bgr24)] - public void GetPixelRowSpan(TestImageProvider provider) + [WithBasicTestPatternImages(1, 1, PixelTypes.Rgba32)] + [WithBasicTestPatternImages(131, 127, PixelTypes.Rgba32)] + [WithBasicTestPatternImages(333, 555, PixelTypes.Bgr24)] + public void GetPixelRowMemory_PixelDataIsCorrect(TestImageProvider provider) where TPixel : struct, IPixel { - using (Image image = provider.GetImage()) - { - var targetBuffer = new TPixel[image.Width * image.Height]; + provider.LimitAllocatorBufferCapacity(); + + using Image image = provider.GetImage(); + for (int y = 0; y < image.Height; y++) + { // Act: - for (int y = 0; y < image.Height; y++) - { - Span rowMemory = image.GetPixelRowSpan(y); - rowMemory.CopyTo(targetBuffer.AsSpan(image.Width * y)); - } + Memory rowMemory = image.GetPixelRowMemory(y); + Span span = rowMemory.Span; // Assert: - using (Image image1 = provider.GetImage()) + for (int x = 0; x < image.Width; x++) { - // We are using a copy of the original image for assertion - image1.ComparePixelBufferTo(targetBuffer); + Assert.Equal(provider.GetExpectedBasicTestPatternPixelAt(x, y), span[x]); } } } + + [Theory] + [WithBasicTestPatternImages(16, 16, PixelTypes.Rgba32)] + public void GetPixelRowMemory_DestructiveMutate_ShouldInvalidateMemory(TestImageProvider provider) + where TPixel : struct, IPixel + { + using Image image = provider.GetImage(); + + Memory memory3 = image.GetPixelRowMemory(3); + Memory memory10 = image.GetPixelRowMemory(10); + + image.Mutate(c => c.Resize(8, 8)); + + Assert.ThrowsAny(() => _ = memory3.Span); + Assert.ThrowsAny(() => _ = memory10.Span); + } + + [Theory] + [WithBlankImages(1, 1, PixelTypes.Rgba32)] + [WithBlankImages(100, 111, PixelTypes.Rgba32)] + [WithBlankImages(400, 600, PixelTypes.Rgba32)] + public void GetPixelRowSpan_ShouldReferenceSpanOfMemory(TestImageProvider provider) + where TPixel : struct, IPixel + { + provider.LimitAllocatorBufferCapacity(); + + using Image image = provider.GetImage(); + + Memory memory = image.GetPixelRowMemory(image.Height - 1); + Span span = image.GetPixelRowSpan(image.Height - 1); + + Assert.True(span == memory.Span); + } } } diff --git a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupIndex.cs b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupIndex.cs index 88824baf28..158428f4b0 100644 --- a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupIndex.cs +++ b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupIndex.cs @@ -91,25 +91,25 @@ namespace SixLabors.ImageSharp.Tests.Memory.DiscontiguousBuffers internal static class MemoryGroupIndexExtensions { - public static T GetElementAt(this MemoryGroup group, MemoryGroupIndex idx) + public static T GetElementAt(this IMemoryGroup group, MemoryGroupIndex idx) where T : struct { return group[idx.BufferIndex].Span[idx.ElementIndex]; } - public static void SetElementAt(this MemoryGroup group, MemoryGroupIndex idx, T value) + public static void SetElementAt(this IMemoryGroup group, MemoryGroupIndex idx, T value) where T : struct { group[idx.BufferIndex].Span[idx.ElementIndex] = value; } - public static MemoryGroupIndex MinIndex(this MemoryGroup group) + public static MemoryGroupIndex MinIndex(this IMemoryGroup group) where T : struct { return new MemoryGroupIndex(group.BufferLength, 0, 0); } - public static MemoryGroupIndex MaxIndex(this MemoryGroup group) + public static MemoryGroupIndex MaxIndex(this IMemoryGroup group) where T : struct { return group.Count == 0 diff --git a/tests/ImageSharp.Tests/TestUtilities/ImageProviders/BasicTestPatternProvider.cs b/tests/ImageSharp.Tests/TestUtilities/ImageProviders/BasicTestPatternProvider.cs index 9100e26e88..1025ed9a15 100644 --- a/tests/ImageSharp.Tests/TestUtilities/ImageProviders/BasicTestPatternProvider.cs +++ b/tests/ImageSharp.Tests/TestUtilities/ImageProviders/BasicTestPatternProvider.cs @@ -11,16 +11,26 @@ namespace SixLabors.ImageSharp.Tests { public abstract partial class TestImageProvider : IXunitSerializable { + public virtual TPixel GetExpectedBasicTestPatternPixelAt(int x, int y) + { + throw new NotSupportedException("GetExpectedBasicTestPatternPixelAt(x,y) only works with BasicTestPattern"); + } + private class BasicTestPatternProvider : BlankProvider { + private static readonly TPixel TopLeftColor = Color.Red.ToPixel(); + private static readonly TPixel TopRightColor = Color.Green.ToPixel(); + private static readonly TPixel BottomLeftColor = Color.Blue.ToPixel(); + + // Transparent purple: + private static readonly TPixel BottomRightColor = GetBottomRightColor(); + public BasicTestPatternProvider(int width, int height) : base(width, height) { } - /// - /// This parameterless constructor is needed for xUnit deserialization - /// + // This parameterless constructor is needed for xUnit deserialization public BasicTestPatternProvider() { } @@ -31,14 +41,6 @@ namespace SixLabors.ImageSharp.Tests { var result = new Image(this.Configuration, this.Width, this.Height); - TPixel topLeftColor = Color.Red.ToPixel(); - TPixel topRightColor = Color.Green.ToPixel(); - TPixel bottomLeftColor = Color.Blue.ToPixel(); - - // Transparent purple: - TPixel bottomRightColor = default; - bottomRightColor.FromVector4(new Vector4(1f, 0f, 1f, 0.5f)); - int midY = this.Height / 2; int midX = this.Width / 2; @@ -46,20 +48,42 @@ namespace SixLabors.ImageSharp.Tests { Span row = result.GetPixelRowSpan(y); - row.Slice(0, midX).Fill(topLeftColor); - row.Slice(midX, this.Width - midX).Fill(topRightColor); + row.Slice(0, midX).Fill(TopLeftColor); + row.Slice(midX, this.Width - midX).Fill(TopRightColor); } for (int y = midY; y < this.Height; y++) { Span row = result.GetPixelRowSpan(y); - row.Slice(0, midX).Fill(bottomLeftColor); - row.Slice(midX, this.Width - midX).Fill(bottomRightColor); + row.Slice(0, midX).Fill(BottomLeftColor); + row.Slice(midX, this.Width - midX).Fill(BottomRightColor); } return result; } + + public override TPixel GetExpectedBasicTestPatternPixelAt(int x, int y) + { + int midY = this.Height / 2; + int midX = this.Width / 2; + + if (y < midY) + { + return x < midX ? TopLeftColor : TopRightColor; + } + else + { + return x < midX ? BottomLeftColor : BottomRightColor; + } + } + + private static TPixel GetBottomRightColor() + { + TPixel bottomRightColor = default; + bottomRightColor.FromVector4(new Vector4(1f, 0f, 1f, 0.5f)); + return bottomRightColor; + } } } -} \ No newline at end of file +}