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

Improve thumbnail quality and size #835

Merged
merged 6 commits into from
Apr 4, 2024
Merged

Improve thumbnail quality and size #835

merged 6 commits into from
Apr 4, 2024

Conversation

onli
Copy link
Member

@onli onli commented Apr 3, 2024

This tackles a couple of issues with the thumbnail creation:

  1. Gives jpeg thumbnails created without imagemagick a reasonable quality setting, so they are no longer as huge as they were before (often bigger than the full sized original).
  2. Tunes imagemagicks parameters suggested in serendipity_config and actually enables those settings, so no longer just the defaults are used.
  3. Finally, adds https://github.com/spatie/image-optimizer to apply image optimizers to the thumbnails, if they are installed.

That last step adds a couple of dependencies, plus the project has a security issue open at spatie/image-optimizer#210. We should wait with this PR until after that one is patched, and I'm also okay if we just revert the last commit and forego that optimizer step. It might better fit as a plugin anyway. But I wanted to present the option. What do you think?

onli added 4 commits April 3, 2024 22:37
@garvinhicking
Copy link
Member

garvinhicking commented Apr 4, 2024

Interesting... the imageoptimizer looks like a huge dependency set.

Where exactly is it utilized? I could only see the "use" statement in functions_images, but how would that affect things?

Ok saw it now, github didnt load completely due to diff size. Hm, I'm torn. Do you have an example image where the optimizing effects show a real impact?

I'm a bit afraid this might be micro-optimizing at a large dependency cost.

(Adjusting quality to 85 and the other things all look good to me, you could happily merge that!)

@onli
Copy link
Member Author

onli commented Apr 4, 2024

Ok saw it now, github didnt load completely due to diff size. Hm, I'm torn.

Me as well.

Do you have an example image where the optimizing effects show a real impact?

Yes, https://onli2.uber.space/s9y_dev/index.php?/archives/16-png-without-image-optimizer.html vs https://onli2.uber.space/s9y_dev/index.php?/archives/15-imageoptimizer-chain-png.html (make it a small browser window so it loads the thumbnail). 1.3MB vs 900KB. But it's a pathological case, one normally would not use .png for such a motive. And for jpg the difference was miniscule.

Probably best to revert the last commit, I think more and more this should be a plugin.

@garvinhicking
Copy link
Member

Agreed. Let's take out the dependency and better factor it into a plugin.

Thanks!

@onli onli merged commit 99ce856 into master Apr 4, 2024
4 checks passed
@onli onli deleted the feature/thumbnailQuality branch April 4, 2024 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants