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

@@ -1,9 +1,8 @@
 <!-- Aseprite -->
+<!-- Copyright (C) 2022 by Igara Studio S.A. -->
 <!-- Copyright (C) 2015-2016 by David Capello -->
 <gui>
   <window id="undo_history" text="@.title">
-    <view id="view" expansive="true" width="80" height="100">
+    <view id="view" expansive="true" width="80" height="100" />
-      <listbox id="actions" />
-    </view>
   </window>
 </gui>

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)
+    ActionsList(UndoHistoryWindow* window)
-      : ui::ListItem(
+      : m_window(window) {
-          (state ?
+      setFocusStop(true);
-           static_cast<Cmd*>(state->cmd())->label()
+      initTheme();
-#if _DEBUG
-           + std::string(" ") + base::get_pretty_memory_size(static_cast<Cmd*>(state->cmd())->memSize())
-#endif
-           : std::string("Initial State"))),
-        m_state(state) {
     }
-    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<void(const undo::UndoState*)> 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<ui::MouseMessage*>(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<ui::MouseMessage*>(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<ui::KeyMessage*>(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 && i<vp.h / m_itemHeight; ++i)
+                  select = select->prev();
+                break;
+              case ui::kKeyPageDown: {
+                int i = 0;
+                if (!select) {
+                  select = m_undoHistory->firstState();
+                  i = 1;
+                }
+                for (; select && i<vp.h / m_itemHeight; ++i)
+                  select = select->next();
+                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<Cmd*>(state->cmd())->label()
+#if _DEBUG
+         + std::string(" ") + base::get_pretty_memory_size(static_cast<Cmd*>(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);
+    m_actions.Change.connect(&UndoHistoryWindow::onChangeAction, this);
-  }
+    view()->attachToView(&m_actions);
-
-  ~UndoHistoryWindow() {
   }
 
 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() {
+  void onChangeAction(const undo::UndoState* state) {
-    Item* item = static_cast<Item*>(
+    if (m_doc && m_doc->undoHistory()->currentState() != state) {
-      actions()->getSelectedChild());
-
-    if (m_document &&
-        m_document->undoHistory()->currentState() != item->state()) {
       try {
-        DocWriter writer(m_document, 100);
+        DocWriter writer(m_doc, 100);
-        m_document->undoHistory()->moveToState(item->state());
+        m_doc->undoHistory()->moveToState(state);
-        m_document->generateMaskBoundaries();
+        m_doc->generateMaskBoundaries();
 
         // TODO this should be an observer of the current document palette
-        set_current_palette(m_document->sprite()->palette(m_frame),
+        set_current_palette(m_doc->sprite()->palette(m_frame), false);
-                            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<Doc*>(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);
+    ++m_nitems;
-    actions()->layout();
+
+    m_actions.invalidate();
     view()->updateView();
-    actions()->selectChild(item);
+
+    selectState(history->currentState());
   }
 
   void onDeleteUndoState(DocUndo* history,
                          undo::UndoState* state) override {
-    for (auto child : actions()->children()) {
+    --m_nitems;
-      Item* item = static_cast<Item*>(child);
-      if (item->state() == state) {
-        actions()->removeChild(item);
-        item->deferDelete();
-        break;
-      }
-    }
-
-    actions()->layout();
-    view()->updateView();
   }
 
   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) {
+  void attachDocument(Doc* doc) {
-    detachDocument();
+    if (m_doc == doc)
-
-    m_document = document;
-    if (!document)
       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_doc->undoHistory()->remove_observer(this);
-    m_document->undoHistory()->remove_observer(this);
+    m_doc = nullptr;
-    m_document = nullptr;
+
+    setUndoHistory(nullptr);
     updateTitle();
   }
 
-  void clearList() {
+  void setUndoHistory(DocUndo* history) {
-    ui::Widget* child;
+    m_nitems = 0;
-    while ((child = actions()->lastChild()))
+    m_actions.setUndoHistory(history);
-      delete child;
-
-    actions()->layout();
     view()->updateView();
-  }
 
-  void refillList(DocUndo* history) {
+    if (history)
-    clearList();
+      m_actions.selectState(history->currentState());
-
-    // 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);
   }
 
   void selectState(const undo::UndoState* state) {
-    for (auto child : actions()->children()) {
+    m_actions.selectState(state);
-      Item* item = static_cast<Item*>(child);
-      if (item->state() == state) {
-        actions()->selectChild(item);
-        break;
-      }
-    }
   }
 
   void updateTitle() {
-    if (!m_document)
+    if (!m_doc)
       setText(m_title);
-    else
+    else {
-      setTextf("%s (%s)",
+      setText(
-               m_title.c_str(),
+        fmt::format(
-               base::get_pretty_memory_size(m_document->undoHistory()->totalUndoSize()).c_str());
+          "{} ({})",
+          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 {

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);
 }

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);