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: https://github.com/rails/rails/blob/master/activestorage/app/models/active_storage/variation.rb#L59-L67. 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 👍 On Thursday, April 5, 2018 at 9:07:34 AM UTC-7, Janko Marohnić wrote: > > 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). > > On Thursday, April 5, 2018 at 5:52:29 PM UTC+2, DHH wrote: >> >> 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. >> >> On Wednesday, April 4, 2018 at 5:18:25 PM UTC-7, Janko Marohnić wrote: >>> >>> Hello everyone, >>> >>> ActiveStorage performs image processing on-the-fly, and for that it >>> currently uses raw MiniMagick. As we can see from CarrierWave >>> <https://github.com/carrierwaveuploader/carrierwave/blob/master/lib/carrierwave/processing/mini_magick.rb> >>> >>> and Refile <https://github.com/refile/refile-mini_magick>, 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* <https://github.com/janko-m/image_processing> >>> 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 >>> <https://github.com/shrinerb/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: >>> >>> - resizing macros: >>> - #resize_to_limit >>> >>> <https://github.com/janko-m/image_processing/blob/master/doc/minimagick.md#resize_to_limit> >>> - #resize_to_fit >>> >>> <https://github.com/janko-m/image_processing/blob/master/doc/minimagick.md#resize_to_fit> >>> - #resize_to_fill >>> >>> <https://github.com/janko-m/image_processing/blob/master/doc/minimagick.md#resize_to_fill> >>> - #resize_and_pad >>> >>> <https://github.com/janko-m/image_processing/blob/master/doc/minimagick.md#resize_and_pad> >>> - automatic orientation >>> <https://www.imagemagick.org/script/command-line-options.php#auto-orient> >>> - automatic thumbnail sharpening >>> >>> <https://github.com/janko-m/image_processing/blob/master/doc/minimagick.md#sharpening> >>> - avoids the complex MiniMagick::Image class >>> >>> But probably the biggest features of the ImageProcessing gem is that it >>> also ships with the *libvips* <http://jcupitt.github.io/libvips/> >>> backend. Libvips is an alternative to ImageMagick. The advantage of using >>> libvips is that in most cases it's* multiple times faster than >>> ImageMagick*. This is a big deal. >>> >>> 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 >>> >> -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscr...@googlegroups.com. To post to this group, send email to email@example.com. Visit this group at https://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/d/optout.