Tool bar configuration stores a tree structure, not a simple sequence

src/toolbars/ToolDock.cpp

@@ -33,6 +33,7 @@
 #include <wx/intl.h>
 #include <wx/panel.h>
 #include <wx/settings.h>
+#include <wx/tokenzr.h>
 #include <wx/window.h>
 #endif  /*  */
 
@@ -54,9 +55,36 @@ const ToolBarConfiguration::Position
 auto ToolBarConfiguration::FindPlace(const ToolBar *bar) const
    -> Iterator
 {
-   return std::find_if(begin(), end(),
+   auto This = const_cast<ToolBarConfiguration*>(this);
-      [=](const Place &place){ return place.pBar == bar; }
+   return std::find_if(This->begin(), This->end(),
+      [=](const Place &place){
+         return place.pTree->pBar == bar;
+      });
+}
+
+auto ToolBarConfiguration::FindParent(const ToolBar *bar)
+   -> std::pair<Forest*, Forest::iterator>
+{
+   auto findTree = [=](Forest &forest){
+      return std::find_if(forest.begin(), forest.end(),
+         [=](const Tree &tree){ return tree.pBar == bar; });
+   };
+
+   auto iter1 = findTree(mForest);
+   if (iter1 != mForest.end())
+      return { &mForest, iter1 };
+
+   Forest::iterator result;
+   auto iter = std::find_if(begin(), end(),
+      [&](const Place &place){
+         auto &children = place.pTree->children;
+         return (result = findTree(children)) != children.end();
+      }
    );
+   if (iter != end())
+      return { &iter->pTree->children, result };
+
+   return { nullptr, Forest::iterator{} };
 }
 
 auto ToolBarConfiguration::Find(const ToolBar *bar) const -> Position
@@ -70,20 +98,105 @@ auto ToolBarConfiguration::Find(const ToolBar *bar) const -> Position
 
 void ToolBarConfiguration::Insert(ToolBar *bar, Position position)
 {
-   if (position == UnspecifiedPosition)
+   if (position == UnspecifiedPosition) {
-      push_back(bar);
+      // Add at the "end" of the layout
+      Forest *pForest = &mForest;
+      while (!pForest->empty())
+         pForest = &pForest->back().children;
+      pForest->push_back( Tree {} );
+      pForest->back().pBar = bar;
+   }
    else {
-      auto index = wxArrayPtrVoid::Index(position.rightOf);
+      auto pForest = &mForest;
-      if (index == wxNOT_FOUND)
+      if (position.rightOf) {
-         push_back(bar);
+         const auto parent = FindPlace(position.rightOf);
+         if (parent != end())
+            pForest = &parent->pTree->children;
+      }
+
+      const auto begin = pForest->begin();
+      auto iter = begin;
+      const auto end = pForest->end();
+      bool adopt = false;
+
+      if (position.below) {
+         iter = std::find_if(begin, end,
+            [=](const Tree &tree){ return tree.pBar == position.below; }
+         );
+         if (iter != end) {
+            ++iter;
+            if (iter != end)
+               adopt = true;
+         }
+         else
+            // Not found, default to topmost
+            iter = begin;
+      }
       else
-         wxArrayPtrVoid::Insert(bar, 1 + index);
+         adopt = (iter != end);
+
+      // Adopt the child only if the insertion point specifies that
+      if (adopt && position.adopt) {
+         // Make new node with one child
+         Tree tree;
+         tree.pBar = bar;
+         tree.children.push_back(Tree{});
+
+         // Do adoption
+         auto &child = tree.children.back();
+         child.pBar = iter->pBar;
+         child.children.swap(iter->children);
+
+         // Put the node in the tree
+         (*iter).swap(tree);
+      }
+      else
+         pForest->insert(iter, Tree {})->pBar = bar;
+   }
+}
+
+void ToolBarConfiguration::InsertAtPath
+   (ToolBar *bar, const std::vector<int> &path)
+{
+   auto pForest = &mForest;
+   Tree *pTree {};
+
+   // Guarantee the existence of nodes
+   for (auto ii : path) {
+      Forest::size_type uu = std::max(0, ii);
+      pForest->resize(std::max(uu + 1, pForest->size()));
+      pTree = &(*pForest)[uu];
+      pForest = &pTree->children;
+   }
+
+   if (pTree)
+      pTree->pBar = bar;
+}
+
+void ToolBarConfiguration::Remove(Forest &forest, Forest::iterator iter)
+{
+   Tree tree;
+   tree.swap(*iter);
+   iter = forest.erase(iter);
+   auto &children = tree.children;
+   auto cIter = children.rbegin(), cEnd = children.rend();
+   while (cIter != cEnd) {
+      iter = forest.insert(iter, Tree{});
+      (*iter).swap(*cIter);
+      ++cIter;
    }
 }
 
 void ToolBarConfiguration::Remove(const ToolBar *bar)
 {
-   wxArrayPtrVoid::Remove(const_cast<ToolBar*>(bar));
+   auto results = FindParent(bar);
+   auto pForest = results.first;
+   if (pForest) {
+      // Reparent all of the children of the deleted node
+      auto iter = results.second;
+      wxASSERT(iter->pBar == bar);
+      Remove(*pForest, iter);
+   }
 }
 
 void ToolBarConfiguration::Show(ToolBar *bar)
@@ -109,8 +222,8 @@ bool ToolBarConfiguration::IsRightmost(const ToolBar *bar) const
    if (++iter == endit)
       // Last of all
       return true;
-   if (bar->GetRect().y != iter->pBar->GetRect().y)
+   if (bar->GetRect().y != iter->pTree->pBar->GetRect().y)
-      // Last in its row
+      // 	
       return true;
    return false;
 }
@@ -118,7 +231,7 @@ bool ToolBarConfiguration::IsRightmost(const ToolBar *bar) const
 bool ToolBarConfiguration::Read
    (ToolBarConfiguration *pConfiguration,
     ToolManager *pManager,
-    Legacy *,
+    Legacy *pLegacy,
     ToolBar *bar, bool &visible, bool defaultVisible)
 {
    bool result = true;
@@ -132,11 +245,27 @@ bool ToolBarConfiguration::Read
          result = false;
       else if (ord >= 0)
       {
-         while(pConfiguration->size () <= ord)
+         // Legacy preferences
-            pConfiguration->push_back(nullptr);
+         while (pLegacy->bars.size() <= ord)
-         (*pConfiguration)[ord] = bar;
+            pLegacy->bars.push_back(nullptr);
+         pLegacy->bars[ord] = bar;
+      }
+      else {
+         wxString strPath;
+         gPrefs->Read( wxT("Path"), &strPath );
+         if (!strPath.empty()) {
+            wxStringTokenizer toker { strPath, wxT(",") };
+            std::vector<int> path;
+            while(toker.HasMoreTokens()) {
+               auto token = toker.GetNextToken();
+               auto ii = wxAtoi(token);
+               path.push_back(ii);
+            }
+            pConfiguration->InsertAtPath(bar, path);
+         }
       }
    }
+
    // Future: might remember visibility in the configuration, not forgetting
    // positions of hidden bars.
    gPrefs->Read( wxT("Show"), &visible, defaultVisible);
@@ -144,21 +273,53 @@ bool ToolBarConfiguration::Read
    return result;
 }
 
-void ToolBarConfiguration::PostRead(Legacy &)
+void ToolBarConfiguration::RemoveNulls(Forest &forest)
 {
-   auto b = wxArrayPtrVoid::begin();
+   for (int ii = 0; ii < forest.size(); ++ii) {
-   auto iter =
+      if(forest[ii].pBar == nullptr)
-      std::remove(b, wxArrayPtrVoid::end(), nullptr);
+         Remove(forest, forest.begin() + ii--);
-   resize(iter - b);
+   }
+
+   // Now do the same recursively
+   for (auto &tree : forest)
+      RemoveNulls(tree.children);
+}
+
+void ToolBarConfiguration::PostRead(Legacy &legacy)
+{
+   // Be sure no nodes contain NULL,
+   // against the case of obsolete preferences, perhaps
+   RemoveNulls(mForest);
+
+   ToolBar *prev {};
+   for (auto pBar : legacy.bars) {
+      if (!pBar)
+         continue;
+
+      Position position{ prev };
+      Insert(pBar, position);
+
+      prev = pBar;
+   }
 }
 
 void ToolBarConfiguration::Write
    (const ToolBarConfiguration *pConfiguration, const ToolBar *bar)
 {
    if (pConfiguration) {
-      auto index = pConfiguration->Index(const_cast<ToolBar*>(bar));
+      wxString strPath;
-      if (index != wxNOT_FOUND)
+      const auto cIter = pConfiguration->FindPlace(bar);
-         gPrefs->Write( wxT("Order"), 1 + index );
+      const auto path = cIter.GetPath();
+      if (!path.empty()) {
+         auto iter = path.begin(), end = path.end();
+         strPath += wxString::Format(wxT("%d"), *iter++);
+         while (iter != end)
+            strPath += wxString::Format(wxT(",%d"), *iter++);
+      }
+      gPrefs->Write(wxT("Path"), strPath);
+
+      // Remove any legacy configuration info.
+      gPrefs->DeleteEntry(wxT("Order"));
    }
    gPrefs->Write( wxT("Show"), bar->IsVisible() );
 }
@@ -251,7 +412,7 @@ void ToolDock::LoadConfig(ToolBar *bars[])
 {
    // Add all ordered toolbars
    for(const auto &place : GetConfiguration()) {
-      auto bar = place.pBar;
+      auto bar = place.pTree->pBar;
       this->Dock(bar, false);
       // Show it -- hidden bars are not (yet) ever saved as part of a
       // configuration
@@ -286,7 +447,7 @@ void ToolDock::LayoutToolBars()
    for (const auto &place : GetConfiguration())
    {
       // Cache toolbar pointer
-      ToolBar *ct = place.pBar;
+      ToolBar *ct = place.pTree->pBar;
 
       // Get and cache the toolbar sizes
       wxSize sz = ct->GetSize();
@@ -402,7 +563,7 @@ ToolBarConfiguration::Position
       else
       {
          // Cache toolbar pointer
-         ToolBar *ct = iter->pBar;
+         ToolBar *ct = iter->pTree->pBar;
 
          // Remember current bars ' dimensions
          sz = ct->GetSize();
@@ -583,7 +744,7 @@ void ToolDock::OnPaint( wxPaintEvent & WXUNUSED(event) )
    // Draw the gap between each bar
    for (const auto &place : GetConfiguration())
    {
-      auto toolbar = place.pBar;
+      auto toolbar = place.pTree->pBar;
       if (!toolbar)
          continue;
 

src/toolbars/ToolDock.h

@@ -13,6 +13,8 @@
 #ifndef __AUDACITY_TOOLDOCK__
 #define __AUDACITY_TOOLDOCK__
 
+#include <vector>
+#include "../MemoryX.h" // for std::move
 #include <wx/defs.h>
 #include <wx/panel.h>
 
@@ -45,24 +47,32 @@ enum
    DockCount = 2
 };
 
-class ToolBarConfiguration : public wxArrayPtrVoid
+class ToolBarConfiguration
 {
+   struct Tree;
+   using Forest = std::vector<Tree>;
+
 public:
+   void Swap(ToolBarConfiguration &that)
+   {
+      mForest.swap(that.mForest);
+   }
+
    struct Position {
       ToolBar *rightOf {};
-      // ToolBar *below {};
+      ToolBar *below {};
-      // bool adopt {true};
+      bool adopt {true};
       bool valid {true};
 
       // Default constructor
       Position() {}
 
       Position(
-         ToolBar *r /*,
+         ToolBar *r,
          ToolBar *b = nullptr,
-         bool shouldAdopt = true */
+         bool shouldAdopt = true
       )
-         : rightOf{ r } // , below{ b }, adopt{ shouldAdopt }
+         : rightOf{ r }, below{ b }, adopt{ shouldAdopt }
       {}
 
       // Constructor for the invalid value
@@ -73,8 +83,8 @@ public:
       { return lhs.valid == rhs.valid &&
          (!lhs.valid ||
           (lhs.rightOf == rhs.rightOf
-           // && lhs.below == rhs.below
+           && lhs.below == rhs.below
-           // && lhs.adopt == rhs.adopt
+           && lhs.adopt == rhs.adopt
          ));
       }
 
@@ -86,10 +96,12 @@ public:
    static const Position UnspecifiedPosition;
 
    struct Place {
-      ToolBar *pBar {};
+      Tree *pTree {};
       Position position;
    };
 
+   // This iterator visits the nodes of the forest in pre-order, and at each
+   // stop, makes the parent, previous sibling, and children accessible.
    class Iterator
       : public std::iterator<std::forward_iterator_tag, Place>
    {
@@ -98,24 +110,67 @@ public:
       const Place *operator -> () const { return &**this; }
       Iterator &operator ++ ()
       {
-         wxASSERT(mIter != mEnd);
-
          // This is a feature:  advance position even at the end
-         mPlace.position.rightOf = mPlace.pBar;
+         mPlace.position =
-         // mPlace.position.below = nullptr;
+            { mPlace.pTree ? mPlace.pTree->pBar : nullptr };
+
+         if (!mIters.empty())
+         {
+            auto triple = &mIters.back();
+            auto &children = triple->current->children;
+            if (children.empty()) {
+               while (++triple->current == triple->end) {
+                  mIters.pop_back();
+                  if (mIters.empty())
+                     break;
+                  triple = &mIters.back();
+               }
+            }
+            else {
+               auto b = children.begin();
+               mIters.push_back( Triple { b, b, children.end() } );
+            }
+         }
+
+         if (mIters.empty()) {
+            mPlace.pTree = nullptr;
+            // Leave mPlace.position as above
+         }
+         else {
+            const auto &triple = mIters.back();
+            mPlace.pTree = &*triple.current;
+
+            if (mIters.size() == 1)
+               mPlace.position.rightOf = nullptr;
+            else
+               mPlace.position.rightOf = (mIters.rbegin() + 1)->current->pBar;
+
+            if (triple.begin == triple.current)
+               mPlace.position.below = nullptr;
+            else
+               mPlace.position.below = (triple.current - 1)->pBar;
+         }
 
-         ++mIter;
-         if (mIter != mEnd)
-            mPlace.pBar = static_cast<ToolBar*>(*mIter);
-         else
-            mPlace.pBar = nullptr;
          return *this;
       }
 
+      // This may be called on the end iterator, and then returns empty
+      std::vector<int> GetPath() const
+      {
+         std::vector<int> path;
+         path.reserve(mIters.size());
+         for (const auto &triple : mIters)
+            path.push_back(triple.current - triple.begin);
+         return std::move(path);
+      }
+
       friend inline bool operator ==
       (const Iterator &lhs, const Iterator &rhs)
       {
-         return lhs.mIter == rhs.mIter;
+         const auto &li = lhs.mIters;
+         const auto &ri = rhs.mIters;
+         return li.size() == ri.size() &&
+            std::equal(li.begin(), li.end(), ri.begin());
       }
 
       friend inline bool operator !=
@@ -126,23 +181,42 @@ public:
 
    private:
       friend ToolBarConfiguration;
-      using iterator = wxArrayPtrVoid::const_iterator;
+      Iterator () {}
-      explicit Iterator(iterator iter, iterator end)
+      explicit Iterator(ToolBarConfiguration &conf)
-         : mIter(iter)
-         , mEnd(end)
       {
-         if (mIter != mEnd)
+         auto &forest = conf.mForest;
-            mPlace.pBar = static_cast<ToolBar*>(*mIter);
+         if (!forest.empty()) {
+            auto b = forest.begin();
+            mIters.push_back( Triple { b, b, forest.end() } );
+            mPlace.pTree = &*b;
+         }
       }
 
-      iterator mIter, mEnd;
       Place mPlace;
+
+      using FIter = Forest::iterator;
+      struct Triple
+      {
+         Triple (FIter b, FIter c, FIter e)
+            : begin{b}, current{c}, end{e} {}
+         FIter begin, current, end;
+
+         friend inline bool operator ==
+         (const Triple &lhs, const Triple &rhs)
+         {
+            // Really need only to compare current
+            return
+               // lhs.begin == rhs.begin &&
+               lhs.current == rhs.current
+               // lhs.end == rhs.end
+            ;
+         }
+      };
+      std::vector<Triple> mIters;
    };
 
-   Iterator begin() const
+   Iterator begin() { return Iterator { *this }; }
-      { return Iterator { wxArrayPtrVoid::begin(), wxArrayPtrVoid::end() }; }
+   Iterator end() const { return Iterator {}; }
-   Iterator end() const
-      { return Iterator { wxArrayPtrVoid::end(), wxArrayPtrVoid::end() }; }
 
    Position Find(const ToolBar *bar) const;
 
@@ -154,6 +228,7 @@ public:
    // Default position inserts at the end
    void Insert(ToolBar *bar,
                Position position = UnspecifiedPosition);
+   void InsertAtPath(ToolBar *bar, const std::vector<int> &path);
    void Remove(const ToolBar *bar);
 
    // Future: might allow a state that the configuration remembers
@@ -166,6 +241,7 @@ public:
    bool IsRightmost(const ToolBar *bar) const;
 
    struct Legacy {
+      std::vector<ToolBar*> bars;
    };
 
    static bool Read
@@ -179,7 +255,26 @@ public:
       (const ToolBarConfiguration *pConfiguration, const ToolBar *bar);
 
 private:
+
+   void Remove(Forest &forest, Forest::iterator iter);
+   void RemoveNulls(Forest &forest);
+
+   struct Tree
+   {
+      ToolBar *pBar {};
+      Forest children;
+
+      void swap(Tree &that)
+      {
+         std::swap(pBar, that.pBar);
+         children.swap(that.children);
+      }
+   };
+
    Iterator FindPlace(const ToolBar *bar) const;
+   std::pair<Forest*, Forest::iterator> FindParent(const ToolBar *bar);
+
+   Forest mForest;
 };
 
 class ToolDock final : public wxPanel

src/toolbars/ToolManager.cpp

@@ -862,13 +862,17 @@ void ToolManager::WriteConfig()
       gPrefs->Write( wxT("W"), sz.x );
       gPrefs->Write( wxT("H"), sz.y );
 
-      // Kill the bar
-      bar->Destroy();
-
       // Change back to the bar root
       gPrefs->SetPath( wxT("..") );
    }
 
+   // Kill the bars
+   for( ndx = 0; ndx < ToolBarCount; ndx++ )
+   {
+      ToolBar *bar = mBars[ ndx ];
+      bar->Destroy();
+   }
+
    // Restore original config path
    gPrefs->SetPath( oldpath );
    gPrefs->Flush();