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

[3.0] Fixes using ActiveStorage adapter with libvips as variant processor #4325

Merged
merged 1 commit into from
Apr 11, 2022

Conversation

waiting-for-dev
Copy link
Contributor

Backports #4317

Rails 6.0 introduced the option of using libvips as an alternative to imagemagick as a backend image processor.

Both processors are handled using the image_processing library as adapter. However, the API for both is not fully compatible. What it's important for us when generating a variant, the strip option needs to be within the saver group to work with both adapters. See https://gist.github.com/brenogazzola/a4369965a1da426d50f11d080fe2e563#1-move-everything-that-has-to-do-with-compression-to-a-saver-hash
for details. We also need to explicitly coerce size values to integers for libvips.

We're also allowing to run tests with that adapter by setting the ACTIVE_STORAGE_VARIANT_PROCESSOR env variable. However, we still default it to :mime_magick (Ruby's wrapper for imagemagick), Rail's default previous to 7.0. We'll update it once we integrate support for Rails 7.0.

I also manually tested that metadata is still stripped when using both
adapters.

Description

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)
  • I have attached screenshots to this PR for visual changes (if needed)

[Rails 6.0
introduced](https://github.com/rails/rails/blob/6-0-stable/activestorage/CHANGELOG.md#rails-600beta1-january-18-2019)
the option of using libvips as an alternative to imagemagick as a
backend image processor.

Both processors are handled using the
[image_processing](https://github.com/janko/image_processing) library as
adapter. However, the API for both is not fully compatible. What
it's important for us when generating a variant, the `strip` option
needs to be within the `saver` group to work with both adapters. See
https://gist.github.com/brenogazzola/a4369965a1da426d50f11d080fe2e563#1-move-everything-that-has-to-do-with-compression-to-a-saver-hash
for details. We also need to explicitly coerce size values to integers
for libvips.

We're also allowing to run tests with that adapter by setting the
`ACTIVE_STORAGE_VARIANT_PROCESSOR` env variable. However, we still
default it to `:mime_magick` (Ruby's wrapper for imagemagick), Rail's
default previous to 7.0. We'll update it once we integrate support for
Rails 7.0.

I also manually tested that metadata is still stripped when using both
adapters.
@waiting-for-dev waiting-for-dev changed the title Fixes using ActiveStorage adapter with libvips as variant processor [3.0] Fixes using ActiveStorage adapter with libvips as variant processor Apr 1, 2022
@waiting-for-dev waiting-for-dev merged commit 6ed3400 into solidusio:v3.0 Apr 11, 2022
@waiting-for-dev waiting-for-dev deleted the waiting-for-dev/libvips_30 branch April 11, 2022 04:01
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.

2 participants