vips-image-editor

vips-image-editor copied to clipboard

Hi, thanks for writing this plugin. I have a huge performance improvement compared to ImageMagick. Unfortunately, when switching to VIPS on my production environment, I noticed that one particular size of thumbnail (650px x 433px - registered by the theme I'm using) is not generated from my pictures (which are admittedly quite big at 4000x2667). Bigger thumbnails are generated, but not this small one.

On my dev environment based on Homestead, it works though. Since the production and dev environment vary greatly (same php version though), my guess is that there is some kind of memory limit / timeout parameter at play. But I'm unable to find it. There is nothing in the php error logs.

How do you advise I debug this problem?

Thanks.

Follow up: it looks like it's the usage of thumbnail in the _resize function that is problematic. If used, for the particular size I'm looking for, the next $resized->crop call gives this error: extract_area: bad extract area

If I change your code to use resize in place of thumbnail then the crop works.

Can you post a complete example of the code you are using and share an example image? i.e. how you are registering the image size. This way I'll be able to look into the issue.

Thanks!

If thumbnail is causing you problems, a workaround would be using vips_ie_thumbnail filter to disable the functionality temporarily:

add_filter('vips_ie_thumbnail', false);

But please do post the code and the image so I can fix the underlying issue 🙂

Hi @joppuyo and thanks for the great plugin. OK, so it happens when trying to get a 650x433px thumbnail of the attached image. This is a custom thumbnail size registered in my WP install by a theme I'm using: add_image_size('custome-thumb', 650, 9999);

The _resize function of this plugin is called for each thumbnail size but it fails only on this particular one. More precisely, it fails with the extract_area: bad extract area error at the crop stage that happens in this function after the call to thumbnail_image has been made first. See attached debug session capture bellow.

If I replace the call to thumbnail_image to a call to resize (by forcing the else part of the if in your function), then crop doesn't fail.

This discussion continued in the other open issue (https://github.com/CreunaFI/vips-image-editor/issues/1) with @jcupitt, who is suggesting that the problem lies with the fact that you compute the $scale ratio with 2 different methods. By running the code step by step for different images, I was not able to pinpoint exactly why in this particular case the extract area is "bad". A rounding issue maybe?

Hey @joppuyo, my thought from a quick look at the code was that scale was being computed in two different ways (one round up, one round down). The code that calls thumbnail computes it differently from the later section that scales for the crop, so in some cases the crop will miss.

Upon further inspection, the error arrises because when called on this particular image with parameters width = 650 and height = 433, the thumbnail_image function return a 649x433 images. Thus, the following call to crop fails for out of bounds cause. So, it's indeed a rounding issue, but it looks that the issue is inside the thumbnail_image implementation itself. @jcupitt what do you think?

I think it's the computation of target_w and target_h. thumbnail uses round-to-nearest, but this code is using round-up.

Off-topic, but thumbnail_image is extremely slow and should be avoided if possible. You'll get something like a 4x speedup if you use plain thumbnail instead. It should give a good quality improvement for formats like SVG and PDF as well.

I don't think that's the problem in this particular case because target_w and target_h are equal to 1. So using ceil or floor doesn't change the outcome. Working on the hypothesis that it is a rounding issue due to image ratio constraints, after reading the VIPS doc, I added 'size' => 'force' argument to the thumbnail_image call. After doing so, the resulting image is not 1 pixel short anymore and the following crop works. Do you think it's a good solution or too dirty?

The resulting call code is

$resized = $this->image->thumbnail_image($target_w, [
                    'height' => $target_h, 'size' => 'force',
                ]);

Too dirty I think :(

I would try to fix the underlying problem.

OK. In any case, putting aside the supposed rounding issue in the code of @joppuyo , it doesn't change the fact that when calling thumbnail_image with width = 650 and height = 433, the thumbnail_image function return a 649x433 image. I'm very sorry for insisting, especially since I don't know VIPS a lot and you are the expert on this, but doesn't that simply point in the direction of libvips and not a problem with the code of this plugin?

Or is it expected behavior because 649x433 fits within 650 x 433 (as per the doc - https://libvips.github.io/libvips/API/current/libvips-resample.html#vips-thumbnail) ?

Again, sorry for the dumb question ;)

Ah I see, sorry. Yes, it's expected behaviour for thumbnail.

Note that imagemagick has the same behaviour:

john@yingna ~/pics $ convert chatelp.jpeg -resize 650x433 x.jpg
john@yingna ~/pics $ identify x.jpg
x.jpg JPEG 649x433 649x433+0+0 8-bit sRGB 382651B 0.020u 0:00.010

OK, then... for the time being, I guess that my solution of slightly breaking the image ratio for this edge case using the 'size' => 'force' option when resizing is not so bad! I'll stick with it for now and we'll see what @joppuyo wants to do in the end. I won't make a push request just for this.

Maybe another solution is not to do a resize and then a crop but to use the crop option of thumbnail_image to do both in one go. I guess that doing so, the output image will be 649x433 but without the crop error.

Also, I won't change thumbnail_image to thumbnail because, reading from the other opened issue, it seems that there is some compatibility issue with wordpress when previous rotation or cropping are applied.

Thanks for looking into it deeper.

In my opinion, it's not so important what the underlying technical issue is, I think the most important thing is to fix this plugin so works correctly without throwing exceptions.

WordPress image processing is pretty complex because you have resizing, cropping, rotation and flipping, and other operations. Also, WordPress image sizes can be fixed or proportional and they can have crop gravity. It's been a while since the last time I worked on this plugin it's going to take some time for me to get up to speed and also test all possible cases.

In any case, I will try to match the functionality with ImageMagick and try to make sure there are no edge cases like this issue.

I would love to use thumbnail directly instead of thumbnail_image but unfortunately, since this class extends the WP_Image_Editor class, I can't control how it's called. WordPress can call class methods like resize , crop, rotate in any order so I need to operate on vips image object and can't simply pass a file from the disk to thumbnail.

In my testing (by running this plugin via WordPress admin interface) thumbnail_file was 11% faster than resize so there's already a benefit of using it if it's available.