Modify PixelsMovementImpl to avoid write-locking the document

in the whole operation.

src/undo_transaction.cpp

@@ -35,7 +35,7 @@
 #include "raster/stock.h"
 #include "raster/undo_history.h"
 
-UndoTransaction::UndoTransaction(DocumentWriter& document, const char* label)
+UndoTransaction::UndoTransaction(Document* document, const char* label)
 {
   ASSERT(label != NULL);
 

src/undo_transaction.h

@@ -23,7 +23,6 @@
 
 class Cel;
 class Document;
-class DocumentWriter;
 class Image;
 class Layer;
 class LayerImage;
@@ -31,12 +30,21 @@ class Mask;
 class Sprite;
 class UndoHistory;
 
-/**
- * Class with high-level set of routines to modify a sprite.
- *
- * In this class, all modifications in the sprite have an undo
- * counterpart (if the sprite's undo is enabled).
- */
+// High-level class to group a set of operations to modify the
+// document atomically, adding information in the undo history to
+// rollback the whole operation if something fails (with an
+// exceptions) in the middle of the procedure.
+//
+// You have to wrap every call to an undo transaction with a
+// DocumentWriter. The preferred usage is as the following:
+// 
+// {
+//   DocumentWriter documentWriter(document);
+//   UndoTransaction undoTransaction(documentWriter, "My big operation");
+//   ...
+//   undoTransaction.commit();
+// }
+// 
 class UndoTransaction
 {
 public:
@@ -44,7 +52,7 @@ public:
   // Starts a undoable sequence of operations in a transaction that
   // can be committed or rollbacked.  All the operations will be
   // grouped in the sprite's undo as an atomic operation.
-  UndoTransaction(DocumentWriter& document, const char* label);
+  UndoTransaction(Document* document, const char* label);
   virtual ~UndoTransaction();
 
   inline Sprite* getSprite() const { return m_sprite;  }

src/widgets/editor/pixels_movement.cpp

@@ -33,7 +33,7 @@ using namespace gfx;
 
 class PixelsMovementImpl
 {
-  DocumentWriter m_documentWriter;
+  const DocumentReader m_documentReader;
   Sprite* m_sprite;
   UndoTransaction m_undoTransaction;
   int m_initial_x, m_initial_y;
@@ -43,17 +43,18 @@ class PixelsMovementImpl
 
 public:
   PixelsMovementImpl(Document* document, Sprite* sprite, const Image* moveThis, int initial_x, int initial_y, int opacity)
-    : m_documentWriter(document)
+    : m_documentReader(document)
     , m_sprite(sprite)
-    , m_undoTransaction(m_documentWriter, "Pixels Movement")
+    , m_undoTransaction(document, "Pixels Movement")
     , m_initial_x(initial_x)
     , m_initial_y(initial_y)
     , m_firstDrop(true)
     , m_isDragging(false)
   {
-    m_documentWriter->prepareExtraCel(initial_x, initial_y, moveThis->w, moveThis->h, opacity);
+    DocumentWriter documentWriter(m_documentReader);
+    documentWriter->prepareExtraCel(initial_x, initial_y, moveThis->w, moveThis->h, opacity);
 
-    Image* extraImage = m_documentWriter->getExtraCelImage();
+    Image* extraImage = documentWriter->getExtraCelImage();
     image_copy(extraImage, moveThis, 0, 0);
   }
 
@@ -63,7 +64,10 @@ public:
 
   void cutMask()
   {
-    m_undoTransaction.clearMask(app_get_color_to_clear_layer(m_sprite->getCurrentLayer()));
+    {
+      DocumentWriter documentWriter(m_documentReader);
+      m_undoTransaction.clearMask(app_get_color_to_clear_layer(m_sprite->getCurrentLayer()));
+    }
 
     copyMask();
   }
@@ -71,11 +75,13 @@ public:
   void copyMask()
   {
     // Hide the mask (do not deselect it, it will be moved them using m_undoTransaction.setMaskPosition)
-    Mask* empty_mask = new Mask();
-    m_documentWriter->generateMaskBoundaries(empty_mask);
-    delete empty_mask;
+    Mask emptyMask;
+    {
+      DocumentWriter documentWriter(m_documentReader);
+      documentWriter->generateMaskBoundaries(&emptyMask);
+    }
 
-    update_screen_for_document(m_documentWriter);
+    update_screen_for_document(m_documentReader);
   }
 
   void catchImage(int x, int y)
@@ -88,7 +94,7 @@ public:
   void catchImageAgain(int x, int y)
   {
     // Create a new UndoTransaction to move the pixels to other position
-    Cel* cel = m_documentWriter->getExtraCel();
+    const Cel* cel = m_documentReader->getExtraCel();
     m_initial_x = cel->x;
     m_initial_y = cel->y;
     m_isDragging = true;
@@ -97,17 +103,20 @@ public:
     m_catch_y = y;
 
     // Hide the mask (do not deselect it, it will be moved them using m_undoTransaction.setMaskPosition)
-    Mask* empty_mask = new Mask();
-    m_documentWriter->generateMaskBoundaries(empty_mask);
-    delete empty_mask;
+    Mask emptyMask;
+    {
+      DocumentWriter documentWriter(m_documentReader);
+      documentWriter->generateMaskBoundaries(&emptyMask);
+    }
 
-    update_screen_for_document(m_documentWriter);
+    update_screen_for_document(m_documentReader);
   }
 
   Rect moveImage(int x, int y)
   {
-    Cel* cel = m_documentWriter->getExtraCel();
-    Image* image = m_documentWriter->getExtraCelImage();
+    DocumentWriter documentWriter(m_documentReader);
+    Cel* cel = documentWriter->getExtraCel();
+    Image* image = documentWriter->getExtraCelImage();
     int x1, y1, x2, y2;
     int u1, v1, u2, v2;
 
@@ -140,31 +149,38 @@ public:
   {
     m_isDragging = false;
 
-    Cel* cel = m_documentWriter->getExtraCel();
+    const Cel* cel = m_documentReader->getExtraCel();
 
-    // Show the mask again in the new position
-    if (m_firstDrop) {
-      m_firstDrop = false;
-      m_undoTransaction.setMaskPosition(cel->x, cel->y);
-    }
-    else {
-      m_documentWriter->getMask()->x = cel->x;
-      m_documentWriter->getMask()->y = cel->y;
-    }
-    m_documentWriter->generateMaskBoundaries();
+    {
+      DocumentWriter documentWriter(m_documentReader);
 
-    update_screen_for_document(m_documentWriter);
+      // Show the mask again in the new position
+      if (m_firstDrop) {
+	m_firstDrop = false;
+	m_undoTransaction.setMaskPosition(cel->x, cel->y);
+      }
+      else {
+	documentWriter->getMask()->x = cel->x;
+	documentWriter->getMask()->y = cel->y;
+      }
+      documentWriter->generateMaskBoundaries();
+    }
+
+    update_screen_for_document(m_documentReader);
   }
 
   void dropImage()
   {
     m_isDragging = false;
 
-    Cel* cel = m_documentWriter->getExtraCel();
-    Image* image = m_documentWriter->getExtraCelImage();
+    const Cel* cel = m_documentReader->getExtraCel();
+    const Image* image = m_documentReader->getExtraCelImage();
 
-    m_undoTransaction.pasteImage(image, cel->x, cel->y, cel->opacity);
-    m_undoTransaction.commit();
+    {
+      DocumentWriter documentWriter(m_documentReader);
+      m_undoTransaction.pasteImage(image, cel->x, cel->y, cel->opacity);
+      m_undoTransaction.commit();
+    }
   }
 
   bool isDragging()
@@ -174,8 +190,8 @@ public:
 
   Rect getImageBounds()
   {
-    Cel* cel = m_documentWriter->getExtraCel();
-    Image* image = m_documentWriter->getExtraCelImage();
+    const Cel* cel = m_documentReader->getExtraCel();
+    const Image* image = m_documentReader->getExtraCelImage();
 
     ASSERT(cel != NULL);
     ASSERT(image != NULL);
@@ -185,13 +201,16 @@ public:
 
   void setMaskColor(ase_uint32 mask_color)
   {
-    Image* image = m_documentWriter->getExtraCelImage();
+    {
+      DocumentWriter documentWriter(m_documentReader);
+      Image* image = documentWriter->getExtraCelImage();
 
-    ASSERT(image != NULL);
+      ASSERT(image != NULL);
 
-    image->mask_color = mask_color;
+      image->mask_color = mask_color;
+    }
 
-    update_screen_for_document(m_documentWriter);
+    update_screen_for_document(m_documentReader);
   }
 
 };