Undo system for painterly paintops.

[Emanuele Tamponi]

Hello,
I attached the patches that implements the method I thought for undoing 
painterly actions.

Explanation of the problem:
Actually, KisTransaction handles only one paint device at once.
Painterly paintops change two paint devices at a time: the paint device 
onto they're putting colors, and the painterly overlay where they write 
painterly data.

Possible solution:

QUndoCommand provides

virtual bool mergeWith(const QUndoCommand *command)

that is used "to merge two commands with the same ids".
The default implementation returns false, so I implemented my own, this 
way: I changed the d-pointer so that it contains

QMap<KisPaintDeviceSP, KisMementoSP> data

I used a standard way of iteration through the values of the map by 
using foreach for undo(), redo() and undoNoUpdate().

Then mergeWith just calls
data.unite(transaction->m_private->data);

This works as expected only when the paint device of the command 
parameter is different from the device of the destination 
KisTransaction. And it's what we want because we don't want to merge two 
KisTransactions for the same paint devices.
Anyway, it would not do anything evil when called with a command on the 
same paint device, since in this case this unite is just ignored.

Next, I added a setSourceLayer(KisLayer *) method to KisPainter. It 
dynamic_cast's it to a KisPaintLayer to have the "source layer" which 
will be used by the paintop to get the "source paint device" which it 
reads the color from, and the "source painterly overlay", that's similar 
use.
Once setSourceLayer is called from the Tool. The KisPainter will decide 
what to do with transactions. In particular:

- If a non-painterly paintop (that's: all paintops except mine for now) 
is set (with setPaintOp), it will do nothing more than what it has 
always done.
- If a painterly paintop is set, it has these possibilities:
      - If m_transaction != 0, then it infers that the developer want to 
keep track of the overlay too, and initializes m_overlayTransaction by 
calling beginOverlayTransaction
      - If m_transaction == 0, don't do a m_overlayTransaction.

Then the paintop, painterly or non-painterly, will do all his things 
until KisPainter::endTransaction() is called. Then, again:
- If a non-painterly paintop is used, do nothing more.
- If a painterly paintop is used, and the transaction is not zero, then 
merge the overlay transaction with the device transaction and return the 
merged command

Advantages:
I liked this solution because it is completely transparent to the final 
developer, who has not to bother with all this things.

For example:
(1)
Emanuele wants to implement a tool and want his tool to be usable with 
painterly paintops.
To let his tool be used from painterly paintops, he has to:

- do *nothing* if his tool inherits from KisToolFreehand and uses its 
initPaint and endPaint functions
- add m_painter->setSourceLayer(sourceLayer)  before calling setPaintOp
- write the same old code to undo commands, without any extra call.

(2)
Bart wants to implement his own painterly paintop.
- It has just to do this check:

if (!painter()->sourceLayer())
    return;

(because the paintop can't be used if there is no data to take from to 
do bidirectionality...)

This means that almost all tools are painterly-paintops-ready, by just 
adding only one line of code, and that all painterly paintops can check 
if the current tool supports them by adding 2 lines of code.

If you agree, I can commit.

Bye,
Emanuele
-------------- next part --------------
Index: kis_painter.cc
===================================================================
--- kis_painter.cc	(revisione 702430)
+++ kis_painter.cc	(copia locale)
@@ -46,6 +46,7 @@
 #include "kis_layer.h"
 #include "kis_paint_device.h"
 #include "kis_painter.h"
+#include "kis_painterly_overlay.h"
 #include "KoColorSpace.h"
 #include "kis_transaction.h"
 #include "kis_types.h"
@@ -77,6 +78,7 @@
 void KisPainter::init()
 {
     m_transaction = 0;
+	m_overlayTransaction = 0;
     m_paintOp = 0;
     m_brush = 0;
     m_pattern= 0;
@@ -96,9 +98,9 @@
 
 KisPainter::~KisPainter()
 {
+    end();
     m_brush = 0;
     delete m_paintOp;
-    end();
 }
 
 void KisPainter::begin(KisPaintDeviceSP device)
@@ -109,6 +111,8 @@
 
     if (m_transaction)
         delete m_transaction;
+	if (m_overlayTransaction)
+		delete m_overlayTransaction;
 
     m_device = device;
     m_colorSpace = device->colorSpace();
@@ -121,6 +125,30 @@
     return endTransaction();
 }
 
+void KisPainter::setSourceLayer(KisLayer *source)
+{
+	if (!source)
+		return;
+	m_sourceLayer = dynamic_cast<KisPaintLayer *>(source);
+
+	if (!m_sourceLayer)
+		return;
+
+	if (!m_sourceLayer->painterlyOverlay())
+		m_sourceLayer->createPainterlyOverlay();
+}
+
+void KisPainter::beginOverlayTransaction()
+{
+	Q_ASSERT(m_sourceLayer);
+	Q_ASSERT(m_sourceLayer->painterlyOverlay());
+
+	if (m_overlayTransaction)
+		delete m_overlayTransaction;
+	m_overlayTransaction = new KisTransaction("Overlay Auto Command", m_sourceLayer->painterlyOverlay());
+	Q_CHECK_PTR(m_overlayTransaction);
+}
+
 void KisPainter::beginTransaction(const QString& customName)
 {
     if (m_transaction)
@@ -136,11 +164,19 @@
     m_transaction = command;
 }
 
-
 QUndoCommand *KisPainter::endTransaction()
 {
     QUndoCommand *command = m_transaction;
+	if (m_transaction && m_overlayTransaction) {
+		if (paintOp() && paintOp()->painterly()) {
+			bool result = command->mergeWith(m_overlayTransaction);
+			Q_ASSERT(result);
+
+			delete m_overlayTransaction;
+		}
+	}
     m_transaction = 0;
+	m_overlayTransaction = 0;
     return command;
 }
 
@@ -177,6 +213,12 @@
 {
     delete m_paintOp;
     m_paintOp = paintOp;
+	if (m_paintOp->painterly() && m_transaction) {
+		if (m_sourceLayer && m_sourceLayer->painterlyOverlay())
+			beginOverlayTransaction();
+		else
+			kDebug() << "You didn't set the source layer - expect problems!";
+	}
 }
 
 void KisPainter::bitBlt(qint32 dx, qint32 dy,
-------------- next part --------------
Index: kis_painter.h
===================================================================
--- kis_painter.h	(revisione 702430)
+++ kis_painter.h	(copia locale)
@@ -71,6 +71,8 @@
     // Implement KisProgressSubject
     virtual void cancel() { m_cancelRequested = true; }
 
+	void beginOverlayTransaction();
+
 public:
     /**
      * Start painting on the specified device. Not undoable.
@@ -504,7 +506,7 @@
     // Returns the current fill color
     KoColor fillColor() const { return m_fillColor; }
 
-	void setSourceLayer(KisPaintLayer *source) { m_sourceLayer = source; }
+	void setSourceLayer(KisLayer *source);
 	KisPaintLayer *sourceLayer() { return m_sourceLayer; }
 	void setComplexColor(KisComplexColor *color) { m_complexColor = color; }
 	KisComplexColor *complexColor() { return m_complexColor; }
@@ -604,7 +606,8 @@
 
 protected:
     KisPaintDeviceSP m_device;
-    KisTransaction  *m_transaction;
+    KisTransaction *m_transaction;
+	KisTransaction *m_overlayTransaction;
 
     QRegion m_dirtyRegion;
     QRect m_dirtyRect;
-------------- next part --------------
Index: kis_transaction.cc
===================================================================
--- kis_transaction.cc	(revisione 702349)
+++ kis_transaction.cc	(copia locale)
@@ -15,6 +15,9 @@
  *  along with this program; if not, write to the Free Software
  *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
  */
+
+#include <QMap>
+
 #include "kis_transaction.h"
 
 #include "kis_types.h"
@@ -26,8 +29,7 @@
 class KisTransaction::Private {
 public:
     QString name;
-    KisPaintDeviceSP device;
-    KisMementoSP memento;
+    QMap<KisPaintDeviceSP, KisMementoSP> data;
     bool firstRedo;
 };
 
@@ -35,17 +37,16 @@
     : QUndoCommand(name, parent)
     , m_private( new Private() )
 {
-    m_private->device = device;
-    m_private->memento = device->dataManager()->getMemento();
+    m_private->data.insert(device, device->dataManager()->getMemento());
     m_private->firstRedo = true;
 }
 
 KisTransaction::~KisTransaction()
 {
-    if (m_private->memento) {
-        // For debugging purposes
-        m_private->memento->setInvalid();
-    }
+	KisMementoSP memento;
+	foreach (memento, m_private->data)
+		if (memento)
+			memento->setInvalid();
     delete m_private;
 }
 
@@ -57,34 +58,51 @@
         m_private->firstRedo = false;
         return;
     }
-    Q_ASSERT(!m_private->memento.isNull());
 
-    m_private->device->dataManager()->rollforward(m_private->memento);
-
-    QRect rc;
-    qint32 x, y, width, height;
-    m_private->memento->extent(x,y,width,height);
-    rc.setRect(x + m_private->device->x(), y + m_private->device->y(), width, height);
-
-    m_private->device->setDirty( rc );
+	KisMementoSP memento;
+	KisPaintDeviceSP device;
+	foreach (memento, m_private->data) {
+		device = m_private->data.key(memento);
+		device->dataManager()->rollforward(memento);
+		QRect rc;
+		qint32 x, y, width, height;
+		memento->extent(x, y, width, height);
+		rc.setRect(x + device->x(), y + device->y(), width, height);
+		device->setDirty( rc );
+	}
 }
 
 void KisTransaction::undo()
 {
-    Q_ASSERT(!m_private->memento.isNull());
-    m_private->device->dataManager()->rollback(m_private->memento);
+	KisMementoSP memento;
+	KisPaintDeviceSP device;
+	foreach (memento, m_private->data) {
+		device = m_private->data.key(memento);
+		device->dataManager()->rollback(memento);
+		QRect rc;
+		qint32 x, y, width, height;
+		memento->extent(x, y, width, height);
+		rc.setRect(x + device->x(), y + device->y(), width, height);
+		device->setDirty( rc );
+	}
+}
 
-    QRect rc;
-    qint32 x, y, width, height;
-    m_private->memento->extent(x,y,width,height);
-    rc.setRect(x + m_private->device->x(), y + m_private->device->y(), width, height);
+bool KisTransaction::mergeWith(const QUndoCommand *command)
+{
+	const KisTransaction *transaction = dynamic_cast<const KisTransaction *>(command);
+	if (!transaction)
+		return false;
 
-    m_private->device->setDirty( rc );
+	m_private->data.unite(transaction->m_private->data);
+	return true;
 }
 
 void KisTransaction::undoNoUpdate()
 {
-    Q_ASSERT(!m_private->memento.isNull());
-
-    m_private->device->dataManager()->rollback(m_private->memento);
+	KisMementoSP memento;
+	KisPaintDeviceSP device;
+	foreach (memento, m_private->data) {
+		device = m_private->data.key(memento);
+		device->dataManager()->rollback(memento);
+	}
 }
-------------- next part --------------
Index: kis_transaction.h
===================================================================
--- kis_transaction.h	(revisione 702349)
+++ kis_transaction.h	(copia locale)
@@ -34,6 +34,7 @@
     virtual void redo();
     virtual void undo();
     virtual void undoNoUpdate();
+	virtual bool mergeWith(const QUndoCommand *command);
 private:
     class Private;
     Private * const m_private;
-------------- next part --------------
Index: kis_tool_freehand.cc
===================================================================
--- kis_tool_freehand.cc	(revisione 702431)
+++ kis_tool_freehand.cc	(copia locale)
@@ -238,14 +238,14 @@
     m_painter = new KisPainter( m_target );
     Q_CHECK_PTR(m_painter);
     m_source = device;
+
     m_painter->beginTransaction(m_transactionText);
 
     m_painter->setPaintColor(currentFgColor());
     m_painter->setBackgroundColor(currentBgColor());
     m_painter->setBrush(currentBrush());
-	if (dynamic_cast<KisPaintLayer *>(currentLayer().data()))
-		m_painter->setSourceLayer(dynamic_cast<KisPaintLayer *>(currentLayer().data()));
 	m_painter->setComplexColor(currentComplexColor());
+	m_painter->setSourceLayer(currentLayer().data());
 
     // if you're drawing on a temporary layer, the layer already sets this
     if (m_paintIncremental) {
@@ -254,8 +254,8 @@
     } else {
         m_painter->setCompositeOp(device->colorSpace()->compositeOp(COMPOSITE_ALPHA_DARKEN));
         m_painter->setOpacity( OPACITY_OPAQUE );
+    }
 
-    }
     m_previousTangent = QPointF(0,0);
     m_previousDrag = QPointF(0,0);
 /*    kDebug(41007) <<"target:" << m_target <<"(" << m_target->name() <<" )"