From e2a80fbd541cda814cde9d7f122dc8b119bb1dea Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 11 Aug 2020 17:39:36 +0100 Subject: [PATCH 01/37] Add GitHub Sponsors (#1311) [skip ci] * Add Github Sponsors and new tier system * Skip CI for trivial things. * Smarter ignore * Try a different skip approach [skip ci] * Fix spacing [skip-ci] * Try again [skip-ci] * Use different query [skip-ci] * Try moving after the matrix --- .github/FUNDING.yml | 3 +- .github/workflows/build-and-test.yml | 2 +- README.md | 109 ++++++++++----------------- 3 files changed, 41 insertions(+), 73 deletions(-) diff --git a/.github/FUNDING.yml b/.github/FUNDING.yml index ee7a10862e..ac30c6f1e2 100644 --- a/.github/FUNDING.yml +++ b/.github/FUNDING.yml @@ -1 +1,2 @@ -open_collective: imagesharp \ No newline at end of file +github: SixLabors +open_collective: sixlabors \ No newline at end of file diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 1e4aaaa85c..a83e194234 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -9,7 +9,6 @@ on: pull_request: branches: - master - jobs: Build: strategy: @@ -37,6 +36,7 @@ jobs: codecov: false runs-on: ${{matrix.options.os}} + if: "!contains(github.event.head_commit.message, '[skip ci]')" steps: - uses: actions/checkout@v2 diff --git a/README.md b/README.md index ae9161f949..adf9647cc2 100644 --- a/README.md +++ b/README.md @@ -1,11 +1,10 @@ -

+

SixLabors.ImageSharp
SixLabors.ImageSharp

-
[![Build Status](https://img.shields.io/github/workflow/status/SixLabors/ImageSharp/Build/master)](https://github.com/SixLabors/ImageSharp/actions) @@ -95,72 +94,40 @@ Please... Spread the word, contribute algorithms, submit performance improvement - [Scott Williams](https://github.com/tocsoft) - [Brian Popow](https://github.com/brianpopow) -### Backers - -Support us with a monthly donation and help us continue our activities. [[Become a backer](https://opencollective.com/imagesharp#backer)] - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -### Sponsors - -Become a sponsor and get your logo on our README on Github with a link to your site. [[Become a sponsor](https://opencollective.com/imagesharp#sponsor)] - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +## Sponsor Six Labors + +Support the efforts of the development of the Six Labors projects. [[Become a sponsor :heart:](https://opencollective.com/sixlabors#sponsor)] + +### Platinum Sponsors +Become a platinum sponsor with a monthly donation of $2000 (providing 32 hours of maintenance and development) and get 2 hours of dedicated support (remote support available through chat or screen-sharing) per month. + +In addition you get your logo (large) on our README on GitHub and the home page (large) of sixlabors.com + + + +### Gold Sponsors +Become a gold sponsor with a monthly donation of $1000 (providing 16 hours of maintenance and development) and get 1 hour of dedicated support (remote support available through chat or screen-sharing) per month. + +In addition you get your logo (large) on our README on GitHub and the home page (medium) of sixlabors.com + + + +### Silver Sponsors +Become a silver sponsor with a monthly donation of $500 (providing 8 hours of maintenance and development) and get your logo (medium) on our README on GitHub and the product pages of sixlabors.com + + + +### Bronze Sponsors +Become a bronze sponsor with a monthly donation of $100 and get your logo (small) on our README on GitHub. + + + + + + + + + + + + \ No newline at end of file From b61366ac005a9e7b08731a1a4db16e161bd4bb3d Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 13 Aug 2020 15:49:45 +0200 Subject: [PATCH 02/37] Add ByteMemoryManager type --- src/ImageSharp/Memory/ByteMemoryManager{T}.cs | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 src/ImageSharp/Memory/ByteMemoryManager{T}.cs diff --git a/src/ImageSharp/Memory/ByteMemoryManager{T}.cs b/src/ImageSharp/Memory/ByteMemoryManager{T}.cs new file mode 100644 index 0000000000..3a9eb34c20 --- /dev/null +++ b/src/ImageSharp/Memory/ByteMemoryManager{T}.cs @@ -0,0 +1,67 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. +using System; +using System.Buffers; +using System.Runtime.InteropServices; + +namespace SixLabors.ImageSharp.Memory +{ + /// + /// A custom that can wrap of instances + /// and cast them to be for any arbitrary unmanaged value type. + /// + /// The value type to use when casting the wrapped instance. + internal sealed class ByteMemoryManager : MemoryManager + where T : unmanaged + { + /// + /// The wrapped of instance. + /// + private readonly Memory memory; + + /// + /// Initializes a new instance of the class. + /// + /// The of instance to wrap. + public ByteMemoryManager(Memory memory) + { + this.memory = memory; + } + + /// + protected override void Dispose(bool disposing) + { + } + + /// + public override Span GetSpan() + { + if (MemoryMarshal.TryGetArray(this.memory, out ArraySegment arraySegment)) + { + return MemoryMarshal.Cast(arraySegment.AsSpan()); + } + + if (MemoryMarshal.TryGetMemoryManager>(this.memory, out MemoryManager memoryManager)) + { + return MemoryMarshal.Cast(memoryManager.GetSpan()); + } + + // This should never be reached, as Memory can currently only be wrapping + // either a byte[] array or a MemoryManager instance in this case. + ThrowHelper.ThrowArgumentException("The input Memory instance was not valid.", nameof(this.memory)); + + return default; + } + + /// + public override MemoryHandle Pin(int elementIndex = 0) + { + return this.memory.Pin(); + } + + /// + public override void Unpin() + { + } + } +} From cd394109840e0a823b65cd569349279c235bf210 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 13 Aug 2020 15:56:24 +0200 Subject: [PATCH 03/37] Add Image.WrapMemory from Memory --- src/ImageSharp/Image.WrapMemory.cs | 65 ++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/src/ImageSharp/Image.WrapMemory.cs b/src/ImageSharp/Image.WrapMemory.cs index 2d3c29ed42..34bdeafdcb 100644 --- a/src/ImageSharp/Image.WrapMemory.cs +++ b/src/ImageSharp/Image.WrapMemory.cs @@ -150,5 +150,70 @@ namespace SixLabors.ImageSharp int height) where TPixel : unmanaged, IPixel => WrapMemory(Configuration.Default, pixelMemoryOwner, width, height); + + /// + /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, + /// allowing to view/manipulate it as an ImageSharp instance. + /// + /// The pixel type + /// The + /// The byte memory representing the pixel data. + /// The width of the memory image. + /// The height of the memory image. + /// The . + /// The configuration is null. + /// The metadata is null. + /// An instance + public static Image WrapMemory( + Configuration configuration, + Memory byteMemory, + int width, + int height, + ImageMetadata metadata) + where TPixel : unmanaged, IPixel + { + Guard.NotNull(configuration, nameof(configuration)); + Guard.NotNull(metadata, nameof(metadata)); + + var memoryManager = new ByteMemoryManager(byteMemory); + var memorySource = MemoryGroup.Wrap(memoryManager.Memory); + return new Image(configuration, memorySource, width, height, metadata); + } + + /// + /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, + /// allowing to view/manipulate it as an ImageSharp instance. + /// + /// The pixel type + /// The + /// The byte memory representing the pixel data. + /// The width of the memory image. + /// The height of the memory image. + /// The configuration is null. + /// An instance. + public static Image WrapMemory( + Configuration configuration, + Memory byteMemory, + int width, + int height) + where TPixel : unmanaged, IPixel + => WrapMemory(configuration, byteMemory, width, height, new ImageMetadata()); + + /// + /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, + /// allowing to view/manipulate it as an ImageSharp instance. + /// The memory is being observed, the caller remains responsible for managing it's lifecycle. + /// + /// The pixel type. + /// The byte memory representing the pixel data. + /// The width of the memory image. + /// The height of the memory image. + /// An instance. + public static Image WrapMemory( + Memory byteMemory, + int width, + int height) + where TPixel : unmanaged, IPixel + => WrapMemory(Configuration.Default, byteMemory, width, height); } } From 129977e4654553bdd1b0fde12a16cb8e8093c3d3 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 13 Aug 2020 16:12:42 +0200 Subject: [PATCH 04/37] Add tests for new Image.WrapMemory APIs --- .../Image/ImageTests.WrapMemory.cs | 107 +++++++++++++++++- 1 file changed, 106 insertions(+), 1 deletion(-) diff --git a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs index 2b30d9459f..c0cd3f56a7 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs @@ -6,10 +6,10 @@ using System.Buffers; using System.Drawing; using System.Drawing.Imaging; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.Common.Helpers; -using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.PixelFormats; using Xunit; @@ -80,6 +80,52 @@ namespace SixLabors.ImageSharp.Tests } } + public sealed class CastMemoryManager : MemoryManager + where TFrom : unmanaged + where TTo : unmanaged + { + private readonly Memory memory; + + public CastMemoryManager(Memory memory) + { + this.memory = memory; + } + + /// + protected override void Dispose(bool disposing) + { + } + + /// + public override Span GetSpan() + { + if (MemoryMarshal.TryGetArray(this.memory, out ArraySegment arraySegment)) + { + return MemoryMarshal.Cast(arraySegment.AsSpan()); + } + + if (MemoryMarshal.TryGetMemoryManager>(this.memory, out MemoryManager memoryManager)) + { + return MemoryMarshal.Cast(memoryManager.GetSpan()); + } + + ThrowHelper.ThrowArgumentException("The input Memory instance was not valid.", nameof(this.memory)); + + return default; + } + + /// + public override MemoryHandle Pin(int elementIndex = 0) + { + return this.memory.Pin(); + } + + /// + public override void Unpin() + { + } + } + [Fact] public void WrapMemory_CreatedImageIsCorrect() { @@ -173,6 +219,65 @@ namespace SixLabors.ImageSharp.Tests } } + [Fact] + public void WrapMemory_FromBytes_CreatedImageIsCorrect() + { + Configuration cfg = Configuration.Default.Clone(); + var metaData = new ImageMetadata(); + + var array = new byte[25 * Unsafe.SizeOf()]; + var memory = new Memory(array); + + using (var image = Image.WrapMemory(cfg, memory, 5, 5, metaData)) + { + Assert.True(image.TryGetSinglePixelSpan(out Span imageSpan)); + ref Rgba32 pixel0 = ref imageSpan[0]; + Assert.True(Unsafe.AreSame(ref Unsafe.As(ref array[0]), ref pixel0)); + + Assert.Equal(cfg, image.GetConfiguration()); + Assert.Equal(metaData, image.Metadata); + } + } + + [Fact] + public void WrapSystemDrawingBitmap_FromBytes_WhenObserved() + { + if (ShouldSkipBitmapTest) + { + return; + } + + using (var bmp = new Bitmap(51, 23)) + { + using (var memoryManager = new BitmapMemoryManager(bmp)) + { + Memory pixelMemory = memoryManager.Memory; + Memory byteMemory = new CastMemoryManager(pixelMemory).Memory; + Bgra32 bg = Color.Red; + Bgra32 fg = Color.Green; + + using (var image = Image.WrapMemory(byteMemory, bmp.Width, bmp.Height)) + { + Assert.Equal(pixelMemory, image.GetRootFramePixelBuffer().GetSingleMemory()); + Assert.True(image.TryGetSinglePixelSpan(out Span imageSpan)); + imageSpan.Fill(bg); + for (var i = 10; i < 20; i++) + { + image.GetPixelRowSpan(i).Slice(10, 10).Fill(fg); + } + } + + Assert.False(memoryManager.IsDisposed); + } + + string fn = System.IO.Path.Combine( + TestEnvironment.ActualOutputDirectoryFullPath, + $"{nameof(this.WrapSystemDrawingBitmap_WhenObserved)}.bmp"); + + bmp.Save(fn, ImageFormat.Bmp); + } + } + private static bool ShouldSkipBitmapTest => !TestEnvironment.Is64BitProcess || (TestHelpers.ImageSharpBuiltAgainst != "netcoreapp3.1" && TestHelpers.ImageSharpBuiltAgainst != "netcoreapp2.1"); } From 8f4417cf61c92cf400d50f6b4bb05022be7882b6 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 13 Aug 2020 16:30:27 +0200 Subject: [PATCH 05/37] Remove unnecessary indirection in ByteMemoryManager --- src/ImageSharp/Memory/ByteMemoryManager{T}.cs | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/src/ImageSharp/Memory/ByteMemoryManager{T}.cs b/src/ImageSharp/Memory/ByteMemoryManager{T}.cs index 3a9eb34c20..924230fc85 100644 --- a/src/ImageSharp/Memory/ByteMemoryManager{T}.cs +++ b/src/ImageSharp/Memory/ByteMemoryManager{T}.cs @@ -36,21 +36,7 @@ namespace SixLabors.ImageSharp.Memory /// public override Span GetSpan() { - if (MemoryMarshal.TryGetArray(this.memory, out ArraySegment arraySegment)) - { - return MemoryMarshal.Cast(arraySegment.AsSpan()); - } - - if (MemoryMarshal.TryGetMemoryManager>(this.memory, out MemoryManager memoryManager)) - { - return MemoryMarshal.Cast(memoryManager.GetSpan()); - } - - // This should never be reached, as Memory can currently only be wrapping - // either a byte[] array or a MemoryManager instance in this case. - ThrowHelper.ThrowArgumentException("The input Memory instance was not valid.", nameof(this.memory)); - - return default; + return MemoryMarshal.Cast(this.memory.Span); } /// From b9dc71d37534810e03120dc94c26bb8dccdf2e85 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 13 Aug 2020 16:46:05 +0200 Subject: [PATCH 06/37] Fix bug in Pin() method, minor code tweaks --- src/ImageSharp/Memory/ByteMemoryManager{T}.cs | 2 +- .../Image/ImageTests.WrapMemory.cs | 16 ++-------------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/ImageSharp/Memory/ByteMemoryManager{T}.cs b/src/ImageSharp/Memory/ByteMemoryManager{T}.cs index 924230fc85..70d1b1c305 100644 --- a/src/ImageSharp/Memory/ByteMemoryManager{T}.cs +++ b/src/ImageSharp/Memory/ByteMemoryManager{T}.cs @@ -42,7 +42,7 @@ namespace SixLabors.ImageSharp.Memory /// public override MemoryHandle Pin(int elementIndex = 0) { - return this.memory.Pin(); + return this.memory.Slice(elementIndex).Pin(); } /// diff --git a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs index c0cd3f56a7..06ee069c67 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs @@ -99,25 +99,13 @@ namespace SixLabors.ImageSharp.Tests /// public override Span GetSpan() { - if (MemoryMarshal.TryGetArray(this.memory, out ArraySegment arraySegment)) - { - return MemoryMarshal.Cast(arraySegment.AsSpan()); - } - - if (MemoryMarshal.TryGetMemoryManager>(this.memory, out MemoryManager memoryManager)) - { - return MemoryMarshal.Cast(memoryManager.GetSpan()); - } - - ThrowHelper.ThrowArgumentException("The input Memory instance was not valid.", nameof(this.memory)); - - return default; + return MemoryMarshal.Cast(this.memory.Span); } /// public override MemoryHandle Pin(int elementIndex = 0) { - return this.memory.Pin(); + return this.memory.Slice(elementIndex).Pin(); } /// From 77a6d08a463265897bc1239de6d1413cbc7ead10 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 13 Aug 2020 17:15:16 +0200 Subject: [PATCH 07/37] Fix relative offsetting in Pin() methods --- src/ImageSharp/Memory/ByteMemoryManager{T}.cs | 6 +++++- tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs | 10 +++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Memory/ByteMemoryManager{T}.cs b/src/ImageSharp/Memory/ByteMemoryManager{T}.cs index 70d1b1c305..223709df65 100644 --- a/src/ImageSharp/Memory/ByteMemoryManager{T}.cs +++ b/src/ImageSharp/Memory/ByteMemoryManager{T}.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. using System; using System.Buffers; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; namespace SixLabors.ImageSharp.Memory @@ -42,7 +43,10 @@ namespace SixLabors.ImageSharp.Memory /// public override MemoryHandle Pin(int elementIndex = 0) { - return this.memory.Slice(elementIndex).Pin(); + // We need to adjust the offset into the wrapped byte segment, + // as the input index refers to the target-cast memory of T. + // We just have to shift this index by the byte size of T. + return this.memory.Slice(elementIndex * Unsafe.SizeOf()).Pin(); } /// diff --git a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs index 06ee069c67..ee8e4b97af 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs @@ -105,7 +105,15 @@ namespace SixLabors.ImageSharp.Tests /// public override MemoryHandle Pin(int elementIndex = 0) { - return this.memory.Slice(elementIndex).Pin(); + int byteOffset = elementIndex * Unsafe.SizeOf(); + int shiftedOffset = Math.DivRem(byteOffset, Unsafe.SizeOf(), out int remainder); + + if (remainder != 0) + { + ThrowHelper.ThrowArgumentException("The input index doesn't result in an aligned item access", nameof(elementIndex)); + } + + return this.memory.Slice(shiftedOffset).Pin(); } /// From 9642d0e14c9f6398abdb0299e5ea16d94bc0e57c Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 13 Aug 2020 17:45:38 +0200 Subject: [PATCH 08/37] Fix comparison in wrapping bytes test --- .../ImageSharp.Tests/Image/ImageTests.WrapMemory.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs index ee8e4b97af..9e1ebbf851 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs @@ -254,8 +254,16 @@ namespace SixLabors.ImageSharp.Tests using (var image = Image.WrapMemory(byteMemory, bmp.Width, bmp.Height)) { - Assert.Equal(pixelMemory, image.GetRootFramePixelBuffer().GetSingleMemory()); - Assert.True(image.TryGetSinglePixelSpan(out Span imageSpan)); + Span pixelSpan = pixelMemory.Span; + Span imageSpan = image.GetRootFramePixelBuffer().GetSingleMemory().Span; + + // We can't compare the two Memory instances directly as they wrap different memory managers. + // To check that the underlying data matches, we can just manually check their lenth, and the + // fact that a reference to the first pixel in both spans is actually the same memory location. + Assert.Equal(pixelSpan.Length, imageSpan.Length); + Assert.True(Unsafe.AreSame(ref pixelSpan.GetPinnableReference(), ref imageSpan.GetPinnableReference())); + + Assert.True(image.TryGetSinglePixelSpan(out imageSpan)); imageSpan.Fill(bg); for (var i = 10; i < 20; i++) { From 0e5ddabb7cd65392315331648a26fe34ac0d90de Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 13 Aug 2020 19:31:04 +0200 Subject: [PATCH 09/37] Replace Default.Clone with CreateDefaultConfiguration --- tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs index 9e1ebbf851..bb22f59a3a 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs @@ -125,7 +125,7 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void WrapMemory_CreatedImageIsCorrect() { - Configuration cfg = Configuration.Default.Clone(); + var cfg = Configuration.CreateDefaultInstance(); var metaData = new ImageMetadata(); var array = new Rgba32[25]; @@ -218,7 +218,7 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void WrapMemory_FromBytes_CreatedImageIsCorrect() { - Configuration cfg = Configuration.Default.Clone(); + var cfg = Configuration.CreateDefaultInstance(); var metaData = new ImageMetadata(); var array = new byte[25 * Unsafe.SizeOf()]; From 5f58bbcba5a8a3684da0bfac4cc07178a0805dfe Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 16 Aug 2020 22:04:07 +0100 Subject: [PATCH 10/37] Fix non-seekable stream reading. --- src/ImageSharp/IO/ChunkedMemoryStream.cs | 544 ++++++++++++++++++ src/ImageSharp/Image.FromStream.cs | 5 +- .../IO/ChunkedMemoryStreamTests.cs | 183 ++++++ ....Load_FromStream_PassLocalConfiguration.cs | 15 +- .../Image/NonSeekableStream.cs | 43 ++ .../Image/NoneSeekableStream.cs | 53 -- 6 files changed, 786 insertions(+), 57 deletions(-) create mode 100644 src/ImageSharp/IO/ChunkedMemoryStream.cs create mode 100644 tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs create mode 100644 tests/ImageSharp.Tests/Image/NonSeekableStream.cs delete mode 100644 tests/ImageSharp.Tests/Image/NoneSeekableStream.cs diff --git a/src/ImageSharp/IO/ChunkedMemoryStream.cs b/src/ImageSharp/IO/ChunkedMemoryStream.cs new file mode 100644 index 0000000000..798ebbdd5d --- /dev/null +++ b/src/ImageSharp/IO/ChunkedMemoryStream.cs @@ -0,0 +1,544 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using System; +using System.IO; +using System.Runtime.CompilerServices; +using SixLabors.ImageSharp.Memory; + +namespace SixLabors.ImageSharp.IO +{ + /// + /// Provides an in-memory stream composed of non-contiguous chunks that doesn't need to be resized. + /// Chunks are allocated by the assigned via the constructor + /// and is designed to take advantage of buffer pooling when available. + /// + internal sealed class ChunkedMemoryStream : Stream + { + /// + /// The default length in bytes of each buffer chunk. + /// + public const int DefaultBufferLength = 4096; + + // The memory allocator. + private readonly MemoryAllocator allocator; + + // Data + private MemoryChunk memoryChunk; + + // The length of each buffer chunk + private readonly int chunkLength; + + // Has the stream been disposed. + private bool isDisposed; + + // Current chunk to write to + private MemoryChunk writeChunk; + + // Offset into chunk to write to + private int writeOffset; + + // Current chunk to read from + private MemoryChunk readChunk; + + // Offset into chunk to read from + private int readOffset; + + /// + /// Initializes a new instance of the class. + /// + public ChunkedMemoryStream(MemoryAllocator allocator) + : this(DefaultBufferLength, allocator) + { + } + + /// + /// Initializes a new instance of the class. + /// + /// The length, in bytes of each buffer chunk. + /// The memory allocator. + public ChunkedMemoryStream(int bufferLength, MemoryAllocator allocator) + { + Guard.MustBeGreaterThan(bufferLength, 0, nameof(bufferLength)); + Guard.NotNull(allocator, nameof(allocator)); + + this.chunkLength = bufferLength; + this.allocator = allocator; + } + + /// + public override bool CanRead => !this.isDisposed; + + /// + public override bool CanSeek => !this.isDisposed; + + /// + public override bool CanWrite => !this.isDisposed; + + /// + public override long Length + { + get + { + this.EnsureNotDisposed(); + + int length = 0; + MemoryChunk chunk = this.memoryChunk; + while (chunk != null) + { + MemoryChunk next = chunk.Next; + if (next != null) + { + length += chunk.Length; + } + else + { + length += this.writeOffset; + } + + chunk = next; + } + + return length; + } + } + + /// + public override long Position + { + get + { + this.EnsureNotDisposed(); + + if (this.readChunk is null) + { + return 0; + } + + int pos = 0; + MemoryChunk chunk = this.memoryChunk; + while (chunk != this.readChunk) + { + pos += chunk.Length; + chunk = chunk.Next; + } + + pos += this.readOffset; + + return pos; + } + + set + { + this.EnsureNotDisposed(); + + if (value < 0) + { + throw new ArgumentOutOfRangeException(nameof(value)); + } + + // Back up current position in case new position is out of range + MemoryChunk backupReadChunk = this.readChunk; + int backupReadOffset = this.readOffset; + + this.readChunk = null; + this.readOffset = 0; + + int leftUntilAtPos = (int)value; + MemoryChunk chunk = this.memoryChunk; + while (chunk != null) + { + if ((leftUntilAtPos < chunk.Length) + || ((leftUntilAtPos == chunk.Length) + && (chunk.Next is null))) + { + // The desired position is in this chunk + this.readChunk = chunk; + this.readOffset = leftUntilAtPos; + break; + } + + leftUntilAtPos -= chunk.Length; + chunk = chunk.Next; + } + + if (this.readChunk is null) + { + // Position is out of range + this.readChunk = backupReadChunk; + this.readOffset = backupReadOffset; + throw new ArgumentOutOfRangeException(nameof(value)); + } + } + } + + /// + public override long Seek(long offset, SeekOrigin origin) + { + this.EnsureNotDisposed(); + + switch (origin) + { + case SeekOrigin.Begin: + this.Position = offset; + break; + + case SeekOrigin.Current: + this.Position += offset; + break; + + case SeekOrigin.End: + this.Position = this.Length + offset; + break; + } + + return this.Position; + } + + /// + public override void SetLength(long value) + => throw new NotSupportedException(); + + /// + protected override void Dispose(bool disposing) + { + if (this.isDisposed) + { + return; + } + + try + { + this.isDisposed = true; + if (disposing) + { + this.ReleaseMemoryChunks(this.memoryChunk); + } + + this.memoryChunk = null; + this.writeChunk = null; + this.readChunk = null; + } + finally + { + base.Dispose(disposing); + } + } + + /// + public override void Flush() + { + } + + /// + public override int Read(byte[] buffer, int offset, int count) + { + Guard.NotNull(buffer, nameof(buffer)); + Guard.MustBeGreaterThanOrEqualTo(offset, 0, nameof(offset)); + Guard.MustBeGreaterThanOrEqualTo(count, 0, nameof(count)); + if (buffer.Length - offset < count) + { + throw new ArgumentException($"{offset} subtracted from the buffer length is less than {count}"); + } + + this.EnsureNotDisposed(); + + if (this.readChunk is null) + { + if (this.memoryChunk is null) + { + return 0; + } + + this.readChunk = this.memoryChunk; + this.readOffset = 0; + } + + byte[] chunkBuffer = this.writeChunk.Buffer.Array; + int chunkSize = this.readChunk.Length; + if (this.readChunk.Next is null) + { + chunkSize = this.writeOffset; + } + + int bytesRead = 0; + + while (count > 0) + { + if (this.readOffset == chunkSize) + { + // Exit if no more chunks are currently available + if (this.readChunk.Next is null) + { + break; + } + + this.readChunk = this.readChunk.Next; + this.readOffset = 0; + chunkBuffer = this.writeChunk.Buffer.Array; + chunkSize = this.readChunk.Length; + if (this.readChunk.Next is null) + { + chunkSize = this.writeOffset; + } + } + + int readCount = Math.Min(count, chunkSize - this.readOffset); + Buffer.BlockCopy(chunkBuffer, this.readOffset, buffer, offset, readCount); + offset += readCount; + count -= readCount; + this.readOffset += readCount; + bytesRead += readCount; + } + + return bytesRead; + } + + /// + public override int ReadByte() + { + this.EnsureNotDisposed(); + + if (this.readChunk is null) + { + if (this.memoryChunk is null) + { + return 0; + } + + this.readChunk = this.memoryChunk; + this.readOffset = 0; + } + + byte[] chunkBuffer = this.writeChunk.Buffer.Array; + int chunkSize = this.readChunk.Length; + if (this.readChunk.Next is null) + { + chunkSize = this.writeOffset; + } + + if (this.readOffset == chunkSize) + { + // Exit if no more chunks are currently available + if (this.readChunk.Next is null) + { + return -1; + } + + this.readChunk = this.readChunk.Next; + this.readOffset = 0; + chunkBuffer = this.writeChunk.Buffer.Array; + } + + return chunkBuffer[this.readOffset++]; + } + + /// + public override void Write(byte[] buffer, int offset, int count) + { + this.EnsureNotDisposed(); + + if (this.memoryChunk is null) + { + this.memoryChunk = this.AllocateMemoryChunk(); + this.writeChunk = this.memoryChunk; + this.writeOffset = 0; + } + + byte[] chunkBuffer = this.writeChunk.Buffer.Array; + int chunkSize = this.writeChunk.Length; + + while (count > 0) + { + if (this.writeOffset == chunkSize) + { + // Allocate a new chunk if the current one is full + this.writeChunk.Next = this.AllocateMemoryChunk(); + this.writeChunk = this.writeChunk.Next; + this.writeOffset = 0; + chunkBuffer = this.writeChunk.Buffer.Array; + chunkSize = this.writeChunk.Length; + } + + int copyCount = Math.Min(count, chunkSize - this.writeOffset); + Buffer.BlockCopy(buffer, offset, chunkBuffer, this.writeOffset, copyCount); + offset += copyCount; + count -= copyCount; + this.writeOffset += copyCount; + } + } + + /// + public override void WriteByte(byte value) + { + this.EnsureNotDisposed(); + + if (this.memoryChunk is null) + { + this.memoryChunk = this.AllocateMemoryChunk(); + this.writeChunk = this.memoryChunk; + this.writeOffset = 0; + } + + byte[] chunkBuffer = this.writeChunk.Buffer.Array; + int chunkSize = this.writeChunk.Length; + + if (this.writeOffset == chunkSize) + { + // Allocate a new chunk if the current one is full + this.writeChunk.Next = this.AllocateMemoryChunk(); + this.writeChunk = this.writeChunk.Next; + this.writeOffset = 0; + chunkBuffer = this.writeChunk.Buffer.Array; + } + + chunkBuffer[this.writeOffset++] = value; + } + + /// + /// Copy entire buffer into an array. + /// + /// The . + public byte[] ToArray() + { + int length = (int)this.Length; // This will throw if stream is closed + byte[] copy = new byte[this.Length]; + + MemoryChunk backupReadChunk = this.readChunk; + int backupReadOffset = this.readOffset; + + this.readChunk = this.memoryChunk; + this.readOffset = 0; + this.Read(copy, 0, length); + + this.readChunk = backupReadChunk; + this.readOffset = backupReadOffset; + + return copy; + } + + /// + /// Write remainder of this stream to another stream. + /// + /// The stream to write to. + public void WriteTo(Stream stream) + { + this.EnsureNotDisposed(); + + Guard.NotNull(stream, nameof(stream)); + + if (this.readChunk is null) + { + if (this.memoryChunk is null) + { + return; + } + + this.readChunk = this.memoryChunk; + this.readOffset = 0; + } + + byte[] chunkBuffer = this.readChunk.Buffer.Array; + int chunkSize = this.readChunk.Length; + if (this.readChunk.Next is null) + { + chunkSize = this.writeOffset; + } + + // Following code mirrors Read() logic (readChunk/readOffset should + // point just past last byte of last chunk when done) + // loop until end of chunks is found + while (true) + { + if (this.readOffset == chunkSize) + { + // Exit if no more chunks are currently available + if (this.readChunk.Next is null) + { + break; + } + + this.readChunk = this.readChunk.Next; + this.readOffset = 0; + chunkBuffer = this.readChunk.Buffer.Array; + chunkSize = this.readChunk.Length; + if (this.readChunk.Next is null) + { + chunkSize = this.writeOffset; + } + } + + int writeCount = chunkSize - this.readOffset; + stream.Write(chunkBuffer, this.readOffset, writeCount); + this.readOffset = chunkSize; + } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private void EnsureNotDisposed() + { + if (this.isDisposed) + { + ThrowDisposed(); + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void ThrowDisposed() + { + throw new ObjectDisposedException(null, "The stream is closed."); + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private MemoryChunk AllocateMemoryChunk() + { + IManagedByteBuffer buffer = this.allocator.AllocateManagedByteBuffer(this.chunkLength); + return new MemoryChunk + { + Buffer = buffer, + Next = null, + Length = this.chunkLength + }; + } + + private void ReleaseMemoryChunks(MemoryChunk chunk) + { + while (chunk != null) + { + chunk.Dispose(); + chunk = chunk.Next; + } + } + + private sealed class MemoryChunk : IDisposable + { + private bool isDisposed; + + public IManagedByteBuffer Buffer { get; set; } + + public MemoryChunk Next { get; set; } + + public int Length { get; set; } + + private void Dispose(bool disposing) + { + if (!this.isDisposed) + { + if (disposing) + { + this.Buffer.Dispose(); + } + + this.Buffer = null; + this.isDisposed = true; + } + } + + public void Dispose() + { + this.Dispose(disposing: true); + GC.SuppressFinalize(this); + } + } + } +} diff --git a/src/ImageSharp/Image.FromStream.cs b/src/ImageSharp/Image.FromStream.cs index ee148cd254..b57fa9a6ca 100644 --- a/src/ImageSharp/Image.FromStream.cs +++ b/src/ImageSharp/Image.FromStream.cs @@ -8,6 +8,7 @@ using System.Text; using System.Threading; using System.Threading.Tasks; using SixLabors.ImageSharp.Formats; +using SixLabors.ImageSharp.IO; using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.PixelFormats; @@ -731,7 +732,7 @@ namespace SixLabors.ImageSharp } // We want to be able to load images from things like HttpContext.Request.Body - using MemoryStream memoryStream = configuration.MemoryAllocator.AllocateFixedCapacityMemoryStream(stream.Length); + using var memoryStream = new ChunkedMemoryStream(configuration.MemoryAllocator); stream.CopyTo(memoryStream, configuration.StreamProcessingBufferSize); memoryStream.Position = 0; @@ -775,7 +776,7 @@ namespace SixLabors.ImageSharp return await action(stream, cancellationToken).ConfigureAwait(false); } - using MemoryStream memoryStream = configuration.MemoryAllocator.AllocateFixedCapacityMemoryStream(stream.Length); + using var memoryStream = new ChunkedMemoryStream(configuration.MemoryAllocator); await stream.CopyToAsync(memoryStream, configuration.StreamProcessingBufferSize, cancellationToken).ConfigureAwait(false); memoryStream.Position = 0; diff --git a/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs b/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs new file mode 100644 index 0000000000..65ad93d5be --- /dev/null +++ b/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs @@ -0,0 +1,183 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using System; +using System.Collections.Generic; +using System.IO; +using SixLabors.ImageSharp.IO; +using SixLabors.ImageSharp.Memory; +using Xunit; + +namespace SixLabors.ImageSharp.Tests.IO +{ + /// + /// Tests for the class. + /// + public class ChunkedMemoryStreamTests + { + private readonly MemoryAllocator allocator; + + public ChunkedMemoryStreamTests() + { + this.allocator = Configuration.Default.MemoryAllocator; + } + + [Fact] + public void MemoryStream_Ctor_InvalidCapacities() + { + Assert.Throws(() => new ChunkedMemoryStream(int.MinValue, this.allocator)); + Assert.Throws(() => new ChunkedMemoryStream(0, this.allocator)); + } + + [Fact] + public void ChunkedPooledMemoryStream_GetPositionTest_Negative() + { + using var ms = new ChunkedMemoryStream(this.allocator); + long iCurrentPos = ms.Position; + for (int i = -1; i > -6; i--) + { + Assert.Throws(() => ms.Position = i); + Assert.Equal(ms.Position, iCurrentPos); + } + } + + [Fact] + public void MemoryStream_ReadTest_Negative() + { + var ms2 = new ChunkedMemoryStream(this.allocator); + + Assert.Throws(() => ms2.Read(null, 0, 0)); + Assert.Throws(() => ms2.Read(new byte[] { 1 }, -1, 0)); + Assert.Throws(() => ms2.Read(new byte[] { 1 }, 0, -1)); + Assert.Throws(null, () => ms2.Read(new byte[] { 1 }, 2, 0)); + Assert.Throws(null, () => ms2.Read(new byte[] { 1 }, 0, 2)); + + ms2.Dispose(); + + Assert.Throws(() => ms2.Read(new byte[] { 1 }, 0, 1)); + } + + [Fact] + public void MemoryStream_WriteToTests() + { + using (var ms2 = new ChunkedMemoryStream(this.allocator)) + { + byte[] bytArrRet; + byte[] bytArr = new byte[] { byte.MinValue, byte.MaxValue, 1, 2, 3, 4, 5, 6, 128, 250 }; + + // [] Write to FileStream, check the filestream + ms2.Write(bytArr, 0, bytArr.Length); + + using var readonlyStream = new ChunkedMemoryStream(this.allocator); + ms2.WriteTo(readonlyStream); + readonlyStream.Flush(); + readonlyStream.Position = 0; + bytArrRet = new byte[(int)readonlyStream.Length]; + readonlyStream.Read(bytArrRet, 0, (int)readonlyStream.Length); + for (int i = 0; i < bytArr.Length; i++) + { + Assert.Equal(bytArr[i], bytArrRet[i]); + } + } + + // [] Write to memoryStream, check the memoryStream + using (var ms2 = new ChunkedMemoryStream(this.allocator)) + using (var ms3 = new ChunkedMemoryStream(this.allocator)) + { + byte[] bytArrRet; + byte[] bytArr = new byte[] { byte.MinValue, byte.MaxValue, 1, 2, 3, 4, 5, 6, 128, 250 }; + + ms2.Write(bytArr, 0, bytArr.Length); + ms2.WriteTo(ms3); + ms3.Position = 0; + bytArrRet = new byte[(int)ms3.Length]; + ms3.Read(bytArrRet, 0, (int)ms3.Length); + for (int i = 0; i < bytArr.Length; i++) + { + Assert.Equal(bytArr[i], bytArrRet[i]); + } + } + } + + [Fact] + public void MemoryStream_WriteToTests_Negative() + { + using var ms2 = new ChunkedMemoryStream(this.allocator); + Assert.Throws(() => ms2.WriteTo(null)); + + ms2.Write(new byte[] { 1 }, 0, 1); + var readonlyStream = new MemoryStream(new byte[1028], false); + Assert.Throws(() => ms2.WriteTo(readonlyStream)); + + readonlyStream.Dispose(); + + // [] Pass in a closed stream + Assert.Throws(() => ms2.WriteTo(readonlyStream)); + } + + [Fact] + public void MemoryStream_CopyTo_Invalid() + { + ChunkedMemoryStream memoryStream; + const string BufferSize = "bufferSize"; + using (memoryStream = new ChunkedMemoryStream(this.allocator)) + { + const string Destination = "destination"; + Assert.Throws(Destination, () => memoryStream.CopyTo(destination: null)); + + // Validate the destination parameter first. + Assert.Throws(Destination, () => memoryStream.CopyTo(destination: null, bufferSize: 0)); + Assert.Throws(Destination, () => memoryStream.CopyTo(destination: null, bufferSize: -1)); + + // Then bufferSize. + Assert.Throws(BufferSize, () => memoryStream.CopyTo(Stream.Null, bufferSize: 0)); // 0-length buffer doesn't make sense. + Assert.Throws(BufferSize, () => memoryStream.CopyTo(Stream.Null, bufferSize: -1)); + } + + // After the Stream is disposed, we should fail on all CopyTos. + Assert.Throws(BufferSize, () => memoryStream.CopyTo(Stream.Null, bufferSize: 0)); // Not before bufferSize is validated. + Assert.Throws(BufferSize, () => memoryStream.CopyTo(Stream.Null, bufferSize: -1)); + + ChunkedMemoryStream disposedStream = memoryStream; + + // We should throw first for the source being disposed... + Assert.Throws(() => memoryStream.CopyTo(disposedStream, 1)); + + // Then for the destination being disposed. + memoryStream = new ChunkedMemoryStream(this.allocator); + Assert.Throws(() => memoryStream.CopyTo(disposedStream, 1)); + memoryStream.Dispose(); + } + + [Theory] + [MemberData(nameof(CopyToData))] + public void CopyTo(Stream source, byte[] expected) + { + using var destination = new ChunkedMemoryStream(this.allocator); + source.CopyTo(destination); + Assert.InRange(source.Position, source.Length, int.MaxValue); // Copying the data should have read to the end of the stream or stayed past the end. + Assert.Equal(expected, destination.ToArray()); + } + + public static IEnumerable CopyToData() + { + // Stream is positioned @ beginning of data + byte[] data1 = new byte[] { 1, 2, 3 }; + var stream1 = new MemoryStream(data1); + + yield return new object[] { stream1, data1 }; + + // Stream is positioned in the middle of data + byte[] data2 = new byte[] { 0xff, 0xf3, 0xf0 }; + var stream2 = new MemoryStream(data2) { Position = 1 }; + + yield return new object[] { stream2, new byte[] { 0xf3, 0xf0 } }; + + // Stream is positioned after end of data + byte[] data3 = data2; + var stream3 = new MemoryStream(data3) { Position = data3.Length + 1 }; + + yield return new object[] { stream3, Array.Empty() }; + } + } +} diff --git a/tests/ImageSharp.Tests/Image/ImageTests.Load_FromStream_PassLocalConfiguration.cs b/tests/ImageSharp.Tests/Image/ImageTests.Load_FromStream_PassLocalConfiguration.cs index c7737ef8b4..17b557f833 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.Load_FromStream_PassLocalConfiguration.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.Load_FromStream_PassLocalConfiguration.cs @@ -2,7 +2,7 @@ // Licensed under the Apache License, Version 2.0. using System.IO; - +using System.Threading.Tasks; using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.PixelFormats; @@ -39,7 +39,7 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void NonSeekableStream() { - var stream = new NoneSeekableStream(this.DataStream); + var stream = new NonSeekableStream(this.DataStream); var img = Image.Load(this.TopLevelConfiguration, stream); Assert.NotNull(img); @@ -47,6 +47,17 @@ namespace SixLabors.ImageSharp.Tests this.TestFormat.VerifySpecificDecodeCall(this.Marker, this.TopLevelConfiguration); } + [Fact] + public async Task NonSeekableStreamAsync() + { + var stream = new NonSeekableStream(this.DataStream); + Image img = await Image.LoadAsync(this.TopLevelConfiguration, stream); + + Assert.NotNull(img); + + this.TestFormat.VerifySpecificDecodeCall(this.Marker, this.TopLevelConfiguration); + } + [Fact] public void Configuration_Stream_Decoder_Specific() { diff --git a/tests/ImageSharp.Tests/Image/NonSeekableStream.cs b/tests/ImageSharp.Tests/Image/NonSeekableStream.cs new file mode 100644 index 0000000000..76b11d148f --- /dev/null +++ b/tests/ImageSharp.Tests/Image/NonSeekableStream.cs @@ -0,0 +1,43 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using System; +using System.IO; + +namespace SixLabors.ImageSharp.Tests +{ + internal class NonSeekableStream : Stream + { + private readonly Stream dataStream; + + public NonSeekableStream(Stream dataStream) + => this.dataStream = dataStream; + + public override bool CanRead => this.dataStream.CanRead; + + public override bool CanSeek => false; + + public override bool CanWrite => false; + + public override long Length => throw new NotSupportedException(); + + public override long Position + { + get { throw new NotSupportedException(); } + set { throw new NotSupportedException(); } + } + + public override void Flush() => this.dataStream.Flush(); + + public override int Read(byte[] buffer, int offset, int count) => this.dataStream.Read(buffer, offset, count); + + public override long Seek(long offset, SeekOrigin origin) + => throw new NotSupportedException(); + + public override void SetLength(long value) + => throw new NotSupportedException(); + + public override void Write(byte[] buffer, int offset, int count) + => throw new NotImplementedException(); + } +} diff --git a/tests/ImageSharp.Tests/Image/NoneSeekableStream.cs b/tests/ImageSharp.Tests/Image/NoneSeekableStream.cs deleted file mode 100644 index 1ae217f0fe..0000000000 --- a/tests/ImageSharp.Tests/Image/NoneSeekableStream.cs +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright (c) Six Labors. -// Licensed under the Apache License, Version 2.0. - -using System; -using System.IO; - -namespace SixLabors.ImageSharp.Tests -{ - internal class NoneSeekableStream : Stream - { - private Stream dataStream; - - public NoneSeekableStream(Stream dataStream) - { - this.dataStream = dataStream; - } - - public override bool CanRead => this.dataStream.CanRead; - - public override bool CanSeek => false; - - public override bool CanWrite => false; - - public override long Length => this.dataStream.Length; - - public override long Position { get => this.dataStream.Position; set => throw new NotImplementedException(); } - - public override void Flush() - { - this.dataStream.Flush(); - } - - public override int Read(byte[] buffer, int offset, int count) - { - return this.dataStream.Read(buffer, offset, count); - } - - public override long Seek(long offset, SeekOrigin origin) - { - throw new NotImplementedException(); - } - - public override void SetLength(long value) - { - throw new NotImplementedException(); - } - - public override void Write(byte[] buffer, int offset, int count) - { - throw new NotImplementedException(); - } - } -} \ No newline at end of file From 3f534b1b15c9560b89972bf891a5be6b29b1e55c Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 16 Aug 2020 23:04:28 +0100 Subject: [PATCH 11/37] Update ChunkedMemoryStream.cs --- src/ImageSharp/IO/ChunkedMemoryStream.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ImageSharp/IO/ChunkedMemoryStream.cs b/src/ImageSharp/IO/ChunkedMemoryStream.cs index 798ebbdd5d..d3e6861d28 100644 --- a/src/ImageSharp/IO/ChunkedMemoryStream.cs +++ b/src/ImageSharp/IO/ChunkedMemoryStream.cs @@ -254,7 +254,7 @@ namespace SixLabors.ImageSharp.IO this.readOffset = 0; } - byte[] chunkBuffer = this.writeChunk.Buffer.Array; + byte[] chunkBuffer = this.readChunk.Buffer.Array; int chunkSize = this.readChunk.Length; if (this.readChunk.Next is null) { @@ -275,7 +275,7 @@ namespace SixLabors.ImageSharp.IO this.readChunk = this.readChunk.Next; this.readOffset = 0; - chunkBuffer = this.writeChunk.Buffer.Array; + chunkBuffer = this.readChunk.Buffer.Array; chunkSize = this.readChunk.Length; if (this.readChunk.Next is null) { @@ -310,7 +310,7 @@ namespace SixLabors.ImageSharp.IO this.readOffset = 0; } - byte[] chunkBuffer = this.writeChunk.Buffer.Array; + byte[] chunkBuffer = this.readChunk.Buffer.Array; int chunkSize = this.readChunk.Length; if (this.readChunk.Next is null) { @@ -327,7 +327,7 @@ namespace SixLabors.ImageSharp.IO this.readChunk = this.readChunk.Next; this.readOffset = 0; - chunkBuffer = this.writeChunk.Buffer.Array; + chunkBuffer = this.readChunk.Buffer.Array; } return chunkBuffer[this.readOffset++]; @@ -497,7 +497,7 @@ namespace SixLabors.ImageSharp.IO { Buffer = buffer, Next = null, - Length = this.chunkLength + Length = buffer.Length() }; } From ceab655ce5d75721508dd91ec415fee252da7c99 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 17 Aug 2020 10:25:37 +0100 Subject: [PATCH 12/37] More tests and remove old stream --- .../IO/FixedCapacityPooledMemoryStream.cs | 69 ------------------- .../Memory/MemoryAllocatorExtensions.cs | 3 - .../IO/ChunkedMemoryStreamTests.cs | 53 +++++++++++++- .../FixedCapacityPooledMemoryStreamTests.cs | 42 ----------- 4 files changed, 52 insertions(+), 115 deletions(-) delete mode 100644 src/ImageSharp/IO/FixedCapacityPooledMemoryStream.cs delete mode 100644 tests/ImageSharp.Tests/IO/FixedCapacityPooledMemoryStreamTests.cs diff --git a/src/ImageSharp/IO/FixedCapacityPooledMemoryStream.cs b/src/ImageSharp/IO/FixedCapacityPooledMemoryStream.cs deleted file mode 100644 index 74864d45e6..0000000000 --- a/src/ImageSharp/IO/FixedCapacityPooledMemoryStream.cs +++ /dev/null @@ -1,69 +0,0 @@ -// Copyright (c) Six Labors. -// Licensed under the Apache License, Version 2.0. - -using System; -using System.Buffers; -using System.IO; -using SixLabors.ImageSharp.Memory; - -namespace SixLabors.ImageSharp.IO -{ - /// - /// A memory stream constructed from a pooled buffer of known length. - /// - internal sealed class FixedCapacityPooledMemoryStream : MemoryStream - { - private readonly IManagedByteBuffer buffer; - private bool isDisposed; - - /// - /// Initializes a new instance of the class. - /// - /// The length of the stream buffer to rent. - /// The allocator to rent the buffer from. - public FixedCapacityPooledMemoryStream(long length, MemoryAllocator allocator) - : this(RentBuffer(length, allocator)) => this.Length = length; - - private FixedCapacityPooledMemoryStream(IManagedByteBuffer buffer) - : base(buffer.Array) => this.buffer = buffer; - - /// - public override long Length { get; } - - /// - public override bool TryGetBuffer(out ArraySegment buffer) - { - if (this.isDisposed) - { - throw new ObjectDisposedException(this.GetType().Name); - } - - buffer = new ArraySegment(this.buffer.Array, 0, this.buffer.Length()); - return true; - } - - /// - protected override void Dispose(bool disposing) - { - if (!this.isDisposed) - { - this.isDisposed = true; - - if (disposing) - { - this.buffer.Dispose(); - } - - base.Dispose(disposing); - } - } - - // In the extrememly unlikely event someone ever gives us a stream - // with length longer than int.MaxValue then we'll use something else. - private static IManagedByteBuffer RentBuffer(long length, MemoryAllocator allocator) - { - Guard.MustBeBetweenOrEqualTo(length, 0, int.MaxValue, nameof(length)); - return allocator.AllocateManagedByteBuffer((int)length); - } - } -} diff --git a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs index 9a56390d89..922088b26d 100644 --- a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs +++ b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs @@ -100,8 +100,5 @@ namespace SixLabors.ImageSharp.Memory AllocationOptions options = AllocationOptions.None) where T : struct => MemoryGroup.Allocate(memoryAllocator, totalLength, bufferAlignment, options); - - internal static MemoryStream AllocateFixedCapacityMemoryStream(this MemoryAllocator allocator, long length) => - new FixedCapacityPooledMemoryStream(length, allocator); } } diff --git a/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs b/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs index 65ad93d5be..7b17a3e663 100644 --- a/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs +++ b/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs @@ -30,7 +30,7 @@ namespace SixLabors.ImageSharp.Tests.IO } [Fact] - public void ChunkedPooledMemoryStream_GetPositionTest_Negative() + public void MemoryStream_GetPositionTest_Negative() { using var ms = new ChunkedMemoryStream(this.allocator); long iCurrentPos = ms.Position; @@ -57,6 +57,48 @@ namespace SixLabors.ImageSharp.Tests.IO Assert.Throws(() => ms2.Read(new byte[] { 1 }, 0, 1)); } + [Theory] + [InlineData(1024)] + [InlineData(1024 * 4)] + [InlineData(1024 * 6)] + [InlineData(1024 * 8)] + public void MemoryStream_ReadByteTest(int length) + { + using MemoryStream ms = this.CreateTestStream(length); + using var cms = new ChunkedMemoryStream(this.allocator); + + ms.CopyTo(cms); + cms.Position = 0; + var expected = ms.ToArray(); + + for (int i = 0; i < expected.Length; i++) + { + Assert.Equal(expected[i], cms.ReadByte()); + } + } + + [Theory] + [InlineData(1024)] + [InlineData(1024 * 4)] + [InlineData(1024 * 6)] + [InlineData(1024 * 8)] + public void MemoryStream_ReadByteBufferTest(int length) + { + using MemoryStream ms = this.CreateTestStream(length); + using var cms = new ChunkedMemoryStream(this.allocator); + + ms.CopyTo(cms); + cms.Position = 0; + var expected = ms.ToArray(); + var buffer = new byte[2]; + for (int i = 0; i < expected.Length; i += 2) + { + cms.Read(buffer); + Assert.Equal(expected[i], buffer[0]); + Assert.Equal(expected[i + 1], buffer[1]); + } + } + [Fact] public void MemoryStream_WriteToTests() { @@ -179,5 +221,14 @@ namespace SixLabors.ImageSharp.Tests.IO yield return new object[] { stream3, Array.Empty() }; } + + private MemoryStream CreateTestStream(int length) + { + var buffer = new byte[length]; + var random = new Random(); + random.NextBytes(buffer); + + return new MemoryStream(buffer); + } } } diff --git a/tests/ImageSharp.Tests/IO/FixedCapacityPooledMemoryStreamTests.cs b/tests/ImageSharp.Tests/IO/FixedCapacityPooledMemoryStreamTests.cs deleted file mode 100644 index 0581a6ee2c..0000000000 --- a/tests/ImageSharp.Tests/IO/FixedCapacityPooledMemoryStreamTests.cs +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright (c) Six Labors. -// Licensed under the Apache License, Version 2.0. - -using System; -using System.IO; -using System.Linq; -using SixLabors.ImageSharp.Memory; -using SixLabors.ImageSharp.Tests.Memory; -using Xunit; - -namespace SixLabors.ImageSharp.Tests.IO -{ - public class FixedCapacityPooledMemoryStreamTests - { - private readonly TestMemoryAllocator memoryAllocator = new TestMemoryAllocator(); - - [Theory] - [InlineData(1)] - [InlineData(512)] - public void RentsManagedBuffer(int length) - { - MemoryStream ms = this.memoryAllocator.AllocateFixedCapacityMemoryStream(length); - Assert.Equal(length, this.memoryAllocator.AllocationLog.Single().Length); - ms.Dispose(); - Assert.Equal(1, this.memoryAllocator.ReturnLog.Count); - } - - [Theory] - [InlineData(42)] - [InlineData(2999)] - public void UsesRentedBuffer(int length) - { - using MemoryStream ms = this.memoryAllocator.AllocateFixedCapacityMemoryStream(length); - ms.TryGetBuffer(out ArraySegment buffer); - byte[] array = buffer.Array; - Assert.Equal(array.GetHashCode(), this.memoryAllocator.AllocationLog.Single().HashCodeOfBuffer); - - ms.Write(new byte[] { 123 }); - Assert.Equal(123, array[0]); - } - } -} From e9b460efcea51102ce5f0264acb77d45f165c906 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 17 Aug 2020 11:18:13 +0100 Subject: [PATCH 13/37] Add WriteByteTests --- .../IO/ChunkedMemoryStreamTests.cs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs b/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs index 7b17a3e663..46afc7e50c 100644 --- a/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs +++ b/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs @@ -141,6 +141,32 @@ namespace SixLabors.ImageSharp.Tests.IO } } + [Fact] + public void MemoryStream_WriteByteTests() + { + using (var ms2 = new ChunkedMemoryStream(this.allocator)) + { + byte[] bytArrRet; + byte[] bytArr = new byte[] { byte.MinValue, byte.MaxValue, 1, 2, 3, 4, 5, 6, 128, 250 }; + + for (int i = 0; i < bytArr.Length; i++) + { + ms2.WriteByte(bytArr[i]); + } + + using var readonlyStream = new ChunkedMemoryStream(this.allocator); + ms2.WriteTo(readonlyStream); + readonlyStream.Flush(); + readonlyStream.Position = 0; + bytArrRet = new byte[(int)readonlyStream.Length]; + readonlyStream.Read(bytArrRet, 0, (int)readonlyStream.Length); + for (int i = 0; i < bytArr.Length; i++) + { + Assert.Equal(bytArr[i], bytArrRet[i]); + } + } + } + [Fact] public void MemoryStream_WriteToTests_Negative() { From 94613d68b37455f57466d43cf1be8df1927207c9 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 19 Aug 2020 13:40:26 +0100 Subject: [PATCH 14/37] Add optimized Read(Span) API --- src/ImageSharp/IO/ChunkedMemoryStream.cs | 24 ++++++---- .../Memory/MemoryOwnerExtensions.cs | 1 + .../IO/ChunkedMemoryStreamTests.cs | 47 ++++++++++++++----- 3 files changed, 53 insertions(+), 19 deletions(-) diff --git a/src/ImageSharp/IO/ChunkedMemoryStream.cs b/src/ImageSharp/IO/ChunkedMemoryStream.cs index d3e6861d28..bd374a3ce4 100644 --- a/src/ImageSharp/IO/ChunkedMemoryStream.cs +++ b/src/ImageSharp/IO/ChunkedMemoryStream.cs @@ -236,11 +236,18 @@ namespace SixLabors.ImageSharp.IO Guard.NotNull(buffer, nameof(buffer)); Guard.MustBeGreaterThanOrEqualTo(offset, 0, nameof(offset)); Guard.MustBeGreaterThanOrEqualTo(count, 0, nameof(count)); - if (buffer.Length - offset < count) - { - throw new ArgumentException($"{offset} subtracted from the buffer length is less than {count}"); - } + Guard.IsFalse(buffer.Length - offset < count, nameof(buffer), $"{offset} subtracted from the buffer length is less than {count}"); + + return this.ReadImpl(buffer.AsSpan().Slice(offset, count)); + } + +#if SUPPORTS_SPAN_STREAM + /// + public override int Read(Span buffer) => this.ReadImpl(buffer); +#endif + private int ReadImpl(Span buffer) + { this.EnsureNotDisposed(); if (this.readChunk is null) @@ -254,7 +261,7 @@ namespace SixLabors.ImageSharp.IO this.readOffset = 0; } - byte[] chunkBuffer = this.readChunk.Buffer.Array; + Span chunkBuffer = this.readChunk.Buffer.GetSpan(); int chunkSize = this.readChunk.Length; if (this.readChunk.Next is null) { @@ -262,7 +269,8 @@ namespace SixLabors.ImageSharp.IO } int bytesRead = 0; - + int offset = 0; + int count = buffer.Length; while (count > 0) { if (this.readOffset == chunkSize) @@ -275,7 +283,7 @@ namespace SixLabors.ImageSharp.IO this.readChunk = this.readChunk.Next; this.readOffset = 0; - chunkBuffer = this.readChunk.Buffer.Array; + chunkBuffer = this.readChunk.Buffer.GetSpan(); chunkSize = this.readChunk.Length; if (this.readChunk.Next is null) { @@ -284,7 +292,7 @@ namespace SixLabors.ImageSharp.IO } int readCount = Math.Min(count, chunkSize - this.readOffset); - Buffer.BlockCopy(chunkBuffer, this.readOffset, buffer, offset, readCount); + chunkBuffer.Slice(this.readOffset, count).CopyTo(buffer.Slice(offset)); offset += readCount; count -= readCount; this.readOffset += readCount; diff --git a/src/ImageSharp/Memory/MemoryOwnerExtensions.cs b/src/ImageSharp/Memory/MemoryOwnerExtensions.cs index 98fd40e65b..aa475a80f1 100644 --- a/src/ImageSharp/Memory/MemoryOwnerExtensions.cs +++ b/src/ImageSharp/Memory/MemoryOwnerExtensions.cs @@ -13,6 +13,7 @@ namespace SixLabors.ImageSharp.Memory /// internal static class MemoryOwnerExtensions { + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Span GetSpan(this IMemoryOwner buffer) => buffer.Memory.Span; diff --git a/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs b/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs index 46afc7e50c..28f1d336d9 100644 --- a/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs +++ b/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs @@ -49,8 +49,8 @@ namespace SixLabors.ImageSharp.Tests.IO Assert.Throws(() => ms2.Read(null, 0, 0)); Assert.Throws(() => ms2.Read(new byte[] { 1 }, -1, 0)); Assert.Throws(() => ms2.Read(new byte[] { 1 }, 0, -1)); - Assert.Throws(null, () => ms2.Read(new byte[] { 1 }, 2, 0)); - Assert.Throws(null, () => ms2.Read(new byte[] { 1 }, 0, 2)); + Assert.Throws(() => ms2.Read(new byte[] { 1 }, 2, 0)); + Assert.Throws(() => ms2.Read(new byte[] { 1 }, 0, 2)); ms2.Dispose(); @@ -58,10 +58,11 @@ namespace SixLabors.ImageSharp.Tests.IO } [Theory] - [InlineData(1024)] - [InlineData(1024 * 4)] - [InlineData(1024 * 6)] - [InlineData(1024 * 8)] + [InlineData(ChunkedMemoryStream.DefaultBufferLength)] + [InlineData((int)(ChunkedMemoryStream.DefaultBufferLength * 1.5))] + [InlineData(ChunkedMemoryStream.DefaultBufferLength * 4)] + [InlineData((int)(ChunkedMemoryStream.DefaultBufferLength * 5.5))] + [InlineData(ChunkedMemoryStream.DefaultBufferLength * 8)] public void MemoryStream_ReadByteTest(int length) { using MemoryStream ms = this.CreateTestStream(length); @@ -78,10 +79,11 @@ namespace SixLabors.ImageSharp.Tests.IO } [Theory] - [InlineData(1024)] - [InlineData(1024 * 4)] - [InlineData(1024 * 6)] - [InlineData(1024 * 8)] + [InlineData(ChunkedMemoryStream.DefaultBufferLength)] + [InlineData((int)(ChunkedMemoryStream.DefaultBufferLength * 1.5))] + [InlineData(ChunkedMemoryStream.DefaultBufferLength * 4)] + [InlineData((int)(ChunkedMemoryStream.DefaultBufferLength * 5.5))] + [InlineData(ChunkedMemoryStream.DefaultBufferLength * 8)] public void MemoryStream_ReadByteBufferTest(int length) { using MemoryStream ms = this.CreateTestStream(length); @@ -99,6 +101,29 @@ namespace SixLabors.ImageSharp.Tests.IO } } + [Theory] + [InlineData(ChunkedMemoryStream.DefaultBufferLength)] + [InlineData((int)(ChunkedMemoryStream.DefaultBufferLength * 1.5))] + [InlineData(ChunkedMemoryStream.DefaultBufferLength * 4)] + [InlineData((int)(ChunkedMemoryStream.DefaultBufferLength * 5.5))] + [InlineData(ChunkedMemoryStream.DefaultBufferLength * 8)] + public void MemoryStream_ReadByteBufferSpanTest(int length) + { + using MemoryStream ms = this.CreateTestStream(length); + using var cms = new ChunkedMemoryStream(this.allocator); + + ms.CopyTo(cms); + cms.Position = 0; + var expected = ms.ToArray(); + Span buffer = new byte[2]; + for (int i = 0; i < expected.Length; i += 2) + { + cms.Read(buffer); + Assert.Equal(expected[i], buffer[0]); + Assert.Equal(expected[i + 1], buffer[1]); + } + } + [Fact] public void MemoryStream_WriteToTests() { @@ -107,7 +132,7 @@ namespace SixLabors.ImageSharp.Tests.IO byte[] bytArrRet; byte[] bytArr = new byte[] { byte.MinValue, byte.MaxValue, 1, 2, 3, 4, 5, 6, 128, 250 }; - // [] Write to FileStream, check the filestream + // [] Write to memoryStream, check the memoryStream ms2.Write(bytArr, 0, bytArr.Length); using var readonlyStream = new ChunkedMemoryStream(this.allocator); From b4b3074738b43d4d4ca47c3d72c29560798411f7 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 19 Aug 2020 14:39:01 +0100 Subject: [PATCH 15/37] Optimize Write(Span) --- src/ImageSharp/IO/ChunkedMemoryStream.cs | 36 ++++++++++++++++++------ 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/src/ImageSharp/IO/ChunkedMemoryStream.cs b/src/ImageSharp/IO/ChunkedMemoryStream.cs index bd374a3ce4..233154e9e0 100644 --- a/src/ImageSharp/IO/ChunkedMemoryStream.cs +++ b/src/ImageSharp/IO/ChunkedMemoryStream.cs @@ -134,7 +134,7 @@ namespace SixLabors.ImageSharp.IO if (value < 0) { - throw new ArgumentOutOfRangeException(nameof(value)); + ThrowArgumentOutOfRange(nameof(value)); } // Back up current position in case new position is out of range @@ -167,12 +167,13 @@ namespace SixLabors.ImageSharp.IO // Position is out of range this.readChunk = backupReadChunk; this.readOffset = backupReadOffset; - throw new ArgumentOutOfRangeException(nameof(value)); + ThrowArgumentOutOfRange(nameof(value)); } } } /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] public override long Seek(long offset, SeekOrigin origin) { this.EnsureNotDisposed(); @@ -231,6 +232,7 @@ namespace SixLabors.ImageSharp.IO } /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] public override int Read(byte[] buffer, int offset, int count) { Guard.NotNull(buffer, nameof(buffer)); @@ -243,6 +245,7 @@ namespace SixLabors.ImageSharp.IO #if SUPPORTS_SPAN_STREAM /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] public override int Read(Span buffer) => this.ReadImpl(buffer); #endif @@ -303,6 +306,7 @@ namespace SixLabors.ImageSharp.IO } /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] public override int ReadByte() { this.EnsureNotDisposed(); @@ -342,7 +346,17 @@ namespace SixLabors.ImageSharp.IO } /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] public override void Write(byte[] buffer, int offset, int count) + => this.WriteImpl(buffer.AsSpan().Slice(offset, count)); + +#if SUPPORTS_SPAN_STREAM + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public override void Write(ReadOnlySpan buffer) => this.WriteImpl(buffer); +#endif + + private void WriteImpl(ReadOnlySpan buffer) { this.EnsureNotDisposed(); @@ -353,9 +367,10 @@ namespace SixLabors.ImageSharp.IO this.writeOffset = 0; } - byte[] chunkBuffer = this.writeChunk.Buffer.Array; + Span chunkBuffer = this.writeChunk.Buffer.GetSpan(); int chunkSize = this.writeChunk.Length; - + int count = buffer.Length; + int offset = 0; while (count > 0) { if (this.writeOffset == chunkSize) @@ -364,12 +379,13 @@ namespace SixLabors.ImageSharp.IO this.writeChunk.Next = this.AllocateMemoryChunk(); this.writeChunk = this.writeChunk.Next; this.writeOffset = 0; - chunkBuffer = this.writeChunk.Buffer.Array; + chunkBuffer = this.writeChunk.Buffer.GetSpan(); chunkSize = this.writeChunk.Length; } int copyCount = Math.Min(count, chunkSize - this.writeOffset); - Buffer.BlockCopy(buffer, offset, chunkBuffer, this.writeOffset, copyCount); + buffer.Slice(offset, copyCount).CopyTo(chunkBuffer.Slice(this.writeOffset)); + offset += copyCount; count -= copyCount; this.writeOffset += copyCount; @@ -493,9 +509,11 @@ namespace SixLabors.ImageSharp.IO [MethodImpl(MethodImplOptions.NoInlining)] private static void ThrowDisposed() - { - throw new ObjectDisposedException(null, "The stream is closed."); - } + => throw new ObjectDisposedException(null, "The stream is closed."); + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void ThrowArgumentOutOfRange(string value) + => throw new ArgumentOutOfRangeException(value); [MethodImpl(MethodImplOptions.AggressiveInlining)] private MemoryChunk AllocateMemoryChunk() From da6ba1d5a3398bab62aac8661b306466a18aa621 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 19 Aug 2020 15:24:35 +0100 Subject: [PATCH 16/37] Increase chunk size, add benchmarks and write span tests --- src/ImageSharp/IO/ChunkedMemoryStream.cs | 2 +- .../General/IO/BufferedStreams.cs | 116 +++++++++++++----- .../IO/ChunkedMemoryStreamTests.cs | 42 +++++++ 3 files changed, 128 insertions(+), 32 deletions(-) diff --git a/src/ImageSharp/IO/ChunkedMemoryStream.cs b/src/ImageSharp/IO/ChunkedMemoryStream.cs index 233154e9e0..77a3174094 100644 --- a/src/ImageSharp/IO/ChunkedMemoryStream.cs +++ b/src/ImageSharp/IO/ChunkedMemoryStream.cs @@ -18,7 +18,7 @@ namespace SixLabors.ImageSharp.IO /// /// The default length in bytes of each buffer chunk. /// - public const int DefaultBufferLength = 4096; + public const int DefaultBufferLength = 81920; // The memory allocator. private readonly MemoryAllocator allocator; diff --git a/tests/ImageSharp.Benchmarks/General/IO/BufferedStreams.cs b/tests/ImageSharp.Benchmarks/General/IO/BufferedStreams.cs index be232c78d6..eb1bc7a851 100644 --- a/tests/ImageSharp.Benchmarks/General/IO/BufferedStreams.cs +++ b/tests/ImageSharp.Benchmarks/General/IO/BufferedStreams.cs @@ -21,8 +21,12 @@ namespace SixLabors.ImageSharp.Benchmarks.IO private MemoryStream stream4; private MemoryStream stream5; private MemoryStream stream6; + private ChunkedMemoryStream chunkedMemoryStream1; + private ChunkedMemoryStream chunkedMemoryStream2; private BufferedReadStream bufferedStream1; private BufferedReadStream bufferedStream2; + private BufferedReadStream bufferedStream3; + private BufferedReadStream bufferedStream4; private BufferedReadStreamWrapper bufferedStreamWrap1; private BufferedReadStreamWrapper bufferedStreamWrap2; @@ -35,8 +39,19 @@ namespace SixLabors.ImageSharp.Benchmarks.IO this.stream4 = new MemoryStream(this.buffer); this.stream5 = new MemoryStream(this.buffer); this.stream6 = new MemoryStream(this.buffer); + this.stream6 = new MemoryStream(this.buffer); + this.chunkedMemoryStream1 = new ChunkedMemoryStream(Configuration.Default.MemoryAllocator); + this.chunkedMemoryStream1.Write(this.buffer); + this.chunkedMemoryStream1.Position = 0; + + this.chunkedMemoryStream2 = new ChunkedMemoryStream(Configuration.Default.MemoryAllocator); + this.chunkedMemoryStream2.Write(this.buffer); + this.chunkedMemoryStream2.Position = 0; + this.bufferedStream1 = new BufferedReadStream(Configuration.Default, this.stream3); this.bufferedStream2 = new BufferedReadStream(Configuration.Default, this.stream4); + this.bufferedStream3 = new BufferedReadStream(Configuration.Default, this.chunkedMemoryStream1); + this.bufferedStream4 = new BufferedReadStream(Configuration.Default, this.chunkedMemoryStream2); this.bufferedStreamWrap1 = new BufferedReadStreamWrapper(this.stream5); this.bufferedStreamWrap2 = new BufferedReadStreamWrapper(this.stream6); } @@ -46,8 +61,12 @@ namespace SixLabors.ImageSharp.Benchmarks.IO { this.bufferedStream1?.Dispose(); this.bufferedStream2?.Dispose(); + this.bufferedStream3?.Dispose(); + this.bufferedStream4?.Dispose(); this.bufferedStreamWrap1?.Dispose(); this.bufferedStreamWrap2?.Dispose(); + this.chunkedMemoryStream1?.Dispose(); + this.chunkedMemoryStream2?.Dispose(); this.stream1?.Dispose(); this.stream2?.Dispose(); this.stream3?.Dispose(); @@ -86,6 +105,21 @@ namespace SixLabors.ImageSharp.Benchmarks.IO return r; } + [Benchmark] + public int BufferedReadStreamChunkedRead() + { + int r = 0; + BufferedReadStream reader = this.bufferedStream3; + byte[] b = this.chunk2; + + for (int i = 0; i < reader.Length / 2; i++) + { + r += reader.Read(b, 0, 2); + } + + return r; + } + [Benchmark] public int BufferedReadStreamWrapRead() { @@ -129,6 +163,20 @@ namespace SixLabors.ImageSharp.Benchmarks.IO return r; } + [Benchmark] + public int BufferedReadStreamChunkedReadByte() + { + int r = 0; + BufferedReadStream reader = this.bufferedStream4; + + for (int i = 0; i < reader.Length; i++) + { + r += reader.ReadByte(); + } + + return r; + } + [Benchmark] public int BufferedReadStreamWrapReadByte() { @@ -167,40 +215,46 @@ namespace SixLabors.ImageSharp.Benchmarks.IO } /* - BenchmarkDotNet=v0.12.0, OS=Windows 10.0.19041 + BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19041.450 (2004/?/20H1) Intel Core i7-8650U CPU 1.90GHz (Kaby Lake R), 1 CPU, 8 logical and 4 physical cores - .NET Core SDK=3.1.301 - [Host] : .NET Core 3.1.5 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.27001), X64 RyuJIT - Job-LKLBOT : .NET Framework 4.8 (4.8.4180.0), X64 RyuJIT - Job-RSTMKF : .NET Core 2.1.19 (CoreCLR 4.6.28928.01, CoreFX 4.6.28928.04), X64 RyuJIT - Job-PZIHIV : .NET Core 3.1.5 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.27001), X64 RyuJIT + .NET Core SDK=3.1.401 + [Host] : .NET Core 3.1.7 (CoreCLR 4.700.20.36602, CoreFX 4.700.20.37001), X64 RyuJIT + Job-OKZLUV : .NET Framework 4.8 (4.8.4084.0), X64 RyuJIT + Job-CPYMXV : .NET Core 2.1.21 (CoreCLR 4.6.29130.01, CoreFX 4.6.29130.02), X64 RyuJIT + Job-BSGVGU : .NET Core 3.1.7 (CoreCLR 4.700.20.36602, CoreFX 4.700.20.37001), X64 RyuJIT IterationCount=3 LaunchCount=1 WarmupCount=3 -| Method | Runtime | Mean | Error | StdDev | Ratio | RatioSD | Gen 0 | Gen 1 | Gen 2 | Allocated | -|------------------------------- |-------------- |----------:|------------:|-----------:|------:|--------:|------:|------:|------:|----------:| -| StandardStreamRead | .NET 4.7.2 | 63.238 us | 49.7827 us | 2.7288 us | 0.66 | 0.13 | - | - | - | - | -| BufferedReadStreamRead | .NET 4.7.2 | 66.092 us | 0.4273 us | 0.0234 us | 0.69 | 0.11 | - | - | - | - | -| BufferedReadStreamWrapRead | .NET 4.7.2 | 26.216 us | 3.0527 us | 0.1673 us | 0.27 | 0.04 | - | - | - | - | -| StandardStreamReadByte | .NET 4.7.2 | 97.900 us | 261.7204 us | 14.3458 us | 1.00 | 0.00 | - | - | - | - | -| BufferedReadStreamReadByte | .NET 4.7.2 | 97.260 us | 1.2979 us | 0.0711 us | 1.01 | 0.15 | - | - | - | - | -| BufferedReadStreamWrapReadByte | .NET 4.7.2 | 19.170 us | 2.2296 us | 0.1222 us | 0.20 | 0.03 | - | - | - | - | -| ArrayReadByte | .NET 4.7.2 | 12.878 us | 11.1292 us | 0.6100 us | 0.13 | 0.02 | - | - | - | - | -| | | | | | | | | | | | -| StandardStreamRead | .NET Core 2.1 | 60.618 us | 131.7038 us | 7.2191 us | 0.78 | 0.10 | - | - | - | - | -| BufferedReadStreamRead | .NET Core 2.1 | 30.006 us | 25.2499 us | 1.3840 us | 0.38 | 0.02 | - | - | - | - | -| BufferedReadStreamWrapRead | .NET Core 2.1 | 29.241 us | 6.5020 us | 0.3564 us | 0.37 | 0.01 | - | - | - | - | -| StandardStreamReadByte | .NET Core 2.1 | 78.074 us | 15.8463 us | 0.8686 us | 1.00 | 0.00 | - | - | - | - | -| BufferedReadStreamReadByte | .NET Core 2.1 | 14.737 us | 20.1510 us | 1.1045 us | 0.19 | 0.01 | - | - | - | - | -| BufferedReadStreamWrapReadByte | .NET Core 2.1 | 13.234 us | 1.4711 us | 0.0806 us | 0.17 | 0.00 | - | - | - | - | -| ArrayReadByte | .NET Core 2.1 | 9.373 us | 0.6108 us | 0.0335 us | 0.12 | 0.00 | - | - | - | - | -| | | | | | | | | | | | -| StandardStreamRead | .NET Core 3.1 | 52.151 us | 19.9456 us | 1.0933 us | 0.65 | 0.03 | - | - | - | - | -| BufferedReadStreamRead | .NET Core 3.1 | 29.217 us | 0.2490 us | 0.0136 us | 0.36 | 0.01 | - | - | - | - | -| BufferedReadStreamWrapRead | .NET Core 3.1 | 32.962 us | 7.1382 us | 0.3913 us | 0.41 | 0.02 | - | - | - | - | -| StandardStreamReadByte | .NET Core 3.1 | 80.310 us | 45.0350 us | 2.4685 us | 1.00 | 0.00 | - | - | - | - | -| BufferedReadStreamReadByte | .NET Core 3.1 | 13.092 us | 0.6268 us | 0.0344 us | 0.16 | 0.00 | - | - | - | - | -| BufferedReadStreamWrapReadByte | .NET Core 3.1 | 13.282 us | 3.8689 us | 0.2121 us | 0.17 | 0.01 | - | - | - | - | -| ArrayReadByte | .NET Core 3.1 | 9.349 us | 2.9860 us | 0.1637 us | 0.12 | 0.00 | - | - | - | - | + | Method | Job | Runtime | Mean | Error | StdDev | Ratio | RatioSD | Gen 0 | Gen 1 | Gen 2 | Allocated | + |---------------------------------- |----------- |-------------- |-----------:|----------:|----------:|------:|--------:|------:|------:|------:|----------:| + | StandardStreamRead | Job-OKZLUV | .NET 4.7.2 | 66.785 us | 15.768 us | 0.8643 us | 0.83 | 0.01 | - | - | - | - | + | BufferedReadStreamRead | Job-OKZLUV | .NET 4.7.2 | 97.389 us | 17.658 us | 0.9679 us | 1.21 | 0.01 | - | - | - | - | + | BufferedReadStreamChunkedRead | Job-OKZLUV | .NET 4.7.2 | 96.006 us | 16.286 us | 0.8927 us | 1.20 | 0.02 | - | - | - | - | + | BufferedReadStreamWrapRead | Job-OKZLUV | .NET 4.7.2 | 37.064 us | 14.640 us | 0.8024 us | 0.46 | 0.02 | - | - | - | - | + | StandardStreamReadByte | Job-OKZLUV | .NET 4.7.2 | 80.315 us | 26.676 us | 1.4622 us | 1.00 | 0.00 | - | - | - | - | + | BufferedReadStreamReadByte | Job-OKZLUV | .NET 4.7.2 | 118.706 us | 38.013 us | 2.0836 us | 1.48 | 0.00 | - | - | - | - | + | BufferedReadStreamChunkedReadByte | Job-OKZLUV | .NET 4.7.2 | 115.437 us | 33.352 us | 1.8282 us | 1.44 | 0.01 | - | - | - | - | + | BufferedReadStreamWrapReadByte | Job-OKZLUV | .NET 4.7.2 | 16.449 us | 11.400 us | 0.6249 us | 0.20 | 0.00 | - | - | - | - | + | ArrayReadByte | Job-OKZLUV | .NET 4.7.2 | 10.416 us | 1.866 us | 0.1023 us | 0.13 | 0.00 | - | - | - | - | + | | | | | | | | | | | | | + | StandardStreamRead | Job-CPYMXV | .NET Core 2.1 | 71.425 us | 50.441 us | 2.7648 us | 0.82 | 0.03 | - | - | - | - | + | BufferedReadStreamRead | Job-CPYMXV | .NET Core 2.1 | 32.816 us | 6.655 us | 0.3648 us | 0.38 | 0.01 | - | - | - | - | + | BufferedReadStreamChunkedRead | Job-CPYMXV | .NET Core 2.1 | 31.995 us | 7.751 us | 0.4249 us | 0.37 | 0.01 | - | - | - | - | + | BufferedReadStreamWrapRead | Job-CPYMXV | .NET Core 2.1 | 31.970 us | 4.170 us | 0.2286 us | 0.37 | 0.01 | - | - | - | - | + | StandardStreamReadByte | Job-CPYMXV | .NET Core 2.1 | 86.909 us | 18.565 us | 1.0176 us | 1.00 | 0.00 | - | - | - | - | + | BufferedReadStreamReadByte | Job-CPYMXV | .NET Core 2.1 | 14.596 us | 10.889 us | 0.5969 us | 0.17 | 0.01 | - | - | - | - | + | BufferedReadStreamChunkedReadByte | Job-CPYMXV | .NET Core 2.1 | 13.629 us | 1.569 us | 0.0860 us | 0.16 | 0.00 | - | - | - | - | + | BufferedReadStreamWrapReadByte | Job-CPYMXV | .NET Core 2.1 | 13.566 us | 1.743 us | 0.0956 us | 0.16 | 0.00 | - | - | - | - | + | ArrayReadByte | Job-CPYMXV | .NET Core 2.1 | 9.771 us | 6.658 us | 0.3650 us | 0.11 | 0.00 | - | - | - | - | + | | | | | | | | | | | | | + | StandardStreamRead | Job-BSGVGU | .NET Core 3.1 | 53.265 us | 65.819 us | 3.6078 us | 0.81 | 0.05 | - | - | - | - | + | BufferedReadStreamRead | Job-BSGVGU | .NET Core 3.1 | 33.163 us | 9.569 us | 0.5245 us | 0.51 | 0.01 | - | - | - | - | + | BufferedReadStreamChunkedRead | Job-BSGVGU | .NET Core 3.1 | 33.001 us | 6.114 us | 0.3351 us | 0.50 | 0.01 | - | - | - | - | + | BufferedReadStreamWrapRead | Job-BSGVGU | .NET Core 3.1 | 29.448 us | 7.120 us | 0.3902 us | 0.45 | 0.01 | - | - | - | - | + | StandardStreamReadByte | Job-BSGVGU | .NET Core 3.1 | 65.619 us | 6.732 us | 0.3690 us | 1.00 | 0.00 | - | - | - | - | + | BufferedReadStreamReadByte | Job-BSGVGU | .NET Core 3.1 | 13.989 us | 3.464 us | 0.1899 us | 0.21 | 0.00 | - | - | - | - | + | BufferedReadStreamChunkedReadByte | Job-BSGVGU | .NET Core 3.1 | 13.806 us | 1.710 us | 0.0938 us | 0.21 | 0.00 | - | - | - | - | + | BufferedReadStreamWrapReadByte | Job-BSGVGU | .NET Core 3.1 | 13.690 us | 1.523 us | 0.0835 us | 0.21 | 0.00 | - | - | - | - | + | ArrayReadByte | Job-BSGVGU | .NET Core 3.1 | 10.792 us | 8.228 us | 0.4510 us | 0.16 | 0.01 | - | - | - | - | */ } diff --git a/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs b/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs index 28f1d336d9..748dcb24ad 100644 --- a/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs +++ b/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs @@ -166,6 +166,48 @@ namespace SixLabors.ImageSharp.Tests.IO } } + [Fact] + public void MemoryStream_WriteToSpanTests() + { + using (var ms2 = new ChunkedMemoryStream(this.allocator)) + { + Span bytArrRet; + Span bytArr = new byte[] { byte.MinValue, byte.MaxValue, 1, 2, 3, 4, 5, 6, 128, 250 }; + + // [] Write to memoryStream, check the memoryStream + ms2.Write(bytArr, 0, bytArr.Length); + + using var readonlyStream = new ChunkedMemoryStream(this.allocator); + ms2.WriteTo(readonlyStream); + readonlyStream.Flush(); + readonlyStream.Position = 0; + bytArrRet = new byte[(int)readonlyStream.Length]; + readonlyStream.Read(bytArrRet, 0, (int)readonlyStream.Length); + for (int i = 0; i < bytArr.Length; i++) + { + Assert.Equal(bytArr[i], bytArrRet[i]); + } + } + + // [] Write to memoryStream, check the memoryStream + using (var ms2 = new ChunkedMemoryStream(this.allocator)) + using (var ms3 = new ChunkedMemoryStream(this.allocator)) + { + Span bytArrRet; + Span bytArr = new byte[] { byte.MinValue, byte.MaxValue, 1, 2, 3, 4, 5, 6, 128, 250 }; + + ms2.Write(bytArr, 0, bytArr.Length); + ms2.WriteTo(ms3); + ms3.Position = 0; + bytArrRet = new byte[(int)ms3.Length]; + ms3.Read(bytArrRet, 0, (int)ms3.Length); + for (int i = 0; i < bytArr.Length; i++) + { + Assert.Equal(bytArr[i], bytArrRet[i]); + } + } + } + [Fact] public void MemoryStream_WriteByteTests() { From b2607e170c1b2036347dbb7ce1e0fe7aa584ef18 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 19 Aug 2020 15:34:34 +0100 Subject: [PATCH 17/37] Add lightweight integration tests. --- .../General/IO/BufferedStreams.cs | 1 + .../Image/ImageTests.Identify.cs | 48 +++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/tests/ImageSharp.Benchmarks/General/IO/BufferedStreams.cs b/tests/ImageSharp.Benchmarks/General/IO/BufferedStreams.cs index eb1bc7a851..f2ff49d4e9 100644 --- a/tests/ImageSharp.Benchmarks/General/IO/BufferedStreams.cs +++ b/tests/ImageSharp.Benchmarks/General/IO/BufferedStreams.cs @@ -40,6 +40,7 @@ namespace SixLabors.ImageSharp.Benchmarks.IO this.stream5 = new MemoryStream(this.buffer); this.stream6 = new MemoryStream(this.buffer); this.stream6 = new MemoryStream(this.buffer); + this.chunkedMemoryStream1 = new ChunkedMemoryStream(Configuration.Default.MemoryAllocator); this.chunkedMemoryStream1.Write(this.buffer); this.chunkedMemoryStream1.Position = 0; diff --git a/tests/ImageSharp.Tests/Image/ImageTests.Identify.cs b/tests/ImageSharp.Tests/Image/ImageTests.Identify.cs index 72de3fcc44..3fbe1f70d8 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.Identify.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.Identify.cs @@ -89,6 +89,29 @@ namespace SixLabors.ImageSharp.Tests } } + [Fact] + public void FromNonSeekableStream_GlobalConfiguration() + { + using var stream = new MemoryStream(this.ActualImageBytes); + using var nonSeekableStream = new NonSeekableStream(stream); + + IImageInfo info = Image.Identify(nonSeekableStream, out IImageFormat type); + + Assert.NotNull(info); + Assert.Equal(ExpectedGlobalFormat, type); + } + + [Fact] + public void FromNonSeekableStream_GlobalConfiguration_NoFormat() + { + using var stream = new MemoryStream(this.ActualImageBytes); + using var nonSeekableStream = new NonSeekableStream(stream); + + IImageInfo info = Image.Identify(nonSeekableStream); + + Assert.NotNull(info); + } + [Fact] public void FromStream_CustomConfiguration() { @@ -140,6 +163,31 @@ namespace SixLabors.ImageSharp.Tests } } + [Fact] + public async Task FromNonSeekableStreamAsync_GlobalConfiguration_NoFormat() + { + using var stream = new MemoryStream(this.ActualImageBytes); + using var nonSeekableStream = new NonSeekableStream(stream); + + var asyncStream = new AsyncStreamWrapper(nonSeekableStream, () => false); + IImageInfo info = await Image.IdentifyAsync(asyncStream); + + Assert.NotNull(info); + } + + [Fact] + public async Task FromNonSeekableStreamAsync_GlobalConfiguration() + { + using var stream = new MemoryStream(this.ActualImageBytes); + using var nonSeekableStream = new NonSeekableStream(stream); + + var asyncStream = new AsyncStreamWrapper(nonSeekableStream, () => false); + (IImageInfo ImageInfo, IImageFormat Format) res = await Image.IdentifyWithFormatAsync(asyncStream); + + Assert.Equal(ExpectedImageSize, res.ImageInfo.Size()); + Assert.Equal(ExpectedGlobalFormat, res.Format); + } + [Fact] public async Task FromPathAsync_CustomConfiguration() { From 0d28a38f72d40831a4aa9807d5991185fc561095 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Wed, 19 Aug 2020 17:40:21 +0200 Subject: [PATCH 18/37] heavyweight integration test --- .../IO/ChunkedMemoryStreamTests.cs | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs b/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs index 748dcb24ad..822e71513b 100644 --- a/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs +++ b/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs @@ -4,8 +4,11 @@ using System; using System.Collections.Generic; using System.IO; +using System.Linq; using SixLabors.ImageSharp.IO; using SixLabors.ImageSharp.Memory; +using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; using Xunit; namespace SixLabors.ImageSharp.Tests.IO @@ -294,6 +297,42 @@ namespace SixLabors.ImageSharp.Tests.IO Assert.Equal(expected, destination.ToArray()); } + public static TheoryData GetAllTestImages() + { + IEnumerable allImageFiles = Directory.EnumerateFiles(TestEnvironment.InputImagesDirectoryFullPath, "*.*", SearchOption.AllDirectories) + .Where(s => !s.ToLower().EndsWith("txt")); + var result = new TheoryData(); + foreach (string path in allImageFiles) + { + result.Add(path); + } + + return result; + } + + [Theory] + [MemberData(nameof(GetAllTestImages))] + public void DecoderIntegrationTest(string testFileFullPath) + { + Image expected = null; + try + { + expected = Image.Load(testFileFullPath); + } + catch + { + // The image is invalid + return; + } + + using FileStream fs = File.OpenRead(testFileFullPath); + using NonSeekableStream nonSeekableStream = new NonSeekableStream(fs); + + var actual = Image.Load(nonSeekableStream); + + ImageComparer.Exact.VerifySimilarity(expected, actual); + } + public static IEnumerable CopyToData() { // Stream is positioned @ beginning of data From 0ff69248ecd033030cd80da9fdd0ef4f3f4310bf Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 19 Aug 2020 17:18:36 +0100 Subject: [PATCH 19/37] Fix read count. --- src/ImageSharp/IO/ChunkedMemoryStream.cs | 2 +- tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/IO/ChunkedMemoryStream.cs b/src/ImageSharp/IO/ChunkedMemoryStream.cs index 77a3174094..2d895245c7 100644 --- a/src/ImageSharp/IO/ChunkedMemoryStream.cs +++ b/src/ImageSharp/IO/ChunkedMemoryStream.cs @@ -295,7 +295,7 @@ namespace SixLabors.ImageSharp.IO } int readCount = Math.Min(count, chunkSize - this.readOffset); - chunkBuffer.Slice(this.readOffset, count).CopyTo(buffer.Slice(offset)); + chunkBuffer.Slice(this.readOffset, readCount).CopyTo(buffer.Slice(offset)); offset += readCount; count -= readCount; this.readOffset += readCount; diff --git a/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs b/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs index 822e71513b..ceef8f7ff5 100644 --- a/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs +++ b/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs @@ -300,7 +300,7 @@ namespace SixLabors.ImageSharp.Tests.IO public static TheoryData GetAllTestImages() { IEnumerable allImageFiles = Directory.EnumerateFiles(TestEnvironment.InputImagesDirectoryFullPath, "*.*", SearchOption.AllDirectories) - .Where(s => !s.ToLower().EndsWith("txt")); + .Where(s => !s.EndsWith("txt", StringComparison.OrdinalIgnoreCase)); var result = new TheoryData(); foreach (string path in allImageFiles) { @@ -314,7 +314,12 @@ namespace SixLabors.ImageSharp.Tests.IO [MemberData(nameof(GetAllTestImages))] public void DecoderIntegrationTest(string testFileFullPath) { - Image expected = null; + if (!TestEnvironment.Is64BitProcess) + { + return; + } + + Image expected; try { expected = Image.Load(testFileFullPath); @@ -326,7 +331,7 @@ namespace SixLabors.ImageSharp.Tests.IO } using FileStream fs = File.OpenRead(testFileFullPath); - using NonSeekableStream nonSeekableStream = new NonSeekableStream(fs); + using var nonSeekableStream = new NonSeekableStream(fs); var actual = Image.Load(nonSeekableStream); From 300273fd11dfb477eeef1ae0cf572e159fe30b75 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 21 Aug 2020 15:02:08 +0100 Subject: [PATCH 20/37] Use TestProvider to load images --- .../IO/ChunkedMemoryStreamTests.cs | 26 ++++++++++++------- .../ImageProviders/FileProvider.cs | 2 +- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs b/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs index ceef8f7ff5..00a178c8fd 100644 --- a/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs +++ b/tests/ImageSharp.Tests/IO/ChunkedMemoryStreamTests.cs @@ -297,32 +297,36 @@ namespace SixLabors.ImageSharp.Tests.IO Assert.Equal(expected, destination.ToArray()); } - public static TheoryData GetAllTestImages() + public static IEnumerable GetAllTestImages() { IEnumerable allImageFiles = Directory.EnumerateFiles(TestEnvironment.InputImagesDirectoryFullPath, "*.*", SearchOption.AllDirectories) .Where(s => !s.EndsWith("txt", StringComparison.OrdinalIgnoreCase)); - var result = new TheoryData(); + + var result = new List(); foreach (string path in allImageFiles) { - result.Add(path); + result.Add(path.Substring(TestEnvironment.InputImagesDirectoryFullPath.Length)); } return result; } + public static IEnumerable AllTestImages = GetAllTestImages(); + [Theory] - [MemberData(nameof(GetAllTestImages))] - public void DecoderIntegrationTest(string testFileFullPath) + [WithFileCollection(nameof(AllTestImages), PixelTypes.Rgba32)] + public void DecoderIntegrationTest(TestImageProvider provider) + where TPixel : unmanaged, IPixel { if (!TestEnvironment.Is64BitProcess) { return; } - Image expected; + Image expected; try { - expected = Image.Load(testFileFullPath); + expected = provider.GetImage(); } catch { @@ -330,10 +334,14 @@ namespace SixLabors.ImageSharp.Tests.IO return; } - using FileStream fs = File.OpenRead(testFileFullPath); + string fullPath = Path.Combine( + TestEnvironment.InputImagesDirectoryFullPath, + ((TestImageProvider.FileProvider)provider).FilePath); + + using FileStream fs = File.OpenRead(fullPath); using var nonSeekableStream = new NonSeekableStream(fs); - var actual = Image.Load(nonSeekableStream); + var actual = Image.Load(nonSeekableStream); ImageComparer.Exact.VerifySimilarity(expected, actual); } diff --git a/tests/ImageSharp.Tests/TestUtilities/ImageProviders/FileProvider.cs b/tests/ImageSharp.Tests/TestUtilities/ImageProviders/FileProvider.cs index 440baaa63b..f57c19f12a 100644 --- a/tests/ImageSharp.Tests/TestUtilities/ImageProviders/FileProvider.cs +++ b/tests/ImageSharp.Tests/TestUtilities/ImageProviders/FileProvider.cs @@ -17,7 +17,7 @@ namespace SixLabors.ImageSharp.Tests public abstract partial class TestImageProvider : IXunitSerializable where TPixel : unmanaged, IPixel { - private class FileProvider : TestImageProvider, IXunitSerializable + internal class FileProvider : TestImageProvider, IXunitSerializable { // Need PixelTypes in the dictionary key, because result images of TestImageProvider.FileProvider // are shared between PixelTypes.Color & PixelTypes.Rgba32 From b85243e0d263847b6915acf08942932ca8cd49f8 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 25 Aug 2020 16:09:25 +0100 Subject: [PATCH 21/37] Bump ChunkedMemoryStream buffer length --- README.md | 2 +- src/ImageSharp/IO/ChunkedMemoryStream.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index adf9647cc2..6b2fa5d0f5 100644 --- a/README.md +++ b/README.md @@ -49,7 +49,7 @@ Install stable releases via Nuget; development releases are available via MyGet. | Package Name | Release (NuGet) | Nightly (MyGet) | |--------------------------------|-----------------|-----------------| -| `SixLabors.ImageSharp` | [![NuGet](https://img.shields.io/nuget/v/SixLabors.ImageSharp.svg)](https://www.nuget.org/packages/SixLabors.ImageSharp/) | [![MyGet](https://img.shields.io/myget/sixlabors/v/SixLabors.ImageSharp.svg)](https://www.myget.org/feed/sixlabors/package/nuget/SixLabors.ImageSharp) | +| `SixLabors.ImageSharp` | [![NuGet](https://img.shields.io/nuget/v/SixLabors.ImageSharp.svg)](https://www.nuget.org/packages/SixLabors.ImageSharp/) | [![MyGet](https://img.shields.io/myget/sixlabors/vpre/SixLabors.ImageSharp.svg)](https://www.myget.org/feed/sixlabors/package/nuget/SixLabors.ImageSharp) | ## Manual build diff --git a/src/ImageSharp/IO/ChunkedMemoryStream.cs b/src/ImageSharp/IO/ChunkedMemoryStream.cs index 2d895245c7..9a2d75276c 100644 --- a/src/ImageSharp/IO/ChunkedMemoryStream.cs +++ b/src/ImageSharp/IO/ChunkedMemoryStream.cs @@ -18,7 +18,7 @@ namespace SixLabors.ImageSharp.IO /// /// The default length in bytes of each buffer chunk. /// - public const int DefaultBufferLength = 81920; + public const int DefaultBufferLength = 128 * 1024; // The memory allocator. private readonly MemoryAllocator allocator; From 29d133c89a189fdaf56c35641b5f18447aa97796 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 25 Aug 2020 17:39:52 +0100 Subject: [PATCH 22/37] Fix build --- .github/workflows/build-and-test.yml | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index a83e194234..412b1d8074 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -52,11 +52,6 @@ jobs: git fetch --prune --unshallow git submodule -q update --init --recursive - - name: Setup DotNet SDK - uses: actions/setup-dotnet@v1 - with: - dotnet-version: "3.1.x" - - name: Build shell: pwsh run: ./ci-build.ps1 "${{matrix.options.framework}}" @@ -95,11 +90,6 @@ jobs: git fetch --prune --unshallow git submodule -q update --init --recursive - - name: Setup DotNet SDK - uses: actions/setup-dotnet@v1 - with: - dotnet-version: "3.1.x" - - name: Pack shell: pwsh run: ./ci-pack.ps1 From 6d90af1a8bcdc53228f714fbe63c9d0e8319849d Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 25 Aug 2020 19:26:30 +0100 Subject: [PATCH 23/37] Remove Guard allocations --- src/ImageSharp/IO/ChunkedMemoryStream.cs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/IO/ChunkedMemoryStream.cs b/src/ImageSharp/IO/ChunkedMemoryStream.cs index 9a2d75276c..c5fc6b9395 100644 --- a/src/ImageSharp/IO/ChunkedMemoryStream.cs +++ b/src/ImageSharp/IO/ChunkedMemoryStream.cs @@ -238,7 +238,9 @@ namespace SixLabors.ImageSharp.IO Guard.NotNull(buffer, nameof(buffer)); Guard.MustBeGreaterThanOrEqualTo(offset, 0, nameof(offset)); Guard.MustBeGreaterThanOrEqualTo(count, 0, nameof(count)); - Guard.IsFalse(buffer.Length - offset < count, nameof(buffer), $"{offset} subtracted from the buffer length is less than {count}"); + + const string BufferMessage = "Offset subtracted from the buffer length is less than count."; + Guard.IsFalse(buffer.Length - offset < count, nameof(buffer), BufferMessage); return this.ReadImpl(buffer.AsSpan().Slice(offset, count)); } @@ -348,7 +350,16 @@ namespace SixLabors.ImageSharp.IO /// [MethodImpl(MethodImplOptions.AggressiveInlining)] public override void Write(byte[] buffer, int offset, int count) - => this.WriteImpl(buffer.AsSpan().Slice(offset, count)); + { + Guard.NotNull(buffer, nameof(buffer)); + Guard.MustBeGreaterThanOrEqualTo(offset, 0, nameof(offset)); + Guard.MustBeGreaterThanOrEqualTo(count, 0, nameof(count)); + + const string BufferMessage = "Offset subtracted from the buffer length is less than count."; + Guard.IsFalse(buffer.Length - offset < count, nameof(buffer), BufferMessage); + + this.WriteImpl(buffer.AsSpan().Slice(offset, count)); + } #if SUPPORTS_SPAN_STREAM /// From 575402d3312be739e0e16d76e3d550589774c2e9 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 25 Aug 2020 22:35:10 +0200 Subject: [PATCH 24/37] Add input size validation in Image.WrapMemory --- src/ImageSharp/Image.WrapMemory.cs | 5 ++ .../Image/ImageTests.WrapMemory.cs | 51 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/src/ImageSharp/Image.WrapMemory.cs b/src/ImageSharp/Image.WrapMemory.cs index 34bdeafdcb..081ed22a14 100644 --- a/src/ImageSharp/Image.WrapMemory.cs +++ b/src/ImageSharp/Image.WrapMemory.cs @@ -38,6 +38,7 @@ namespace SixLabors.ImageSharp { Guard.NotNull(configuration, nameof(configuration)); Guard.NotNull(metadata, nameof(metadata)); + Guard.IsTrue(pixelMemory.Length == width * height, nameof(pixelMemory), "The length of the input memory doesn't match the specified image size"); var memorySource = MemoryGroup.Wrap(pixelMemory); return new Image(configuration, memorySource, width, height, metadata); @@ -105,6 +106,7 @@ namespace SixLabors.ImageSharp { Guard.NotNull(configuration, nameof(configuration)); Guard.NotNull(metadata, nameof(metadata)); + Guard.IsTrue(pixelMemoryOwner.Memory.Length == width * height, nameof(pixelMemoryOwner), "The length of the input memory doesn't match the specified image size"); var memorySource = MemoryGroup.Wrap(pixelMemoryOwner); return new Image(configuration, memorySource, width, height, metadata); @@ -176,6 +178,9 @@ namespace SixLabors.ImageSharp Guard.NotNull(metadata, nameof(metadata)); var memoryManager = new ByteMemoryManager(byteMemory); + + Guard.IsTrue(memoryManager.Memory.Length == width * height, nameof(byteMemory), "The length of the input memory doesn't match the specified image size"); + var memorySource = MemoryGroup.Wrap(memoryManager.Memory); return new Image(configuration, memorySource, width, height, metadata); } diff --git a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs index bb22f59a3a..7dc7dbb30c 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs @@ -282,6 +282,57 @@ namespace SixLabors.ImageSharp.Tests } } + [Theory] + [InlineData(0, 5, 5)] + [InlineData(20, 5, 5)] + [InlineData(26, 5, 5)] + [InlineData(2, 1, 1)] + [InlineData(1023, 32, 32)] + public void WrapMemory_MemoryOfT_InvalidSize(int size, int height, int width) + { + var array = new Rgba32[size]; + var memory = new Memory(array); + + Assert.Throws(() => Image.WrapMemory(memory, height, width)); + } + + private class TestMemoryOwner : IMemoryOwner + { + public Memory Memory { get; set; } + + public void Dispose() + { + } + } + + [Theory] + [InlineData(0, 5, 5)] + [InlineData(20, 5, 5)] + [InlineData(26, 5, 5)] + [InlineData(2, 1, 1)] + [InlineData(1023, 32, 32)] + public void WrapMemory_IMemoryOwnerOfT_InvalidSize(int size, int height, int width) + { + var array = new Rgba32[size]; + var memory = new TestMemoryOwner { Memory = array }; + + Assert.Throws(() => Image.WrapMemory(memory, height, width)); + } + + [Theory] + [InlineData(0, 5, 5)] + [InlineData(20, 5, 5)] + [InlineData(26, 5, 5)] + [InlineData(2, 1, 1)] + [InlineData(1023, 32, 32)] + public void WrapMemory_MemoryOfByte_InvalidSize(int size, int height, int width) + { + var array = new byte[size * Unsafe.SizeOf()]; + var memory = new Memory(array); + + Assert.Throws(() => Image.WrapMemory(memory, height, width)); + } + private static bool ShouldSkipBitmapTest => !TestEnvironment.Is64BitProcess || (TestHelpers.ImageSharpBuiltAgainst != "netcoreapp3.1" && TestHelpers.ImageSharpBuiltAgainst != "netcoreapp2.1"); } From 5397ab328fa51c5a0d07924b9bdec9773fd0d2d3 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 25 Aug 2020 22:39:21 +0200 Subject: [PATCH 25/37] Minor optimizations, improve XML docs and annotations --- .../Memory/MemoryOwnerExtensions.cs | 37 +++++++++++++++++-- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Memory/MemoryOwnerExtensions.cs b/src/ImageSharp/Memory/MemoryOwnerExtensions.cs index 98fd40e65b..0cff94110c 100644 --- a/src/ImageSharp/Memory/MemoryOwnerExtensions.cs +++ b/src/ImageSharp/Memory/MemoryOwnerExtensions.cs @@ -3,6 +3,7 @@ using System; using System.Buffers; +using System.Diagnostics.Contracts; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -13,12 +14,29 @@ namespace SixLabors.ImageSharp.Memory /// internal static class MemoryOwnerExtensions { + /// + /// Gets a from an instance. + /// + /// The buffer + /// The + [Pure] + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Span GetSpan(this IMemoryOwner buffer) - => buffer.Memory.Span; + { + return buffer.Memory.Span; + } + /// + /// Gets the length of an internal buffer. + /// + /// The buffer + /// The length of the buffer + [Pure] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static int Length(this IMemoryOwner buffer) - => buffer.GetSpan().Length; + { + return buffer.Memory.Length; + } /// /// Gets a to an offsetted position inside the buffer. @@ -26,6 +44,7 @@ namespace SixLabors.ImageSharp.Memory /// The buffer /// The start /// The + [Pure] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Span Slice(this IMemoryOwner buffer, int start) { @@ -39,6 +58,7 @@ namespace SixLabors.ImageSharp.Memory /// The start /// The length of the slice /// The + [Pure] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Span Slice(this IMemoryOwner buffer, int start, int length) { @@ -55,8 +75,17 @@ namespace SixLabors.ImageSharp.Memory buffer.GetSpan().Clear(); } + /// + /// Gets a reference to the first item in the internal buffer for an instance. + /// + /// The buffer + /// A reference to the first item within the memory wrapped by + [Pure] + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static ref T GetReference(this IMemoryOwner buffer) - where T : struct => - ref MemoryMarshal.GetReference(buffer.GetSpan()); + where T : struct + { + return ref MemoryMarshal.GetReference(buffer.GetSpan()); + } } } From 88a93fafe2b4f7db2086561e6195d3998533be9c Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 25 Aug 2020 22:41:43 +0200 Subject: [PATCH 26/37] Remove unnecessary Memory.Span access --- src/ImageSharp/Processing/Processors/Dithering/OrderedDither.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Processing/Processors/Dithering/OrderedDither.cs b/src/ImageSharp/Processing/Processors/Dithering/OrderedDither.cs index b11411e32a..448eb3833b 100644 --- a/src/ImageSharp/Processing/Processors/Dithering/OrderedDither.cs +++ b/src/ImageSharp/Processing/Processors/Dithering/OrderedDither.cs @@ -262,7 +262,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Dithering this.source = source; this.bounds = bounds; this.scale = processor.DitherScale; - this.bitDepth = ImageMaths.GetBitsNeededForColorDepth(processor.Palette.Span.Length); + this.bitDepth = ImageMaths.GetBitsNeededForColorDepth(processor.Palette.Length); } [MethodImpl(InliningOptions.ShortMethod)] From 16c9dba5bc4ed336d375efd9270f0a4f8bd54413 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Wed, 26 Aug 2020 01:25:02 +0200 Subject: [PATCH 27/37] Simplify XML docs for WrapMemory APIs --- src/ImageSharp/Image.WrapMemory.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/ImageSharp/Image.WrapMemory.cs b/src/ImageSharp/Image.WrapMemory.cs index 081ed22a14..d89c44dc56 100644 --- a/src/ImageSharp/Image.WrapMemory.cs +++ b/src/ImageSharp/Image.WrapMemory.cs @@ -17,7 +17,7 @@ namespace SixLabors.ImageSharp { /// /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, - /// allowing to view/manipulate it as an ImageSharp instance. + /// allowing to view/manipulate it as an instance. /// /// The pixel type /// The @@ -46,7 +46,7 @@ namespace SixLabors.ImageSharp /// /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, - /// allowing to view/manipulate it as an ImageSharp instance. + /// allowing to view/manipulate it as an instance. /// /// The pixel type /// The @@ -65,7 +65,7 @@ namespace SixLabors.ImageSharp /// /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, - /// allowing to view/manipulate it as an ImageSharp instance. + /// allowing to view/manipulate it as an instance. /// The memory is being observed, the caller remains responsible for managing it's lifecycle. /// /// The pixel type. @@ -82,7 +82,7 @@ namespace SixLabors.ImageSharp /// /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, - /// allowing to view/manipulate it as an ImageSharp instance. + /// allowing to view/manipulate it as an instance. /// The ownership of the is being transferred to the new instance, /// meaning that the caller is not allowed to dispose . /// It will be disposed together with the result image. @@ -114,7 +114,7 @@ namespace SixLabors.ImageSharp /// /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, - /// allowing to view/manipulate it as an ImageSharp instance. + /// allowing to view/manipulate it as an instance. /// The ownership of the is being transferred to the new instance, /// meaning that the caller is not allowed to dispose . /// It will be disposed together with the result image. @@ -136,7 +136,7 @@ namespace SixLabors.ImageSharp /// /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, - /// allowing to view/manipulate it as an ImageSharp instance. + /// allowing to view/manipulate it as an instance. /// The ownership of the is being transferred to the new instance, /// meaning that the caller is not allowed to dispose . /// It will be disposed together with the result image. @@ -155,7 +155,7 @@ namespace SixLabors.ImageSharp /// /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, - /// allowing to view/manipulate it as an ImageSharp instance. + /// allowing to view/manipulate it as an instance. /// /// The pixel type /// The @@ -187,7 +187,7 @@ namespace SixLabors.ImageSharp /// /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, - /// allowing to view/manipulate it as an ImageSharp instance. + /// allowing to view/manipulate it as an instance. /// /// The pixel type /// The @@ -206,7 +206,7 @@ namespace SixLabors.ImageSharp /// /// Wraps an existing contiguous memory area of 'width' x 'height' pixels, - /// allowing to view/manipulate it as an ImageSharp instance. + /// allowing to view/manipulate it as an instance. /// The memory is being observed, the caller remains responsible for managing it's lifecycle. /// /// The pixel type. From c061df958422b527edb5de248ad26fdb73f33e6f Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sun, 30 Aug 2020 13:04:03 +0200 Subject: [PATCH 28/37] Added unit tests for bokeh blur constructor --- .../Processors/Convolution/BokehBlurTest.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/ImageSharp.Tests/Processing/Processors/Convolution/BokehBlurTest.cs b/tests/ImageSharp.Tests/Processing/Processors/Convolution/BokehBlurTest.cs index 490a6ea493..50b8782e47 100644 --- a/tests/ImageSharp.Tests/Processing/Processors/Convolution/BokehBlurTest.cs +++ b/tests/ImageSharp.Tests/Processing/Processors/Convolution/BokehBlurTest.cs @@ -35,6 +35,19 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Convolution 0.02565295+0.01611732j 0.0153483+0.01605112j 0.00698622+0.01370844j 0.00135338+0.00998296j -0.00152245+0.00604545j -0.00227282+0.002851j ]]"; + [Theory] + [InlineData(-10, 2, 3f)] + [InlineData(-1, 2, 3f)] + [InlineData(0, 2, 3f)] + [InlineData(20, -1, 3f)] + [InlineData(20, -0, 3f)] + [InlineData(20, 4, -10f)] + [InlineData(20, 4, 0f)] + public void VerifyBokehBlurProcessorArguments_Fail(int radius, int components, float gamma) + { + Assert.Throws(() => new BokehBlurProcessor(radius, components, gamma)); + } + [Fact] public void VerifyComplexComponents() { From 141abca768c45352a24b2d31bab7c37ba86079a3 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sun, 30 Aug 2020 13:07:06 +0200 Subject: [PATCH 29/37] Added checks to bokeh blur constructor --- .../Processing/Processors/Convolution/BokehBlurProcessor.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor.cs b/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor.cs index 27eca523c0..d329641e52 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor.cs @@ -47,6 +47,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution /// public BokehBlurProcessor(int radius, int components, float gamma) { + Guard.MustBeGreaterThan(radius, 0, nameof(radius)); + Guard.MustBeBetweenOrEqualTo(components, 1, 6, nameof(components)); Guard.MustBeGreaterThanOrEqualTo(gamma, 1, nameof(gamma)); this.Radius = radius; From c69a41fe626e651298a0b56d107fef66d0aa9009 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sun, 30 Aug 2020 13:07:36 +0200 Subject: [PATCH 30/37] Skipped checks in default bokeh blur constructor --- .../Processing/Processors/Convolution/BokehBlurProcessor.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor.cs b/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor.cs index d329641e52..8a4c703e0c 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor.cs @@ -29,8 +29,10 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution /// Initializes a new instance of the class. /// public BokehBlurProcessor() - : this(DefaultRadius, DefaultComponents, DefaultGamma) { + this.Radius = DefaultRadius; + this.Components = DefaultComponents; + this.Gamma = DefaultGamma; } /// From f446f2513c3168d206a5f8874789656591451b37 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sun, 30 Aug 2020 13:37:30 +0200 Subject: [PATCH 31/37] Improved codegen in ImageSharp.Guard --- src/ImageSharp/Common/Helpers/Guard.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Common/Helpers/Guard.cs b/src/ImageSharp/Common/Helpers/Guard.cs index 751920683e..8ce6c4b4d6 100644 --- a/src/ImageSharp/Common/Helpers/Guard.cs +++ b/src/ImageSharp/Common/Helpers/Guard.cs @@ -20,10 +20,12 @@ namespace SixLabors [MethodImpl(InliningOptions.ShortMethod)] public static void MustBeValueType(TValue value, string parameterName) { - if (!value.GetType().GetTypeInfo().IsValueType) + if (value.GetType().GetTypeInfo().IsValueType) { - ThrowHelper.ThrowArgumentException("Type must be a struct.", parameterName); + return; } + + ThrowHelper.ThrowArgumentException("Type must be a struct.", parameterName); } } } From f0e8a0f6f502aa2c81352308cc5dd4d9d0aa3083 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 1 Sep 2020 00:23:53 +0100 Subject: [PATCH 32/37] Add missing SaveAsync method --- .../Advanced/AdvancedImageExtensions.cs | 4 +- src/ImageSharp/ImageExtensions.cs | 63 ++++++++++++++++--- .../Image/ImageTests.SaveAsync.cs | 31 +++++++++ 3 files changed, 90 insertions(+), 8 deletions(-) diff --git a/src/ImageSharp/Advanced/AdvancedImageExtensions.cs b/src/ImageSharp/Advanced/AdvancedImageExtensions.cs index c5abbda61b..54a773be05 100644 --- a/src/ImageSharp/Advanced/AdvancedImageExtensions.cs +++ b/src/ImageSharp/Advanced/AdvancedImageExtensions.cs @@ -23,7 +23,9 @@ namespace SixLabors.ImageSharp.Advanced /// /// The source image. /// The target file path to save the image to. - /// The matching encoder. + /// The file path is null. + /// No encoder available for provided path. + /// The matching . public static IImageEncoder DetectEncoder(this Image source, string filePath) { Guard.NotNull(filePath, nameof(filePath)); diff --git a/src/ImageSharp/ImageExtensions.cs b/src/ImageSharp/ImageExtensions.cs index d40c5c271b..75cd321067 100644 --- a/src/ImageSharp/ImageExtensions.cs +++ b/src/ImageSharp/ImageExtensions.cs @@ -18,27 +18,29 @@ namespace SixLabors.ImageSharp public static partial class ImageExtensions { /// - /// Writes the image to the given stream using the currently loaded image format. + /// Writes the image to the given file path using an encoder detected from the path. /// /// The source image. /// The file path to save the image to. /// The path is null. + /// No encoder available for provided path. public static void Save(this Image source, string path) => source.Save(path, source.DetectEncoder(path)); /// - /// Writes the image to the given stream using the currently loaded image format. + /// Writes the image to the given file path using an encoder detected from the path. /// /// The source image. /// The file path to save the image to. /// The token to monitor for cancellation requests. /// The path is null. + /// No encoder available for provided path. /// A representing the asynchronous operation. public static Task SaveAsync(this Image source, string path, CancellationToken cancellationToken = default) => source.SaveAsync(path, source.DetectEncoder(path), cancellationToken); /// - /// Writes the image to the given stream using the currently loaded image format. + /// Writes the image to the given file path using the given image encoder. /// /// The source image. /// The file path to save the image to. @@ -56,7 +58,7 @@ namespace SixLabors.ImageSharp } /// - /// Writes the image to the given stream using the currently loaded image format. + /// Writes the image to the given file path using the given image encoder. /// /// The source image. /// The file path to save the image to. @@ -73,12 +75,15 @@ namespace SixLabors.ImageSharp { Guard.NotNull(path, nameof(path)); Guard.NotNull(encoder, nameof(encoder)); - using Stream fs = source.GetConfiguration().FileSystem.Create(path); - await source.SaveAsync(fs, encoder, cancellationToken).ConfigureAwait(false); + + using (Stream fs = source.GetConfiguration().FileSystem.Create(path)) + { + await source.SaveAsync(fs, encoder, cancellationToken).ConfigureAwait(false); + } } /// - /// Writes the image to the given stream using the currently loaded image format. + /// Writes the image to the given stream using the given image format. /// /// The source image. /// The stream to save the image to. @@ -115,6 +120,50 @@ namespace SixLabors.ImageSharp source.Save(stream, encoder); } + /// + /// Writes the image to the given stream using the given image format. + /// + /// The source image. + /// The stream to save the image to. + /// The format to save the image in. + /// The token to monitor for cancellation requests. + /// The stream is null. + /// The format is null. + /// The stream is not writable. + /// No encoder available for provided format. + /// A representing the asynchronous operation. + public static Task SaveAsync( + this Image source, + Stream stream, + IImageFormat format, + CancellationToken cancellationToken = default) + { + Guard.NotNull(stream, nameof(stream)); + Guard.NotNull(format, nameof(format)); + + if (!stream.CanWrite) + { + throw new NotSupportedException("Cannot write to the stream."); + } + + IImageEncoder encoder = source.GetConfiguration().ImageFormatsManager.FindEncoder(format); + + if (encoder is null) + { + var sb = new StringBuilder(); + sb.AppendLine("No encoder was found for the provided mime type. Registered encoders include:"); + + foreach (KeyValuePair val in source.GetConfiguration().ImageFormatsManager.ImageEncoders) + { + sb.AppendFormat(" - {0} : {1}{2}", val.Key.Name, val.Value.GetType().Name, Environment.NewLine); + } + + throw new NotSupportedException(sb.ToString()); + } + + return source.SaveAsync(stream, encoder, cancellationToken); + } + /// /// Returns a Base64 encoded string from the given image. /// The result is prepended with a Data URI diff --git a/tests/ImageSharp.Tests/Image/ImageTests.SaveAsync.cs b/tests/ImageSharp.Tests/Image/ImageTests.SaveAsync.cs index 40c3b65b53..4e6b002d0d 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.SaveAsync.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.SaveAsync.cs @@ -72,6 +72,37 @@ namespace SixLabors.ImageSharp.Tests } } + [Theory] + [InlineData("test.png", "image/png")] + [InlineData("test.tga", "image/tga")] + [InlineData("test.bmp", "image/bmp")] + [InlineData("test.jpg", "image/jpeg")] + [InlineData("test.gif", "image/gif")] + public async Task SaveStreamWithMime(string filename, string mimeType) + { + using (var image = new Image(5, 5)) + { + string ext = Path.GetExtension(filename); + IImageFormat format = image.GetConfiguration().ImageFormatsManager.FindFormatByFileExtension(ext); + Assert.Equal(mimeType, format.DefaultMimeType); + + using (var stream = new MemoryStream()) + { + var asyncStream = new AsyncStreamWrapper(stream, () => false); + await image.SaveAsync(asyncStream, format); + + stream.Position = 0; + + (Image Image, IImageFormat Format) imf = await Image.LoadWithFormatAsync(stream); + + Assert.Equal(format, imf.Format); + Assert.Equal(mimeType, imf.Format.DefaultMimeType); + + imf.Image.Dispose(); + } + } + } + [Fact] public async Task ThrowsWhenDisposed() { From 1aa26a0e17709f7a6d51f07c2fb5c87b85445fed Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 1 Sep 2020 07:48:17 +0100 Subject: [PATCH 33/37] Fix Codecov --- .github/workflows/build-and-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 412b1d8074..0e093a8347 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -64,7 +64,7 @@ jobs: XUNIT_PATH: .\tests\ImageSharp.Tests # Required for xunit - name: Update Codecov - uses: codecov/codecov-action@v1.0.7 + uses: codecov/codecov-action@v1 if: matrix.options.codecov == true && startsWith(github.repository, 'SixLabors') with: flags: unittests From 34522eb62cdecaa18069493bad9d14e59bd810e0 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 1 Sep 2020 13:18:45 +0200 Subject: [PATCH 34/37] Improved Guard.MustBeValueType codegen Can now be JITted to a constant on .NET 5, see https://github.com/dotnet/runtime/pull/1157 --- src/ImageSharp/Common/Helpers/Guard.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/ImageSharp/Common/Helpers/Guard.cs b/src/ImageSharp/Common/Helpers/Guard.cs index 8ce6c4b4d6..0b5cc21cb8 100644 --- a/src/ImageSharp/Common/Helpers/Guard.cs +++ b/src/ImageSharp/Common/Helpers/Guard.cs @@ -2,7 +2,9 @@ // Licensed under the Apache License, Version 2.0. using System; +#if NETSTANDARD1_3 using System.Reflection; +#endif using System.Runtime.CompilerServices; using SixLabors.ImageSharp; @@ -20,7 +22,11 @@ namespace SixLabors [MethodImpl(InliningOptions.ShortMethod)] public static void MustBeValueType(TValue value, string parameterName) { - if (value.GetType().GetTypeInfo().IsValueType) + if (value.GetType() +#if NETSTANDARD1_3 + .GetTypeInfo() +#endif + .IsValueType) { return; } From d6e3f9fbbc02715c14139075805d9d84a68dc885 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 1 Sep 2020 13:21:56 +0200 Subject: [PATCH 35/37] Remove [Pure] attributes --- src/ImageSharp/Memory/MemoryOwnerExtensions.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/ImageSharp/Memory/MemoryOwnerExtensions.cs b/src/ImageSharp/Memory/MemoryOwnerExtensions.cs index 0cff94110c..c2551ccf2c 100644 --- a/src/ImageSharp/Memory/MemoryOwnerExtensions.cs +++ b/src/ImageSharp/Memory/MemoryOwnerExtensions.cs @@ -3,7 +3,6 @@ using System; using System.Buffers; -using System.Diagnostics.Contracts; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -19,7 +18,6 @@ namespace SixLabors.ImageSharp.Memory /// /// The buffer /// The - [Pure] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Span GetSpan(this IMemoryOwner buffer) { @@ -31,7 +29,6 @@ namespace SixLabors.ImageSharp.Memory /// /// The buffer /// The length of the buffer - [Pure] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static int Length(this IMemoryOwner buffer) { @@ -44,7 +41,6 @@ namespace SixLabors.ImageSharp.Memory /// The buffer /// The start /// The - [Pure] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Span Slice(this IMemoryOwner buffer, int start) { @@ -58,7 +54,6 @@ namespace SixLabors.ImageSharp.Memory /// The start /// The length of the slice /// The - [Pure] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Span Slice(this IMemoryOwner buffer, int start, int length) { @@ -80,7 +75,6 @@ namespace SixLabors.ImageSharp.Memory /// /// The buffer /// A reference to the first item within the memory wrapped by - [Pure] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static ref T GetReference(this IMemoryOwner buffer) where T : struct From 163a3eb6ca571cdbd0fd92aedb032aadc059241a Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 1 Sep 2020 17:42:31 +0100 Subject: [PATCH 36/37] Fix compatibility with NET 5 SDK InternalsVisibleTo --- src/Directory.Build.props | 8 ++++---- src/Directory.Build.targets | 4 ++-- tests/ImageSharp.Tests/ImageSharp.Tests.csproj | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Directory.Build.props b/src/Directory.Build.props index bdf1ff49cb..650f30fe1c 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -41,10 +41,10 @@ - - - - + + + + diff --git a/src/Directory.Build.targets b/src/Directory.Build.targets index 1eeedecd2c..d1875262d3 100644 --- a/src/Directory.Build.targets +++ b/src/Directory.Build.targets @@ -46,10 +46,10 @@ Condition="'$(Language)' == 'VB' or '$(Language)' == 'C#'" Inputs="$(MSBuildAllProjects)" Outputs="$(GeneratedInternalsVisibleToFile)"> - + - + diff --git a/tests/ImageSharp.Tests/ImageSharp.Tests.csproj b/tests/ImageSharp.Tests/ImageSharp.Tests.csproj index 0761b0978d..ba849ab251 100644 --- a/tests/ImageSharp.Tests/ImageSharp.Tests.csproj +++ b/tests/ImageSharp.Tests/ImageSharp.Tests.csproj @@ -14,7 +14,7 @@ - + From dd66efa5d8358d57304046f7f9778013ced28ed6 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 4 Sep 2020 14:13:42 +0100 Subject: [PATCH 37/37] Use interest for target bounds. Fixes #1342 --- .../Resize/ResizeProcessor{TPixel}.cs | 16 ++++++++----- .../Processors/Transforms/ResizeTests.cs | 24 +++++++++++++++++++ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs index 9908d4f799..3c033d89e4 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs @@ -147,6 +147,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms var operation = new NNRowOperation( sourceRectangle, destinationRectangle, + interest, widthFactor, heightFactor, source, @@ -197,6 +198,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms { private readonly Rectangle sourceBounds; private readonly Rectangle destinationBounds; + private readonly Rectangle interest; private readonly float widthFactor; private readonly float heightFactor; private readonly ImageFrame source; @@ -206,6 +208,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms public NNRowOperation( Rectangle sourceBounds, Rectangle destinationBounds, + Rectangle interest, float widthFactor, float heightFactor, ImageFrame source, @@ -213,6 +216,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms { this.sourceBounds = sourceBounds; this.destinationBounds = destinationBounds; + this.interest = interest; this.widthFactor = widthFactor; this.heightFactor = heightFactor; this.source = source; @@ -224,19 +228,19 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms { int sourceX = this.sourceBounds.X; int sourceY = this.sourceBounds.Y; - int destX = this.destinationBounds.X; - int destY = this.destinationBounds.Y; - int destLeft = this.destinationBounds.Left; - int destRight = this.destinationBounds.Right; + int destOriginX = this.destinationBounds.X; + int destOriginY = this.destinationBounds.Y; + int destLeft = this.interest.Left; + int destRight = this.interest.Right; // Y coordinates of source points - Span sourceRow = this.source.GetPixelRowSpan((int)(((y - destY) * this.heightFactor) + sourceY)); + Span sourceRow = this.source.GetPixelRowSpan((int)(((y - destOriginY) * this.heightFactor) + sourceY)); Span targetRow = this.destination.GetPixelRowSpan(y); for (int x = destLeft; x < destRight; x++) { // X coordinates of source points - targetRow[x] = sourceRow[(int)(((x - destX) * this.widthFactor) + sourceX)]; + targetRow[x] = sourceRow[(int)(((x - destOriginX) * this.widthFactor) + sourceX)]; } } } diff --git a/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeTests.cs b/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeTests.cs index 51b8ee0264..f40b8d11a0 100644 --- a/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeTests.cs +++ b/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeTests.cs @@ -621,5 +621,29 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Transforms })); } } + + [Theory] + [InlineData(1, 1)] + [InlineData(4, 6)] + [InlineData(2, 10)] + [InlineData(8, 1)] + [InlineData(3, 7)] + public void Issue1342(int width, int height) + { + using (var image = new Image(1, 1)) + { + var size = new Size(width, height); + image.Mutate(x => x + .Resize( + new ResizeOptions + { + Size = size, + Sampler = KnownResamplers.NearestNeighbor + })); + + Assert.Equal(width, image.Width); + Assert.Equal(height, image.Height); + } + } } }