image->size with disabled upscaling -> wrong aspect ratio

[thomasaull]

Short description of the issue

Default Image field with one image with Dimensions of 1200x1400 px (Width x Height). In my template I'm exporting this image in different resolutions to use it as an responsive image but with a fixed aspect ratio of 16:9

$options = ['upscaling' => false];
$image->size(1920, 1080, $options);
$image->size(960, 540, $options);
$image->size(480, 270, $options);

Expected behavior

Images which are to small in dimensions for the target image size should still maintain the expected aspect ratio. The two smaller images can be cropped without problems, the larger image would have to be upscaled to meet the required dimensions (which is disabled). I would expect this image to have dimensions of 1200x675 (crop to maximum possible dimensions without upscaling but still maintain the aspect ratio of 16:9)

Actual behavior

The image just uses the original dimensions as a maximum independently. The image with a target size of 1920x1080 gets cropped to 1200x1080. So I end up with the two smaller images in the correct aspect ratio of 16:9 and with the largest image with a completely arbitrary aspect ratio.

Setup/Environment

• ProcessWire version: 3.0.105

[ryancramerdesign]

I hear what you are saying, and it seems like it would be a preferable result for this case, but in terms of what is the correct return value for the size() function, I'm not so sure. You've got upscaling turned off, so it is impossible to achieve your requested dimension. As a result, it either has to throw an Exception, or return an image that doesn't match the request. While it can't achieve your width dimension, it can achieve your height dimension. So it's returning a image that at least matches your requested height dimension. On the other hand, if it were to return what you suggested, then it would be returning an image that doesn't match either of your width or height dimension. Which result is more correct, I'm not really sure. It comes down to a question of whether it's the requested dimensions that are more important, or the proportion created by those combined dimensions. The coder side of me wants preference to the numbers in the arguments (current behavior), while the designer side of me wants preference to the proportion. I suppose it depends on context, but maybe proportion fits our context better? @horst-n what do you think?

[ryancramerdesign]

Btw, here's where the logic where it determines what to do with this particular case: https://github.com/processwire/processwire/blob/dev/wire/core/ImageSizerEngine.php#L672

[horst-n]

@ryancramerdesign My opinion is: when it can't be rendered because it is to small, it is an exception. I would expect that a designer / developer defines the min settings for those input fields to match at least the max needed ones! Or he allows upscaling.

Without that, you ever get the wrong and maybe totally useless results: f.e., if you want to get variations of 900x600 and 1800x1200, but your original image only is 600x500, what should be done then? Cropping it two times to 600x400 and serve it?

Maybe I'm to simple minded, but I don't see any advandtages. In every site, where I have special interest in serving fine images, (that's the case when I disable upscaling!) I restrict the users only to be able to upload images matching the needed min dimensions.

[thomasaull]

In my use case it was for responsive images, in my frontend I define a srcset for small, medium, high, ultra-high resolution. But not for every picture there is an ultra-high resolution available, so to save my clients frustration (and phone calls) I would allow to upload pictures with a minimum of medium/high resolution. For the responsive image I do not want to upscale the image to high-resolution, since there would be more bandwidth needed, without any benefit

Without that, you ever get the wrong and maybe totally useless results: f.e., if you want to get variations of 900x600 and 1800x1200, but your original image only is 600x500, what should be done then? Cropping it two times to 600x400 and serve it?

In my opinion and if upscaling is not allowed: Yes

[horst-n]

@thomasaull Using another imagefield at places where you need highres variations is not possible?

[horst-n]

@thomasaull Another question in this regard: Have you compared a medium sized variation upscaled through GD or Imagick to highres and displayed fullwidth in the browser, against a mediumsized image that gets upscaled to fullwidth by the browser directly? I can't see better results here on desktop monitors with the latter.

Why not optionally using enabled upscaling for the Highres only:

$image->size(1920, 1080, ['upscaling'=>true, 'sharpening'=>'none']);
$image->size(960, 540, ['upscaling'=>false]);
$image->size(480, 270, ['upscaling'=>false]);

[thomasaull]

@horst-n I can sure implement workarounds for my cases, that's not the issue here. It is more about the way the system behaves in general, which for me, is leading a bit to unpredictable results

[LostKobrakai]

I'd support @thomasaull standpoint. I feel like maintaining the aspect ratio is the way more common expectation to "at least have one pixel dimension correct", aspect ratio is just the implicit dimension, which is currently incorrect after the resizing. In responsive designs images are hardly displayed at their native resolution anyways, so differences at the pixel dimensions are imho less a problem than differences at the aspect ratio, which will break any layout which depends on images being of a certain aspect ratio.

[horst-n]

I'm aware of all these arguments, but I don't get the advantage of disabling upscaling when the image gets upscaled through the browser anyways. At least both looks the same to me, so that we also then can override the disabled upscaling option.

[thomasaull]

Filesize. The image get's upscaled without any additional information, so it does not really looks better, but the user has to download more data without any real benefit

[ryancramerdesign]

Thanks for the feedback. It does seem like maintaining the aspect ratio would be better for the context of most PW installs (responsive web sites) when upscaling is disabled. But there might be a different context and good use case for the current behavior, where it will try to at least keep one of the dimensions if possible, when upscaling is disabled. I don't know what the use case is, but it seems like there could be one. If there is, then chances are someone (or many), are relying upon it. In which case, we should keep it (since it's the current behavior), and if important, maybe add an alternative option that sticks to the proportion. On the other hand, if there is no use case for the current behavior, then it's probably okay to change it.

I haven't tried it yet, but also wondering what the behavior is if upscaling is off and neither width nor height dimension can be achieved.

I'm aware of all these arguments, but I don't get the advantage of disabling upscaling when the image gets upscaled through the browser anyways. At least both looks the same to me, so that we also then can override the disabled upscaling option.

I think the reason someone might disable upscaling is so that they aren't creating images that consume more space (and thus bandwidth) than what is necessary. Letting the browser upscale will produce the same result visually, but with fewer bytes/bandwidth. I could be wrong though, if an upscaled image consumes the same amount of bytes as one that isn't, then there's probably no value in disabling upscaling for a case like this.

[thomasaull]

I just tested it with Sketch: original: 202kb scaled x3: 905kb

Should be the same for any other upscaling by php libraries, more pixels = more data

[horst-n]

@thomasaull Filesize is a valid argument. (I hadn't thought on)

@ryancramerdesign When implementing the new behaviour, there seems to be no reason to keep the old one besides it. At least one get an unwanted result. It was thought as a compromise to not break sites completly by throwing exceptions. But it definetly is a handling of misconfigurations. One sort seems to be enough to me. And then, after the discussion, better that with keeping aspect ratios. :)

[horst-n]

@thomasaull, @ryancramerdesign I sent a fix for this to the repo: https://github.com/processwire/processwire/pull/118

[horst-n]

@netcarver The status should be changed to suggest a possible fix: PR #118 :)

[ryancramerdesign]

Thanks @horst-n and @thomasaull — I have added Horst's PR and tested it here and seems to work nicely!