[patch] ngx_http_image_filter_module incorrectly calculates size

[Maxim Dounin]

Hello!

On Thu, Apr 19, 2012 at 09:29:07PM +0400, Maxim Bublis wrote:

> Hi.
> 
> There is a bug in ngx_http_image_resize function from
> ngx_http_image_filter_module with incorrect calculation of new image
> sizes after resizing with crop in case of original image is capable to
> resize with preservation of original proportions (i.e. crop is not
> required). For example, resizing image with original sizes of 360x203
> to 304x171 with crop, will surprisingly give us resized and cropped
> image with sizes of 303x171, though resizing the same image without
> crop will give us correct image with sizes of 304x171.
> Attached image_filter_crop.patch fixes this undesired behavior.

You patch just moves processing from one code path to another in a 
case where nginx (due to use of fixed-point calculations) doesn't 
see the difference between original image aspect ratio (~ 1.773) 
and desired resulting image aspect ratio (~ 1.778).  Normally both 
code paths should result in identical images matching desired 
sizes, but in the case in question this doesn't happen due to 
rounding effects.  With your patch the problem goes away for a 
particular image, but will instead appear in other cases. 

Something like this should be correct solution to eliminate the 
problem:

--- a/src/http/modules/ngx_http_image_filter_module.c
+++ b/src/http/modules/ngx_http_image_filter_module.c
@@ -817,9 +817,7 @@ transparent:
 
         resize = 0;
 
-        if ((ngx_uint_t) (dx * 100 / dy)
-            < ctx->max_width * 100 / ctx->max_height)
-        {
+        if (dx * ctx->max_height < dy * ctx->max_width) {
             if ((ngx_uint_t) dx > ctx->max_width) {
                 dy = dy * ctx->max_width / dx;
                 dy = dy ? dy : 1;


> In the same time i've found that algorithm for calculation of new
> image sizes in both resize and crop cases could be generalized.
> Attached image_filter.patch uses this idea and reduces number of
> duplicated source lines (also includes bugfix from
> image_filter_crop.patch).

No, thanks.  We generally prefer code readability over 
generalization.

Maxim Dounin