diff --git a/src/ImageSharp/Memory/Buffer2D{T}.cs b/src/ImageSharp/Memory/Buffer2D{T}.cs index ba39a924ea..42dce2def5 100644 --- a/src/ImageSharp/Memory/Buffer2D{T}.cs +++ b/src/ImageSharp/Memory/Buffer2D{T}.cs @@ -56,7 +56,7 @@ namespace SixLabors.ImageSharp.Memory /// It's public counterpart is , /// which only exposes the view of the MemoryGroup. /// - internal MemoryGroup FastMemoryGroup { get; } + internal MemoryGroup FastMemoryGroup { get; private set; } /// /// Gets a reference to the element at the specified position. @@ -168,25 +168,34 @@ namespace SixLabors.ImageSharp.Memory /// Swaps the contents of 'destination' with 'source' if the buffers are owned (1), /// copies the contents of 'source' to 'destination' otherwise (2). Buffers should be of same size in case 2! /// - internal static void SwapOrCopyContent(Buffer2D destination, Buffer2D source) + internal static bool SwapOrCopyContent(Buffer2D destination, Buffer2D source) { - MemoryGroup.SwapOrCopyContent(destination.FastMemoryGroup, source.FastMemoryGroup); - SwapOwnData(destination, source); + bool swapped = false; + if (MemoryGroup.CanSwapContent(destination.FastMemoryGroup, source.FastMemoryGroup)) + { + (destination.FastMemoryGroup, source.FastMemoryGroup) = + (source.FastMemoryGroup, destination.FastMemoryGroup); + destination.FastMemoryGroup.RecreateViewAfterSwap(); + source.FastMemoryGroup.RecreateViewAfterSwap(); + swapped = true; + } + else + { + if (destination.FastMemoryGroup.TotalLength != source.FastMemoryGroup.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.FastMemoryGroup.CopyTo(destination.MemoryGroup); + } + + (destination.Width, source.Width) = (source.Width, destination.Width); + (destination.Height, source.Height) = (source.Height, destination.Height); + return swapped; } [MethodImpl(InliningOptions.ShortMethod)] private Memory GetRowMemoryCore(int y) => this.FastMemoryGroup.GetBoundedSlice(y * (long)this.Width, this.Width); - - private static void SwapOwnData(Buffer2D a, Buffer2D b) - { - Size aSize = a.Size(); - Size bSize = b.Size(); - - b.Width = aSize.Width; - b.Height = aSize.Height; - - a.Width = bSize.Width; - a.Height = bSize.Height; - } } } diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs index 3b92413833..b578b417f9 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs @@ -122,6 +122,12 @@ namespace SixLabors.ImageSharp.Memory } } + public override void RecreateViewAfterSwap() + { + this.View.Invalidate(); + this.View = new MemoryGroupView(this); + } + /// IEnumerator> IEnumerable>.GetEnumerator() { diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs index cdd8e6a758..1f18d91692 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs @@ -241,30 +241,11 @@ namespace SixLabors.ImageSharp.Memory return new Owned(source, bufferLength, totalLength, false); } - /// - /// 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 bool SwapOrCopyContent(MemoryGroup target, MemoryGroup source) - { - if (source is Owned ownedSrc && ownedSrc.Swappable && - target is Owned ownedTarget && ownedTarget.Swappable) - { - Owned.SwapContents(ownedTarget, ownedSrc); - return true; - } - 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."); - } + public static bool CanSwapContent(MemoryGroup target, MemoryGroup source) => + source is Owned { Swappable: true } && target is Owned { Swappable: true }; - source.CopyTo(target); - return false; - } + public virtual void RecreateViewAfterSwap() + { } public virtual void IncreaseRefCounts() diff --git a/tests/ImageSharp.Tests/Memory/Buffer2DTests.SwapOrCopyContent.cs b/tests/ImageSharp.Tests/Memory/Buffer2DTests.SwapOrCopyContent.cs new file mode 100644 index 0000000000..f58136f738 --- /dev/null +++ b/tests/ImageSharp.Tests/Memory/Buffer2DTests.SwapOrCopyContent.cs @@ -0,0 +1,160 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using System; +using System.Linq; +using SixLabors.ImageSharp.Memory; +using SixLabors.ImageSharp.PixelFormats; +using Xunit; + +namespace SixLabors.ImageSharp.Tests.Memory +{ + public partial class Buffer2DTests + { + public class SwapOrCopyContent + { + private readonly TestMemoryAllocator MemoryAllocator = new TestMemoryAllocator(); + + [Fact] + public void SwapOrCopyContent_WhenBothAllocated() + { + using (Buffer2D a = this.MemoryAllocator.Allocate2D(10, 5, AllocationOptions.Clean)) + using (Buffer2D b = this.MemoryAllocator.Allocate2D(3, 7, AllocationOptions.Clean)) + { + a[1, 3] = 666; + b[1, 3] = 444; + + Memory aa = a.FastMemoryGroup.Single(); + Memory bb = b.FastMemoryGroup.Single(); + + Buffer2D.SwapOrCopyContent(a, b); + + Assert.Equal(bb, a.FastMemoryGroup.Single()); + Assert.Equal(aa, b.FastMemoryGroup.Single()); + + Assert.Equal(new Size(3, 7), a.Size()); + Assert.Equal(new Size(10, 5), b.Size()); + + Assert.Equal(666, b[1, 3]); + Assert.Equal(444, a[1, 3]); + } + } + + [Fact] + public void SwapOrCopyContent_WhenDestinationIsOwned_ShouldNotSwapInDisposedSourceBuffer() + { + using var destData = MemoryGroup.Wrap(new int[100]); + using var dest = new Buffer2D(destData, 10, 10); + + using (Buffer2D source = this.MemoryAllocator.Allocate2D(10, 10, AllocationOptions.Clean)) + { + source[0, 0] = 1; + dest[0, 0] = 2; + + Buffer2D.SwapOrCopyContent(dest, source); + } + + int actual1 = dest.DangerousGetRowSpan(0)[0]; + int actual2 = dest.DangerousGetRowSpan(0)[0]; + int actual3 = dest.GetSafeRowMemory(0).Span[0]; + int actual5 = dest[0, 0]; + + Assert.Equal(1, actual1); + Assert.Equal(1, actual2); + Assert.Equal(1, actual3); + Assert.Equal(1, actual5); + } + + [Fact] + public void WhenBothAreMemoryOwners_ShouldSwap() + { + this.MemoryAllocator.BufferCapacityInBytes = sizeof(int) * 50; + using Buffer2D a = this.MemoryAllocator.Allocate2D(48, 2); + using Buffer2D b = this.MemoryAllocator.Allocate2D(50, 2); + + Memory a0 = a.FastMemoryGroup[0]; + Memory a1 = a.FastMemoryGroup[1]; + Memory b0 = b.FastMemoryGroup[0]; + Memory b1 = b.FastMemoryGroup[1]; + + bool swap = Buffer2D.SwapOrCopyContent(a, b); + Assert.True(swap); + + Assert.Equal(b0, a.FastMemoryGroup[0]); + Assert.Equal(b1, a.FastMemoryGroup[1]); + Assert.Equal(a0, b.FastMemoryGroup[0]); + Assert.Equal(a1, b.FastMemoryGroup[1]); + Assert.NotEqual(a.FastMemoryGroup[0], b.FastMemoryGroup[0]); + } + + [Fact] + public void WhenBothAreMemoryOwners_ShouldReplaceViews() + { + using Buffer2D a = this.MemoryAllocator.Allocate2D(100, 1); + using Buffer2D b = this.MemoryAllocator.Allocate2D(100, 2); + + a.FastMemoryGroup[0].Span[42] = 1; + b.FastMemoryGroup[0].Span[33] = 2; + MemoryGroupView aView0 = (MemoryGroupView)a.MemoryGroup; + MemoryGroupView bView0 = (MemoryGroupView)b.MemoryGroup; + + Buffer2D.SwapOrCopyContent(a, b); + Assert.False(aView0.IsValid); + Assert.False(bView0.IsValid); + Assert.ThrowsAny(() => _ = aView0[0].Span); + Assert.ThrowsAny(() => _ = bView0[0].Span); + + Assert.True(a.MemoryGroup.IsValid); + Assert.True(b.MemoryGroup.IsValid); + Assert.Equal(2, a.MemoryGroup[0].Span[33]); + Assert.Equal(1, b.MemoryGroup[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 = new Buffer2D(MemoryGroup.Wrap(destOwner.Memory), 21, 1); + + using Buffer2D source = this.MemoryAllocator.Allocate2D(21, 1); + + source.FastMemoryGroup[0].Span[10] = color; + + // Act: + bool swap = Buffer2D.SwapOrCopyContent(dest, source); + + // Assert: + Assert.False(swap); + Assert.Equal(color, dest.MemoryGroup[0].Span[10]); + Assert.NotEqual(source.FastMemoryGroup[0], dest.FastMemoryGroup[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); + using var dest = new Buffer2D(MemoryGroup.Wrap(destOwner.Memory), 21, 1); + + using Buffer2D source = this.MemoryAllocator.Allocate2D(22, 1); + + source.FastMemoryGroup[0].Span[10] = color; + + // Act: + Assert.ThrowsAny(() => Buffer2D.SwapOrCopyContent(dest, source)); + + Assert.Equal(color, source.MemoryGroup[0].Span[10]); + Assert.NotEqual(color, dest.MemoryGroup[0].Span[10]); + } + } + } +} diff --git a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs index 73a0f4d60e..72b4ccadf0 100644 --- a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs +++ b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs @@ -4,7 +4,6 @@ using System; using System.Buffers; using System.Diagnostics; -using System.Linq; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using SixLabors.ImageSharp.Memory; @@ -14,7 +13,7 @@ using Xunit; // ReSharper disable InconsistentNaming namespace SixLabors.ImageSharp.Tests.Memory { - public class Buffer2DTests + public partial class Buffer2DTests { // ReSharper disable once ClassNeverInstantiated.Local private class Assert : Xunit.Assert @@ -229,56 +228,6 @@ namespace SixLabors.ImageSharp.Tests.Memory } } - [Fact] - public void SwapOrCopyContent_WhenBothAllocated() - { - using (Buffer2D a = this.MemoryAllocator.Allocate2D(10, 5, AllocationOptions.Clean)) - using (Buffer2D b = this.MemoryAllocator.Allocate2D(3, 7, AllocationOptions.Clean)) - { - a[1, 3] = 666; - b[1, 3] = 444; - - Memory aa = a.FastMemoryGroup.Single(); - Memory bb = b.FastMemoryGroup.Single(); - - Buffer2D.SwapOrCopyContent(a, b); - - Assert.Equal(bb, a.FastMemoryGroup.Single()); - Assert.Equal(aa, b.FastMemoryGroup.Single()); - - Assert.Equal(new Size(3, 7), a.Size()); - Assert.Equal(new Size(10, 5), b.Size()); - - Assert.Equal(666, b[1, 3]); - Assert.Equal(444, a[1, 3]); - } - } - - [Fact] - public void SwapOrCopyContent_WhenDestinationIsOwned_ShouldNotSwapInDisposedSourceBuffer() - { - using var destData = MemoryGroup.Wrap(new int[100]); - using var dest = new Buffer2D(destData, 10, 10); - - using (Buffer2D source = this.MemoryAllocator.Allocate2D(10, 10, AllocationOptions.Clean)) - { - source[0, 0] = 1; - dest[0, 0] = 2; - - Buffer2D.SwapOrCopyContent(dest, source); - } - - int actual1 = dest.DangerousGetRowSpan(0)[0]; - int actual2 = dest.DangerousGetRowSpan(0)[0]; - int actual3 = dest.GetSafeRowMemory(0).Span[0]; - int actual5 = dest[0, 0]; - - Assert.Equal(1, actual1); - Assert.Equal(1, actual2); - Assert.Equal(1, actual3); - Assert.Equal(1, actual5); - } - [Theory] [InlineData(100, 20, 0, 90, 10)] [InlineData(100, 3, 0, 50, 50)] diff --git a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.SwapOrCopyContent.cs b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.SwapOrCopyContent.cs deleted file mode 100644 index 61b9f7a895..0000000000 --- a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.SwapOrCopyContent.cs +++ /dev/null @@ -1,107 +0,0 @@ -// Copyright (c) Six Labors. -// 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]; - - bool swap = MemoryGroup.SwapOrCopyContent(a, b); - - Assert.True(swap); - 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: - bool swap = MemoryGroup.SwapOrCopyContent(dest, source); - - // Assert: - Assert.False(swap); - 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]); - } - } - } -}