[3516] Aspect ratio bug

[Charlie82]

This bug has passed surprisingly 6 years unnoticed. That can cause resized avatars to be 1 pixel less high or less wide.

Version: all

File: Subs-Graphics.php

Line: 349 (2.0 RC 1-1)

Replace:

Code Select Expand

$dst_height = floor($src_height * $max_width / $src_width);

With:

Code Select Expand

$dst_height = round($src_height * $max_width / $src_width);

Line: 353 (2.0 RC 1-1)

Replace:

Code Select Expand

$dst_width = floor($src_width * $max_height / $src_height);

With:

Code Select Expand

$dst_width = round($src_width * $max_height / $src_height);

Line: 346 (2.0 RC 1-1)

Replace:

Code Select Expand

if (!empty($max_width) && (empty($max_height) || $src_height * $max_width / $src_width <= $max_height))

With:

Code Select Expand

if (!empty($max_width) && (empty($max_height) || round($src_height * $max_width / $src_width) <= $max_height))

Regards.

[SleePy]

What size Avatars are you uploading?

I did a quick test, and the only way I could get round and floor to produce a different result is when I had src_height as an odd number and src_width way different.

Code Select Expand


<?php
$src_height = 183;
$max_width = 100;
$src_width = 183;

$floor_height = floor($src_height * $max_width / $src_width);
$round_height = round($src_height * $max_width / $src_width);

echo $floor_height . ':' . $round_height;
?>


As a note, that last replacement wouldn't work to tell. That code you put into round returns true or false, so round is trying to round true of false.

[Charlie82]

What size Avatars are you uploading?

I did a quick test, and the only way I could get round and floor to produce a different result is when I had src_height as an odd number and src_width way different.

I'm uploading 1024x768, with maxwidth = 85.

But it also produce different results when:

Code Select Expand


$src_width = 1024 (4:3 aspect ratio)
$src_height = 768
$max_width = 65, 70, 85 or 90

Or:

Code Select Expand

$src_width = 160 (16:9 aspect ratio)
$src_height = 90
$max_width = 65, 90, 95 or 100

To say some.

As a note, that last replacement wouldn't work to tell. That code you put into round returns true or false, so round is trying to round true of false.

Sorry, my mistake. Now the first post is edited to fix the last replacement.

[SleePy]

Whats the max_height you have set? Is it following the same aspect ratio?

In each of those numbers, Some of them produce the same result, others come out 1 pixel higher. So your changed code would result in some resized images being 1 pixel wider.
Round works differently than floor. floor will just take any number and return the next lowest whole number (ie 3.9 would become 3). While round will round it up or down depending on the number, mode and precision.

The whole issue is that with the division, we have a decimal number in some ratios and we can't have a 39.5 pixel wide image. So it needs fixed. If we round up, then there is .5 of a pixel that doesn't really exist. If we round down, we trim off that last .5 of that pixel. I am in favor of rounding down really.

[Charlie82]

Whats the max_height you have set? Is it following the same aspect ratio?

Yes max_height = 85, the same.

If we round up, then there is .5 of a pixel that doesn't really exist. If we round down, we trim off that last .5 of that pixel. I am in favor of rounding down really.

The less you invent the better. In that regard rounding (not ceiling) is the best way to do it, as Photoshop does.

Rounding up or down (ceil or floor function) you are less accurate unless the number to round ends with .5, that is the only situation where rounding up and down is the same. But the rest it is better to round to the nearest integer:
4.51 closer to 5
3.75 closer to 4
3.25 closer to 3

That can only be done with round(), not floor or ceil().

[SleePy]

Yes, but we are still "inventing" those bits that don't really exist. I don't think this is causing any real issues is it? We use floor on both the height and width. So since its consistent it should be fine?
Forgive me with all the go around, it has been a while since I had to actually think about mathematics in such a fashion. I am just trying to wrap my head around how this is causing any issues. Do you have any test images that prove this?

[Charlie82]

It causes real issues that you can see in the attached images.

[Charlie82]

I think it does't matter if it is more or less visible. The fact is that it is a bug.

I think that [Unknown] developed SMF first with theory in mind, the practice comes second. Anyway it has real consecuences (1px) and the development team take only 1 minute to fix the bug.

[karlbenson]

I've changed my mind on this one a few dozen times.

I've tracked it for now
http://dev.simplemachines.org/mantis/view.php?id=3516

[karlbenson]

The devs have decided not to fix this, as round *can* theoretically make the value end up too high

[Charlie82]

I wont give my opinion, but:

Why don't you stick with the standard?

We all agree that Photoshop is the standard (and it uses round). Do we like reinvent the wheel?

[Nao 尚]

I remember SMF Media Gallery used SMF's code for resizing, and it did generate issues with height or width always being set to 99x100 or 100x99 instead of 100x100.
I fixed it a few months ago, with round() and everything. Believe me, I haven't had a thumbnail size issue ever since.
I'm not sure my fix is in SMG1 but if it isn't, I can share my SMG2 code block here of course.

[Joshua Dickerson]

What would be the issue with it being too high?

[Charlie82]

The same issue that you have viewing a 16:9 movie in a 4:3 TV without the black bars. In that case people in the movie are taller.

[Joshua Dickerson]

Then they would be shorter if we rounded down, right?

[Charlie82]

Shorter than the distorted version (in the example 4:3 Fullscreen without black bars).