From 7b639e814b041107ff4bec2f2081c9ca9d79297e Mon Sep 17 00:00:00 2001 From: David Capello Date: Tue, 9 Aug 2011 22:40:09 -0300 Subject: [PATCH] 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 | 1 - src/commands/cmd_palette_editor.cpp | 47 ++++++++++++++++++---- src/undo/undo_history.cpp | 62 +++++++++++++++++++++++------ src/undo/undo_history.h | 7 ++++ src/undo/undoers_stack.h | 2 + 5 files changed, 97 insertions(+), 22 deletions(-) diff --git a/TODO.txt b/TODO.txt index d719225d3..b1f117c01 100644 --- a/TODO.txt +++ b/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. diff --git a/src/commands/cmd_palette_editor.cpp b/src/commands/cmd_palette_editor.cpp index 53e101356..c66a2c044 100644 --- a/src/commands/cmd_palette_editor.cpp +++ b/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::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(); + } } ////////////////////////////////////////////////////////////////////// diff --git a/src/undo/undo_history.cpp b/src/undo/undo_history.cpp index dd1d6975d..f008ab449 100644 --- a/src/undo/undo_history.cpp +++ b/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; +} diff --git a/src/undo/undo_history.h b/src/undo/undo_history.h index 4e89f1aec..4a6f102a9 100644 --- a/src/undo/undo_history.h +++ b/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; diff --git a/src/undo/undoers_stack.h b/src/undo/undoers_stack.h index be5cdfe82..bfc40e99c 100644 --- a/src/undo/undoers_stack.h +++ b/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; };