Browse Source

fix bug found by DetectEdges_WorksOnWrappedMemoryImage

pull/1109/head
Anton Firszov 6 years ago
parent
commit
334f16bf7c
  1. 10
      src/ImageSharp/Memory/Allocators/ArrayPoolMemoryAllocator.Buffer{T}.cs
  2. 15
      src/ImageSharp/Memory/Buffer2D{T}.cs
  3. 4
      src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs
  4. 29
      tests/ImageSharp.Tests/Memory/Buffer2DTests.cs
  5. 6
      tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.SwapOrCopyContent.cs

10
src/ImageSharp/Memory/Allocators/ArrayPoolMemoryAllocator.Buffer{T}.cs

@ -46,7 +46,15 @@ namespace SixLabors.ImageSharp.Memory
protected byte[] Data { get; private set; }
/// <inheritdoc />
public override Span<T> GetSpan() => MemoryMarshal.Cast<byte, T>(this.Data.AsSpan()).Slice(0, this.length);
public override Span<T> GetSpan()
{
if (this.Data == null)
{
throw new ObjectDisposedException("ArrayPoolMemoryAllocator.Buffer<T>");
}
return MemoryMarshal.Cast<byte, T>(this.Data.AsSpan()).Slice(0, this.length);
}
/// <inheritdoc />
protected override void Dispose(bool disposing)

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

@ -149,14 +149,14 @@ namespace SixLabors.ImageSharp.Memory
/// </summary>
internal static void SwapOrCopyContent(Buffer2D<T> destination, Buffer2D<T> source)
{
MemoryGroup<T>.SwapOrCopyContent(destination.MemoryGroup, source.MemoryGroup);
SwapOwnData(destination, source);
bool swap = MemoryGroup<T>.SwapOrCopyContent(destination.MemoryGroup, source.MemoryGroup);
SwapOwnData(destination, source, swap);
}
[MethodImpl(InliningOptions.ColdPath)]
private Memory<T> GetRowMemorySlow(int y) => this.MemoryGroup.GetBoundedSlice(y * this.Width, this.Width);
private static void SwapOwnData(Buffer2D<T> a, Buffer2D<T> b)
private static void SwapOwnData(Buffer2D<T> a, Buffer2D<T> b, bool swapCachedMemory)
{
Size aSize = a.Size();
Size bSize = b.Size();
@ -167,9 +167,12 @@ namespace SixLabors.ImageSharp.Memory
a.Width = bSize.Width;
a.Height = bSize.Height;
Memory<T> aCached = a.cachedMemory;
a.cachedMemory = b.cachedMemory;
b.cachedMemory = aCached;
if (swapCachedMemory)
{
Memory<T> aCached = a.cachedMemory;
a.cachedMemory = b.cachedMemory;
b.cachedMemory = aCached;
}
}
}
}

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

@ -166,12 +166,13 @@ namespace SixLabors.ImageSharp.Memory
/// copies the contents of 'source' to 'target' otherwise (2).
/// Groups should be of same TotalLength in case 2.
/// </summary>
public static void SwapOrCopyContent(MemoryGroup<T> target, MemoryGroup<T> source)
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
{
@ -182,6 +183,7 @@ namespace SixLabors.ImageSharp.Memory
}
source.CopyTo(target);
return false;
}
}
}

29
tests/ImageSharp.Tests/Memory/Buffer2DTests.cs

@ -130,7 +130,7 @@ namespace SixLabors.ImageSharp.Tests.Memory
}
[Fact]
public void SwapOrCopyContent()
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))
@ -154,6 +154,33 @@ namespace SixLabors.ImageSharp.Tests.Memory
}
}
[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.GetRowSpanUnchecked(0)[0];
int actual2 = dest.GetRowSpan(0)[0];
int actual3 = dest.GetRowMemorySafe(0).Span[0];
int actual4 = dest.GetRowMemoryFast(0).Span[0];
int actual5 = dest[0, 0];
Assert.Equal(1, actual1);
Assert.Equal(1, actual2);
Assert.Equal(1, actual3);
Assert.Equal(1, actual4);
Assert.Equal(1, actual5);
}
[Theory]
[InlineData(100, 20, 0, 90, 10)]
[InlineData(100, 3, 0, 50, 50)]

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

@ -24,8 +24,9 @@ namespace SixLabors.ImageSharp.Tests.Memory.DiscontiguousBuffers
Memory<int> b0 = b[0];
Memory<int> b1 = b[1];
MemoryGroup<int>.SwapOrCopyContent(a, b);
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]);
@ -72,9 +73,10 @@ namespace SixLabors.ImageSharp.Tests.Memory.DiscontiguousBuffers
source[0].Span[10] = color;
// Act:
MemoryGroup<Rgba32>.SwapOrCopyContent(dest, source);
bool swap = MemoryGroup<Rgba32>.SwapOrCopyContent(dest, source);
// Assert:
Assert.False(swap);
Assert.Equal(color, dest[0].Span[10]);
Assert.NotEqual(source[0], dest[0]);
}

Loading…
Cancel
Save