diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupView{T}.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupView{T}.cs index ec801015a..3f39ba12f 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupView{T}.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupView{T}.cs @@ -21,12 +21,11 @@ namespace SixLabors.ImageSharp.Memory internal class MemoryGroupView : IMemoryGroup where T : struct { - private readonly Memory.MemoryGroup owner; + private MemoryGroup owner; private readonly MemoryOwnerWrapper[] memoryWrappers; - public MemoryGroupView(Memory.MemoryGroup owner) + public MemoryGroupView(MemoryGroup owner) { - this.IsValid = true; this.owner = owner; this.memoryWrappers = new MemoryOwnerWrapper[owner.Count]; @@ -36,20 +35,68 @@ namespace SixLabors.ImageSharp.Memory } } - public int Count => this.owner.Count; + public int Count + { + get + { + this.EnsureIsValid(); + return this.owner.Count; + } + } - public int BufferLength => this.owner.BufferLength; + public int BufferLength + { + get + { + this.EnsureIsValid(); + return this.owner.BufferLength; + } + } - public long TotalLength => this.owner.TotalLength; + public long TotalLength + { + get + { + this.EnsureIsValid(); + return this.owner.TotalLength; + } + } - public bool IsValid { get; internal set; } + public bool IsValid => this.owner != null; - public Memory this[int index] => throw new NotImplementedException(); + public Memory this[int index] + { + get + { + this.EnsureIsValid(); + return this.memoryWrappers[index].Memory; + } + } - public IEnumerator> GetEnumerator() => throw new NotImplementedException(); + public IEnumerator> GetEnumerator() + { + this.EnsureIsValid(); + for (int i = 0; i < this.Count; i++) + { + yield return this.memoryWrappers[i].Memory; + } + } IEnumerator IEnumerable.GetEnumerator() => this.GetEnumerator(); + internal void Invalidate() + { + this.owner = null; + } + + private void EnsureIsValid() + { + if (!this.IsValid) + { + throw new InvalidMemoryOperationException("Can not access an invalidated MemoryGroupView!"); + } + } + private class MemoryOwnerWrapper : MemoryManager { private readonly MemoryGroupView view; @@ -68,17 +115,20 @@ namespace SixLabors.ImageSharp.Memory public override Span GetSpan() { - if (!this.view.IsValid) - { - throw new InvalidOperationException(); - } - - return this.view[this.index].Span; + this.view.EnsureIsValid(); + return this.view.owner[this.index].Span; } - public override MemoryHandle Pin(int elementIndex = 0) => throw new NotImplementedException(); + public override MemoryHandle Pin(int elementIndex = 0) + { + this.view.EnsureIsValid(); + return this.view.owner[this.index].Pin(); + } - public override void Unpin() => throw new NotImplementedException(); + public override void Unpin() + { + throw new NotSupportedException(); + } } } } diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Consumed.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Consumed.cs index 4b7f8acae..b16692bd5 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Consumed.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Consumed.cs @@ -17,6 +17,7 @@ namespace SixLabors.ImageSharp.Memory : base(bufferLength, totalLength) { this.source = source; + this.View = new MemoryGroupView(this); } public override int Count => this.source.Length; @@ -33,7 +34,7 @@ namespace SixLabors.ImageSharp.Memory public override void Dispose() { - // No ownership nothing to dispose + this.View.Invalidate(); } } } diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs index e0dfc6396..6f325d0b2 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs @@ -19,15 +19,9 @@ namespace SixLabors.ImageSharp.Memory : base(bufferLength, totalLength) { this.memoryOwners = memoryOwners; + this.View = new MemoryGroupView(this); } - public override IEnumerator> GetEnumerator() - { - this.EnsureNotDisposed(); - return this.memoryOwners.Select(mo => mo.Memory).GetEnumerator(); - } - - public override int Count { get @@ -46,6 +40,12 @@ namespace SixLabors.ImageSharp.Memory } } + public override IEnumerator> GetEnumerator() + { + this.EnsureNotDisposed(); + return this.memoryOwners.Select(mo => mo.Memory).GetEnumerator(); + } + public override void Dispose() { if (this.memoryOwners == null) @@ -53,6 +53,8 @@ namespace SixLabors.ImageSharp.Memory return; } + this.View.Invalidate(); + foreach (IMemoryOwner memoryOwner in this.memoryOwners) { memoryOwner.Dispose(); @@ -69,6 +71,29 @@ namespace SixLabors.ImageSharp.Memory throw new ObjectDisposedException(nameof(MemoryGroup)); } } + + internal static void SwapContents(Owned a, Owned b) + { + a.EnsureNotDisposed(); + b.EnsureNotDisposed(); + + IMemoryOwner[] tempOwners = a.memoryOwners; + long tempTotalLength = a.TotalLength; + int tempBufferLength = a.BufferLength; + + a.memoryOwners = b.memoryOwners; + a.TotalLength = b.TotalLength; + a.BufferLength = b.BufferLength; + + b.memoryOwners = tempOwners; + b.TotalLength = tempTotalLength; + b.BufferLength = tempBufferLength; + + a.View.Invalidate(); + b.View.Invalidate(); + a.View = new MemoryGroupView(a); + b.View = new MemoryGroupView(b); + } } } } diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs index 3883a111d..072b7e3e7 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs @@ -26,41 +26,60 @@ namespace SixLabors.ImageSharp.Memory this.TotalLength = totalLength; } + /// public abstract int Count { get; } - public int BufferLength { get; } + /// + public int BufferLength { get; private set; } - public long TotalLength { get; } + /// + public long TotalLength { get; private set; } + /// public bool IsValid { get; private set; } = true; + public MemoryGroupView View { get; private set; } + + /// public abstract Memory this[int index] { get; } + /// public abstract void Dispose(); + /// public abstract IEnumerator> GetEnumerator(); + /// IEnumerator IEnumerable.GetEnumerator() => this.GetEnumerator(); - // bufferLengthAlignment == image.Width in row-major images + /// + /// Creates a new memory group, allocating it's buffers with the provided allocator. + /// + /// The to use. + /// The total length of the buffer. + /// The expected alignment (eg. to make sure image rows fit into single buffers). + /// The . + /// A new . + /// Thrown when 'blockAlignment' converted to bytes is greater than the buffer capacity of the allocator. public static MemoryGroup Allocate( MemoryAllocator allocator, long totalLength, - int blockAlignment, - AllocationOptions allocationOptions = AllocationOptions.None) + int bufferAlignment, + AllocationOptions options = AllocationOptions.None) { Guard.NotNull(allocator, nameof(allocator)); Guard.MustBeGreaterThanOrEqualTo(totalLength, 0, nameof(totalLength)); - Guard.MustBeGreaterThan(blockAlignment, 0, nameof(blockAlignment)); + Guard.MustBeGreaterThan(bufferAlignment, 0, nameof(bufferAlignment)); int blockCapacityInElements = allocator.GetBufferCapacityInBytes() / ElementSize; - if (blockAlignment > blockCapacityInElements) + if (bufferAlignment > blockCapacityInElements) { - throw new InvalidMemoryOperationException(); + throw new InvalidMemoryOperationException( + $"The buffer capacity of the provided MemoryAllocator is insufficient for the requested buffer alignment: {bufferAlignment}."); } - int numberOfAlignedSegments = blockCapacityInElements / blockAlignment; - int bufferLength = numberOfAlignedSegments * blockAlignment; + int numberOfAlignedSegments = blockCapacityInElements / bufferAlignment; + int bufferLength = numberOfAlignedSegments * bufferAlignment; if (totalLength > 0 && totalLength < bufferLength) { bufferLength = (int)totalLength; @@ -81,12 +100,12 @@ namespace SixLabors.ImageSharp.Memory var buffers = new IMemoryOwner[bufferCount]; for (int i = 0; i < buffers.Length - 1; i++) { - buffers[i] = allocator.Allocate(bufferLength, allocationOptions); + buffers[i] = allocator.Allocate(bufferLength, options); } if (bufferCount > 0) { - buffers[^1] = allocator.Allocate(sizeOfLastBuffer, allocationOptions); + buffers[^1] = allocator.Allocate(sizeOfLastBuffer, options); } return new Owned(buffers, bufferLength, totalLength); @@ -115,10 +134,27 @@ namespace SixLabors.ImageSharp.Memory return new Consumed(source, bufferLength, totalLength); } - // Analogous to current MemorySource.SwapOrCopyContent() - public static void SwapOrCopyContent(MemoryGroup destination, MemoryGroup source) + /// + /// Swaps the contents of 'target' with 'source' if the buffers are allocated (1), + /// copies the contents of 'source' to 'target' otherwise (2). + /// Groups should be of same TotalLength in case 2. + /// + public static void SwapOrCopyContent(MemoryGroup target, MemoryGroup source) { - throw new NotImplementedException(); + if (source is Owned ownedSrc && target is Owned ownedTarget) + { + Owned.SwapContents(ownedTarget, ownedSrc); + } + else + { + if (target.TotalLength != source.TotalLength) + { + throw new InvalidMemoryOperationException( + "Trying to copy/swap incompatible buffers. This is most likely caused by applying an unsupported processor to wrapped-memory images."); + } + + source.CopyTo(target); + } } } } diff --git a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs index 6e317bb8f..b9a0d2536 100644 --- a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs +++ b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs @@ -17,7 +17,7 @@ namespace SixLabors.ImageSharp.Memory /// The type of buffer items to allocate. /// The memory allocator. /// The buffer width. - /// The buffer heght. + /// The buffer height. /// The allocation options. /// The . public static Buffer2D Allocate2D( @@ -50,13 +50,13 @@ namespace SixLabors.ImageSharp.Memory Allocate2D(memoryAllocator, size.Width, size.Height, options); /// - /// Allocates padded buffers for BMP encoder/decoder. (Replacing old PixelRow/PixelArea) + /// Allocates padded buffers for BMP encoder/decoder. (Replacing old PixelRow/PixelArea). /// - /// The + /// The . /// Pixel count in the row - /// The pixel size in bytes, eg. 3 for RGB - /// The padding - /// A + /// The pixel size in bytes, eg. 3 for RGB. + /// The padding. + /// A . internal static IManagedByteBuffer AllocatePaddedPixelRowBuffer( this MemoryAllocator memoryAllocator, int width, @@ -66,5 +66,22 @@ namespace SixLabors.ImageSharp.Memory int length = (width * pixelSizeInBytes) + paddingInBytes; return memoryAllocator.AllocateManagedByteBuffer(length); } + + /// + /// Allocates a . + /// + /// The to use. + /// The total length of the buffer. + /// The expected alignment (eg. to make sure image rows fit into single buffers). + /// The . + /// A new . + /// Thrown when 'blockAlignment' converted to bytes is greater than the buffer capacity of the allocator. + internal static MemoryGroup AllocateGroup( + this MemoryAllocator memoryAllocator, + long totalLength, + int bufferAlignment, + AllocationOptions options = AllocationOptions.None) + where T : struct + => MemoryGroup.Allocate(memoryAllocator, totalLength, bufferAlignment, options); } } diff --git a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupIndex.cs b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupIndex.cs index d619aec29..88824baf2 100644 --- a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupIndex.cs +++ b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupIndex.cs @@ -112,12 +112,9 @@ namespace SixLabors.ImageSharp.Tests.Memory.DiscontiguousBuffers public static MemoryGroupIndex MaxIndex(this MemoryGroup group) where T : struct { - if (group.Count == 0) - { - return new MemoryGroupIndex(group.BufferLength, 0, 0); - } - - return new MemoryGroupIndex(group.BufferLength, group.Count - 1, group[^1].Length - 1); + return group.Count == 0 + ? new MemoryGroupIndex(group.BufferLength, 0, 0) + : new MemoryGroupIndex(group.BufferLength, group.Count - 1, group[^1].Length); } } } diff --git a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs index 0c96f3d78..972f6cb26 100644 --- a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs +++ b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. +using System; using System.Collections.Generic; using System.Linq; using SixLabors.ImageSharp.Memory; diff --git a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.SwapOrCopyContent.cs b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.SwapOrCopyContent.cs new file mode 100644 index 000000000..b3a522cc6 --- /dev/null +++ b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.SwapOrCopyContent.cs @@ -0,0 +1,105 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + +using System; +using SixLabors.ImageSharp.Memory; +using SixLabors.ImageSharp.PixelFormats; +using Xunit; + +namespace SixLabors.ImageSharp.Tests.Memory.DiscontiguousBuffers +{ + public partial class MemoryGroupTests + { + public class SwapOrCopyContent : MemoryGroupTestsBase + { + [Fact] + public void WhenBothAreMemoryOwners_ShouldSwap() + { + this.MemoryAllocator.BufferCapacityInBytes = sizeof(int) * 50; + using MemoryGroup a = this.MemoryAllocator.AllocateGroup(100, 50); + using MemoryGroup b = this.MemoryAllocator.AllocateGroup(120, 50); + + Memory a0 = a[0]; + Memory a1 = a[1]; + Memory b0 = b[0]; + Memory b1 = b[1]; + + MemoryGroup.SwapOrCopyContent(a, b); + + Assert.Equal(b0, a[0]); + Assert.Equal(b1, a[1]); + Assert.Equal(a0, b[0]); + Assert.Equal(a1, b[1]); + Assert.NotEqual(a[0], b[0]); + } + + [Fact] + public void WhenBothAreMemoryOwners_ShouldReplaceViews() + { + using MemoryGroup a = this.MemoryAllocator.AllocateGroup(100, 100); + using MemoryGroup b = this.MemoryAllocator.AllocateGroup(120, 100); + + a[0].Span[42] = 1; + b[0].Span[33] = 2; + MemoryGroupView aView0 = a.View; + MemoryGroupView bView0 = b.View; + + MemoryGroup.SwapOrCopyContent(a, b); + Assert.False(aView0.IsValid); + Assert.False(bView0.IsValid); + Assert.ThrowsAny(() => _ = aView0[0].Span); + Assert.ThrowsAny(() => _ = bView0[0].Span); + + Assert.True(a.View.IsValid); + Assert.True(b.View.IsValid); + Assert.Equal(2, a.View[0].Span[33]); + Assert.Equal(1, b.View[0].Span[42]); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void WhenDestIsNotAllocated_SameSize_ShouldCopy(bool sourceIsAllocated) + { + var data = new Rgba32[21]; + var color = new Rgba32(1, 2, 3, 4); + + using var destOwner = new TestMemoryManager(data); + using var dest = MemoryGroup.Wrap(destOwner.Memory); + + using MemoryGroup source = this.MemoryAllocator.AllocateGroup(21, 30); + + source[0].Span[10] = color; + + // Act: + MemoryGroup.SwapOrCopyContent(dest, source); + + // Assert: + Assert.Equal(color, dest[0].Span[10]); + Assert.NotEqual(source[0], dest[0]); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void WhenDestIsNotMemoryOwner_DifferentSize_Throws(bool sourceIsOwner) + { + var data = new Rgba32[21]; + var color = new Rgba32(1, 2, 3, 4); + + using var destOwner = new TestMemoryManager(data); + var dest = MemoryGroup.Wrap(destOwner.Memory); + + using MemoryGroup source = this.MemoryAllocator.AllocateGroup(22, 30); + + source[0].Span[10] = color; + + // Act: + Assert.ThrowsAny(() => MemoryGroup.SwapOrCopyContent(dest, source)); + + Assert.Equal(color, source[0].Span[10]); + Assert.NotEqual(color, dest[0].Span[10]); + } + } + } +} diff --git a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.View.cs b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.View.cs new file mode 100644 index 000000000..8884037a5 --- /dev/null +++ b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.View.cs @@ -0,0 +1,84 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + +using System; +using SixLabors.ImageSharp.Memory; +using Xunit; + +namespace SixLabors.ImageSharp.Tests.Memory.DiscontiguousBuffers +{ + public partial class MemoryGroupTests + { + public class View : MemoryGroupTestsBase + { + [Fact] + public void RefersToOwnerGroupContent() + { + using MemoryGroup group = this.CreateTestGroup(240, 80, true); + + MemoryGroupView view = group.View; + Assert.True(view.IsValid); + Assert.Equal(group.Count, view.Count); + Assert.Equal(group.BufferLength, view.BufferLength); + Assert.Equal(group.TotalLength, view.TotalLength); + int cnt = 1; + foreach (Memory memory in view) + { + Span span = memory.Span; + foreach (int t in span) + { + Assert.Equal(cnt, t); + cnt++; + } + } + } + + [Fact] + public void IsInvalidatedOnOwnerGroupDispose() + { + MemoryGroupView view; + using (MemoryGroup group = this.CreateTestGroup(240, 80, true)) + { + view = group.View; + } + + Assert.False(view.IsValid); + + Assert.ThrowsAny(() => + { + _ = view.Count; + }); + + Assert.ThrowsAny(() => + { + _ = view.BufferLength; + }); + + Assert.ThrowsAny(() => + { + _ = view.TotalLength; + }); + + Assert.ThrowsAny(() => + { + _ = view[0]; + }); + } + + [Fact] + public void WhenInvalid_CanNotUseMemberMemory() + { + Memory memory; + using (MemoryGroup group = this.CreateTestGroup(240, 80, true)) + { + memory = group.View[0]; + } + + Assert.ThrowsAny(() => + { + _ = memory.Span; + }); + } + } + } +} diff --git a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.cs b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.cs index f9f725fb4..6b0737742 100644 --- a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.cs +++ b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. using System; +using System.Buffers; using System.Runtime.InteropServices; using SixLabors.ImageSharp.Memory; using Xunit; @@ -103,8 +104,6 @@ namespace SixLabors.ImageSharp.Tests.Memory.DiscontiguousBuffers Assert.True(b == 2 * a, $"Mismatch @ {pos} Expected: {a} Actual: {b}"); } - - } [Fact] @@ -117,7 +116,6 @@ namespace SixLabors.ImageSharp.Tests.Memory.DiscontiguousBuffers } } - [Theory] [InlineData(100, 5)] [InlineData(100, 101)] @@ -136,6 +134,26 @@ namespace SixLabors.ImageSharp.Tests.Memory.DiscontiguousBuffers } } + [Fact] + public void Wrap() + { + int[] data0 = { 1, 2, 3, 4 }; + int[] data1 = { 5, 6, 7, 8 }; + int[] data2 = { 9, 10 }; + using var mgr0 = new TestMemoryManager(data0); + using var mgr1 = new TestMemoryManager(data1); + + using var group = MemoryGroup.Wrap(mgr0.Memory, mgr1.Memory, data2); + + Assert.Equal(3, group.Count); + Assert.Equal(4, group.BufferLength); + Assert.Equal(10, group.TotalLength); + + Assert.True(group[0].Span.SequenceEqual(data0)); + Assert.True(group[1].Span.SequenceEqual(data1)); + Assert.True(group[2].Span.SequenceEqual(data2)); + } + private static void MultiplyAllBy2(ReadOnlySpan source, Span target) { Assert.Equal(source.Length, target.Length); diff --git a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTestsBase.cs b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTestsBase.cs index acd24e343..8dd28653c 100644 --- a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTestsBase.cs +++ b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTestsBase.cs @@ -9,6 +9,9 @@ namespace SixLabors.ImageSharp.Tests.Memory.DiscontiguousBuffers { internal readonly TestMemoryAllocator MemoryAllocator = new TestMemoryAllocator(); + /// + /// Create a group, either uninitialized or filled with incrementing numbers starting with 1. + /// internal MemoryGroup CreateTestGroup(long totalLength, int bufferLength, bool fillSequence = false) { this.MemoryAllocator.BufferCapacityInBytes = bufferLength * sizeof(int);