jwagner / smartcrop.js

Content aware image cropping
http://29a.ch/2014/04/03/smartcrop-content-aware-image-cropping
MIT License
12.83k stars 580 forks source link

Cropped images are sometimes 1 pixel too small #123

Open wave-dash opened 1 year ago

wave-dash commented 1 year ago

smartcrop --width 100 --height 400 %1 %~n1_out%~x1

When I run this command on kitty.jpg from the test suite, the output image is 99x400.

2023-03-19_22-47-36_dopus_crop

I assume this happens because of the skewed aspect ratio of the requested crop, I've encountered a similar "missing pixel" problem when cropping in Photoshop. Is there a way to force exact dimensions for the output?

jwagner commented 1 year ago

Hey @wave-dash thanks for the detailed report. I don't think this is the correct repo though. Can you please report it against https://github.com/jwagner/smartcrop-cli ? Looks like a rounding error somewhere. I don't have time to go digging for it right now but I should get to it eventually. :)

wave-dash commented 1 year ago

Sure thing, sorry about that. This repo seemed more active, and I figured the code for the two would be mostly the same.

kamiyo commented 1 year ago

I actually have a similar problem using smartcrop-sharp. When passing it options like this:

const { topCrop } = await smartcrop.crop(imageFile, {
            minScale: 1.0,
            width: 3,
            height: 2,
        });

OR

const { topCrop } = await smartcrop.crop(imageFile, {
            minScale: 1.0,
            width: 4000,
            height: 2666,
        });

It always returns a height of 2656 instead. I think the issues are here:

https://github.com/jwagner/smartcrop.js/blob/de2d1438b1f46490452f8e8b196974b19b0b0dcf/smartcrop.js#L120-L121 https://github.com/jwagner/smartcrop.js/blob/de2d1438b1f46490452f8e8b196974b19b0b0dcf/smartcrop.js#L147-L148

In the first pair of lines, I believe you are scaling down the dimensions for the downsampled image (for processing ease, I'm assuming). But since it's floored, once you scale back up in the second pair of lines, it won't be at the correct width, especially if the initial wasn't a multiple of 256.

To prove the above example in pseudocode,

prescale = 0.064
downsampledCropHeight = floor(2666 * 0.064) = 170
finalCropHeight = floor(170 / 0.064) = 2656

I don't know what the solution should be. You could force based on aspect ratio. But as an end user, I guess I can just use the x and y of the crop, and force the dimensions myself.

Just letting you know where I think the issue lies.