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.

Reply via email to