moved SwapOrCopyContent responsibility entirely to Buffer2D

src/ImageSharp/Memory/Buffer2D{T}.cs

@@ -56,7 +56,7 @@ namespace SixLabors.ImageSharp.Memory
         /// It's public counterpart is <see cref="MemoryGroup"/>,
         /// which only exposes the view of the MemoryGroup.
         /// </remarks>
-        internal MemoryGroup<T> FastMemoryGroup { get; }
+        internal MemoryGroup<T> FastMemoryGroup { get; private set; }
 
         /// <summary>
         /// 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!
         /// </summary>
-        internal static void SwapOrCopyContent(Buffer2D<T> destination, Buffer2D<T> source)
+        internal static bool SwapOrCopyContent(Buffer2D<T> destination, Buffer2D<T> source)
         {
-            MemoryGroup<T>.SwapOrCopyContent(destination.FastMemoryGroup, source.FastMemoryGroup);
+            bool swapped = false;
-            SwapOwnData(destination, source);
+            if (MemoryGroup<T>.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<T> GetRowMemoryCore(int y) => this.FastMemoryGroup.GetBoundedSlice(y * (long)this.Width, this.Width);
-
-        private static void SwapOwnData(Buffer2D<T> a, Buffer2D<T> 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;
-        }
     }
 }

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<T>(this);
+            }
+
             /// <inheritdoc/>
             IEnumerator<Memory<T>> IEnumerable<Memory<T>>.GetEnumerator()
             {

src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs

@@ -241,30 +241,11 @@ namespace SixLabors.ImageSharp.Memory
             return new Owned(source, bufferLength, totalLength, false);
         }
 
-        /// <summary>
+        public static bool CanSwapContent(MemoryGroup<T> target, MemoryGroup<T> source) =>
-        /// Swaps the contents of 'target' with 'source' if the buffers are allocated (1),
+            source is Owned { Swappable: true } && target is Owned { Swappable: true };
-        /// copies the contents of 'source' to 'target' otherwise (2).
-        /// Groups should be of same TotalLength in case 2.
-        /// </summary>
-        public static bool SwapOrCopyContent(MemoryGroup<T> target, MemoryGroup<T> 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.");
-                }
 
-                source.CopyTo(target);
+        public virtual void RecreateViewAfterSwap()
-                return false;
+        {
-            }
         }
 
         public virtual void IncreaseRefCounts()

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<int> a = this.MemoryAllocator.Allocate2D<int>(10, 5, AllocationOptions.Clean))
+                using (Buffer2D<int> b = this.MemoryAllocator.Allocate2D<int>(3, 7, AllocationOptions.Clean))
+                {
+                    a[1, 3] = 666;
+                    b[1, 3] = 444;
+
+                    Memory<int> aa = a.FastMemoryGroup.Single();
+                    Memory<int> bb = b.FastMemoryGroup.Single();
+
+                    Buffer2D<int>.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<int>.Wrap(new int[100]);
+                using var dest = new Buffer2D<int>(destData, 10, 10);
+
+                using (Buffer2D<int> source = this.MemoryAllocator.Allocate2D<int>(10, 10, AllocationOptions.Clean))
+                {
+                    source[0, 0] = 1;
+                    dest[0, 0] = 2;
+
+                    Buffer2D<int>.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<int> a = this.MemoryAllocator.Allocate2D<int>(48, 2);
+                using Buffer2D<int> b = this.MemoryAllocator.Allocate2D<int>(50, 2);
+
+                Memory<int> a0 = a.FastMemoryGroup[0];
+                Memory<int> a1 = a.FastMemoryGroup[1];
+                Memory<int> b0 = b.FastMemoryGroup[0];
+                Memory<int> b1 = b.FastMemoryGroup[1];
+
+                bool swap = Buffer2D<int>.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<int> a = this.MemoryAllocator.Allocate2D<int>(100, 1);
+                using Buffer2D<int> b = this.MemoryAllocator.Allocate2D<int>(100, 2);
+
+                a.FastMemoryGroup[0].Span[42] = 1;
+                b.FastMemoryGroup[0].Span[33] = 2;
+                MemoryGroupView<int> aView0 = (MemoryGroupView<int>)a.MemoryGroup;
+                MemoryGroupView<int> bView0 = (MemoryGroupView<int>)b.MemoryGroup;
+
+                Buffer2D<int>.SwapOrCopyContent(a, b);
+                Assert.False(aView0.IsValid);
+                Assert.False(bView0.IsValid);
+                Assert.ThrowsAny<InvalidOperationException>(() => _ = aView0[0].Span);
+                Assert.ThrowsAny<InvalidOperationException>(() => _ = 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<Rgba32>(data);
+                using var dest = new Buffer2D<Rgba32>(MemoryGroup<Rgba32>.Wrap(destOwner.Memory), 21, 1);
+
+                using Buffer2D<Rgba32> source = this.MemoryAllocator.Allocate2D<Rgba32>(21, 1);
+
+                source.FastMemoryGroup[0].Span[10] = color;
+
+                // Act:
+                bool swap = Buffer2D<Rgba32>.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<Rgba32>(data);
+                using var dest = new Buffer2D<Rgba32>(MemoryGroup<Rgba32>.Wrap(destOwner.Memory), 21, 1);
+
+                using Buffer2D<Rgba32> source = this.MemoryAllocator.Allocate2D<Rgba32>(22, 1);
+
+                source.FastMemoryGroup[0].Span[10] = color;
+
+                // Act:
+                Assert.ThrowsAny<InvalidOperationException>(() => Buffer2D<Rgba32>.SwapOrCopyContent(dest, source));
+
+                Assert.Equal(color, source.MemoryGroup[0].Span[10]);
+                Assert.NotEqual(color, dest.MemoryGroup[0].Span[10]);
+            }
+        }
+    }
+}

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<int> a = this.MemoryAllocator.Allocate2D<int>(10, 5, AllocationOptions.Clean))
-            using (Buffer2D<int> b = this.MemoryAllocator.Allocate2D<int>(3, 7, AllocationOptions.Clean))
-            {
-                a[1, 3] = 666;
-                b[1, 3] = 444;
-
-                Memory<int> aa = a.FastMemoryGroup.Single();
-                Memory<int> bb = b.FastMemoryGroup.Single();
-
-                Buffer2D<int>.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<int>.Wrap(new int[100]);
-            using var dest = new Buffer2D<int>(destData, 10, 10);
-
-            using (Buffer2D<int> source = this.MemoryAllocator.Allocate2D<int>(10, 10, AllocationOptions.Clean))
-            {
-                source[0, 0] = 1;
-                dest[0, 0] = 2;
-
-                Buffer2D<int>.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)]

tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.SwapOrCopyContent.cs

@@ -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<int> a = this.MemoryAllocator.AllocateGroup<int>(100, 50);
-                using MemoryGroup<int> b = this.MemoryAllocator.AllocateGroup<int>(120, 50);
-
-                Memory<int> a0 = a[0];
-                Memory<int> a1 = a[1];
-                Memory<int> b0 = b[0];
-                Memory<int> b1 = b[1];
-
-                bool swap = MemoryGroup<int>.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<int> a = this.MemoryAllocator.AllocateGroup<int>(100, 100);
-                using MemoryGroup<int> b = this.MemoryAllocator.AllocateGroup<int>(120, 100);
-
-                a[0].Span[42] = 1;
-                b[0].Span[33] = 2;
-                MemoryGroupView<int> aView0 = a.View;
-                MemoryGroupView<int> bView0 = b.View;
-
-                MemoryGroup<int>.SwapOrCopyContent(a, b);
-                Assert.False(aView0.IsValid);
-                Assert.False(bView0.IsValid);
-                Assert.ThrowsAny<InvalidOperationException>(() => _ = aView0[0].Span);
-                Assert.ThrowsAny<InvalidOperationException>(() => _ = 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<Rgba32>(data);
-                using var dest = MemoryGroup<Rgba32>.Wrap(destOwner.Memory);
-
-                using MemoryGroup<Rgba32> source = this.MemoryAllocator.AllocateGroup<Rgba32>(21, 30);
-
-                source[0].Span[10] = color;
-
-                // Act:
-                bool swap = MemoryGroup<Rgba32>.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<Rgba32>(data);
-                var dest = MemoryGroup<Rgba32>.Wrap(destOwner.Memory);
-
-                using MemoryGroup<Rgba32> source = this.MemoryAllocator.AllocateGroup<Rgba32>(22, 30);
-
-                source[0].Span[10] = color;
-
-                // Act:
-                Assert.ThrowsAny<InvalidOperationException>(() => MemoryGroup<Rgba32>.SwapOrCopyContent(dest, source));
-
-                Assert.Equal(color, source[0].Span[10]);
-                Assert.NotEqual(color, dest[0].Span[10]);
-            }
-        }
-    }
-}