Fix issue #106 - Trim is not working for certain images.

Two pixels with alpha=0 should be treated as the same color (even when
they RGB values are different).

src/commands/cmd_crop.cpp

@@ -91,7 +91,7 @@ protected:
 
 AutocropSpriteCommand::AutocropSpriteCommand()
   : Command("AutocropSprite",
-            "Autocrop Sprite",
+            "Trim Sprite",
             CmdRecordableFlag)
 {
 }
@@ -109,8 +109,8 @@ void AutocropSpriteCommand::onExecute(Context* context)
   {
     int bgcolor = color_utils::color_for_image(app_get_colorbar()->getBgColor(), sprite->getPixelFormat());
 
-    UndoTransaction undoTransaction(document, "Sprite Autocrop");
-    undoTransaction.autocropSprite(bgcolor);
+    UndoTransaction undoTransaction(document, "Trim Sprite");
+    undoTransaction.trimSprite(bgcolor);
     undoTransaction.commit();
   }
   document->generateMaskBoundaries();

src/raster/image.cpp

@@ -634,46 +634,85 @@ int image_count_diff(const Image* i1, const Image* i2)
   return diff;
 }
 
-bool image_shrink_rect(Image *image, int *x1, int *y1, int *x2, int *y2, int refpixel)
+static bool is_same_pixel(PixelFormat pixelFormat, int pixel1, int pixel2)
 {
-#define SHRINK_SIDE(u_begin, u_op, u_final, u_add,              \
-                    v_begin, v_op, v_final, v_add, U, V, var)   \
-  do {                                                          \
-    for (u = u_begin; u u_op u_final; u u_add) {                \
-      for (v = v_begin; v v_op v_final; v v_add) {              \
-        if (image->getpixel(U, V) != refpixel)                  \
-          break;                                                \
-      }                                                         \
-      if (v == v_final)                                         \
-        var;                                                    \
-      else                                                      \
-        break;                                                  \
-    }                                                           \
-  } while (0)
+  switch (pixelFormat) {
+    case IMAGE_RGB:
+      if (_rgba_geta(pixel1) == 0 && _rgba_geta(pixel2) == 0)
+        return true;
+      break;
+    case IMAGE_GRAYSCALE:
+      if (_graya_geta(pixel1) == 0 && _graya_geta(pixel2) == 0)
+        return true;
+      break;
+  }
+  return pixel1 == pixel2;
+}
 
+bool image_shrink_rect(Image *image, gfx::Rect& bounds, int refpixel)
+{
+  bool shrink;
   int u, v;
 
-  *x1 = 0;
-  *y1 = 0;
-  *x2 = image->w-1;
-  *y2 = image->h-1;
+  bounds = gfx::Rect(0, 0, image->w, image->h);
 
-  SHRINK_SIDE(0, <, image->w, ++,
-              0, <, image->h, ++, u, v, (*x1)++);
+  // Shrink left side
+  for (u=bounds.x; u<bounds.x+bounds.w; ++u) {
+    shrink = true;
+    for (v=bounds.y; v<bounds.y+bounds.h; ++v) {
+      if (!is_same_pixel(image->getPixelFormat(), image->getpixel(u, v), refpixel)) {
+        shrink = false;
+        break;
+      }
+    }
+    if (!shrink)
+      break;
+    ++bounds.x;
+    --bounds.w;
+  }
 
-  SHRINK_SIDE(0, <, image->h, ++,
-              0, <, image->w, ++, v, u, (*y1)++);
+  // Shrink right side
+  for (u=bounds.x+bounds.w-1; u>=bounds.x; --u) {
+    shrink = true;
+    for (v=bounds.y; v<bounds.y+bounds.h; ++v) {
+      if (!is_same_pixel(image->getPixelFormat(), image->getpixel(u, v), refpixel)) {
+        shrink = false;
+        break;
+      }
+    }
+    if (!shrink)
+      break;
+    --bounds.w;
+  }
 
-  SHRINK_SIDE(image->w-1, >, 0, --,
-              0, <, image->h, ++, u, v, (*x2)--);
+  // Shrink top side
+  for (v=bounds.y; v<bounds.y+bounds.h; ++v) {
+    shrink = true;
+    for (u=bounds.x; u<bounds.x+bounds.w; ++u) {
+      if (!is_same_pixel(image->getPixelFormat(), image->getpixel(u, v), refpixel)) {
+        shrink = false;
+        break;
+      }
+    }
+    if (!shrink)
+      break;
+    ++bounds.y;
+    --bounds.h;
+  }
 
-  SHRINK_SIDE(image->h-1, >, 0, --,
-              0, <, image->w, ++, v, u, (*y2)--);
+  // Shrink bottom side
+  for (v=bounds.y+bounds.h-1; v>=bounds.y; --v) {
+    shrink = true;
+    for (u=bounds.x; u<bounds.x+bounds.w; ++u) {
+      if (!is_same_pixel(image->getPixelFormat(), image->getpixel(u, v), refpixel)) {
+        shrink = false;
+        break;
+      }
+    }
+    if (!shrink)
+      break;
+    --bounds.h;
+  }
 
-  if ((*x1 > *x2) || (*y1 > *y2))
-    return false;
-  else
-    return true;
-
-#undef SHRINK_SIDE
+  return (!bounds.isEmpty());
 }

src/raster/image.h

@@ -19,14 +19,16 @@
 #ifndef RASTER_IMAGE_H_INCLUDED
 #define RASTER_IMAGE_H_INCLUDED
 
+#include "gfx/rect.h"
 #include "raster/blend.h"
 #include "raster/gfxobj.h"
 #include "raster/pixel_format.h"
 
 #include <allegro/color.h>
 
-class Palette;
+struct BITMAP;
 
+class Palette;
 class Pen;
 class RgbMap;
 
@@ -35,8 +37,6 @@ enum ResizeMethod {
   RESIZE_METHOD_BILINEAR,
 };
 
-struct BITMAP;
-
 class Image : public GfxObj
 {
 public:
@@ -98,7 +98,7 @@ void image_to_allegro(const Image* image, BITMAP* bmp, int x, int y, const Palet
 void image_fixup_transparent_colors(Image* image);
 void image_resize(const Image* src, Image* dst, ResizeMethod method, const Palette* palette, const RgbMap* rgbmap);
 int image_count_diff(const Image* i1, const Image* i2);
-bool image_shrink_rect(Image *image, int *x1, int *y1, int *x2, int *y2, int refpixel);
+bool image_shrink_rect(Image *image, gfx::Rect& bounds, int refpixel);
 
 #include "raster/image_traits.h"
 

src/undo_transaction.cpp

@@ -202,42 +202,32 @@ void UndoTransaction::cropSprite(const gfx::Rect& bounds, int bgcolor)
                     m_document->getMask()->getBounds().y-bounds.y);
 }
 
-void UndoTransaction::autocropSprite(int bgcolor)
+void UndoTransaction::trimSprite(int bgcolor)
 {
   int old_frame = m_sprite->getCurrentFrame();
-  int x1, y1, x2, y2;
-  int u1, v1, u2, v2;
+  gfx::Rect bounds;
 
-  x1 = y1 = INT_MAX;
-  x2 = y2 = INT_MIN;
-
-  Image* image = Image::create(m_sprite->getPixelFormat(),
-                               m_sprite->getWidth(),
-                               m_sprite->getHeight());
+  UniquePtr<Image> image_wrap(Image::create(m_sprite->getPixelFormat(),
+                                            m_sprite->getWidth(),
+                                            m_sprite->getHeight()));
+  Image* image = image_wrap.get();
 
   for (int frame=0; frame<m_sprite->getTotalFrames(); ++frame) {
+    image->clear(0);
+
     m_sprite->setCurrentFrame(frame);
     m_sprite->render(image, 0, 0);
 
     // TODO configurable (what color pixel to use as "refpixel",
     // here we are using the top-left pixel by default)
-    if (image_shrink_rect(image, &u1, &v1, &u2, &v2,
-                          image_getpixel(image, 0, 0))) {
-      x1 = MIN(x1, u1);
-      y1 = MIN(y1, v1);
-      x2 = MAX(x2, u2);
-      y2 = MAX(y2, v2);
-    }
+    gfx::Rect frameBounds;
+    if (image_shrink_rect(image, frameBounds, image_getpixel(image, 0, 0)))
+      bounds = bounds.createUnion(frameBounds);
   }
   m_sprite->setCurrentFrame(old_frame);
 
-  image_free(image);
-
-  // do nothing
-  if (x1 > x2 || y1 > y2)
-    return;
-
-  cropSprite(gfx::Rect(x1, y1, x2-x1+1, y2-y1+1), bgcolor);
+  if (!bounds.isEmpty())
+    cropSprite(bounds, bgcolor);
 }
 
 void UndoTransaction::setPixelFormat(PixelFormat newFormat, DitheringMethod dithering_method)

src/undo_transaction.h

@@ -79,7 +79,7 @@ public:
   void setCurrentLayer(Layer* layer);
   void setSpriteSize(int w, int h);
   void cropSprite(const gfx::Rect& bounds, int bgcolor);
-  void autocropSprite(int bgcolor);
+  void trimSprite(int bgcolor);
   void setPixelFormat(PixelFormat newFormat, DitheringMethod dithering_method);
 
   // for images in stock