Shrink-on-load broken on v8.8.0-rc1 #1297
Comments
|
Argh, it should be fixed now, sorry about that. thumbnail shrink-on-load still fails for multi-page images, but that's something to fix later. |
|
Seems to work now, thanks! Indeed, shrink-on-load fails for multi-page images when a height is specified (because it shrinks the whole height instead of just the page height). See for example these test-cases: |
|
I'd like to get 8.8 out of the door, so I'm inclined to leave this unimplemented for now. Do you think we should add this feature too? I think autorotate will probably break too. |
|
I agree, lets leave this unimplemented for now. It's just a minor issue that I've encountered, and it only happens with PDF files that are explicitly loaded with A quick solution would be to skip shrink-on-load for multi-page images. |
After further testing, this assumption appears to be incorrect: DetailsHowever, it will break when writing to animated WebP: Details |
|
I found a fix: --- libvips/foreign/pdfload.c
+++ libvips/foreign/pdfload.c
@@ -278,6 +278,9 @@
return( -1 );
}
+ if (pdf->scale != 1.0)
+ pdf->scale *= pdf->n;
+
/* Lay out the pages in our output image.
*/
if( !(pdf->pages = VIPS_ARRAY( pdf, pdf->n, VipsRect )) )This multiplies the amount of scale by the number of pages to be loaded. Before this patch: After: Writing to an animated WebP now also seems to work: (notice the pre-shrunk size is |
|
Nice! But I think it should go into vips_thumbnail_open() instead -- this doesn't really fit in the loader. Thumbnail open needs a bit of reorganising :( |
|
Indeed, a neater place would be to move this logic to Details--- libvips/resample/thumbnail.c
+++ libvips/resample/thumbnail.c
@@ -476,9 +476,17 @@
factor = 1.0 /
vips_thumbnail_calculate_common_shrink( thumbnail,
thumbnail->input_width,
- thumbnail->input_height );
+ thumbnail->input_height ) * thumbnail->n_pages;
- g_info( "loading PDF/SVG with factor %g pre-scale", factor );
+ g_info( "loading PDF with factor %g pre-scale", factor );
+ }
+ else if( vips_isprefix( "VipsForeignLoadSvg", thumbnail->loader ) ) {
+ factor = 1.0 /
+ vips_thumbnail_calculate_common_shrink( thumbnail,
+ thumbnail->input_width,
+ thumbnail->input_height );
+
+ g_info( "loading SVG with factor %g pre-scale", factor );
}
else if( vips_isprefix( "VipsForeignLoadHeif", thumbnail->loader ) ) {
/* 'factor' is a gboolean which enables thumbnail load insteadBut that will fail if you thumbnail a single page ( By the way, It gives Poppler and tile errors when doing Non-animated WebP seems also to work (with this patch): Details--- libvips/foreign/vips2webp.c
+++ libvips/foreign/vips2webp.c
@@ -373,10 +373,10 @@
{
int page_height = vips_image_get_page_height( image );
- if( page_height < image->Ysize )
+ /*if( page_height < image->Ysize )
return( write_webp_anim( write, image, page_height ) );
- else
- return( write_webp_single( write, image ) );
+ else*/
+ return( write_webp_single( write, image ) );
}
static voidI could open a new issue, if you want. |
shrink-on-load should now work for multipage PDF thumbnailing see #1297
|
I think I've fixed it. I tried to make the logic simpler too. Making an anim gif of a large PDF is surprisingly fast! I'll have a look at webp write. |
|
It seems to work with webp as well: And the image looks OK. |
|
I reenabled webp shrink-on-load as well. This fails: But only because this also fails: ie. shrink-on-load is broken in webpload for animated webp images. |
|
Thanks! Specifying a height of 900 works now. I tried some other dimensions, it looks like it's breaking at x500 now. :( See: Also, does libvips/libvips/resample/thumbnail.c Line 243 in 7326a40 Regarding shrink-on-load for animated webp images, perhaps we could disable the shrink option within |
|
Huh that's wierd. It should shrink-on-load to exactly the correct size. I'll have a look. |
We were doing simple round down for page size with @scale param. But this makes it very sensitive to rounding errors, so do rint() instead. vips-resize() does rint() on the output size as well for the same reason. See #1297 (comment)
|
pdf shrink-on-load was rounding image size down instead of rounding to nearest, so it was very sensitive to rounding errors. It seems to work now: |
|
I think It should pick a layer >=200% larger than the target size, which I think it's not doing. |
|
Simple demo of the webp shrink-on-load issues:
|
|
Phew, it seems to all work now, for animated webp too. I'll close. |
|
I finally found some time to test this, it seems to be working now. Thanks! #839 can also be closed, I think. |
It seems shrink-on-load is broken for pyramidal tiff images: [kleisauke@pc-kaw fixtures]$ wget http://merovingio.c2rmf.cnrs.fr/iipimage/PalaisDuLouvre.tif
[kleisauke@pc-kaw fixtures]$ vipsthumbnail PalaisDuLouvre.tif -s 500x -o tn_%s.jpg
[kleisauke@pc-kaw fixtures]$ vipsheader tn_PalaisDuLouvre.jpg
tn_PalaisDuLouvre.jpg: 500x104 uchar, 3 bands, srgb, jpegloadThat should produce a I also tried to load all pages explicitly (to ensure (I could open a new issue, if you want) |
thumbnail was not testing pyramidal tiff images for pyramidness correctly. see #1297
|
Oooop!! You're right, adding thumbnail of PDF and GIF had broken thumbnail of pyramid. I now see: |
|
And thank you for reporting this! It'll be in 8.8.1. |
|
Seems to work now, thanks! 👍 |
It seems that the shrink-on-load feature is broken on v8.8.0-rc1:
Possible due to:
libvips/libvips/resample/thumbnail.c
Line 362 in 120c3b6
page_heightis uninitialized when doing pre-shrink:libvips/libvips/resample/thumbnail.c
Lines 584 to 587 in 120c3b6
The weird thing is that I couldn't reproduce this on Windows:
However, the output seems to be wrong (should be
100x100):This was found while testing the Windows binaries. The libvips test-suite failed on this line:
libvips/test/test-suite/test_resample.py
Line 151 in 120c3b6
The text was updated successfully, but these errors were encountered: