Imagesharp: Image.WrapMemory methods using Memory<T> or IMemoryOwner<T> do not allow blocks larger than the image size

Created on 26 Jan 2021 · 4Comments · Source: SixLabors/ImageSharp

Prerequisites

[X] I have written a descriptive issue title
[X] I have verified that I am running the latest version of ImageSharp
[X] I have verified if the problem exist in both DEBUG and RELEASE mode
[X] I have searched open and closed issues to ensure it has not already been reported

Description

Version 1.0.2 added parameter guards that check the length of the allocated Memory/IMemoryOwner which will throw if the length is not _exactly_ width * height:

Guard.IsTrue(pixelMemory.Length == width * height, nameof(pixelMemory), "The length of the input memory doesn't match the specified image size");

This prevents passing a Memory/IMemoryOwner allocated larger than needed. For example, I can no longer use memory from a MemoryPool to wrap an image. Worked in version 1.0.1.

Thinking the guard should be >= instead. I'd be happy to submit a PR.

Steps to Reproduce

var memowner = MemoryPool<L8>.Shared.Rent(wid * ht);
var image = ISImage.WrapMemory(memowner, wid, ht);  // <- throws if pool block is larger than width * height

System Configuration

Windows 10, Azure pipelines (unit tests)

ImageSharp version: 1.0.2
Other ImageSharp packages and versions:
Environment (Operating system, version and so on): Windows 10, Azure Pipelines
.NET Framework version: .NET 5.0
Additional information:

up-for-grabs

Source

jeffska

Most helpful comment

A bit of a weird scenario, but I can understand how this would make sense, sure. I'd have been against this given that callers could just pre-slice the input Memory<T> to the right length, but since that's not doable for IMemoryOwner<T> I guess keeping a consistent behavior between the various overloads makes sense. This is conceptually the same behavior the void* overload has too, if that matters. So yeah, I'd say this seems ok to me as well 🙂