Add Image<T>.LoadPixelData variant that allows reusing an image object

[fredlllll]

for my project i used system.drawing for prototyping, but now it has to run on linux and i wanted to use imagesharp.

in my implementation i pooled bitmaps to prevent memory shooting up and take load of the GC. i reused bitmap instances of the fitting size and set their pixels using the LockBits[etc...] method to then save the image to disc and then return the bitmap into the pool

when changing to imagesharp i didnt find a way to reuse an image when directly settings its pixels using LoadPixelData.
here you can see that a new image is created every time i call this method to convert my pixel data into an image

ImageSharp/src/ImageSharp/Image.LoadPixelData.cs

Line 132 in 78a584e

var image = new Image<TPixel>(configuration, width, height);

doing this caused memory spikes as seen here

it would be nice to have a variant of LoadPixelData that reuses a passed in image instance instead of creating a new one every time

i tried copying the logic from the LoadPixelData and changing it to my needs, but alas, some important bits are internal and cant be accessed by me =/

[antonfirsov]

@fredlllll we have an internal pooling mechanism implemented by the type ArrayPoolMemoryAllocator, so normally there is no benefit from bothering with manual reusing/pooling strategies, all you need is to make sure you properly dispose all images being created. Note that the Image's buffer is not the only one being created during processing, there are temporary buffers created+destroyed behind the scenes, so proper pooling is a must for getting this right, you cant achieve it by just reusing the image instance. And again -- pooling is provided out of the box by the library. See the following article for more details:
https://docs.sixlabors.com/articles/imagesharp/memorymanagement.html

However, based on the memory timeline you posted something might got wrong in your app, but without code/more info I can't advise. A lot depends on image sizes + what operations you do.

But first, I suggest to rethink your code based on the article above.

[fredlllll]

i just tried making a minimal example of the behaviour and for the love of it i cant recreate it. i also moved the code i used for image saving into a seperate class in my project and it stopped the behaviour. but it is still occuring in the commit 72e91928d6385305bf8fde2807c4e00c8797d2b0 in my repo https://github.com/fredlllll/MandelbrotCSharp in case you want to check that out. i must be hitting an edge case.

commenting out the using clause and moving the arraypool.Put outside so its still called removes any memory spikes https://github.com/fredlllll/MandelbrotCSharp/blob/72e91928d6385305bf8fde2807c4e00c8797d2b0/MandelbrotCSharp/MandelField.cs#L229

anyway my problem is solved, but if you want to investigate that edge case (or prove that im mistaken) id be really intersted in what caused this

[DaZombieKiller]

I've run into a similar form of this while experimenting with using ImageSharp in a Unity project. The idea was to use ImageSharp to load external images, as Unity just exposes a blocking main-thread-only LoadImage(byte[]) method with support for PNG and JPG only. Consider the following example:

static Texture2D LoadImage<T>(Configuration configuration, Stream input)
    where T : unmanaged, IPixel<T>
{
    // Identify the image without loading it so we can create the Texture2D with an appropriate size.
    IImageInfo info = Image.Identify(configuration, input);
    var texture     = new Texture2D(info.Width, info.Height, GetTextureFormat<T>(), mipChain: false);
    
    // Unity allows access to the pixel data using GetPixelData<T>, which can
    // then be wrapped into a Memory<T> instance using a custom MemoryManager.
    NativeArray<T> pixels = texture.GetPixelData<T>(mipLevel: 0);
    
    // There doesn't appear to be a way to load the pixels into the image without needing to
    // allocate a new one and then copying the pixels across, which is a slow operation (omitted here).
    var image = Image.WrapMemory(configuration, pixels.AsMemory(), info.Width, info.Height, info.Metadata);

    return texture;
}

As seen above, I have no control over the allocation of the memory for the pixel data and am forced to pay the price of a copy via Image.Load. Is there perhaps an API that I've missed somewhere for loading pixel data directly into a Memory<T> instance?

[antonfirsov]

This idea makes sense for me, I would consider opening a tracking issue "Decode into existing Memory<T>" to discuss and elaborate the details. I don't think we are able to handle it with high priority at the moment though, but I would be happy to accept community contributions for the feature. @JimBobSquarePants thoughts?

When it comes to API shape, I would prefer a solution that works with existing decoder infrastructure, and exposed through trivially discoverable methods, hiding the if (decoder is IImageMemoryDecoder memoryDecoder) ... else ... (and similar) stuff from the user:

• Either by having static overloads Image.Load[Async](...., Memory<T>)
• Or an instance method image.Load(stream) which can be used in combination with WrapMemory.

The IImageMemoryDecoder or whatever underlying interface should be designed in a way that supports the main API the best.

@DaZombieKiller are you interested in PR-ing this stuff?

[DaZombieKiller]

@antonfirsov I'd certainly be happy to take a shot at PR-ing this at some point. I've opened a new discussion for the feature here: #1487

[JimBobSquarePants]

@antonfirsov I don't like it. We already have the new Image.WrapMemory overloads via #1419 available in the nightlies which performs a copy free wrap.

[DaZombieKiller]

WrapMemory works when you want to use an existing buffer of decoded pixels as an Image, but there's currently no way to use ImageSharp to decode directly into an existing buffer or Image, thus necessitating copies.

[JimBobSquarePants]

We're talking in two different places now. You should be petitioning Unity to add means to create textures from buffers without copying.