-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
feat(gatsby-plugin-sharp): Add support for pngquant speed option #9563
Conversation
Was checking linked issue and seems like speed 0 is just using default (which now seems to be 4?) - might be worth to adjust docs before merging this? |
@ascorbic I wanted to update your PR so we can get this merged but I don't have access. could you enable this feature https://blog.github.com/2016-09-07-improving-collaboration-with-forks/ or could you do the changes yourself. Here's a patch file:
you can apply it with |
@wardpeet I do have the "allow edits" option enabled, so I'm not sure why you can't edit. I'll apply the patch. We need to be clear that this is a breaking change, but so will upgrading to the fixed version of pngquant as it no longer supports |
probably something wrong with my setup than. (thanks you!) I'm not certain why this is a breaking change, we're not upgrading pngquant and by default this is 4, if you set it to 0. |
It was a bug in pngquant: it was supposed to default to 4, but until the change was made recently it was mistakenly defaulting to 0. |
Please don't try to use speed 0 for anything. I'm going to make it a hard error in the next version. 0 never existed as a speed setting in pngquant. It never did anything. It never was a default. The whole 0 rumor is a misinterpretation of pngquant's behavior when parsing command-line arguments, where invalid arguments (such as nonsense speed 0) were ignored and did nothing. To get the default speed, don't set any (i.e don't set |
If you want to have an integration test that verifies that pngquant works well quality-wise, then I recommend using an image comparison tool. I use dssim which is pretty robust and gives reliable results for images that pngquant outputs, including dithering. Other tools may be fine too, but be careful if you compare with a tool that computes difference naively per pixel (like imagemagick's
|
@kornelski What confused me was that as my file list here showed, the previous behaviour when it was passed no arguments was to generate files that were not the same as any other speed option - and were smaller than |
@ascorbic as I've said, it was a misinterpretation of what happened. The
|
@kornelski No, I realise that 0 wasn't the default and was just an invalid value - I just still don't understand what it was actually defaulting to, as there was no valid speed value that gave the same output as leaving off the |
Yes, that was a documentation being out of date. At some point I changed the default value without updating the documentation about what speed setting is closest to the internal default setting. |
OK, but is there a speed setting that is the same as the internal default? When I checked before, I couldn't find one that was. |
Before 2.12 it was 3, after 2.12 it's 4: |
OK, I think what threw me was that my test image was smaller when compressed with the default speed than it was with speed 3, so I assumed it must be a slower speed setting. Anyway, thanks for making pngquant: it's awesome, and thanks for clearing that up. |
The speed setting affects amount of effort put into the algorithm, which mostly but not always means higher visual quality. However, higher-quality image may take less space because there's less dithering noise, OR it may take more space, because there's more detail. So there's no certain relationship between speed setting and file size. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…sbyjs#9563) This adds a `pngCompressionSpeed` option to image queries, which is passed through to pngquant as the `speed` option. In most cases this shouldn't be used, but in some cases with large numbers of PNGs this can make a significant difference in build times. As explained in [imagemin-pngquant](https://github.com/imagemin/imagemin-pngquant#imageminpngquantoptionsinput); _"Speed 10 has 5% lower quality, but is 8 times faster than the default."_ [Contrary to the docs](kornelski/pngquant#313) in pngquant and imagemin-pngquant, the default seems to be 0, not 3. I initially defaulted to 3, but the snapshots broke, which is how I saw that the docs were wrong.
This adds a
pngCompressionSpeed
option to image queries, which is passed through to pngquant as thespeed
option. In most cases this shouldn't be used, but in some cases with large numbers of PNGs this can make a significant difference in build times.As explained in imagemin-pngquant; "Speed 10 has 5% lower quality, but is 8 times faster than the default."
Contrary to the docs in pngquant and imagemin-pngquant, the default seems to be 0, not 3. I initially defaulted to 3, but the snapshots broke, which is how I saw that the docs were wrong.