* added workaround for better gif handling

* added basic test for new crop method
* added tests for croping the right part of various images with various attachment formats
  it needs a little help because of code duplication. How can i access a method in the processor to test if it works? I duplicated the method into the testclass in fixtures/attachment

lib/technoweenie/attachment_fu/processors/mini_magick_processor.rb

@@ -7,7 +7,7 @@ module Technoweenie # :nodoc:
           base.send :extend, ClassMethods
           base.alias_method_chain :process_attachment, :processing
         end
-
+ 
         module ClassMethods
           # Yields a block containing an MiniMagick Image for the given binary data.
           def with_image(file, &block)
@@ -23,23 +23,29 @@ module Technoweenie # :nodoc:
             !binary_data.nil?
           end
         end
-
+ 
       protected
         def process_attachment_with_processing
           return unless process_attachment_without_processing
           with_image do |img|
             resize_image_or_thumbnail! img
-            self.width  = img[:width] if respond_to?(:width)
-            self.height = img[:height]  if respond_to?(:height)
+            self.width = img[:width] if respond_to?(:width)
+            self.height = img[:height] if respond_to?(:height)
             callback_with_args :after_resize, img
           end if image?
         end
-
+ 
         # Performs the actual resizing operation for a thumbnail
         def resize_image(img, size)
           size = size.first if size.is_a?(Array) && size.length == 1
           img.combine_options do |commands|
             commands.strip unless attachment_options[:keep_profile]
+
+            # gif are not handled correct, this is a hack, but it seems to work.
+            if img.output =~ / GIF /
+              img.format("png")
+            end           
+            
             if size.is_a?(Fixnum) || (size.is_a?(Array) && size.first.is_a?(Fixnum))
               if size.is_a?(Fixnum)
                 size = [size, size]
@@ -69,27 +75,8 @@ module Technoweenie # :nodoc:
               
               # only crop if image is not smaller in both dimensions
               unless image_width < thumb_width and image_height < thumb_height
-              
-                # special cases, image smaller in one dimension then thumbsize
-                if image_width < thumb_width
-                  offset = (image_height / 2) - (thumb_height / 2)
-                  command = "#{image_width}x#{thumb_height}+0+#{offset}"
-                elsif image_height < thumb_height
-                  offset = (image_width / 2) - (thumb_width / 2)
-                  command = "#{thumb_width}x#{image_height}+#{offset}+0"
-                
-                # normal thumbnail generation
-                # calculate height and offset y, width is fixed                 
-                elsif (image_aspect <= thumb_aspect or image_width < thumb_width) and image_height > thumb_height
-                  height = image_width / thumb_aspect
-                  offset = (image_height / 2) - (height / 2)
-                  command = "#{image_width}x#{height}+0+#{offset}"
-                # calculate width and offset x, height is fixed
-                else
-                  width = image_height * thumb_aspect
-                  offset = (image_width / 2) - (width / 2)
-                  command = "#{width}x#{image_height}+#{offset}+0"
-                end
+                command = calculate_offset(image_width,image_height,image_aspect,thumb_width,thumb_height,thumb_aspect)
+
                 # crop image
                 commands.extract(command)
               end
@@ -103,14 +90,43 @@ module Technoweenie # :nodoc:
               else
                 commands.resize("#{size.to_s}")
               end
-						# crop end
+            # crop end
             else
               commands.resize(size.to_s)
             end
           end
           temp_paths.unshift img
         end
+
+        def calculate_offset(image_width,image_height,image_aspect,thumb_width,thumb_height,thumb_aspect)
+        # only crop if image is not smaller in both dimensions
+
+          # special cases, image smaller in one dimension then thumbsize
+          if image_width < thumb_width
+            offset = (image_height / 2) - (thumb_height / 2)
+            command = "#{image_width}x#{thumb_height}+0+#{offset}"
+          elsif image_height < thumb_height
+            offset = (image_width / 2) - (thumb_width / 2)
+            command = "#{thumb_width}x#{image_height}+#{offset}+0"
+
+          # normal thumbnail generation
+          # calculate height and offset y, width is fixed                 
+          elsif (image_aspect <= thumb_aspect or image_width < thumb_width) and image_height > thumb_height
+            height = image_width / thumb_aspect
+            offset = (image_height / 2) - (height / 2)
+            command = "#{image_width}x#{height}+0+#{offset}"
+          # calculate width and offset x, height is fixed
+          else
+            width = image_height * thumb_aspect
+            offset = (image_width / 2) - (width / 2)
+            command = "#{width}x#{image_height}+#{offset}+0"
+          end
+          # crop image
+          command
+        end
+
+
       end
     end
   end
-end
+end

test/fixtures/attachment.rb

@@ -35,7 +35,7 @@ end
 class ImageWithThumbsAttachment < Attachment
   has_attachment :thumbnails => { :thumb => [50, 50], :geometry => 'x50' }, :resize_to => [55,55]
   after_resize do |record, img|
-    record.aspect_ratio = img.columns.to_f / img.rows.to_f
+   # record.aspect_ratio = img.columns.to_f / img.rows.to_f
   end
 end
 
@@ -53,7 +53,7 @@ class ImageWithThumbsFileAttachment < FileAttachment
   has_attachment :path_prefix => 'vendor/plugins/attachment_fu/test/files',
     :thumbnails => { :thumb => [50, 50], :geometry => 'x50' }, :resize_to => [55,55]
   after_resize do |record, img|
-    record.aspect_ratio = img.columns.to_f / img.rows.to_f
+  #  record.aspect_ratio = img.columns.to_f / img.rows.to_f
   end
 end
 
@@ -130,9 +130,44 @@ begin
     has_attachment :path_prefix => 'vendor/plugins/attachment_fu/test/files',
       :processor => :mini_magick, :thumbnails => { :thumb => [50, 51], :geometry => '31>' }, :resize_to => 55
   end
+  class ImageThumbnailCrop < MiniMagickAttachment
+    has_attachment :path_prefix => 'vendor/plugins/attachment_fu/test/files',
+    :thumbnails => { :square => "50x50c", :vertical => "30x60c", :horizontal => "60x30c"}
+    
+    # TODO this is a bad duplication, this method is in the MiniMagick Processor
+    def self.calculate_offset(image_width,image_height,image_aspect,thumb_width,thumb_height,thumb_aspect)
+    # only crop if image is not smaller in both dimensions
+
+      # special cases, image smaller in one dimension then thumbsize
+      if image_width < thumb_width
+        offset = (image_height / 2) - (thumb_height / 2)
+        command = "#{image_width}x#{thumb_height}+0+#{offset}"
+      elsif image_height < thumb_height
+        offset = (image_width / 2) - (thumb_width / 2)
+        command = "#{thumb_width}x#{image_height}+#{offset}+0"
+
+      # normal thumbnail generation
+      # calculate height and offset y, width is fixed                 
+      elsif (image_aspect <= thumb_aspect or image_width < thumb_width) and image_height > thumb_height
+        height = image_width / thumb_aspect
+        offset = (image_height / 2) - (height / 2)
+        command = "#{image_width}x#{height}+0+#{offset}"
+      # calculate width and offset x, height is fixed
+      else
+        width = image_height * thumb_aspect
+        offset = (image_width / 2) - (width / 2)
+        command = "#{width}x#{image_height}+#{offset}+0"
+      end
+      # crop image
+      command
+    end
+  end
+
 rescue MissingSourceFile
 end
 
+
+
 begin
   class S3Attachment < ActiveRecord::Base
     has_attachment :storage => :s3, :processor => :rmagick, :s3_config_path => File.join(File.dirname(__FILE__), '../amazon_s3.yml')

test/processors/mini_magick_test.rb

@@ -23,9 +23,81 @@ class MiniMagickTest < Test::Unit::TestCase
       assert_equal 31, geo.width
       assert_equal 40, geo.height
     end
+
+    def test_should_crop_image(klass = ImageThumbnailCrop)
+      attachment_model klass
+      attachment = upload_file :filename => '/files/rails.png'
+      assert_valid attachment
+      assert  attachment.image?
+    #  has_attachment :thumbnails => { :square => "50x50c", :vertical => "30x60c", :horizontal => "60x30c"}
+
+      square      = attachment.thumbnails.detect { |t| t.filename =~ /_square/ }
+      vertical    = attachment.thumbnails.detect { |t| t.filename =~ /_vertical/ }
+      horizontal  = attachment.thumbnails.detect { |t| t.filename =~ /_horizontal/ }
+      
+      # test excat resize
+      assert_equal 50, square.width
+      assert_equal 50, square.height
+
+      assert_equal 30, vertical.width
+      assert_equal 60, vertical.height
+
+      assert_equal 60, horizontal.width
+      assert_equal 30, horizontal.height
+    end
+    
+    # tests the first step in resize, crop the image in original size to right format
+    def test_should_crop_image_right(klass = ImageThumbnailCrop)      
+      @@testcases.collect do |testcase| 
+        image_width, image_height, thumb_width, thumb_height = testcase[:data]
+        image_aspect, thumb_aspect = image_width/image_height, thumb_width/thumb_height
+        crop_comand = klass.calculate_offset(image_width, image_height, image_aspect, thumb_width, thumb_height,thumb_aspect)
+        # pattern matching on crop command
+        if testcase.has_key?(:height) 
+          assert crop_comand.match(/^#{image_width}x#{testcase[:height]}\+0\+#{testcase[:yoffset]}$/)
+        else 
+          assert crop_comand.match(/^#{testcase[:width]}x#{image_height}\+#{testcase[:xoffset]}\+0$/)
+        end
+      end
+    end
+
   else
     def test_flunk
       puts "MiniMagick not loaded, tests not running"
     end
   end
+
+  @@testcases = [
+    # image_aspect <= 1 && thumb_aspect >= 1  
+    {:data => [10.0,40.0,2.0,1.0], :height => 5.0, :yoffset => 17.5}, #   1b
+    {:data => [10.0,40.0,1.0,1.0], :height => 10.0, :yoffset => 15.0}, #  1b
+
+    # image_aspect < 1 && thumb_aspect < 1
+    {:data => [10.0,40.0,1.0,2.0], :height => 20.0, :yoffset => 10.0}, # 1a
+    {:data => [2.0,3.0,1.0,2.0], :width => 1.5, :xoffset => 0.25}, # 1a
+
+    # image_aspect = thumb_aspect
+    {:data => [10.0,10.0,1.0,1.0], :height => 10.0, :yoffset => 0.0}, # QUADRAT 1c
+
+    # image_aspect >= 1 && thumb_aspect > 1     && image_aspect < thumb_aspect
+    {:data => [6.0,3.0,4.0,1.0], :height => 1.5, :yoffset => 0.75}, # 2b  
+    {:data => [6.0,6.0,4.0,1.0], :height => 1.5, :yoffset => 2.25}, # 2b  
+
+    # image_aspect > 1 && thumb_aspect > 1     && image_aspect > thumb_aspect
+    {:data => [9.0,3.0,2.0,1.0], :width => 6.0, :xoffset => 1.5}, # 2a
+
+    # image_aspect > 1 && thumb_aspect < 1 && image_aspect < thumb_aspect
+    {:data => [10.0,5.0,0.1,2.0], :width => 0.25, :xoffset => 4.875}, # 4
+    {:data => [10.0,5.0,1.0,2.0], :width => 2.5, :xoffset => 3.75}, # 4
+
+    # image_aspect > 1 && thumb_aspect > 1     && image_aspect > thumb_aspect
+    {:data => [9.0,3.0,2.0,1.0], :width => 6.0, :xoffset => 1.5}, # 3a    
+    # image_aspect > 1 && thumb_aspect > 1     && image_aspect < thumb_aspect
+    {:data => [9.0,3.0,5.0,1.0], :height => 1.8, :yoffset => 0.6} # 3a
+  ]
+
+
+
+
+
 end