Imagesharp: System.Memory API integration

[antonfirsov]

Goals

Thanks to #558, we are now able to expose System.Memory API-s allowing efficient interop operations. However, we need to distinguish between different use-cases, because they raise design problems of different difficulty levels. In some use-cases we have to decide whether we should expose Span<T>, Memory<T> or both.

"Official" guideline on System.Memory

Managed to find a WIP (?) documentation on Memory<T>. (It was mentioned in coreclr/#17340)

• Memory API documentation and samples
• Memory usage guidelines

Tasks

• [x] Basic ReadOnlySpan<T> overloads of LoadPixelData (done in #567)
• [x] ReadOnlySpan<T> overloads for Image.Load() and Image.DetectFormat(). See @jherby2k's proposal.
  
  
  
  • We can pin the span and use UnmanagedMemoryStream for this.
  
  
• [x] Integrate System.Memory.IMemoryOwner<T> and/or System.Memory.MemoryManager<T> with SL.ImageSharp.Memory.IBuffer<T>
  
  
  
  • Prerequisite for using Memory<T>, thus for the following 2 points
  
  
  • ~Notice the name collision with SL.ImageSharp.Memory.MemoryManager<T> :P~ UPDATE: SixLabors stuff is now called MemoryAllocator
  
  
• [x] Image.Wrap(existingMemoryArea) or similar.
  
  
  
  • Very useful for interop scenarios, see #435 or #88
  
  
  • ~Already added as internal, need to validate the API through a real-life use-case.~ DONE
  
  
• [x] Expose image.GetPixelSpan() and image.GetPixelRowSpan()
  
  
  
  • Note: this API can lead to unintended heap capture issues many users fail to understand
  
  
• [x] Expose image.GetPixelSpan() and image.GetPixelRowSpan()
  
  
  
  • Only if possible/allowed by the guideline. See these issues for more information about the blockers!
  
  
  • We decided to not expose Memory<T> for now
  
  
• [x] Move the core memory management classes out of this repo. They should live in SixLabors.Core so we can integrate them across the whole SixLabors stack. (#619)

[jherby2k]

I'd really like to see these, as a consumer of this library. I recognize that in most cases you're going to have to just copy the span to an array (at least you can ArrayPool) to use with MemoryStream, but eventually (.NET Standard 3, apparently) streams will be able to read from Spans directly.

public static IImageFormat DetectFormat(ReadOnlySpan<byte> data);

public static IImageFormat DetectFormat(Configuration config, ReadOnlySpan<byte> data);

public static Image<Rgba32> Load(ReadOnlySpan<byte> data);

public static Image<Rgba32> Load(ReadOnlySpan<byte> data, out IImageFormat format);

public static Image<Rgba32> Load(Configuration config, ReadOnlySpan<byte> data);

public static Image<Rgba32> Load(Configuration config, ReadOnlySpan<byte> data, out IImageFormat format);

public static Image<Rgba32> Load(ReadOnlySpan<byte> data, IImageDecoder decoder);

public static Image<Rgba32> Load(Configuration config, ReadOnlySpan<byte> data, IImageDecoder decoder);

public static Image<TPixel> Load<TPixel>(ReadOnlySpan<byte> data);

public static Image<TPixel> Load<TPixel>(ReadOnlySpan<byte> data, out IImageFormat format);

public static Image<TPixel> Load<TPixel>(Configuration config, ReadOnlySpan<byte> data);

public static Image<TPixel> Load<TPixel>(Configuration config, ReadOnlySpan<byte> data, out IImageFormat format);

public static Image<TPixel> Load<TPixel>(ReadOnlySpan<byte> data, IImageDecoder decoder);

public static Image<TPixel> Load<TPixel>(Configuration config, ReadOnlySpan<byte> data, IImageDecoder decoder);

[antonfirsov]

@jherby2k I like your API proposal pretty much, we should implement it.
But it's important to make sure we have good docs on these methods, because currently it's very easy to confuse Load() with LoadPixelData() from the beginners POV in my opinion.

Our current docs on Image.Load(byte[] data) aren't really helpful:

Create a new instance of the <see cref="Image{Rgba32}"/> class from the given byte array.

[hypeartist]

@antonfirsov @JimBobSquarePants Additional info on Span and Memory stuff:
C# - All About Span: Exploring a New .NET Mainstay

[antonfirsov]

607 has been merged. We are progressing, but there are many issues I failed to notice when making the proposal:

• Exposing image.GetPixelMemory() is not trivial
• Exposing Image.Load<TPixel>(ReadOnlySpan<byte>) API-s also not trivial (/cc @jherby2k)

See the updated issue description for more details!

[jherby2k]

Well I alluded to that in my request... at the time it was announced that the spanified Steams would be coming to netstandard 3.0 / .net 4.8 but now it sounds like that isn’t going to be the case. My suggestion is to start targeting netcoreapp2.1 directly, and for netstandard just copy the span to a buffer from the array pool. At least then the API is consistent, even if there is no performance benefit when using the netstandard lib.

[antonfirsov]

@jherby2k unfortunately, "spanified stream" isn't a thing that helps here, but I figured it out!
We can pin the span and use UnmanagedMemoryStream.

Gonna PR it soon.

[jherby2k]

Did not know about that one! That’s great.

Just to clarify though, by “spanified stream” I’m referring to .net core 2.1’s Stream.Write(Span buffer) overload. Which is exactly what you’re looking for, but obviously isn’t available for .net standard.

[antonfirsov]

@jherby2k ahh, you mean creating a MemoryStream and writing the span into it? I wanted to avoid this, it's a huge allocation :)

[antonfirsov]

We may need to introduce further API changes to allow handling very large unmanaged buffers and memory mapped files, but the topic is not yet clear for me. I'm leaving a few links here, so we can find them later:

• An article on the new API-s
• Memory-mapped files and Memory<T>: dotnet/corefx#26603

[antonfirsov]

Best System.Memory article so far:
https://www.codemag.com/Article/1807051/Introducing-.NET-Core-2.1-Flagship-Types-Span-T-and-Memory-T

[antonfirsov]

All tasks are done, so I'm closing this epic.