Rectangle.Contains() throws when it should not

[smino-dev]

We are using ImageSharp in our application to allow customers to edit the pictures they're uploading.

We make sure that the cropping dimensions of the instructions are not bigger than the image itself:

    private static void CropImage(ImageProcessingInstructions instructions, IImageProcessingContext img)
    {
        var (cropStartXInstruction, cropStartYInstruction, cropEndXInstruction, cropEndYInstruction) = instructions.Crop.GetValueOrDefault();
        var (currentWidth, currentHeight) = img.GetCurrentSize();

        // We ensure that cropping bounds are not out of image bounds
        var cropStartX = Math.Max(cropStartXInstruction, 0);
        var cropStartY = Math.Max(cropStartYInstruction, 0);
        var cropEndX = Math.Min(currentWidth, cropEndXInstruction);
        var cropEndY = Math.Min(currentHeight, cropEndYInstruction);

        img.Crop(new Rectangle(cropStartX, cropStartY, cropEndX, cropEndY));
    }

However, some images still cause ImageSharp to throw this exception: Crop rectangle should be smaller than the source bounds. (Parameter 'cropRectangle'). It is thrown here.

We log the instructions the customer used and they appear to be fine.
Here are 3 examples that threw the above exception:

• Quality=90;RotateDegrees=90;FitIntoContainer.width=4096;FitIntoContainer.height=4096;Crop.x=22;Crop.y=316;Crop.width=2972;Crop.height=2972;ExtractMetadata=True
• Quality=90;RotateDegrees=90;FitIntoContainer.width=4096;FitIntoContainer.height=4096;Crop.x=407;Crop.y=716;Crop.width=1862;Crop.height=1863;ExtractMetadata=True
• Quality=90;RotateDegrees=90;FitIntoContainer.width=4096;FitIntoContainer.height=4096;Crop.x=0;Crop.y=1007;Crop.width=3024;Crop.height=3024;ExtractMetadata=True

x + width and y + height of Crop are always smaller than width and height of FitIntoContainer. Meaning the cropping should fit into the image.
We also tested these values with the Rectangle.Contains method and it always returned true.
Unfortunately we do not have the original images, which caused the exceptions.

Does anyone have an idea, what the issue could be here?

[JimBobSquarePants]

Are you measuring before or after rotating?

[smino-dev]

Do you mean like this (using the data from the first example)?

// We get source from the image the user is trying to upload
Stream source = null;
var config = Configuration.Default.Clone();
config.MemoryAllocator = MemoryAllocator.Create();
using var image = Image.Load(config, source);

var cropStartXInstruction = 22;
var cropStartYInstruction = 316;
var cropEndXInstruction = 2972;
var cropEndYInstruction = 2972;
var rotateDegrees = 90;

image.Mutate(img =>
{
    img.AutoOrient();

    img.Rotate(rotateDegrees);

    var (currentWidth, currentHeight) = img.GetCurrentSize();

    var cropStartX = Math.Max(cropStartXInstruction, 0);
    var cropStartY = Math.Max(cropStartYInstruction, 0);
    var cropEndX = Math.Min(currentWidth, cropEndXInstruction);
    var cropEndY = Math.Min(currentHeight, cropEndYInstruction);

    img.Crop(new Rectangle(cropStartX, cropStartY, cropEndX, cropEndY));

    var containerWidth = 4096;
    var containerHeight = 4096;

    if (currentWidth > containerWidth || currentHeight > containerHeight)
    {
        img.Resize(new ResizeOptions
        {
            Mode = ResizeMode.Max,
            Size = new Size(containerWidth, containerHeight)
        });
    }
});

We do not have the original images, so we cannot know for sure exactly how big they are. But cropStartX/Y and cropEndX/Y and the Resize at the end should invalidate that problem.

[antonfirsov]

@smino-dev maybe you can add some logging to your app to see with what dimensions does this happen? You were able to catch and report the InvalidOperationException, maybe you can rethrow it with the dimension+parameter info included? It's hard to understand the problem without a runnable repro.

[smino-dev]

We did extend our logging with the dimensions of the image. However that version has only been released for about a week now and it seems that a user has yet to trigger that exception with the new logging.
And since we do not have the original images, we unfortunately cannot trigger it ourselves.
I will immediately update you, as soon as we get the image dimensions.
Sorry for not being able to include all info just yet.

[dlemstra]

Both cropStartXInstruction and cropStartYInstruction can be higher than the width/height of the image? With an image that is 1x1 it is correct that the exception is being thrown?

[smino-dev]

cropStartXInstruction and cropStartYInstruction should never be higher than the image dimensions, since our frontend is configured to now allow that. I could also not reproduce a cropping that was bigger than the image using our frontend. But I guess it could technically be possible.
Assuming the image, which the user tried to upload with the instructions used above, was 1x1 pixels, then that could have caused the exception.
Since we do not currently know the dimensions of the users' images, we cannot determine if that's indeed the problem but it certainly is worth to add a check for that in our code.

I would be okey with closing this discussion for now, if I have the option to reopen it in case the exception starts reoccurring.