From 3ac4ca8869c9616b375cb4801e748756103ff95b Mon Sep 17 00:00:00 2001 From: David Capello Date: Mon, 23 May 2022 16:19:06 -0300 Subject: [PATCH] Create a custom widget for UndoHistory (fix #3281) Several performance issues fixed (as we avoid keeping a ListBox with ListItem in sync with the UndoHistory/UndoStates). There is still some room for improvement: e.g. grouping several ui::View::updateView() in just one if several onAddUndoState() will be called (e.g. when we are running a script without transactions). --- data/widgets/undo_history.xml | 5 +- src/app/commands/cmd_undo_history.cpp | 413 +++++++++++++++++++------- src/app/doc_undo.cpp | 5 + src/app/doc_undo.h | 4 +- 4 files changed, 321 insertions(+), 106 deletions(-) diff --git a/data/widgets/undo_history.xml b/data/widgets/undo_history.xml index f44bc3928..9b621b0d5 100644 --- a/data/widgets/undo_history.xml +++ b/data/widgets/undo_history.xml @@ -1,9 +1,8 @@ + - - - + diff --git a/src/app/commands/cmd_undo_history.cpp b/src/app/commands/cmd_undo_history.cpp index 1f871ef98..75657f76e 100644 --- a/src/app/commands/cmd_undo_history.cpp +++ b/src/app/commands/cmd_undo_history.cpp @@ -15,52 +15,292 @@ #include "app/context.h" #include "app/context_observer.h" #include "app/doc.h" +#include "app/doc_access.h" #include "app/doc_undo.h" #include "app/doc_undo_observer.h" #include "app/docs_observer.h" -#include "app/doc_access.h" #include "app/modules/gui.h" #include "app/modules/palettes.h" #include "app/site.h" +#include "app/ui/skin/skin_theme.h" #include "base/mem_utils.h" +#include "fmt/format.h" +#include "ui/init_theme_event.h" #include "ui/listitem.h" #include "ui/message.h" +#include "ui/paint_event.h" +#include "ui/size_hint_event.h" +#include "ui/view.h" #include "undo/undo_state.h" #include "undo_history.xml.h" namespace app { +using namespace app::skin; + class UndoHistoryWindow : public app::gen::UndoHistory, public ContextObserver, public DocsObserver, public DocUndoObserver { public: - class Item : public ui::ListItem { + class ActionsList final : public ui::Widget { public: - Item(const undo::UndoState* state) - : ui::ListItem( - (state ? - static_cast(state->cmd())->label() -#if _DEBUG - + std::string(" ") + base::get_pretty_memory_size(static_cast(state->cmd())->memSize()) -#endif - : std::string("Initial State"))), - m_state(state) { + ActionsList(UndoHistoryWindow* window) + : m_window(window) { + setFocusStop(true); + initTheme(); } - const undo::UndoState* state() { return m_state; } + + void setUndoHistory(DocUndo* history) { + m_undoHistory = history; + invalidate(); + } + + void selectState(const undo::UndoState* state) { + auto view = ui::View::getView(this); + if (!view) + return; + + invalidate(); + + gfx::Point scroll = view->viewScroll(); + if (state) { + const gfx::Rect vp = view->viewportBounds(); + const gfx::Rect bounds = this->bounds(); + + gfx::Rect itemBounds(bounds.x, bounds.y, bounds.w, m_itemHeight); + + // Jump first "Initial State" + itemBounds.y += itemBounds.h; + + const undo::UndoState* s = m_undoHistory->firstState(); + while (s) { + if (s == state) + break; + itemBounds.y += itemBounds.h; + s = s->next(); + } + + if (itemBounds.y < vp.y) + scroll.y = itemBounds.y - bounds.y; + else if (itemBounds.y > vp.y + vp.h - itemBounds.h) + scroll.y = (itemBounds.y - bounds.y - vp.h + itemBounds.h); + } + else { + scroll = gfx::Point(0, 0); + } + + view->setViewScroll(scroll); + } + + obs::signal Change; + + protected: + void onInitTheme(ui::InitThemeEvent& ev) override { + Widget::onInitTheme(ev); + auto theme = SkinTheme::get(this); + m_itemHeight = + textHeight() + + theme->calcBorder(this, theme->styles.listItem()).height(); + } + + bool onProcessMessage(ui::Message* msg) override { + switch (msg->type()) { + + case ui::kMouseDownMessage: + captureMouse(); + + case ui::kMouseMoveMessage: + if (hasCapture()) { + auto mouseMsg = static_cast(msg); + const gfx::Rect bounds = this->bounds(); + + // Mouse position in client coordinates + const gfx::Point mousePos = mouseMsg->position(); + gfx::Rect itemBounds(bounds.x, bounds.y, bounds.w, m_itemHeight); + + // First state + if (itemBounds.contains(mousePos)) { + Change(nullptr); + break; + } + itemBounds.y += itemBounds.h; + + const undo::UndoState* state = m_undoHistory->firstState(); + while (state) { + if (itemBounds.contains(mousePos)) { + Change(state); + break; + } + itemBounds.y += itemBounds.h; + state = state->next(); + } + } + break; + + case ui::kMouseUpMessage: + releaseMouse(); + break; + + case ui::kMouseWheelMessage: { + auto view = ui::View::getView(this); + if (view) { + auto mouseMsg = static_cast(msg); + gfx::Point scroll = view->viewScroll(); + + if (mouseMsg->preciseWheel()) + scroll += mouseMsg->wheelDelta(); + else + scroll += mouseMsg->wheelDelta() * 3*(m_itemHeight+4*ui::guiscale()); + + view->setViewScroll(scroll); + } + break; + } + + case ui::kKeyDownMessage: + if (hasFocus() && m_undoHistory) { + const undo::UndoState* current = m_undoHistory->currentState(); + const undo::UndoState* select = current; + auto view = ui::View::getView(this); + const gfx::Rect vp = view->viewportBounds(); + ui::KeyMessage* keymsg = static_cast(msg); + ui::KeyScancode scancode = keymsg->scancode(); + + if (keymsg->onlyCmdPressed()) { + if (scancode == ui::kKeyUp) scancode = ui::kKeyHome; + if (scancode == ui::kKeyDown) scancode = ui::kKeyEnd; + } + + switch (scancode) { + case ui::kKeyUp: + if (select) + select = select->prev(); + else + select = m_undoHistory->lastState(); + break; + case ui::kKeyDown: + if (select) + select = select->next(); + else + select = m_undoHistory->firstState(); + break; + case ui::kKeyHome: + select = nullptr; + break; + case ui::kKeyEnd: + select = m_undoHistory->lastState(); + break; + case ui::kKeyPageUp: + for (int i=0; select && iprev(); + break; + case ui::kKeyPageDown: { + int i = 0; + if (!select) { + select = m_undoHistory->firstState(); + i = 1; + } + for (; select && inext(); + break; + } + default: + return Widget::onProcessMessage(msg); + } + + if (select != current) + Change(select); + return true; + } + break; + + } + return Widget::onProcessMessage(msg); + } + + void onPaint(ui::PaintEvent& ev) override { + ui::Graphics* g = ev.graphics(); + auto theme = SkinTheme::get(this); + gfx::Rect bounds = clientBounds(); + + g->fillRect(theme->colors.background(), bounds); + + if (!m_undoHistory) + return; + + const undo::UndoState* currentState = m_undoHistory->currentState(); + gfx::Rect itemBounds(bounds.x, bounds.y, bounds.w, m_itemHeight); + + // First state + { + const bool selected = (currentState == nullptr); + paintItem(g, theme, nullptr, itemBounds, selected); + itemBounds.y += itemBounds.h; + } + + const undo::UndoState* state = m_undoHistory->firstState(); + while (state) { + const bool selected = (state == currentState); + paintItem(g, theme, state, itemBounds, selected); + itemBounds.y += itemBounds.h; + state = state->next(); + } + } + + void onSizeHint(ui::SizeHintEvent& ev) override { + if (m_window->m_nitems == 0) { + int size = 0; + if (m_undoHistory) { + ++size; + const undo::UndoState* state = m_undoHistory->firstState(); + while (state) { + ++size; + state = state->next(); + } + } + m_window->m_nitems = size; + } + ev.setSizeHint(gfx::Size(1, m_itemHeight * m_window->m_nitems)); + } + private: - const undo::UndoState* m_state; + void paintItem(ui::Graphics* g, + SkinTheme* theme, + const undo::UndoState* state, + const gfx::Rect& itemBounds, + const bool selected) { + const std::string itemText = + (state ? static_cast(state->cmd())->label() +#if _DEBUG + + std::string(" ") + base::get_pretty_memory_size(static_cast(state->cmd())->memSize()) +#endif + : std::string("Initial State")); + + if ((g->getClipBounds() & itemBounds).isEmpty()) + return; + + auto style = theme->styles.listItem(); + + ui::PaintWidgetPartInfo info; + info.text = &itemText; + info.styleFlags = (selected ? ui::Style::Layer::kSelected: 0); + theme->paintWidgetPart(g, style, itemBounds, info); + } + + UndoHistoryWindow* m_window; + DocUndo* m_undoHistory = nullptr; + int m_itemHeight; }; UndoHistoryWindow(Context* ctx) : m_ctx(ctx) - , m_document(nullptr) { + , m_doc(nullptr) + , m_actions(this) { m_title = text(); - actions()->Change.connect(&UndoHistoryWindow::onChangeAction, this); - } - - ~UndoHistoryWindow() { + m_actions.Change.connect(&UndoHistoryWindow::onChangeAction, this); + view()->attachToView(&m_actions); } private: @@ -77,12 +317,14 @@ private: attachDocument(m_ctx->activeDocument()); } + + view()->invalidate(); break; case ui::kCloseMessage: save_window_pos(this, "UndoHistory"); - if (m_document) + if (m_doc) detachDocument(); m_ctx->documents().remove_observer(this); m_ctx->remove_observer(this); @@ -91,25 +333,21 @@ private: return app::gen::UndoHistory::onProcessMessage(msg); } - void onChangeAction() { - Item* item = static_cast( - actions()->getSelectedChild()); - - if (m_document && - m_document->undoHistory()->currentState() != item->state()) { + void onChangeAction(const undo::UndoState* state) { + if (m_doc && m_doc->undoHistory()->currentState() != state) { try { - DocWriter writer(m_document, 100); - m_document->undoHistory()->moveToState(item->state()); - m_document->generateMaskBoundaries(); + DocWriter writer(m_doc, 100); + m_doc->undoHistory()->moveToState(state); + m_doc->generateMaskBoundaries(); // TODO this should be an observer of the current document palette - set_current_palette(m_document->sprite()->palette(m_frame), - false); + set_current_palette(m_doc->sprite()->palette(m_frame), false); - m_document->notifyGeneralUpdate(); + m_doc->notifyGeneralUpdate(); + m_actions.invalidate(); } catch (const std::exception& ex) { - selectState(m_document->undoHistory()->currentState()); + selectState(m_doc->undoHistory()->currentState()); Console::showException(ex); } } @@ -119,7 +357,7 @@ private: void onActiveSiteChange(const Site& site) override { m_frame = site.frame(); - if (m_document == site.document()) + if (m_doc == site.document()) return; attachDocument(const_cast(site.document())); @@ -127,33 +365,25 @@ private: // DocsObserver void onRemoveDocument(Doc* doc) override { - if (m_document && m_document == doc) + if (m_doc && m_doc == doc) detachDocument(); } // DocUndoObserver void onAddUndoState(DocUndo* history) override { ASSERT(history->currentState()); - Item* item = new Item(history->currentState()); - actions()->addChild(item); - actions()->layout(); + + ++m_nitems; + + m_actions.invalidate(); view()->updateView(); - actions()->selectChild(item); + + selectState(history->currentState()); } void onDeleteUndoState(DocUndo* history, undo::UndoState* state) override { - for (auto child : actions()->children()) { - Item* item = static_cast(child); - if (item->state() == state) { - actions()->removeChild(item); - item->deferDelete(); - break; - } - } - - actions()->layout(); - view()->updateView(); + --m_nitems; } void onCurrentUndoStateChange(DocUndo* history) override { @@ -161,92 +391,71 @@ private: } void onClearRedo(DocUndo* history) override { - refillList(history); + setUndoHistory(history); } void onTotalUndoSizeChange(DocUndo* history) override { updateTitle(); } - void attachDocument(Doc* document) { - detachDocument(); - - m_document = document; - if (!document) + void attachDocument(Doc* doc) { + if (m_doc == doc) return; - DocUndo* history = m_document->undoHistory(); + detachDocument(); + m_doc = doc; + if (!doc) + return; + + DocUndo* history = m_doc->undoHistory(); history->add_observer(this); - refillList(history); + setUndoHistory(history); updateTitle(); } void detachDocument() { - if (!m_document) + if (!m_doc) return; - clearList(); - m_document->undoHistory()->remove_observer(this); - m_document = nullptr; + m_doc->undoHistory()->remove_observer(this); + m_doc = nullptr; + + setUndoHistory(nullptr); updateTitle(); } - void clearList() { - ui::Widget* child; - while ((child = actions()->lastChild())) - delete child; - - actions()->layout(); + void setUndoHistory(DocUndo* history) { + m_nitems = 0; + m_actions.setUndoHistory(history); view()->updateView(); - } - void refillList(DocUndo* history) { - clearList(); - - // Create an item to reference the initial state (undo state == nullptr) - Item* current = new Item(nullptr); - actions()->addChild(current); - - const undo::UndoState* state = history->firstState(); - while (state) { - Item* item = new Item(state); - actions()->addChild(item); - if (state == history->currentState()) - current = item; - - state = state->next(); - } - - actions()->layout(); - view()->updateView(); - if (current) - actions()->selectChild(current); + if (history) + m_actions.selectState(history->currentState()); } void selectState(const undo::UndoState* state) { - for (auto child : actions()->children()) { - Item* item = static_cast(child); - if (item->state() == state) { - actions()->selectChild(item); - break; - } - } + m_actions.selectState(state); } void updateTitle() { - if (!m_document) + if (!m_doc) setText(m_title); - else - setTextf("%s (%s)", - m_title.c_str(), - base::get_pretty_memory_size(m_document->undoHistory()->totalUndoSize()).c_str()); + else { + setText( + fmt::format( + "{} ({})", + m_title, + base::get_pretty_memory_size(m_doc->undoHistory()->totalUndoSize()))); + } } Context* m_ctx; - Doc* m_document; + Doc* m_doc; doc::frame_t m_frame; std::string m_title; + ActionsList m_actions; + int m_nitems = 0; }; class UndoHistoryCommand : public Command { diff --git a/src/app/doc_undo.cpp b/src/app/doc_undo.cpp index c19947d2d..f31ee1f85 100644 --- a/src/app/doc_undo.cpp +++ b/src/app/doc_undo.cpp @@ -1,4 +1,5 @@ // Aseprite +// Copyright (C) 2022 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -131,6 +132,10 @@ void DocUndo::redo() void DocUndo::clearRedo() { + // Do nothing + if (currentState() == lastState()) + return; + m_undoHistory.clearRedo(); notify_observers(&DocUndoObserver::onClearRedo, this); } diff --git a/src/app/doc_undo.h b/src/app/doc_undo.h index 6cb648e0b..99367e9ba 100644 --- a/src/app/doc_undo.h +++ b/src/app/doc_undo.h @@ -1,4 +1,5 @@ // Aseprite +// Copyright (C) 2022 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -59,7 +60,8 @@ namespace app { int* savedCounter() { return &m_savedCounter; } - const undo::UndoState* firstState() const { return m_undoHistory.firstState(); } + const undo::UndoState* firstState() const { return m_undoHistory.firstState(); } + const undo::UndoState* lastState() const { return m_undoHistory.lastState(); } const undo::UndoState* currentState() const { return m_undoHistory.currentState(); } void moveToState(const undo::UndoState* state);