It's great to hear that you're interested! Btw, here is the PR: https://github.com/rails/rails/pull/32471
Once Rails 5.2 is released, we can discuss plans of getting this in for Rails 6.0. On Friday, April 6, 2018 at 7:34:21 PM UTC+2, DHH wrote: > > 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 rubyonrails-core@googlegroups.com. Visit this group at https://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/d/optout.
