Delegate MiniMagick processing to ImageProcessing gem

README.md

@@ -280,7 +280,7 @@ You no longer need to do this manually.
 
 Often you'll want to add different versions of the same file. The classic example is image thumbnails. There is built in support for this*:
 
-*Note:* You must have Imagemagick and MiniMagick installed to do image resizing. MiniMagick is a Ruby interface for Imagemagick which is a C program. This is why MiniMagick fails on 'bundle install' without Imagemagick installed.
+*Note:* You must have Imagemagick installed to do image resizing.
 
 Some documentation refers to RMagick instead of MiniMagick but MiniMagick is recommended.
 
@@ -362,7 +362,7 @@ private
   end
 
   def is_landscape? picture
-    image = MiniMagick::Image.open(picture.path)
+    image = MiniMagick::Image.new(picture.path)
     image[:width] > image[:height]
   end
 
@@ -871,8 +871,8 @@ manipulation methods.
 
 ## Using MiniMagick
 
-MiniMagick is similar to RMagick but performs all the operations using the 'mogrify'
-command which is part of the standard ImageMagick kit. This allows you to have the power
+MiniMagick is similar to RMagick but performs all the operations using the 'convert'
+CLI which is part of the standard ImageMagick kit. This allows you to have the power
 of ImageMagick without having to worry about installing all the RMagick libraries.
 
 See the MiniMagick site for more details:

carrierwave.gemspec

@@ -24,6 +24,7 @@ Gem::Specification.new do |s|
   s.add_dependency "activesupport", ">= 5.0.0"
   s.add_dependency "activemodel", ">= 5.0.0"
   s.add_dependency "mime-types", ">= 1.16"
+  s.add_dependency "image_processing", "~> 1.1"
   if RUBY_ENGINE == 'jruby'
     s.add_development_dependency 'activerecord-jdbcpostgresql-adapter'
   else

lib/carrierwave/processing/mini_magick.rb

@@ -21,53 +21,42 @@ module CarrierWave
   #       process :resize_to_fit => [200, 200]
   #     end
   #
-  # Or create your own helpers with the powerful manipulate! method. Check
-  # out the ImageMagick docs at http://www.imagemagick.org/script/command-line-options.php for more
-  # info
+  # Or create your own helpers with the powerful minimagick! method, which
+  # yields an ImageProcessing::Builder object. Check out the ImageProcessing
+  # docs at http://github.com/janko-m/image_processing and the list of all
+  # available ImageMagick options at
+  # http://www.imagemagick.org/script/command-line-options.php for more info.
   #
   #     class MyUploader < CarrierWave::Uploader::Base
   #       include CarrierWave::MiniMagick
   #
   #       process :radial_blur => 10
   #
   #       def radial_blur(amount)
-  #         manipulate! do |img|
-  #           img.radial_blur(amount)
-  #           img = yield(img) if block_given?
-  #           img
+  #         minimagick! do |builder|
+  #           builder.radial_blur(amount)
+  #           builder = yield(builder) if block_given?
+  #           builder
   #         end
   #       end
   #     end
   #
   # === Note
   #
-  # MiniMagick is a mini replacement for RMagick that uses the command line
-  # tool "mogrify" for image manipulation.
+  # The ImageProcessing gem uses MiniMagick, a mini replacement for RMagick
+  # that uses ImageMagick command-line tools, to build a "convert" command that
+  # performs the processing.
   #
   # You can find more information here:
   #
-  # http://mini_magick.rubyforge.org/
-  # and
   # https://github.com/minimagick/minimagick/
   #
   #
   module MiniMagick
     extend ActiveSupport::Concern
 
     included do
-      begin
-        require "mini_magick"
-      rescue LoadError => e
-        e.message << " (You may need to install the mini_magick gem)"
-        raise e
-      end
-
-      prepend Module.new {
-        def initialize(*)
-          super
-          @format = nil
-        end
-      }
+      require "image_processing/mini_magick"
     end
 
     module ClassMethods
@@ -109,12 +98,11 @@ def resize_and_pad(width, height, background=:transparent, gravity='Center')
     #
     #     image.convert(:png)
     #
-    def convert(format, page=nil)
-      @format = format
-      @page = page
-      manipulate! do |img|
-        img = yield(img) if block_given?
-        img
+    def convert(format, page=nil, &block)
+      minimagick!(block) do |builder|
+        builder = builder.convert(format)
+        builder = builder.loader(page: page) if page
+        builder
       end
     end
 
@@ -128,21 +116,18 @@ def convert(format, page=nil)
     #
     # [width (Integer)] the width to scale the image to
     # [height (Integer)] the height to scale the image to
+    # [combine_options (Hash)] additional ImageMagick options to apply before resizing
     #
     # === Yields
     #
     # [MiniMagick::Image] additional manipulations to perform
     #
-    def resize_to_limit(width, height, combine_options: {})
-      width = dimension_from width
-      height = dimension_from height
-      manipulate! do |img|
-        img.combine_options do |cmd|
-          cmd.resize "#{width}x#{height}>"
-          append_combine_options cmd, combine_options
-        end
-        img = yield(img) if block_given?
-        img
+    def resize_to_limit(width, height, combine_options: {}, &block)
+      width, height = resolve_dimensions(width, height)
+
+      minimagick!(block) do |builder|
+        builder.resize_to_limit(width, height)
+          .apply(combine_options)
       end
     end
 
@@ -155,21 +140,18 @@ def resize_to_limit(width, height, combine_options: {})
     #
     # [width (Integer)] the width to scale the image to
     # [height (Integer)] the height to scale the image to
+    # [combine_options (Hash)] additional ImageMagick options to apply before resizing
     #
     # === Yields
     #
     # [MiniMagick::Image] additional manipulations to perform
     #
-    def resize_to_fit(width, height, combine_options: {})
-      width = dimension_from width
-      height = dimension_from height
-      manipulate! do |img|
-        img.combine_options do |cmd|
-          cmd.resize "#{width}x#{height}"
-          append_combine_options cmd, combine_options
-        end
-        img = yield(img) if block_given?
-        img
+    def resize_to_fit(width, height, combine_options: {}, &block)
+      width, height = resolve_dimensions(width, height)
+
+      minimagick!(block) do |builder|
+        builder.resize_to_fit(width, height)
+          .apply(combine_options)
       end
     end
 
@@ -183,37 +165,18 @@ def resize_to_fit(width, height, combine_options: {})
     # [width (Integer)] the width to scale the image to
     # [height (Integer)] the height to scale the image to
     # [gravity (String)] the current gravity suggestion (default: 'Center'; options: 'NorthWest', 'North', 'NorthEast', 'West', 'Center', 'East', 'SouthWest', 'South', 'SouthEast')
+    # [combine_options (Hash)] additional ImageMagick options to apply before resizing
     #
     # === Yields
     #
     # [MiniMagick::Image] additional manipulations to perform
     #
-    def resize_to_fill(width, height, gravity = 'Center', combine_options: {})
-      width = dimension_from width
-      height = dimension_from height
-      manipulate! do |img|
-        cols, rows = img[:dimensions]
-        img.combine_options do |cmd|
-          if width != cols || height != rows
-            scale_x = width/cols.to_f
-            scale_y = height/rows.to_f
-            if scale_x >= scale_y
-              cols = (scale_x * (cols + 0.5)).round
-              rows = (scale_x * (rows + 0.5)).round
-              cmd.resize "#{cols}"
-            else
-              cols = (scale_y * (cols + 0.5)).round
-              rows = (scale_y * (rows + 0.5)).round
-              cmd.resize "x#{rows}"
-            end
-          end
-          cmd.gravity gravity
-          cmd.background "rgba(255,255,255,0.0)"
-          cmd.extent "#{width}x#{height}" if cols != width || rows != height
-          append_combine_options cmd, combine_options
-        end
-        img = yield(img) if block_given?
-        img
+    def resize_to_fill(width, height, gravity = 'Center', combine_options: {}, &block)
+      width, height = resolve_dimensions(width, height)
+
+      minimagick!(block) do |builder|
+        builder.resize_to_fill(width, height, gravity: gravity)
+          .apply(combine_options)
       end
     end
 
@@ -232,28 +195,18 @@ def resize_to_fill(width, height, gravity = 'Center', combine_options: {})
     # [height (Integer)] the height to scale the image to
     # [background (String, :transparent)] the color of the background as a hexcode, like "#ff45de"
     # [gravity (String)] how to position the image
+    # [combine_options (Hash)] additional ImageMagick options to apply before resizing
     #
     # === Yields
     #
     # [MiniMagick::Image] additional manipulations to perform
     #
-    def resize_and_pad(width, height, background=:transparent, gravity='Center', combine_options: {})
-      width = dimension_from width
-      height = dimension_from height
-      manipulate! do |img|
-        img.combine_options do |cmd|
-          cmd.thumbnail "#{width}x#{height}>"
-          if background == :transparent
-            cmd.background "rgba(255, 255, 255, 0.0)"
-          else
-            cmd.background background
-          end
-          cmd.gravity gravity
-          cmd.extent "#{width}x#{height}"
-          append_combine_options cmd, combine_options
-        end
-        img = yield(img) if block_given?
-        img
+    def resize_and_pad(width, height, background=:transparent, gravity='Center', combine_options: {}, &block)
+      width, height = resolve_dimensions(width, height)
+
+      minimagick!(block) do |builder|
+        builder.resize_and_pad(width, height, background: background, gravity: gravity)
+          .apply(combine_options)
       end
     end
 
@@ -284,6 +237,9 @@ def height
     # and then pass each of its frames to the supplied block. It will then
     # save the image to disk.
     #
+    # NOTE: This method exists mostly for backwards compatibility, you should
+    # probably use #minimagick!.
+    #
     # === Gotcha
     #
     # This method assumes that the object responds to +current_path+.
@@ -303,20 +259,55 @@ def manipulate!
       cache_stored_file! if !cached?
       image = ::MiniMagick::Image.open(current_path)
 
-      begin
-        image.format(@format.to_s.downcase, @page) if @format
-        image = yield(image)
-        image.write(current_path)
+      image = yield(image)
+      FileUtils.mv image.path, current_path
 
-        if @format
-          move_to = current_path.chomp(File.extname(current_path)) + ".#{@format}"
-          file.content_type = ::MIME::Types.type_for(move_to).first.to_s
-          file.move_to(move_to, permissions, directory_permissions)
-        end
+      image.run_command("identify", current_path)
+    rescue ::MiniMagick::Error, ::MiniMagick::Invalid => e
+      message = I18n.translate(:"errors.messages.mini_magick_processing_error", :e => e)
+      raise CarrierWave::ProcessingError, message
+    ensure
+      image.destroy! if image
+    end
 
-        image.run_command("identify", current_path)
-      ensure
-        image.destroy!
+    # Process the image with MiniMagick, using the ImageProcessing gem. This
+    # method will build a "convert" ImageMagick command and execute it on the
+    # current image.
+    #
+    # === Gotcha
+    #
+    # This method assumes that the object responds to +current_path+.
+    # Any class that this module is mixed into must have a +current_path+ method.
+    # CarrierWave::Uploader does, so you won't need to worry about this in
+    # most cases.
+    #
+    # === Yields
+    #
+    # [ImageProcessing::Builder] use it to define processing to be performed
+    #
+    # === Raises
+    #
+    # [CarrierWave::ProcessingError] if processing failed.
+    def minimagick!(block = nil)
+      builder = ImageProcessing::MiniMagick.source(current_path)
+      builder = yield(builder)
+
+      result = builder.call
+      result.close
+
+      # backwards compatibility (we want to eventually move away from MiniMagick::Image)
+      if block
+        image  = MiniMagick::Image.new(result.path, result)
+        image  = block.call(image)
+        result = image.instance_variable_get(:@tempfile)
+      end
+
+      FileUtils.mv result.path, current_path
+
+      if File.extname(result.path) != File.extname(current_path)
+        move_to = current_path.chomp(File.extname(current_path)) + File.extname(result.path)
+        file.content_type = ::MIME::Types.type_for(move_to).first.to_s
+        file.move_to(move_to, permissions, directory_permissions)
       end
     rescue ::MiniMagick::Error, ::MiniMagick::Invalid => e
       message = I18n.translate(:"errors.messages.mini_magick_processing_error", :e => e)
@@ -325,21 +316,13 @@ def manipulate!
 
     private
 
-      def append_combine_options(cmd, combine_options)
-        combine_options.each do |method, options|
-          if options.nil?
-            cmd.send(method)
-          else
-            cmd.send(method, options)
-          end
+      def resolve_dimensions(*dimensions)
+        dimensions.map do |value|
+          next value unless value.instance_of?(Proc)
+          value.arity >= 1 ? value.call(self) : value.call
         end
       end
 
-      def dimension_from(value)
-        return value unless value.instance_of?(Proc)
-        value.arity >= 1 ? value.call(self) : value.call
-      end
-
       def mini_magick_image
         ::MiniMagick::Image.read(read)
       end

spec/processing/mini_magick_spec.rb

@@ -23,14 +23,20 @@
       expect(instance.file.content_type).to eq('image/png')
     end
 
-    it "converts all pages when no page number is specified" do
-      expect_any_instance_of(::MiniMagick::Image).to receive(:format).with('png', nil).once
-      instance.convert('png')
-    end
+    it "respects the page parameter" do
+      # create a multi-layer image
+      tiff = Tempfile.new(["file", ".tiff"])
+      MiniMagick::Tool::Convert.new do |convert|
+        convert.merge! [landscape_file_path, landscape_file_path, landscape_file_path]
+        convert << tiff.path
+      end
 
-    it "converts specific page" do
-      expect_any_instance_of(::MiniMagick::Image).to receive(:format).with('png', 1).once
-      instance.convert('png', 1)
+      allow(instance).to receive(:file).and_return(CarrierWave::SanitizedFile.new(tiff.path))
+
+      instance.convert('png', 0)
+      expect(instance.file.extension).to eq('png')
+      expect(instance).to be_format('png')
+      expect(instance.file.size).not_to eq(0)
     end
   end