Fix problems generating a lot of consecutive "undo" entries when the palette were modified.

+ Added Undo::graftUndoerInLastGroup() member function.
+ Modify the PaletteEntryEditor to group a sequence of SetPaletteColors
  undoers in the same group using the new Undo::graftUndoerInLastGroup().

TODO.txt

@@ -3,7 +3,6 @@ For next release
 
 + Add IntEntry class in src/gui/ with spin-buttons.
 + Add feedback to "Shift+S" shortcut to switch "snap to grid".
-+ Fix palette editor to avoid generating a lot of consecutive Undo actions.
 + Convert jaccel::key_list to std::vector<>
 + Integrate "copy & paste" loop with selection moving loop.
 + Shade drawing mode.

src/commands/cmd_palette_editor.cpp

@@ -45,6 +45,8 @@
 #include "skin/skin_slider_property.h"
 #include "ui_context.h"
 #include "undo/undo_history.h"
+#include "undoers/close_group.h"
+#include "undoers/open_group.h"
 #include "undoers/set_palette_colors.h"
 #include "widgets/color_bar.h"
 #include "widgets/color_sliders.h"
@@ -116,6 +118,15 @@ private:
   int m_redrawTimerId;
   bool m_redrawAll;
 
+  // True if the palette change must be graft in the UndoHistory
+  // (e.g. when two or more changes in the palette are made in short
+  // time).
+  bool m_graftChange;
+
+  // True if the PaletteChange signal is generated by the same
+  // PaletteEntryEditor instance.
+  bool m_selfPalChange;
+
   Signal0<void>::SlotType* m_palChangeSlot;
 };
 
@@ -239,6 +250,8 @@ PaletteEntryEditor::PaletteEntryEditor()
   , m_quantizeButton("Quantize")
   , m_disableHexUpdate(false)
   , m_redrawAll(false)
+  , m_graftChange(false)
+  , m_selfPalChange(false)
 {
   m_redrawTimerId = jmanager_add_timer(this, 250);
 
@@ -369,10 +382,13 @@ bool PaletteEntryEditor::onProcessMessage(Message* msg)
     // Redraw all editors
     if (m_redrawAll) {
       m_redrawAll = false;
+      m_graftChange = false;
       jmanager_stop_timer(m_redrawTimerId);
 
       // Call all listener of PaletteChange event.
+      m_selfPalChange = true;
       App::instance()->PaletteChange();
+      m_selfPalChange = false;
 
       // Redraw all editors
       try {
@@ -705,10 +721,22 @@ void PaletteEntryEditor::updateCurrentSpritePalette(const char* operationName)
       if (from >= 0 && to >= from) {
 	// Add undo information to save the range of pal entries that will be modified.
 	if (undo->isEnabled()) {
-	  undo->setLabel(operationName);
-	  undo->setModification(undo::ModifyDocument);
-	  undo->pushUndoer(new undoers::SetPaletteColors(undo->getObjects(),
-	      sprite, currentSpritePalette, from, to));
+	  if (m_graftChange && strcmp(undo->getLabel(), operationName) == 0) {
+	    undo->setLabel(operationName);
+	    undo->setModification(undo::ModifyDocument);
+
+	    undo->graftUndoerInLastGroup(new undoers::SetPaletteColors(undo->getObjects(),
+								       sprite, currentSpritePalette, from, to));
+	  }
+	  else {
+	    undo->setLabel(operationName);
+	    undo->setModification(undo::ModifyDocument);
+
+	    undo->pushUndoer(new undoers::OpenGroup());
+	    undo->pushUndoer(new undoers::SetPaletteColors(undo->getObjects(),
+							   sprite, currentSpritePalette, from, to));
+	    undo->pushUndoer(new undoers::CloseGroup());
+	  }
 	}
 
 	// Change the sprite palette
@@ -727,6 +755,7 @@ void PaletteEntryEditor::updateCurrentSpritePalette(const char* operationName)
     jmanager_start_timer(m_redrawTimerId);
 
   m_redrawAll = false;
+  m_graftChange = true;
 }
 
 void PaletteEntryEditor::updateColorBar()
@@ -736,11 +765,13 @@ void PaletteEntryEditor::updateColorBar()
 
 void PaletteEntryEditor::onPalChange()
 {
-  PaletteView* palette_editor = app_get_colorbar()->getPaletteView();
-  setColor(Color::fromIndex(palette_editor->getSelectedEntry()));
+  if (!m_selfPalChange) {
+    PaletteView* palette_editor = app_get_colorbar()->getPaletteView();
+    setColor(Color::fromIndex(palette_editor->getSelectedEntry()));
 
-  // Redraw the window
-  invalidate();
+    // Redraw the window
+    invalidate();
+  }
 }
 
 //////////////////////////////////////////////////////////////////////

src/undo/undo_history.cpp

@@ -194,16 +194,36 @@ void UndoHistory::discardTail()
 
 void UndoHistory::pushUndoer(Undoer* undoer)
 {
-  // TODO Replace this with the following implementation:
-  // * Add the undo limit to UndoHistory class as a normal member (non-static).
-  // * Add App signals to listen changes in settings
-  // * Document should listen changes in the undo limit, 
-  // * When a change is produced, Document calls getUndoHistory()->setUndoLimit().
-  int undo_size_limit = (int)get_config_int("Options", "UndoSizeLimit", 8)*1024*1024;
-
   // Add the undoer in the undoers stack
   m_undoers->pushUndoer(undoer);
 
+  postUndoerAddedEvent(undoer);
+}
+
+bool UndoHistory::graftUndoerInLastGroup(Undoer* undoer)
+{
+  Undoer* lastUndoer = m_undoers->popUndoer(UndoersStack::PopFromHead);
+  bool result;
+
+  if (lastUndoer == NULL || !lastUndoer->isCloseGroup()) {
+    if (lastUndoer)
+      m_undoers->pushUndoer(lastUndoer);
+    m_undoers->pushUndoer(undoer);
+
+    result = false;
+  }
+  else {
+    m_undoers->pushUndoer(undoer);
+    m_undoers->pushUndoer(lastUndoer);
+    result = true;
+  }
+
+  postUndoerAddedEvent(undoer);
+  return result;
+}
+
+void UndoHistory::postUndoerAddedEvent(Undoer* undoer)
+{
   // Reset the "redo" stack.
   clearRedo();
 
@@ -222,11 +242,27 @@ void UndoHistory::pushUndoer(Undoer* undoer)
     if (m_modification == ModifyDocument)
       m_diffCount++;
 
-    // Is undo history too big?
-    int groups = m_undoers->countUndoGroups();
-    while (groups > 1 && m_undoers->getMemSize() > undo_size_limit) {
-      discardTail();
-      groups--;
-    }
+    checkSizeLimit();
   }
 }
+
+// Discards undoers in in case the UndoHistory is bigger than the given limit.
+void UndoHistory::checkSizeLimit()
+{
+  // Is undo history too big?
+  int groups = m_undoers->countUndoGroups();
+  while (groups > 1 && m_undoers->getMemSize() > getUndoSizeLimit()) {
+    discardTail();
+    groups--;
+  }
+}
+
+int UndoHistory::getUndoSizeLimit()
+{
+  // TODO Replace this with the following implementation:
+  // * Add the undo limit to UndoHistory class as a normal member (non-static).
+  // * Add App signals to listen changes in settings
+  // * Document should listen changes in the undo limit, 
+  // * When a change is produced, Document calls getUndoHistory()->setUndoLimit().
+  return (int)get_config_int("Options", "UndoSizeLimit", 8)*1024*1024;
+}

src/undo/undo_history.h

@@ -55,12 +55,19 @@ public:
   // UndoersCollector interface
   void pushUndoer(Undoer* undoer);
 
+  // Special method to add new undoers inside the last added group.
+  // Returns true if the undoer was added in a group.
+  bool graftUndoerInLastGroup(Undoer* undoer);
+
 private:
   enum Direction { UndoDirection, RedoDirection };
 
   void runUndo(Direction direction);
   void discardTail();
   void updateUndo();
+  void postUndoerAddedEvent(Undoer* undoer);
+  void checkSizeLimit();
+  static int getUndoSizeLimit();
 
   ObjectsContainer* m_objects;	// Container of objects to insert & retrieve objects by ID
   UndoersStack* m_undoers;

src/undo/undoers_stack.h

@@ -83,6 +83,8 @@ public:
 private:
   UndoHistory* m_undoHistory;
   Items m_items;
+
+  // Bytes occupied by all undoers in the stack.
   int m_size;
 };