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

• Basic ReadOnlySpan<T> overloads of LoadPixelData (done in Expose Span methods on LoadPixelData and SavePixelData  #567)
• ReadOnlySpan<T> overloads for Image.Load() and Image.DetectFormat(). See @jherby2k's proposal.
  • We can pin the span and use UnmanagedMemoryStream for this.
• 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
• Image.Wrap(existingMemoryArea) or similar.
  • Very useful for interop scenarios, see Library Inter-op extensions #435 or Passing Unmanaged Pointer as Pixels buffer in ImageSharp #88
  • Already added as internal, need to validate the API through a real-life use-case. DONE
• Expose image.GetPixelSpan() and image.GetPixelRowSpan()
  • Note: this API can lead to unintended heap capture issues many users fail to understand
• 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
• 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. (Migrate memory primitives to SixLabors.Core #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<byte> buffer) overload. Which is exactly what you’re looking for, but obviously isn’t available for .net standard.