Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error: shrinkv: image has shrunk to nothing #1782

Closed
MartijnKooij opened this issue Jul 5, 2019 · 15 comments
Closed

Error: shrinkv: image has shrunk to nothing #1782

MartijnKooij opened this issue Jul 5, 2019 · 15 comments

Comments

@MartijnKooij
Copy link

We are seeing an issue that looks a lot like #1003 while using your library through the AWS serverless image handler: https://github.com/awslabs/serverless-image-handler/tree/master/source/image-handler

We have a source image of 180x4 pixels which we are trying to resize to a width of 25 (and we don't specify the height). We are passing the following JSON edit options to the serverless solution:
"resize": { "width": 25, "fit": "inside", "withoutEnlargement": true }

I think I was able to write a unit test proving this bug using the latest master here (0.22.1) using the attached 180x4 pixels image saved in the test fixtures. (Please note that I am not yet familiar with your library so I am trying to resize it in the way "I think" the AWS serverless image handler does it)

it('Image should not shrink to nothing', function (done) {
    sharp(fixtures.inputLine)
      .resize({
        width: 25,
        withoutEnlargement: true,
        fit: sharp.fit.inside 
      })
      .toBuffer(function (err, data, info) {
        if (err) throw err;
        assert.strictEqual(true, data.length > 0);
        assert.strictEqual(25, info.width);
        done();
      });
  });

line

@lovell
Copy link
Owner

lovell commented Jul 5, 2019

Hi, I can reproduce this error via the following code, and yes, this relates to the same problem as #1003 so I suspect we'll have to tighten this logic further.

sharp({
  create: {
    width: 180,
    height: 4,
    channels: 3,
    background: "red"
  }
}).resize(29)...

@lovell lovell added the triage label Jul 5, 2019
@MartijnKooij
Copy link
Author

Yes, sorry, this one does indeed fail as well.

  it('Ensure shortest edge (height) is at least 1 pixel', function () {
    return sharp({
      create: {
        width: 180,
        height: 4,
        channels: 3,
        background: 'red'
      }
    })
      .resize(29)
      .toBuffer({ resolveWithObject: true })
      .then(function (output) {
        assert.strictEqual(2, output.info.width);
        assert.strictEqual(1, output.info.height);
      });
  });

@kleisauke
Copy link
Contributor

fwiw, this works fine:

$ node -e "require('sharp')({create: { width: 180, height: 4, channels: 3, background: 'red'}}).toFile('x.jpg')"
$ node -e "require('sharp')('x.jpg').resize(29).toFile('x2.jpg')"
$ vipsthumbnail x.jpg --size 29x

@lovell
Copy link
Owner

lovell commented Jul 6, 2019

@jcupitt I can reproduce this using vips resize at the command line using a 180x4 input and a scale of 0.161, which I think should produce target dimensions of 29x1.

$ G_MESSAGES_DEBUG=all vips resize 180x4.png out.png 0.161
VIPS-INFO: 13:00:11.633: shrinkv by 3
VIPS-INFO: 13:00:11.633: shrinkv sequential line cache
VIPS-INFO: 13:00:11.633: shrinkh by 3
VIPS-INFO: 13:00:11.633: residual reducev by 0.483
VIPS-INFO: 13:00:11.633: reducev: 13 point mask
VIPS-INFO: 13:00:11.635: reducev: using vector path
reducev: image has shrunk to nothing

jcupitt added a commit to libvips/libvips that referenced this issue Jul 7, 2019
vips_resize() will break the aspect ratio and limit resize on an axis if
it would result in an image of less than 1px on that axis

see lovell/sharp#1782 (comment)
@jcupitt
Copy link
Contributor

jcupitt commented Jul 7, 2019

I think I found a simple fix, what do you think? I now see:

$ vips black x.v 180 4
$ vipsthumbnail x.v -s 29x
$ vipsheader tn_x.jpg 
tn_x.jpg: 29x1 uchar, 3 bands, srgb, jpegload

I made the change to 8.8.1, but perhaps it's too large? Should this be for 8.9?

@lovell
Copy link
Owner

lovell commented Jul 7, 2019

@jcupitt Amazing, thanks John, I think this is OK for a minor release (a similar fix in sharp #1003 went out in a minor release also).

@jcupitt
Copy link
Contributor

jcupitt commented Jul 7, 2019

OK, let's push out 8.8.1 with this change now. I'll make a release.

@lovell lovell added this to the v0.23.0 milestone Jul 8, 2019
@lovell lovell added bug and removed triage labels Jul 8, 2019
@jcupitt
Copy link
Contributor

jcupitt commented Jul 8, 2019

8.8.1 is out now with this fix, thanks for the report!

@lovell
Copy link
Owner

lovell commented Jul 29, 2019

sharp v0.23.0 / libvips v8.8.1 now available. Thanks for reporting this.

@lovell lovell closed this as completed Jul 29, 2019
@kleisauke
Copy link
Contributor

Should we do a similar logic for shrink-on-load? For example:

$ wget https://i.imgur.com/hn28QTS.jpg
$ vipsheader hn28QTS.jpg
hn28QTS.jpg: 1x151 uchar, 3 bands, srgb, jpegload
$ vipsthumbnail hn28QTS.jpg -s x30 --vips-info
VIPS-INFO: 12:43:44.278: thumbnailing hn28QTS.jpg
VIPS-INFO: 12:43:44.279: selected loader is VipsForeignLoadJpegFile
VIPS-INFO: 12:43:44.279: input size is 1 x 151
VIPS-INFO: 12:43:44.279: loading jpeg with factor 2 pre-shrink
VIPS-INFO: 12:43:44.279: pre-shrunk size is 0 x 75
VIPS-INFO: 12:43:44.279: converting to processing space srgb
VIPS-INFO: 12:43:44.279: residual reducev by 0.4
VIPS-INFO: 12:43:44.279: reducev: 16 point mask

(vipsthumbnail:8563): GLib-GObject-WARNING **: 12:43:44.279: value "0" of type 'gint' is invalid or out of range for property 'width' of type 'gint'
vipsthumbnail: unable to thumbnail hn28QTS.jpg
embed: parameter width not set

jcupitt added a commit to libvips/libvips that referenced this issue Aug 27, 2019
We could pre-shrink so much that an axis went to 0.

See lovell/sharp#1782 (comment)
@jcupitt
Copy link
Contributor

jcupitt commented Aug 27, 2019

Oh, good point, I added a test.

@kleisauke
Copy link
Contributor

Wouldn't it be better to do this logic in vips_thumbnail_calculate_common_shrink? For example:

@@ -370,11 +370,20 @@
 {
 	double hshrink;
 	double vshrink;
+	double shrink;
 
 	vips_thumbnail_calculate_shrink( thumbnail, width, height, 
 		&hshrink, &vshrink ); 
 
-	return( VIPS_MIN( hshrink, vshrink ) );
+	shrink = VIPS_MIN( hshrink, vshrink );
+
+	/* We don't want to pre-shrink so much that we send an axis to 0.
+	*/
+	if( shrink > thumbnail->input_width ||
+		shrink > thumbnail->input_height )
+		shrink = 1.0;
+
+	return( shrink );
 }
 
 /* Find the best jpeg preload shrink.
@@ -503,12 +512,6 @@
 				thumbnail->input_width, 
 				thumbnail->page_height ); 
 
-	/* We don't want to pre-shrink so much that we send an axis to 0.
-	 */
-	if( factor > thumbnail->input_width ||
-		factor > thumbnail->input_height )
-		factor = 1.0;
-
 	g_info( "loading with factor %g pre-shrink", factor ); 
 
 	if( !(im = class->open( thumbnail, factor )) )

Otherwise, I don't think the shrink-in-load will work for tiff/openslide images where "factor" is a level or page number.

@jcupitt
Copy link
Contributor

jcupitt commented Aug 27, 2019

I think it's harmless in those cases, since factor (page number) will (almost?) be never be greater than unzoomed width / height.

You can get the same shrink-to-zero behaviour with PDF, SVG and WEBP, so putting the shrink-to-zero test at the end seemed to make sense.

@kleisauke
Copy link
Contributor

Note that PDF, SVG and WebP is a scale factor rather than a shrink factor. For example (tested on master):

$ cat x.svg
<svg width="1" height="10"></svg>
$ vipsthumbnail x.svg -s x5 --vips-info
VIPS-INFO: 17:51:18.084: thumbnailing x.svg
VIPS-INFO: 17:51:18.088: selected loader is VipsForeignLoadSvgFile
VIPS-INFO: 17:51:18.088: input size is 1 x 10
VIPS-INFO: 17:51:18.088: loading with factor 0.5 pre-shrink

(vipsthumbnail:6690): GLib-GObject-WARNING **: 17:51:18.088: value "0" of type 'gint' is invalid or out of range for property 'width' of type 'gint'
VIPS-INFO: 17:51:18.088: pre-shrunk size is 1 x 5
VIPS-INFO: 17:51:18.088: converting to processing space srgb
VIPS-INFO: 17:51:18.088: premultiplying alpha
VIPS-INFO: 17:51:18.088: unpremultiplying alpha
VIPS-INFO: 17:51:18.088: thumbnailing x.svg as ./tn_x.jpg

(vipsthumbnail:6690): GLib-GObject-WARNING **: 17:51:18.089: value "0" of type 'gint' is invalid or out of range for property 'width' of type 'gint'
$ vips copy x.svg x.webp
$ vipsthumbnail x.webp -s x5 --vips-info
VIPS-INFO: 17:56:27.632: thumbnailing x.webp
VIPS-INFO: 17:56:27.633: selected loader is VipsForeignLoadWebpFile
VIPS-INFO: 17:56:27.633: input size is 1 x 10
VIPS-INFO: 17:56:27.633: loading with factor 0.5 pre-shrink
vipsthumbnail: unable to thumbnail x.webp
webp: bad image dimensions

Applying the above patch will not produce these warnings / errors.

@jcupitt
Copy link
Contributor

jcupitt commented Aug 27, 2019

Oh, you're right, sorry, I hadn't read your patch carefully enough. Yes, that is neater.

I also changed it to always use shrink (rather than a mix of shrink and scale) so maybe it'll be a bit less confusing in future.

Sorry, I should have done this in a branch :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants