resized image dimensions incorrectly floored instead of rounded

[_ck_]

Image dimensions are incorrectly floored instead of rounded to a proper number. Some attempt at "fixup" is done later in some processes to move them by a pixel, but still the way it's initially calculated is wrong.

For example:

A typical photo has a 3:2 (3/2 = 1.5) ratio.

If it's resized to 600px width, WP will make the height 399px - this is wrong.

It happens because of function wp_constrain_dimensions in media.php (~line 304) which floors the float using intval, which is wrong.

	$w = intval( $current_width  * $ratio );
	$h = intval( $current_height * $ratio );

I propose

	$w = round( $current_width  * $ratio );
	$h = round( $current_height * $ratio );

Earlier in the function, both $ratio and $current_width are already handled as numbers, so they have to be valid numbers, no need to use intval for that reason, it's just being used to floor, which is bad in this case.

Using round is not a performance issue because this function is not used in realtime output for templates but typically in admin area.

Optionally while you are at it, put a filter on wp_constrain_dimensions and pass the filter all the values passed to the function before the array is returned at the end.

[_ck_]

Oops, I made an oversight in the first post (ticket).

The statement that compares the upper and lower bounds must also use round and not intval to test the result. This is the complete change:

	if ( round( $current_width * $larger_ratio ) > $max_width || round( $current_height * $larger_ratio ) > $max_height )
 		// The larger ratio is too big. It would result in an overflow.
		$ratio = $smaller_ratio;
	else
		// The larger ratio fits, and is likely to be a more "snug" fit.
		$ratio = $larger_ratio;

	$w = round( $current_width  * $ratio );
	$h = round( $current_height * $ratio );

This is tested working on a very basic level, for example 3/2 images become 600:400 now instead of 600:399

However it has not been tested robustly.

[SergeyBiryukov]

Updated the patch.

[kitchin]

Note round() makes a float not an int.

[wonderboymusic]

The default precision for round() is zero, which means it will return an int:
​http://php.net/manual/en/function.round.php#refsect1-function.round-examples

[wonderboymusic]

Patch is blowing up.

[kitchin]

Note, has xxx comments and disables some Imagick tests.

[kitchin]

Patch 18532k.diff does this:

1. Adjusts resize tests to match new behavior where it is off by one pixel from old behavior. I forgot to make a test to fail the bug report, so we need to do that still. The new numbers agree with IrfanView, Photoshop, any web browser, etc., of course. Because they round. This shows we need the patch.

2. Changes a bunch of assertEquals() to assertSame() because the function should continue to return integers, not floats that look like integers.

3. I put the same rounding fix in image_resize_dimensions() for cropping.

4. I have some xxx comments that need to be resolved or deleted.

5. Getting scary looking console errors on my CentOS 5 system cPanel with ImageMagick 6.2.8 (yum up to date), so I skipped-out the unit tests that try to load a PNG or GIF when Wordpress decides ImageMagick only supports JPG (iterator support). They try to load a WP_Error object back into ImageMagick. Instead I did markTestSkipped() when

5a. WP_Error reached in PNG or GIF test.

5b. Any time test_resize_bad_image() is called with ImageMagick.

We will probably want a patch without the (5b) change, or condition (5b) on PNG support. There is no need for the (5a) console errors at all, they need to be either assertions or skips.

I kept the assert on ImageMagick PNG support in general, so you still get one Fail. (That makes it skip the assert for GIF support.)

6. Existing code could use some style cleanup.

Proposed changes for next patch:

• Make a test this bug fails.
• Condition (5b) Skip on no ImageMagick PNG support.
• Fix xxx comments.

[kitchin]

Wordpress should protect ImageMagick and GD from non-string input. New ticket for that?

[kitchin]

I can do a clean patch with no xxx comments and minimal changes to the unit tests if that is better for review.

[wonderboymusic]

In 30660:

Use round() instead of floor() when resizing image dimensions.

Updates unit tests.

Props SergeyBiryukov, kitchin.
Fixes #18532.

[maniu]

I just wanted to note that this fix breaks linking to some old images (old enough to use previous method of calculating size) when importing. It replaces the link but dimensions are different by one pixel and post content ends up with broken image.