Add cmd::PatchCel to reduce undo space after a ExpandCelCanvas

src/app/CMakeLists.txt

@@ -120,6 +120,7 @@ add_library(app-lib
   cmd/layer_from_background.cpp
   cmd/move_cel.cpp
   cmd/move_layer.cpp
+  cmd/patch_cel.cpp
   cmd/remap_colors.cpp
   cmd/remove_cel.cpp
   cmd/remove_frame.cpp

src/app/cmd/copy_region.cpp

@@ -20,7 +20,7 @@ namespace cmd {
 
 CopyRegion::CopyRegion(Image* dst, const Image* src,
                        const gfx::Region& region,
-                       int dst_dx, int dst_dy,
+                       const gfx::Point& dstPos,
                        bool alreadyCopied)
   : WithImage(dst)
   , m_size(0)
@@ -29,7 +29,7 @@ CopyRegion::CopyRegion(Image* dst, const Image* src,
   // Create region to save/swap later
   for (const auto& rc : region) {
     gfx::Clip clip(
-      rc.x+dst_dx, rc.y+dst_dy,
+      rc.x+dstPos.x, rc.y+dstPos.y,
       rc.x, rc.y, rc.w, rc.h);
     if (!clip.clip(
           dst->width(), dst->height(),
@@ -43,7 +43,8 @@ CopyRegion::CopyRegion(Image* dst, const Image* src,
   for (const auto& rc : m_region) {
     for (int y=0; y<rc.h; ++y) {
       m_stream.write(
-        (const char*)src->getPixelAddress(rc.x, rc.y+y),
+        (const char*)src->getPixelAddress(rc.x-dstPos.x,
+                                          rc.y-dstPos.y+y),
         src->getRowStrideSize(rc.w));
     }
   }

src/app/cmd/copy_region.h

@@ -11,6 +11,7 @@
 
 #include "app/cmd.h"
 #include "app/cmd/with_image.h"
+#include "gfx/point.h"
 #include "gfx/region.h"
 
 #include <sstream>
@@ -28,7 +29,7 @@ namespace cmd {
     // (so we use "src" as the original image).
     CopyRegion(Image* dst, const Image* src,
                const gfx::Region& region,
-               int dst_dx, int dst_dy,
+               const gfx::Point& dstPos,
                bool alreadyCopied = false);
 
   protected:

src/app/cmd/patch_cel.cpp

@@ -0,0 +1,57 @@
+// Aseprite
+// Copyright (C) 2016  David Capello
+//
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License version 2 as
+// published by the Free Software Foundation.
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include "app/cmd/patch_cel.h"
+
+#include "app/cmd/copy_region.h"
+#include "app/cmd/crop_cel.h"
+#include "app/cmd/trim_cel.h"
+#include "doc/cel.h"
+
+namespace app {
+namespace cmd {
+
+using namespace doc;
+
+PatchCel::PatchCel(doc::Cel* dstCel,
+                   const doc::Image* patch,
+                   const gfx::Region& patchedRegion,
+                   const gfx::Point& patchPos)
+  : WithCel(dstCel)
+  , m_patch(patch)
+  , m_region(patchedRegion)
+  , m_pos(patchPos)
+{
+}
+
+void PatchCel::onExecute()
+{
+  Cel* cel = this->cel();
+
+  executeAndAdd(
+    new CropCel(cel,
+                cel->bounds() |
+                gfx::Rect(m_region.bounds()).offset(m_pos)));
+
+  executeAndAdd(
+    new CopyRegion(cel->image(),
+                   m_patch,
+                   m_region,
+                   m_pos - cel->position()));
+
+  executeAndAdd(
+    new TrimCel(cel));
+
+  m_patch = nullptr;
+}
+
+} // namespace cmd
+} // namespace app

src/app/cmd/patch_cel.h

@@ -0,0 +1,44 @@
+// Aseprite
+// Copyright (C) 2001-2016  David Capello
+//
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License version 2 as
+// published by the Free Software Foundation.
+
+#ifndef APP_CMD_PATCH_CEL_H_INCLUDED
+#define APP_CMD_PATCH_CEL_H_INCLUDED
+#pragma once
+
+#include "app/cmd/with_cel.h"
+#include "app/cmd_sequence.h"
+#include "gfx/fwd.h"
+#include "gfx/point.h"
+
+namespace doc {
+  class Cel;
+  class Image;
+}
+
+namespace app {
+namespace cmd {
+
+  class PatchCel : public CmdSequence
+                 , public WithCel {
+  public:
+    PatchCel(doc::Cel* dstCel,
+             const doc::Image* patch,
+             const gfx::Region& patchedRegion,
+             const gfx::Point& patchPos);
+
+  protected:
+    void onExecute() override;
+
+    const doc::Image* m_patch;
+    const gfx::Region& m_region;
+    gfx::Point m_pos;
+  };
+
+} // namespace cmd
+} // namespace app
+
+#endif

src/app/script/image_wrap.cpp

@@ -37,7 +37,8 @@ void ImageWrap::commit()
   sprite()->transaction().execute(
     new cmd::CopyRegion(m_image,
                         m_backup.get(),
-                        m_modifiedRegion, 0, 0,
+                        m_modifiedRegion,
+                        gfx::Point(0, 0),
                         true));
 
   m_backup.reset(nullptr);

src/app/util/expand_cel_canvas.cpp

@@ -14,10 +14,7 @@
 #include "app/app.h"
 #include "app/cmd/add_cel.h"
 #include "app/cmd/clear_cel.h"
-#include "app/cmd/copy_region.h"
-#include "app/cmd/replace_image.h"
-#include "app/cmd/set_cel_position.h"
-#include "app/cmd/trim_cel.h"
+#include "app/cmd/patch_cel.h"
 #include "app/context.h"
 #include "app/document.h"
 #include "app/transaction.h"
@@ -183,62 +180,16 @@ void ExpandCelCanvas::commit()
     gfx::Point newPos = m_cel->position();
     m_cel->setPosition(m_origCelPos);
 
-    // If the size of each image is the same, we can create an undo
-    // with only the differences between both images.
-    //
-    // TODO check if the valid destination region is inside the
-    // m_celImage bounds, so we try to use CopyRegion instead of
-    // ReplaceImage in more cases.
-    if (newPos == m_origCelPos &&
-        m_bounds.origin() == m_origCelPos &&
-        m_celImage->width() == m_dstImage->width() &&
-        m_celImage->height() == m_dstImage->height()) {
-      int dx = m_origCelPos.x - m_bounds.x;
-      int dy = m_origCelPos.y - m_bounds.y;
+    ASSERT(m_cel->image() == m_celImage.get());
 
-      if ((m_flags & UseModifiedRegionAsUndoInfo) != UseModifiedRegionAsUndoInfo) {
-        // TODO Reduce m_validDstRegion to modified areas between
-        //      m_celImage and m_dstImage
-      }
-
-      // Copy the destination to the cel image.
-      m_transaction.execute(new cmd::CopyRegion(
-          m_celImage.get(), m_dstImage.get(), m_validDstRegion, dx, dy));
-
-      ASSERT(m_cel);
-      ASSERT(m_cel->layer());
-      if (m_cel &&
-          m_cel->layer() &&
-          !m_cel->layer()->isBackground()) {
-        m_transaction.execute(new cmd::TrimCel(m_cel));
-      }
-    }
-    // If the size of both images are different, we have to
-    // replace the entire image.
-    else {
-      // Validate the whole m_dstImage copying invalid areas from m_celImage
-      validateDestCanvas(gfx::Region(m_bounds));
-
-      gfx::Rect trimBounds = getTrimDstImageBounds();
-      if (!trimBounds.isEmpty()) {
-        newPos += trimBounds.origin();
-
-        // Replace the image in the stock. We need to create a copy of
-        // image because m_dstImage's ImageBuffer cannot be shared.
-        ImageRef newImage(trimDstImage(trimBounds));
-        ASSERT(newImage);
-
-        if (newPos != m_origCelPos) {
-          m_transaction.execute(new cmd::SetCelPosition(m_cel, newPos.x, newPos.y));
-        }
-
-        m_transaction.execute(
-          new cmd::ReplaceImage(m_sprite, m_celImage, newImage));
-      }
-      else {
-        m_transaction.execute(new cmd::ClearCel(m_cel));
-      }
-    }
+    // TODO create a new "dirty dst region" which is the region to be
+    // patched as m_validDstRegion includes more than it's needed.
+    m_transaction.execute(
+      new cmd::PatchCel(
+        m_cel,
+        m_dstImage.get(),
+        m_validDstRegion,
+        m_bounds.origin()));
   }
   else {
     ASSERT(false);