Skip scaling if image is smaller than requested

[orian]

If original image has width/height smaller than requested and no upscaling then skip resizing.

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please let us know the company's name.

[willnorris]

I don't think this will quite work as intended... if both the requested height and width are larger than the original, then this h == 0 will be false, and w won't get set to zero. Instead, I think it would work (and would read a little easier) to leave the current checks alone, and then add after them:

// if requested width and height match the original, skip resizing
if w == imgW && h == imgH {
    w, h = 0, 0
}

In fact, I'd move that outside the if !opt.ScaleUp block entirely. In any case that the requested width and height match the original, then we don't need to do anything.

Not sure how easy it would be to write a test case for this as well, since it's hard to know if the resize methods were called 😕

[orian]

You're right, the patch is wrong. But I think the correct condition would be:

// if requested width and height match the original, skip resizing
if (w == imgW && (h == imgH || h == 0)) || (w==0 && h == imgH) {
    w, h = 0, 0
}

or more readable:

// if requested width and height match the original, skip resizing
if (w == imgW && h == imgH) || (w == imgW && h == 0) || (w == 0 && h == imgH) {
    w, h = 0, 0
}

It needs to handle the case where only one of the dimentions to scale is set.

About testing: I would probably extract the logic reponsible for choosing right w,h.

It would be smth like:

func ResizeParams(m image.Image, opt *Options) (w,h int, resize bool) {
    ...
}

The Line 115 would look:

if w,h,ok:=ResizeParams(m,&opt); ok {
  // resize
}

What do you think?

[willnorris]

Couldn't that be simplified to:

if (w == imgW || w == 0) && (h == imgH || h == 0) {
    w, h = 0, 0
}

Basically, if both height and width or either zero or the original value, then no resize is needed.

Yeah, pulling this out into a separate func would certainly make testing it a bit easier. Though I don't think that func would need to be exported (so resizeParams), and using the name ok seems confusing here... maybe just resize or something?

[willnorris]

Given the refactor, maybe break this into two commits (but in this same pull request)... one that refactors out the resizeParams func and adds some test cases for what it currently supports, and then a second commit that adds this new logic as well as a few additional tests cases.

[orian]

Done ;-)

[googlebot]

CLAs look good, thanks!

[willnorris]

sorry it took me so long to get this merged. But thanks for the contribution, this looks great!