search

nicolashmln / strapi-plugin-responsive-image

Custom responsive image formats for https://strapi.io
154 stars 28 forks source link

Global quality setting doesn't have effect #49

Open ufoch opened 4 months ago

ufoch commented 4 months ago

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 commented 4 months ago

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 #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 commented 2 months ago

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