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

Implement scale option and automatic SVG scaling up #2787

Closed

Conversation

ahukkanen
Copy link

There was earlier some discussion about automatic SVG scaling at: #1421

This proposes two changes to sharp:

  1. Add the scale option for vector images (SVG and PDF)
  1. Automatically calculate the scale option for SVGs that are rendered in lower resolution than the target resolution

The automatic scaling only happens if the following conditions are met:

  • The image is either an SVG or PDF
  • The SVG rendered resolution (either it's width and height attributes or its viewbox attribute) is lower than the target resolution

The SVG will be rendered twice if these conditions are met because only after the first load of the image, the rendered size is known to sharp. If there are any better ways to do this, I'm glad to hear alternatives. For SVGs you could also read it from the XML document but it would require some extra logic too (considering both viewbox and width/height attributes). For PDFs I believe an additional library would be needed.

@ahukkanen ahukkanen force-pushed the feature/vector-image-scaling branch from 39776c7 to 6caf521 Compare July 7, 2021 15:58
@ahukkanen
Copy link
Author

ahukkanen commented Jul 7, 2021

And just for reference, if you want to test this in action, take the example from: itgalaxy/favicons#264

  1. Create the following SVG image, e.g. input.svg:
<svg width="64" height="64" clip-rule="evenodd" image-rendering="optimizeQuality" shape-rendering="geometricPrecision" text-rendering="geometricPrecision" version="1.1" viewBox="0 0 64 64" xmlns="http://www.w3.org/2000/svg"><rect width="64" height="64" fill="#fff" style="paint-order:stroke fill markers"/><path d="m8.9671 31.428 5.4166 5.5052a20.452 20.452 0 0 1 1.8508-1.8969c3.9921-3.5988 9.522-5.8252 15.644-5.8252v0.0153h0.0012v-0.0153c6.1217 0 11.653 2.2264 15.645 5.8252 0.73699 0.6662 1.4221 1.3772 2.047 2.1308l5.4638-5.4627a28.333 28.333 0 0 0-2.3575-2.393c-5.3351-4.8083-12.691-7.7847-20.798-7.7847v-0.0142h-0.0012v0.0142c-8.1083 0-15.463 2.9764-20.797 7.7847a27.925 27.925 0 0 0-2.1142 2.1166zm23.147 2.719v0.0153c5.5512 1e-3 10.59 2.0386 14.25 5.3327v1e-3l-4.4647 4.4658-1.0299 0.9626c-2.2866-1.9099-5.361-3.0768-8.7555-3.0768v0.0154-0.0154c-3.5658 0-6.7807 1.2886-9.0969 3.3721-0.0578 0.052-0.1145 0.1051-0.1724 0.1583l-5.2076-5.6753c0.0745-0.0697 0.15-0.1394 0.2268-0.2079 3.6591-3.2941 8.7-5.3315 14.25-5.3327v-0.0154zm0.144 12.045c2.2206 0 4.2403 0.8586 5.7474 2.2606l-6.4937 6.6875-5.5158-6.1655c1.5449-1.709 3.7784-2.7827 6.2622-2.7827zm-27.047-19.252a38.128 38.128 0 0 1 1.0063-0.9437c6.4689-5.83 15.421-9.4371 25.323-9.4371v0.0142-0.0142c9.9013 0 18.854 3.6071 25.324 9.4371a35.059 35.06 0 0 1 1.689 1.6335l5.4485-5.445a42.135 42.135 0 0 0-1.9843-1.9134c-7.8143-7.0406-18.59-11.397-30.477-11.397v-0.0153 0.0153c-11.888 0-22.663 4.356-30.475 11.397a42.522 42.522 0 0 0-1.0654 0.9934l5.2111 5.6752z" fill="#295682" stroke-width=".1"/></svg>
  1. Create the following resize script at the root of the project resize.js:
const sharp = require('./lib/index');

sharp('input.svg')
  .resize(1024)
  .toFile('output.png');
  1. Run the script with and without this fix:
$ node resize.js
  1. Take a look at the output.png image

Example of the outputs before and after the fix

@ahukkanen ahukkanen force-pushed the feature/vector-image-scaling branch from 6caf521 to 6a96fcc Compare July 7, 2021 21:03
@kleisauke
Copy link
Contributor

If there are any better ways to do this, I'm glad to hear alternatives.

I think a better way to achieve this is to have shrink-on-load take SVG images into account, see for example commit kleisauke@62b2113 which ports the vipsthumbnail calculations to sharp. This should also resolve issue #2275.

However, this commit has discovered a possible error in the intermediate shrink step when the fastShrinkOnLoad option is enabled, see:

2725x2225 (input dimensions) -> 340x278 (8 pre-shrink, fastShrinkOnLoad) -> 320x262 (target 320x320)
2725x2225 (input dimensions) -> 681x556 (4 pre-shrink) -> 320x261 (target 320x320)

The previous implementation returned both 320x261 with fastShrinkOnLoad enabled or disabled, vipsthumbnail returns 320x262 as dimension when patching libvips like this:

--- a/libvips/resample/thumbnail.c
+++ b/libvips/resample/thumbnail.c
@@ -490,11 +490,11 @@ vips_thumbnail_find_jpegshrink( VipsThumbnail *thumbnail,
      *
      * Leave at least a factor of two for the final resize step.
      */
-    if( shrink >= 16 )
+    if( shrink >= 8 )
         return( 8 );
-    else if( shrink >= 8 )
-        return( 4 );
     else if( shrink >= 4 )
+        return( 4 );
+    else if( shrink >= 2 )
         return( 2 );
     else 
         return( 1 );
vipsthumbnail test/fixtures/2569067123_aca715a2ee_o.jpg -s "320x320" -o x.jpg --vips-info

I still haven't figured out the best way to handle this, maybe fastShrinkOnLoad should produce the same "normalised" dimensions if it wasn't enabled. Another option would be to just deprecate fastShrinkOnLoad and always leave at least a factor of two for the final resize step.

/cc @jcupitt

@jcupitt
Copy link
Contributor

jcupitt commented Jul 8, 2021

Hi @kleisauke I'm not sure I understand the thumbnail patch you suggest. Won't that stop it using lanczos3 for the final 200% of JPEG resize? I'm probably misunderstanding the context here.

It's certainly true that you don't want that behaviour for PDF / SVG / etc., since they will render well with no extra resize step required.

@kleisauke
Copy link
Contributor

Sorry for the confusion, that patch was only to reproduce the dimension issue I had with sharp's fastShrinkOnLoad option, it reflects this logic:
https://github.com/kleisauke/sharp/blob/62b2113d2e8d6f09fc522069b7457c2c30f51b18/src/pipeline.cc#L153-L162
(i.e., take greater advantage of the JPEG shrink-on-load feature by not leaving at least a factor of two for the final resizing step)

Indeed, WebP, PDF and SVG images can resize in one go without requiring a intermediate resize step. sharp currently uses the deprecated shrink option for webpload, so I expect WebP images to be resized much faster within the above commit.

@ahukkanen
Copy link
Author

ahukkanen commented Jul 8, 2021

I'm not quite following (or understanding) the shrink-on-load method @kleisauke suggested above. Do you mean a shrink value between 0..1 (i.e. shrink = 1 / scale) causes the vector image to be rendered in larger size as needed in the sample? So are you suggesting that after calculating the shrink factors here the SVG/PDF image would be reloaded similarly to the JPEG and WebP reloading here?

But that does not solve the original problem I had which is:
How we can know the original SVG/PDF size before the image is rendered at all? The most convenient to me would seem to be passing the correct "scale" option already when the image is loaded for the first time. Even if the suggested shrink method works, it would require re-rendering the vector image, which was the problem I had.

Also, I'm not sure if that works with the other operations done before the shrink calculations (rotate, trim, extraction). Wouldn't these operations be lost in case the image is re-rendered after them?

Regarding speed, the method I implemented seems to be faster than running the same sample script (as presented above) with the current sharp source code. But that's not a surprise to me because the rendered bitmap does not need to be resized up pixel-by-pixel anymore when it is already rendered at the target size in the beginning of the process.

Also, looking at the vipsthumbnail source code, I can see that they are also passing the scale option when loading PDF, SVG or WebP images:
https://github.com/libvips/libvips/blob/6dd6fafa2a65310ca7cef7681a77a3d430261d6d/libvips/resample/thumbnail.c#L1127

The factor is calculated here based on the input and target dimensions:
https://github.com/libvips/libvips/blob/6dd6fafa2a65310ca7cef7681a77a3d430261d6d/libvips/resample/thumbnail.c#L604-L606

So I'm not sure if there's much different with their approach either. To me, they also seem to be loading the source image twice for these calculations.

@jcupitt
Copy link
Contributor

jcupitt commented Jul 8, 2021

@ahukkanen when you open an image with libvips, it only reads the header, no pixels are rendered, so it's always fast. Pixels are computed much later when they are actually needed.

The vipsthumbnail code is opening the SVG once to get the "natural" pixel dimensions, then computing a scale and loading again. This will only render the SVG once and always render it directly to the correct number of pixels (hopefully). The same technique works for webp, pdf, jpeg, etc.

Lazy rendering of pixels is nice because you don't need separate PingImage / ReadImage API entry points (the imagemagick solution). Separate code paths for header read and pixel read has often caused me trouble since for many formats they will return images with different metadata.

@ahukkanen
Copy link
Author

@jcupitt Aha OK, thanks for the explanation!

It makes sense and I think in sharp there's also no rendering happening then twice if the image is re-loaded after the first load, so the problem I had in mind does not really exist.

But the question regarding the "lost" operations done prior to resize is still open for me... I can investigate that too but I'd guess you guys have a better understanding about those than me.

@lovell
Copy link
Owner

lovell commented Jul 9, 2021

@ahukkanen Thank you for the PR. Are you able to test this useful auto-scaling logic on larger target dimensions e.g. sharp('input.svg').resize(65536)... as I suspect we might start hitting librsvg limits.

@kleisauke Was your comment about the existing shrink-on-load feature that it should really be more of a generic "scale-on-load"? I'd be happy to deprecate the former for the power of the latter.

Perhaps we should split this PR into a first part that exposes scale then a follow-up to add auto-scale-on-load?

@jcupitt
Copy link
Contributor

jcupitt commented Jul 9, 2021

Yes, recent librsvg and cairo versions have very strict 32k x 32k pixels limits, unfortunately :( Perhaps we should consider another SVG library, though I don't know which one.

@kleisauke kleisauke mentioned this pull request Jul 9, 2021
2 tasks
@kleisauke
Copy link
Contributor

Also, I'm not sure if that works with the other operations done before the shrink calculations (rotate, trim, extraction). Wouldn't these operations be lost in case the image is re-rendered after them?

Currently, in sharp's pipeline, some operations (i.e., gamma correction, pre-resize extraction and trimming) needs to be done on the original image, so scale-on-load is skipped in case this happens.

Was your comment about the existing shrink-on-load feature that it should really be more of a generic "scale-on-load"? I'd be happy to deprecate the former for the power of the latter.

Yep, instead of another specific pass for SVG and PDF images we could adjust the current shrink-on-load logic to a more generic "scale-on-load", just like vips_thumbnail. I just opened draft PR #2789 to discuss this further.

Perhaps we should split this PR into a first part that exposes scale then a follow-up to add auto-scale-on-load?

I'm not sure if the scale option of webpload, svgload, etc. should be exposed to the user, as it will probably collide with the auto-scale-on-load logic, see for example:

$ curl -LO https://github.com/lovell/sharp/raw/master/test/fixtures/circle.svg
$ vipsthumbnail circle.svg[scale=100] -s 1024x -o output.png --vips-info
VIPS-INFO: 13:30:35.842: thumbnailing circle.svg[scale=100]
VIPS-INFO: 13:30:35.845: selected loader is VipsForeignLoadSvgFile
VIPS-INFO: 13:30:35.845: input size is 800 x 800
VIPS-INFO: 13:30:35.845: loading with factor 0.78125 pre-shrink
VIPS-INFO: 13:30:35.846: pre-shrunk size is 10 x 10
VIPS-INFO: 13:30:35.846: converting to processing space srgb
VIPS-INFO: 13:30:35.846: premultiplying alpha
VIPS-INFO: 13:30:35.846: residual scale 102.4 x 102.4
VIPS-INFO: 13:30:35.847: unpremultiplying alpha
VIPS-INFO: 13:30:35.847: thumbnailing circle.svg[scale=100] as ./output.png

@ahukkanen
Copy link
Author

I would be more than happy with the solution provided by @kleisauke at #2789.

I tested it for the presented use case and it works correctly.

@tomasklaen
Copy link

Any update on this? Hacking it with density is awkward, and prone to errors when too big upscaling is required :(

@lovell
Copy link
Owner

lovell commented Nov 16, 2021

I think the approach proposed in #2789 should cover all the scenarios here. Thank you very much @ahukkanen for taking the time to submit this PR in the first place. Let's continue to track this at #2789.

@lovell lovell closed this Nov 16, 2021
@ahukkanen ahukkanen deleted the feature/vector-image-scaling branch December 12, 2024 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants