Provide Image.LoadPixelData(Type pixelFormatType, ...) API

[tzographos]

Hello,

It would be beneficial to add overloads for Image.LoadPixelData<TPixel>(...) as Image.LoadPixelData(Type pixelFormatType, ...).

Here's what someone has to go through when trying to load a byte[] containing raw pixel info with pixelformat as a variable (Rgba32 in the example)

MethodInfo mi = _typeOfImage.GetMethod("LoadPixelData", new[] { typeof(byte[]), typeof(int), typeof(int) }).MakeGenericMethod(typeof(Rgba32));
                        images[i] = mi.Invoke(null, new object[] { imagesData[i].ImageData, imagesData[i].ImageMetadata.Width, imagesData[i].ImageMetadata.Height }) as Image;

All this reflection should not be needed.

Thanks,
--Theodore

[antonfirsov]

This might be valuable, but I would like to see more justification. Can you elaborate a bit more on your use case? Is there a reason you need a dynamic pixel type descriptor and you cannot specify the pixel type at compile time with Load<TPixel>? I'm wondering what do you do with the pixel buffer afterwards, which is then specific to the pixel type. (i.e. if you end up having if or switch statements on the pixel type, I'm skeptical the dynamic pixel type is that valuable.)

[JimBobSquarePants]

I wouldn’t recommend ever adding it to our API. That would require us to use reflection which we really don’t want.

[tzographos]

@antonfirsov I am using ImageSharp to create an image stitcher. It takes multiple images and produces a new combined image.
The images being added are RAW with various pixel formats.
First of all, let me say that I don't see a need for LoadPixelData<TPixel>(byte[] data, int width, int height) as an API method alone.

For me, I would like to see a LoadPixelData(byte[] data, int width, int height, PixelFormat pixelFormat) method as well.
The caller would simply pass me the correct pixelFormat enum and I would be done with the loading in 1 line of code.

What we have now, is asking for a PixelFormat type. So as a user of the library I have 2 options.

1. Ask for a PixelFormat Type and call the relevant LoadPixelData<TPixel> like this:

MethodInfo mi = typeof(Image)
    .GetMethod("LoadPixelData", new[] { typeof(byte[]), typeof(int), typeof(int) })
    .MakeGenericMethod(typeof(Rgba32));
Image image = mi.Invoke(null, new object[] { byteData, width, height }) as Image;

Not to elegant due to the ugly reflections

2. Ask for a PixelFormat enum and make all the necessary mapping in my code.

switch case pixelFormat {
  case PixelFormat.RGBA32:
     image = LoadPixelData<Rgba32>(byte[], width, height);
    break;
......  and so on.............
}

Again not to elegant, because all the if-s could be hidden inside the library.

Thanks,
--Theodore

[tzographos]

@JimBobSquarePants I'm a lazy programmer and having to write a bunch of if/else-s is exactly what I"m trying to avoid
@br3aker I understand the argument about users wanting to create their own PixelFormats which would render the Enum proposal not flexible.
I think the best option would be to expect a PixelFormat Type and having an overload of the LoadPixelData method as

LoadPixelData(TPixel pixelFormat, byte[] data, int width, int height)

[br3aker]

I don't think that laziness is a good justification to make others work :)

I still don't understand why you find it 'a lot of work' to do this code snippet once and reuse it as copypasta or move it to a 'utility' dll:

public static class ImageLoader
{
    private static readonly ConcurrentDictionary<Type, MethodInfo> Methods = new();

    public static Image LoadImage(Type pixelType, byte[] data, int width, int height)
    {
        if (!Methods.TryGetValue(pixelType, out MethodInfo mi))
        {
            _ = Methods.TryAdd(pixelType, mi = CreateImageLoader(pixelType));
        }

        return mi.Invoke(null, new object[] { data, width, height }) as Image;

        static MethodInfo CreateImageLoader(Type pixelType) =>
            typeof(Image)
                .GetMethod("LoadPixelData", new[] { typeof(byte[]), typeof(int), typeof(int) })
                .MakeGenericMethod(pixelType);
    }
}

Reflection is slow?
You can make it a lot faster.

[tzographos]

This is a proposals discussion area. That piece of code (close to what I'm already using) is exactly my proposal to be integrated into the library instead of having a "helper" for something that is useful and could be offered to the library users right off the bat.

If you doubt my proposal's usefulness, then that's fine! I'm only writing here to showcase the need. :)

[br3aker]

I understand the need, I don't understand the reasoning to fulfill this need on the ImageSharp side.

Can you please provide hypothetical usage example with how user would get pixel Type to pass into ImageSharp API if proposed method existed?

[JimBobSquarePants]

I'm a lazy programmer and having to write a bunch of if/else-s is exactly what I"m trying to avoid

Yeah no mate. This discussion is over.