Browse Source

implement review suggestions

af/octree-no-pixelmap
Anton Firszov 6 years ago
parent
commit
7d8fbf6df8
  1. 2
      src/ImageSharp/Advanced/AdvancedImageExtensions.cs
  2. 2
      src/ImageSharp/Image.Decode.cs
  3. 2
      src/ImageSharp/Image.LoadPixelData.cs
  4. 2
      src/ImageSharp/ImageFrame.LoadPixelData.cs
  5. 2
      src/ImageSharp/ImageFrameCollection{TPixel}.cs
  6. 10
      src/ImageSharp/ImageFrame{TPixel}.cs
  7. 10
      src/ImageSharp/Memory/Buffer2DExtensions.cs
  8. 27
      src/ImageSharp/Memory/Buffer2D{T}.cs
  9. 4
      src/ImageSharp/Memory/BufferArea{T}.cs
  10. 2
      src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Consumed.cs
  11. 6
      src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs
  12. 39
      tests/ImageSharp.Tests/Memory/Buffer2DTests.cs
  13. 2
      tests/ImageSharp.Tests/TestUtilities/ImageProviders/SolidProvider.cs
  14. 2
      tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs

2
src/ImageSharp/Advanced/AdvancedImageExtensions.cs

@ -61,7 +61,7 @@ namespace SixLabors.ImageSharp.Advanced
/// </remarks> /// </remarks>
public static IMemoryGroup<TPixel> GetPixelMemoryGroup<TPixel>(this ImageFrame<TPixel> source) public static IMemoryGroup<TPixel> GetPixelMemoryGroup<TPixel>(this ImageFrame<TPixel> source)
where TPixel : struct, IPixel<TPixel> where TPixel : struct, IPixel<TPixel>
=> source?.PixelBuffer.MemoryGroup.View ?? throw new ArgumentNullException(nameof(source)); => source?.PixelBuffer.FastMemoryGroup.View ?? throw new ArgumentNullException(nameof(source));
/// <summary> /// <summary>
/// Gets the representation of the pixels as a <see cref="IMemoryGroup{T}"/> containing the backing pixel data of the image /// Gets the representation of the pixels as a <see cref="IMemoryGroup{T}"/> containing the backing pixel data of the image

2
src/ImageSharp/Image.Decode.cs

@ -36,7 +36,7 @@ namespace SixLabors.ImageSharp
{ {
Buffer2D<TPixel> uninitializedMemoryBuffer = Buffer2D<TPixel> uninitializedMemoryBuffer =
configuration.MemoryAllocator.Allocate2D<TPixel>(width, height); configuration.MemoryAllocator.Allocate2D<TPixel>(width, height);
return new Image<TPixel>(configuration, uninitializedMemoryBuffer.MemoryGroup, width, height, metadata); return new Image<TPixel>(configuration, uninitializedMemoryBuffer.FastMemoryGroup, width, height, metadata);
} }
/// <summary> /// <summary>

2
src/ImageSharp/Image.LoadPixelData.cs

@ -120,7 +120,7 @@ namespace SixLabors.ImageSharp
var image = new Image<TPixel>(config, width, height); var image = new Image<TPixel>(config, width, height);
data = data.Slice(0, count); data = data.Slice(0, count);
data.CopyTo(image.Frames.RootFrame.PixelBuffer.MemoryGroup); data.CopyTo(image.Frames.RootFrame.PixelBuffer.FastMemoryGroup);
return image; return image;
} }

2
src/ImageSharp/ImageFrame.LoadPixelData.cs

@ -45,7 +45,7 @@ namespace SixLabors.ImageSharp
var image = new ImageFrame<TPixel>(configuration, width, height); var image = new ImageFrame<TPixel>(configuration, width, height);
data = data.Slice(0, count); data = data.Slice(0, count);
data.CopyTo(image.PixelBuffer.MemoryGroup); data.CopyTo(image.PixelBuffer.FastMemoryGroup);
return image; return image;
} }

2
src/ImageSharp/ImageFrameCollection{TPixel}.cs

@ -351,7 +351,7 @@ namespace SixLabors.ImageSharp
this.parent.GetConfiguration(), this.parent.GetConfiguration(),
source.Size(), source.Size(),
source.Metadata.DeepClone()); source.Metadata.DeepClone());
source.CopyPixelsTo(result.PixelBuffer.MemoryGroup); source.CopyPixelsTo(result.PixelBuffer.FastMemoryGroup);
return result; return result;
} }
} }

10
src/ImageSharp/ImageFrame{TPixel}.cs

@ -131,7 +131,7 @@ namespace SixLabors.ImageSharp
Guard.NotNull(source, nameof(source)); Guard.NotNull(source, nameof(source));
this.PixelBuffer = this.GetConfiguration().MemoryAllocator.Allocate2D<TPixel>(source.PixelBuffer.Width, source.PixelBuffer.Height); this.PixelBuffer = this.GetConfiguration().MemoryAllocator.Allocate2D<TPixel>(source.PixelBuffer.Width, source.PixelBuffer.Height);
source.PixelBuffer.MemoryGroup.CopyTo(this.PixelBuffer.MemoryGroup); source.PixelBuffer.FastMemoryGroup.CopyTo(this.PixelBuffer.FastMemoryGroup);
} }
/// <summary> /// <summary>
@ -186,7 +186,7 @@ namespace SixLabors.ImageSharp
throw new ArgumentException("ImageFrame<TPixel>.CopyTo(): target must be of the same size!", nameof(target)); throw new ArgumentException("ImageFrame<TPixel>.CopyTo(): target must be of the same size!", nameof(target));
} }
this.PixelBuffer.MemoryGroup.CopyTo(target.MemoryGroup); this.PixelBuffer.FastMemoryGroup.CopyTo(target.FastMemoryGroup);
} }
/// <summary> /// <summary>
@ -222,7 +222,7 @@ namespace SixLabors.ImageSharp
{ {
if (typeof(TPixel) == typeof(TDestinationPixel)) if (typeof(TPixel) == typeof(TDestinationPixel))
{ {
this.PixelBuffer.MemoryGroup.TransformTo(destination, (s, d) => this.PixelBuffer.FastMemoryGroup.TransformTo(destination, (s, d) =>
{ {
Span<TPixel> d1 = MemoryMarshal.Cast<TDestinationPixel, TPixel>(d); Span<TPixel> d1 = MemoryMarshal.Cast<TDestinationPixel, TPixel>(d);
s.CopyTo(d1); s.CopyTo(d1);
@ -230,7 +230,7 @@ namespace SixLabors.ImageSharp
return; return;
} }
this.PixelBuffer.MemoryGroup.TransformTo(destination, (s, d) => this.PixelBuffer.FastMemoryGroup.TransformTo(destination, (s, d) =>
{ {
PixelOperations<TPixel>.Instance.To(this.GetConfiguration(), s, d); PixelOperations<TPixel>.Instance.To(this.GetConfiguration(), s, d);
}); });
@ -291,7 +291,7 @@ namespace SixLabors.ImageSharp
/// <param name="value">The value to initialize the bitmap with.</param> /// <param name="value">The value to initialize the bitmap with.</param>
internal void Clear(TPixel value) internal void Clear(TPixel value)
{ {
MemoryGroup<TPixel> group = this.PixelBuffer.MemoryGroup; MemoryGroup<TPixel> group = this.PixelBuffer.FastMemoryGroup;
if (value.Equals(default)) if (value.Equals(default))
{ {

10
src/ImageSharp/Memory/Buffer2DExtensions.cs

@ -24,7 +24,7 @@ namespace SixLabors.ImageSharp.Memory
where T : struct where T : struct
{ {
Guard.NotNull(buffer, nameof(buffer)); Guard.NotNull(buffer, nameof(buffer));
return buffer.MemoryGroup.View; return buffer.FastMemoryGroup.View;
} }
/// <summary> /// <summary>
@ -42,12 +42,12 @@ namespace SixLabors.ImageSharp.Memory
where T : struct where T : struct
{ {
Guard.NotNull(buffer, nameof(buffer)); Guard.NotNull(buffer, nameof(buffer));
if (buffer.MemoryGroup.Count > 1) if (buffer.FastMemoryGroup.Count > 1)
{ {
throw new InvalidOperationException("GetSingleSpan is only valid for a single-buffer group!"); throw new InvalidOperationException("GetSingleSpan is only valid for a single-buffer group!");
} }
return buffer.MemoryGroup.Single().Span; return buffer.FastMemoryGroup.Single().Span;
} }
/// <summary> /// <summary>
@ -65,12 +65,12 @@ namespace SixLabors.ImageSharp.Memory
where T : struct where T : struct
{ {
Guard.NotNull(buffer, nameof(buffer)); Guard.NotNull(buffer, nameof(buffer));
if (buffer.MemoryGroup.Count > 1) if (buffer.FastMemoryGroup.Count > 1)
{ {
throw new InvalidOperationException("GetSingleMemory is only valid for a single-buffer group!"); throw new InvalidOperationException("GetSingleMemory is only valid for a single-buffer group!");
} }
return buffer.MemoryGroup.Single(); return buffer.FastMemoryGroup.Single();
} }
/// <summary> /// <summary>

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

@ -28,7 +28,7 @@ namespace SixLabors.ImageSharp.Memory
/// <param name="height">The number of rows.</param> /// <param name="height">The number of rows.</param>
internal Buffer2D(MemoryGroup<T> memoryGroup, int width, int height) internal Buffer2D(MemoryGroup<T> memoryGroup, int width, int height)
{ {
this.MemoryGroup = memoryGroup; this.FastMemoryGroup = memoryGroup;
this.Width = width; this.Width = width;
this.Height = height; this.Height = height;
@ -49,14 +49,20 @@ namespace SixLabors.ImageSharp.Memory
public int Height { get; private set; } public int Height { get; private set; }
/// <summary> /// <summary>
/// Gets the backing <see cref="MemoryGroup{T}"/>. /// Gets the backing <see cref="IMemoryGroup{T}"/>.
/// </summary>
/// <returns>The MemoryGroup.</returns>
public IMemoryGroup<T> MemoryGroup => this.FastMemoryGroup.View;
/// <summary>
/// Gets the backing <see cref="MemoryGroup{T}"/> without the view abstraction.
/// </summary> /// </summary>
/// <remarks> /// <remarks>
/// This property has been kept internal intentionally. /// This property has been kept internal intentionally.
/// It's public counterpart is <see cref="Buffer2DExtensions.GetMemoryGroup{T}"/>, /// It's public counterpart is <see cref="MemoryGroup"/>,
/// which only exposes the view of the MemoryGroup. /// which only exposes the view of the MemoryGroup.
/// </remarks> /// </remarks>
internal MemoryGroup<T> MemoryGroup { get; } internal MemoryGroup<T> FastMemoryGroup { get; }
/// <summary> /// <summary>
/// Gets a reference to the element at the specified position. /// Gets a reference to the element at the specified position.
@ -64,9 +70,10 @@ namespace SixLabors.ImageSharp.Memory
/// <param name="x">The x coordinate (row)</param> /// <param name="x">The x coordinate (row)</param>
/// <param name="y">The y coordinate (position at row)</param> /// <param name="y">The y coordinate (position at row)</param>
/// <returns>A reference to the element.</returns> /// <returns>A reference to the element.</returns>
internal ref T this[int x, int y] /// <exception cref="IndexOutOfRangeException">When index is out of range of the buffer.</exception>
public ref T this[int x, int y]
{ {
[MethodImpl(MethodImplOptions.AggressiveInlining)] [MethodImpl(InliningOptions.ShortMethod)]
get get
{ {
DebugGuard.MustBeGreaterThanOrEqualTo(x, 0, nameof(x)); DebugGuard.MustBeGreaterThanOrEqualTo(x, 0, nameof(x));
@ -83,7 +90,7 @@ namespace SixLabors.ImageSharp.Memory
/// </summary> /// </summary>
public void Dispose() public void Dispose()
{ {
this.MemoryGroup.Dispose(); this.FastMemoryGroup.Dispose();
this.cachedMemory = default; this.cachedMemory = default;
} }
@ -148,7 +155,7 @@ namespace SixLabors.ImageSharp.Memory
{ {
DebugGuard.MustBeGreaterThanOrEqualTo(y, 0, nameof(y)); DebugGuard.MustBeGreaterThanOrEqualTo(y, 0, nameof(y));
DebugGuard.MustBeLessThan(y, this.Height, nameof(y)); DebugGuard.MustBeLessThan(y, this.Height, nameof(y));
return this.MemoryGroup.View.GetBoundedSlice(y * this.Width, this.Width); return this.FastMemoryGroup.View.GetBoundedSlice(y * this.Width, this.Width);
} }
/// <summary> /// <summary>
@ -157,12 +164,12 @@ namespace SixLabors.ImageSharp.Memory
/// </summary> /// </summary>
internal static void SwapOrCopyContent(Buffer2D<T> destination, Buffer2D<T> source) internal static void SwapOrCopyContent(Buffer2D<T> destination, Buffer2D<T> source)
{ {
bool swap = MemoryGroup<T>.SwapOrCopyContent(destination.MemoryGroup, source.MemoryGroup); bool swap = MemoryGroup<T>.SwapOrCopyContent(destination.FastMemoryGroup, source.FastMemoryGroup);
SwapOwnData(destination, source, swap); SwapOwnData(destination, source, swap);
} }
[MethodImpl(InliningOptions.ColdPath)] [MethodImpl(InliningOptions.ColdPath)]
private Memory<T> GetRowMemorySlow(int y) => this.MemoryGroup.GetBoundedSlice(y * this.Width, this.Width); private Memory<T> GetRowMemorySlow(int y) => this.FastMemoryGroup.GetBoundedSlice(y * this.Width, this.Width);
[MethodImpl(InliningOptions.ColdPath)] [MethodImpl(InliningOptions.ColdPath)]
private ref T GetElementSlow(int x, int y) private ref T GetElementSlow(int x, int y)

4
src/ImageSharp/Memory/BufferArea{T}.cs

@ -94,7 +94,7 @@ namespace SixLabors.ImageSharp.Memory
int xx = this.Rectangle.X; int xx = this.Rectangle.X;
int width = this.Rectangle.Width; int width = this.Rectangle.Width;
return this.DestinationBuffer.MemoryGroup.GetBoundedSlice(yy + xx, width).Span; return this.DestinationBuffer.FastMemoryGroup.GetBoundedSlice(yy + xx, width).Span;
} }
/// <summary> /// <summary>
@ -140,7 +140,7 @@ namespace SixLabors.ImageSharp.Memory
// Optimization for when the size of the area is the same as the buffer size. // Optimization for when the size of the area is the same as the buffer size.
if (this.IsFullBufferArea) if (this.IsFullBufferArea)
{ {
this.DestinationBuffer.MemoryGroup.Clear(); this.DestinationBuffer.FastMemoryGroup.Clear();
return; return;
} }

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

@ -11,7 +11,7 @@ namespace SixLabors.ImageSharp.Memory
internal abstract partial class MemoryGroup<T> internal abstract partial class MemoryGroup<T>
{ {
// Analogous to the "consumed" variant of MemorySource // Analogous to the "consumed" variant of MemorySource
private class Consumed : MemoryGroup<T> private sealed class Consumed : MemoryGroup<T>
{ {
private readonly Memory<T>[] source; private readonly Memory<T>[] source;

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

@ -11,7 +11,7 @@ namespace SixLabors.ImageSharp.Memory
// Analogous to the "owned" variant of MemorySource // Analogous to the "owned" variant of MemorySource
internal abstract partial class MemoryGroup<T> internal abstract partial class MemoryGroup<T>
{ {
private class Owned : MemoryGroup<T> private sealed class Owned : MemoryGroup<T>
{ {
private IMemoryOwner<T>[] memoryOwners; private IMemoryOwner<T>[] memoryOwners;
@ -25,6 +25,8 @@ namespace SixLabors.ImageSharp.Memory
public bool Swappable { get; } public bool Swappable { get; }
private bool IsDisposed => this.memoryOwners == null;
public override int Count public override int Count
{ {
get get
@ -51,7 +53,7 @@ namespace SixLabors.ImageSharp.Memory
public override void Dispose() public override void Dispose()
{ {
if (this.memoryOwners == null) if (this.IsDisposed)
{ {
return; return;
} }

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

@ -46,8 +46,8 @@ namespace SixLabors.ImageSharp.Tests.Memory
{ {
Assert.Equal(width, buffer.Width); Assert.Equal(width, buffer.Width);
Assert.Equal(height, buffer.Height); Assert.Equal(height, buffer.Height);
Assert.Equal(width * height, buffer.MemoryGroup.TotalLength); Assert.Equal(width * height, buffer.FastMemoryGroup.TotalLength);
Assert.True(buffer.MemoryGroup.BufferLength % width == 0); Assert.True(buffer.FastMemoryGroup.BufferLength % width == 0);
} }
} }
@ -64,7 +64,7 @@ namespace SixLabors.ImageSharp.Tests.Memory
{ {
Assert.Equal(width, buffer.Width); Assert.Equal(width, buffer.Width);
Assert.Equal(height, buffer.Height); Assert.Equal(height, buffer.Height);
Assert.Equal(0, buffer.MemoryGroup.TotalLength); Assert.Equal(0, buffer.FastMemoryGroup.TotalLength);
Assert.Equal(0, buffer.GetSingleSpan().Length); Assert.Equal(0, buffer.GetSingleSpan().Length);
} }
} }
@ -76,7 +76,7 @@ namespace SixLabors.ImageSharp.Tests.Memory
this.MemoryAllocator.BufferCapacityInBytes = sizeof(int) * bufferCapacity; this.MemoryAllocator.BufferCapacityInBytes = sizeof(int) * bufferCapacity;
using Buffer2D<int> buffer = this.MemoryAllocator.Allocate2DOveraligned<int>(width, height, alignmentMultiplier); using Buffer2D<int> buffer = this.MemoryAllocator.Allocate2DOveraligned<int>(width, height, alignmentMultiplier);
MemoryGroup<int> memoryGroup = buffer.MemoryGroup; MemoryGroup<int> memoryGroup = buffer.FastMemoryGroup;
int expectedAlignment = width * alignmentMultiplier; int expectedAlignment = width * alignmentMultiplier;
Assert.Equal(expectedAlignment, memoryGroup.BufferLength); Assert.Equal(expectedAlignment, memoryGroup.BufferLength);
@ -113,8 +113,8 @@ namespace SixLabors.ImageSharp.Tests.Memory
Assert.Equal(width, span.Length); Assert.Equal(width, span.Length);
int expectedSubBufferOffset = (width * y) - (expectedBufferIndex * buffer.MemoryGroup.BufferLength); int expectedSubBufferOffset = (width * y) - (expectedBufferIndex * buffer.FastMemoryGroup.BufferLength);
Assert.SpanPointsTo(span, buffer.MemoryGroup[expectedBufferIndex], expectedSubBufferOffset); Assert.SpanPointsTo(span, buffer.FastMemoryGroup[expectedBufferIndex], expectedSubBufferOffset);
} }
} }
@ -172,10 +172,10 @@ namespace SixLabors.ImageSharp.Tests.Memory
using (Buffer2D<TestStructs.Foo> buffer = this.MemoryAllocator.Allocate2D<TestStructs.Foo>(width, height)) using (Buffer2D<TestStructs.Foo> buffer = this.MemoryAllocator.Allocate2D<TestStructs.Foo>(width, height))
{ {
int bufferIndex = (width * y) / buffer.MemoryGroup.BufferLength; int bufferIndex = (width * y) / buffer.FastMemoryGroup.BufferLength;
int subBufferStart = (width * y) - (bufferIndex * buffer.MemoryGroup.BufferLength); int subBufferStart = (width * y) - (bufferIndex * buffer.FastMemoryGroup.BufferLength);
Span<TestStructs.Foo> span = buffer.MemoryGroup[bufferIndex].Span.Slice(subBufferStart); Span<TestStructs.Foo> span = buffer.FastMemoryGroup[bufferIndex].Span.Slice(subBufferStart);
ref TestStructs.Foo actual = ref buffer[x, y]; ref TestStructs.Foo actual = ref buffer[x, y];
@ -194,13 +194,13 @@ namespace SixLabors.ImageSharp.Tests.Memory
a[1, 3] = 666; a[1, 3] = 666;
b[1, 3] = 444; b[1, 3] = 444;
Memory<int> aa = a.MemoryGroup.Single(); Memory<int> aa = a.FastMemoryGroup.Single();
Memory<int> bb = b.MemoryGroup.Single(); Memory<int> bb = b.FastMemoryGroup.Single();
Buffer2D<int>.SwapOrCopyContent(a, b); Buffer2D<int>.SwapOrCopyContent(a, b);
Assert.Equal(bb, a.MemoryGroup.Single()); Assert.Equal(bb, a.FastMemoryGroup.Single());
Assert.Equal(aa, b.MemoryGroup.Single()); Assert.Equal(aa, b.FastMemoryGroup.Single());
Assert.Equal(new Size(3, 7), a.Size()); Assert.Equal(new Size(3, 7), a.Size());
Assert.Equal(new Size(10, 5), b.Size()); Assert.Equal(new Size(10, 5), b.Size());
@ -287,5 +287,18 @@ namespace SixLabors.ImageSharp.Tests.Memory
} }
} }
} }
[Fact]
public void PublicMemoryGroup_IsMemoryGroupView()
{
using Buffer2D<int> buffer1 = this.MemoryAllocator.Allocate2D<int>(10, 10);
using Buffer2D<int> buffer2 = this.MemoryAllocator.Allocate2D<int>(10, 10);
IMemoryGroup<int> mgBefore = buffer1.MemoryGroup;
Buffer2D<int>.SwapOrCopyContent(buffer1, buffer2);
Assert.False(mgBefore.IsValid);
Assert.NotSame(mgBefore, buffer1.MemoryGroup);
}
} }
} }

2
tests/ImageSharp.Tests/TestUtilities/ImageProviders/SolidProvider.cs

@ -54,7 +54,7 @@ namespace SixLabors.ImageSharp.Tests
Image<TPixel> image = base.GetImage(); Image<TPixel> image = base.GetImage();
Color color = new Rgba32(this.r, this.g, this.b, this.a); Color color = new Rgba32(this.r, this.g, this.b, this.a);
image.GetRootFramePixelBuffer().MemoryGroup.Fill(color.ToPixel<TPixel>()); image.GetRootFramePixelBuffer().FastMemoryGroup.Fill(color.ToPixel<TPixel>());
return image; return image;
} }

2
tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs

@ -54,7 +54,7 @@ namespace SixLabors.ImageSharp.Tests.TestUtilities.ReferenceCodecs
{ {
using var magickImage = new MagickImage(stream); using var magickImage = new MagickImage(stream);
var result = new Image<TPixel>(configuration, magickImage.Width, magickImage.Height); var result = new Image<TPixel>(configuration, magickImage.Width, magickImage.Height);
MemoryGroup<TPixel> resultPixels = result.GetRootFramePixelBuffer().MemoryGroup; MemoryGroup<TPixel> resultPixels = result.GetRootFramePixelBuffer().FastMemoryGroup;
using (IPixelCollection pixels = magickImage.GetPixelsUnsafe()) using (IPixelCollection pixels = magickImage.GetPixelsUnsafe())
{ {

Loading…
Cancel
Save