Updating ActiveStorage to use the ImageProcessing gem

rubyonrails-core
1

Hello everyone,

ActiveStorage performs image processing on-the-fly, and for that it currently uses raw MiniMagick. As we can see from CarrierWave and Refile, for common image resizing it’s not enough to just use simple ImageMagick options, certain resizers like “resize_to_fill” require a combination of ImageMagick options to achieve the desired effect.

To avoid reimplementing this functionality, I propose that ActiveStorage uses the ImageProcessing gem, as a layer on top of MiniMagick. The ImageProcessing gem has been developing for almost 3 years now, and it’s gotten pretty good. It was primarily written to be used with Shrine, but I deliberately made it generic so that other file attachment libraries can use it as well and avoid reimplementing the same functionality.

Some features that are added on top of MiniMagick:

The ImageProcessing gem provides a uniform API regardless of which backend is used, just the operations/options specific to ImageMagick and libvips differ of course. That means the potential ActiveStorage integration would work for both ImageMagick and libvips, allowing the user to swap backends with just a single line of code.

So, my question is: would you be open to accepting a pull request that replaces MiniMagick with ImageProcessing?

Kind regards,

Janko

2

I like that an idea as an optional upgrade. The Rails 5.2 ship has already sailed, so we’ll want to make sure that any optional upgrade honors the existing transformation API. But I could see making the Variation#transform call a strategy that’s configurable. And it’ll just start with what we have now, but can be upgraded/replaced with this solution.

3

Thanks for sharing your thoughts. I was hoping that ImageProcessing would be able to replace the mini_magick gem, as that would allow removing a lot of code. Having the option for ImageProcessing to be an optional upgrade would mean the Rails team would still have to keep and maintain the pure-MiniMagick code, which would be less than ideal as there is no advantage in using pure MiniMagick. Note that switching to ImageProcessing should still retain 100% compatibility with the current ActiveStorage API, from what I’ve observed.

If Rails 5.2 ship really has sailed, then I don’t see a way how we’ll be able to replace the MiniMagick code later on without breaking backwards compatibility, as at the moment MiniMagick is an optional dependency. Only if we make ImageProcessing a runtime dependency instead of optional, but ImageProcessing pulls in both mini_magick and ruby-vips gems (though it does not require them).

4

5.2 ship has indeed sailed. We’re very close to a final release after months of RC’ing. I don’t think it’s a big issue maintaining the 8 lines of code that’s related to using MiniMagick for transformations: rails/variation.rb at main · rails/rails · GitHub. We have to weigh that against taking on another dependency. On that note alone, I don’t think the trade is worth it. But I do like the idea of having easier access to common transformations. And it also sounds nice to have a faster way of doing the transformations.

Happy to see us target a refactoring for Rails 6.0 on this :+1:

5

It’s great to hear that you’re interested! Btw, here is the PR: Use ImageProcessing gem for ActiveStorage variants by janko · Pull Request #32471 · rails/rails · GitHub

Once Rails 5.2 is released, we can discuss plans of getting this in for Rails 6.0.