PngEncoder ignores FilterMethod

melnikov77 commented 2 months ago

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

ImageSharp version

3.1.4

Other ImageSharp packages and versions

NA

Environment (Operating system, version and so on)

NA

.NET Framework version

net8

Description

When a new PngEncoder is created with custom FilterMethod it always produces the same file size, i.e. the FilterMethod is being ignored.

Example:

var encoder = new PngEncoder
{
    FilterMethod = PngFilterMethod.Paeth,
};

I found only one usage of this property here https://github.com/SixLabors/ImageSharp/blob/10e9c49e133c4b44b4d70220ceb8cd8f65c3bce8/src/ImageSharp/Formats/Png/PngEncoderCore.cs#L1522-L1526

i.e. filterMethod is assigned only if encoder.FilterMethod is null, otherwise, it is always 0 (PngFilterMethod.None).

Steps to Reproduce

    SixLabors.ImageSharp.Image img = null!;

    var paethFilterStream = new MemoryStream();
    img.Save(paethFilterStream, new PngEncoder
    {
        FilterMethod = PngFilterMethod.Paeth,
    });

    var noneFilterStream = new MemoryStream();
    img.Save(noneFilterStream , new PngEncoder
    {
        FilterMethod = PngFilterMethod.None,
    });

    Assert.NotEquals(paethFilterStream.Length, noneFilterStream.Length);

Images

No response

JimBobSquarePants commented 1 month ago

@melnikov77 The filter method is used in the release/3.1.x branch here.

https://github.com/SixLabors/ImageSharp/blob/d7ef0e2bb939fc6f3026b34d555ad1f5076ad8a5/src/ImageSharp/Formats/Png/PngEncoderCore.cs#L574-L600

Your test confused me given you are use null! to define the source image.

SixLabors.ImageSharp.Image img = null!;

However, you are correct! We are not assigning the correct filter on encode, your finding actually uncovers a bug in our SIMD accelerated filtering methods where we suffered from an unsigned integer overflow defect!

Many thanks for raising this.

I've opened a PR #2775 with the fix.

JimBobSquarePants commented 1 month ago

Fixed with 3.1.5

SixLabors / ImageSharp