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

Global quality setting doesn't have effect #49

Open
ufoch opened this issue Mar 7, 2024 · 2 comments
Open

Global quality setting doesn't have effect #49

ufoch opened this issue Mar 7, 2024 · 2 comments

Comments

@ufoch
Copy link

ufoch commented Mar 7, 2024

The global quality setting appears to be broken.
E. g. when I change the value between two test uploads of the same file from 80 to 20, I expect the output file sizes to change, but they don't.
Everything else works fine, all configured formats are generated, just the configured quality value is not taken into account.

My strapi version is v4.20.0.
It's running on Node v20.11.0.
I set up the TypeScript version with strapi-server.ts (the JavaScript version didn't work at all; maybe this is also related to this issue).

@ufoch
Copy link
Author

ufoch commented Mar 8, 2024

I found the root of the problem by myself:

The resizeFileTo() function in the image-manipulation module
https://github.com/nicolashmln/strapi-plugin-responsive-image/blob/main/server/services/image-manipulation.js
always executes the default case of the contained switch statement, because the format argument is not a string, but an object at runtime. So the quality argument is never passed to the format-specific calls like jpeg(), webp() etc., because they're actually never called and the conversion only happens, because of the call to toFormat() above.

Also, the format argument is actually not needed, as options.convertToFormat will always hold the intended value at this point.
So an easy fix would be to replace format with options.convertToFormat as the switch expression:
... switch (options.convertToFormat) { ...

For everyone waiting for a quick fix, you could just use an utility like https://github.com/ds300/patch-package to easily patch the package until the problem is also fixed in this repo.

A few other points to consider:

  1. When choosing the output format "Same as source", the quality will still not be taken into account, because options.convertToFormat will be empty in this case. Maybe there should be a warning to the users until this would be fixed?

  2. sharp supports the usage of the mozjpeg encoder, which produces smaller file sizes for jpg files. It can be activated by setting mozjpeg to true in the options of jpeg():
    sharpInstance.jpeg({ quality, mozjpeg: true, progressive, force: false });

  3. The use of sharp's toFormat() and the format-specific functions like jpeg() right after each other potentially decreases performance, because the format step is performed twice.

  4. The format argument of resizeFileTo is actually not needed as stated above.

  5. The idea behind the PR Add quality to individual formats #35 makes perfectly sense, because e. g. avif produces the same quality as jpg when a smaller quality argument is passed. Setting only a single global quality, which is used for all formats, doesn't produce optimal results.

  6. The avif output shows a slightly different color than the other ones, possibly due to a stripped color profile. In this case, it could be fixed, by applying keepIccProfile() to the sharp instance as soon as plugin-upload is updated to use sharp a version >=0.33.0.

In any case thanks a lot for the great plugin idea! It saved me from implementing an awful workflow for reponsive images.

@svenruegg
Copy link

@ufoch sadly there is no info on when my PR #35 will be reviewed and merged.

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

No branches or pull requests

2 participants