esimov / caire

Content aware image resize library
MIT License
10.36k stars 384 forks source link

"cannot rescale to this size preserving the image aspect ratio" #54

Closed JanPokorny closed 4 years ago

JanPokorny commented 4 years ago

When I try to rescale this image: https://wallhaven.cc/w/dgpj33 from its original size 3840x1080 to new size 3046x1050, this happens: obrazek

"cannot rescale to this size preserving the image aspect ratio" seems to always show up. I was under the impression that the whole purpose of this utility is to change the aspect ratio of the image?

esimov commented 4 years ago

It means that you cannot use the scale option to resize the image to the desired dimension. In case you are using the scale option, first the image is rescaled by preserving the image aspect ratio and only afterwards the smart resize algorithm is invoked. This way the content aware image resize will be much faster. So i'm suggesting to remove the scale option. Also please read the documentation.

JanPokorny commented 4 years ago

@esimov Please see in the screenshot that I tried both with and without the scale option, getting the same result.

esimov commented 4 years ago

Looks like there is a condition which is not handled correctly. I will investigate further away.

micahcantor commented 4 years ago

I'm not sure if this impacts the issue that the @JanPokorny had, but I came across a similar issue when scaling down an image with no change in aspect ratio. For this case, I found the issue in this excerpt from Processer.Resize:

if p.Scale {
    if p.NewWidth > img.Bounds().Max.X || p.NewHeight > img.Bounds().Max.Y {
        return nil, errors.New("scale option can not be used on image enlargement")
    }
    // Preserve the aspect ratio on horizontal or vertical axes.
    if p.NewWidth > p.NewHeight {
        newWidth = 0
        newImg = resize.Resize(uint(p.NewWidth), 0, img, resize.Lanczos3)
        if p.NewHeight < newImg.Bounds().Dy() {
            newHeight = newImg.Bounds().Dy() - p.NewHeight
        } else {
            return nil, errors.New("cannot rescale to this size preserving the image aspect ratio")
        }
    } else {
        newHeight = 0
        newImg = resize.Resize(0, uint(p.NewHeight), img, resize.Lanczos3)
        if p.NewWidth < newImg.Bounds().Dx() {
            newWidth = newImg.Bounds().Dx() - p.NewWidth
        } else {
            return nil, errors.New("cannot rescale to this size preserving the image aspect ratio")
        }
    }
    dst := image.NewNRGBA(newImg.Bounds())
    draw.Draw(dst, newImg.Bounds(), newImg, image.ZP, draw.Src)
    img = dst
}

After it resizes the image, it only check if p.NewHeight or p.NewWidth is less than the resized image's respective dimensions, ignoring the case that they are the same. When I change it to if p.NewHeight <= newImg.Bounds().Dy() and if p.NewWidth <= newImg.Bounds().Dy() then it simply writes this rescaled image without any need for content aware resizing.

micahcantor commented 4 years ago

So I continued to run into the original issue in my project and I've come up with a fix, but I'm not sure if it is optimal.

Problem The problem lies in how the program handles the scaling before applying caire. Take for example a resize from 3600x2000 to 1920x1080.

  1. The width > height in the original so 3600 is scaled to 1920 (scale factor of 1.875), and the height is scaled by the same factor, to 1067.
  2. The program checks if the original height < scaled height. In this case, if 1080 < 1067.
  3. It isn't, so the program throws an error that the ratio can't be resized.

The way this proportional scaling works, this error will always be thrown if the aspect ratio of the original image (3600/2000 = 1.8) is greater than the aspect ratio of the desired dimensions (1920/1080 = 1.77). This is the same issue that @JanPokorny ran into resizing from a ratio of 3.56 to 2.9. Additionally, this error will sometimes happen even if the p.Scale flag is set to false, since the program sets that flag to true anyways if the resize amount is sufficiently large.

Solutions From my point of view, this issue could potentially be solved in two ways:

  1. Allow the scaled dimension to be less than the desired dimension, then use caire to expand that dimension back to the desired value. For example, with the above dimensions, after the image is scaled to 1920x1067, the height is enlarged the remaining 13 pixels using caire to 1080.

This seems optimal for this scenario when the scaling gets really close to the desired value, since enlarging by a small amount will almost certainly not be noticeable. This is also the solution I implemented, and I can share the changes I made if anyone wants.

  1. Don't scale the size of the larger side directly to the desired value when the aspect ratio of the original is greater than that of the desired dimensions. Instead it could be scaled "close enough" to the size of the desired dimension, without letting the shorter side be scaled below the desired value. Then both sides could be resized using caire as normal.

This seems like probably a better solution, but I'm not exactly sure how you would choose what to resize the larger side to.

esimov commented 4 years ago

Thanks again for the investigation. I will check it in a more detail in the following days.

JanPokorny commented 4 years ago

Wouldn't in this case (target aspect ratio is lower) the correct course of action be to rescale the shorter side to fit, instead of the larger side?

micahcantor commented 4 years ago

That's what I was thinking. Compare the aspect ratios of the input/target then rescale to the according side. Then remove the check for whether the original side is less than the scaled side.

esimov commented 4 years ago

Guys if you think that the proposed solution will work make a fork, test it and if all's well create a pull request and I will merge it back to the master branch. Otherwise I can work on it only a couple of days later.

micahcantor commented 4 years ago

Sounds good, I'll work on it and probably send a pull request later today.

esimov commented 4 years ago

Closing this issue because it has been resolved with the following pull request: https://github.com/esimov/caire/pull/60.