PNG Gray16 Decode(Encode(x)) not an identity

[Sheyne]

Prerequisites

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

Description

When using the the PNG Encoder / Decoder, I've run a test Gray16 image with a pixel value of 2017 in the top left corner. This incorrectly decodes as 2056.

Steps to Reproduce

using var memoryStream = new MemoryStream();

{
    var width = 2;
    var height = 2;
    var data = new ushort[width * height];
    data[0] = 2017;
    var image = Image.LoadPixelData<Gray16>(MemoryMarshal.Cast<ushort, byte>(data.AsSpan()), width, height);

    Console.WriteLine($"image[0] = {image.GetPixelSpan()[0]}");

    IImageEncoder encoder = new PngEncoder();
    encoder.Encode(image, memoryStream);
    memoryStream.Flush();
}

memoryStream.Seek(0, SeekOrigin.Begin);

{
    var imageLoaded = Image.Load<Gray16>(memoryStream);
    Console.WriteLine($"image[0] = {imageLoaded.GetPixelSpan()[0]}");
}

Prints:

image[0] = Gray16(2017)
image[0] = Gray16(2056)

System Configuration

MacBook Pro

• ImageSharp version: 1.0.0-beta0007
• Other ImageSharp packages and versions: N/A
• Environment (Operating system, version and so on): macOS 10.14.6
• .NET Framework version: Core 3.0

[JimBobSquarePants]

You’re encoding the png as Rgba32. If you use the encoding options to set the correct pixel format you will get the correct result

[JimBobSquarePants]

Ps. Thanks for filling in the issue template. It makes triaging issues much easier and we really appreciate it! 👍

[Sheyne]

This fixes my issue. Thanks for the quick response!

Does it make sense to use reflection on the type parameter to detect the expected encoding type? That might make the API more discoverable to a new user of the API like myself. It could be made efficient by caching the various type parameters we expect receive.

[antonfirsov]

@JimBobSquarePants ignoring the implementation difficulties, and purely from usability POV:
Shouldn't we set the default values of PngEncoder.ColorType and PngEncoder.BitDepth based on the pixel type of the encoded Image instance?

[JimBobSquarePants]

@Sheyne I wouldn't use reflection to do that no, a simple switch probably works over the type of TPixel.

@antonfirsov That's actually easier than you think. Since the two properties are nullable we can simply switch through the TPixel type in the encoder and if it's Rgba32 compatible make an educated guess.

[JimBobSquarePants]

Fixed by #1019