refactor(splits): Store container nodes as shared pointers (#6435)

Reviewed-by: pajlada <rasmus.karlsson@pajlada.com>

CHANGELOG.md

@@ -35,6 +35,7 @@
 - Dev: Fixed compilation error in tests with Clang 21. (#6519)
 - Dev: The LuaLS meta files moved from `docs/plugin-meta.lua` to `docs/lua-meta/globals.lua`. (#6530)
 - Dev: Compile time definitions for `Windows.h` are now conditional based on `WIN32` instead of `MSVC`. (#6534)
+- Dev: Refactored split container nodes to use shared pointers. (#6435)
 
 ## 2.5.4
 

src/singletons/WindowManager.cpp

@@ -690,7 +690,7 @@ void WindowManager::encodeNodeRecursively(SplitNode *node, QJsonObject &obj)
                            : "vertical");
 
             QJsonArray itemsArr;
-            for (const std::unique_ptr<SplitNode> &n : node->getChildren())
+            for (const auto &n : node->getChildren())
             {
                 QJsonObject subObj;
                 WindowManager::encodeNodeRecursively(n.get(), subObj);

src/widgets/splits/SplitContainer.cpp

@@ -32,6 +32,7 @@ SplitContainer::SplitContainer(Notebook *parent)
     : BaseWidget(parent)
     , overlay_(this)
     , mouseOverPoint_(-10000, -10000)
+    , baseNode_(std::make_shared<Node>())
     , tab_(nullptr)
 {
     this->refreshTabTitle();
@@ -158,7 +159,7 @@ void SplitContainer::insertSplit(Split *split, InsertOptions &&options)
         assert(!options.relativeNode);
 
         Node *node =
-            this->baseNode_.findNodeContainingSplit(options.relativeSplit);
+            this->baseNode_->findNodeContainingSplit(options.relativeSplit);
         assert(node != nullptr);
 
         options.relativeNode = node;
@@ -169,22 +170,22 @@ void SplitContainer::insertSplit(Split *split, InsertOptions &&options)
 
     if (relativeTo == nullptr)
     {
-        if (this->baseNode_.type_ == Node::Type::EmptyRoot)
+        if (this->baseNode_->type_ == Node::Type::EmptyRoot)
         {
-            this->baseNode_.setSplit(split);
+            this->baseNode_->setSplit(split);
         }
-        else if (this->baseNode_.type_ == Node::Type::Split)
+        else if (this->baseNode_->type_ == Node::Type::Split)
         {
-            this->baseNode_.nestSplitIntoCollection(split, direction);
+            this->baseNode_->nestSplitIntoCollection(split, direction);
         }
         else
         {
-            this->baseNode_.insertSplitRelative(split, direction);
+            this->baseNode_->insertSplitRelative(split, direction);
         }
     }
     else
     {
-        assert(this->baseNode_.isOrContainsNode(relativeTo));
+        assert(this->baseNode_->isOrContainsNode(relativeTo));
 
         relativeTo->insertSplitRelative(split, direction);
     }
@@ -310,7 +311,7 @@ void SplitContainer::setSelected(Split *split)
 
     this->selected_ = split;
 
-    if (Node *node = this->baseNode_.findNodeContainingSplit(split))
+    if (Node *node = this->baseNode_->findNodeContainingSplit(split))
     {
         this->focusSplitRecursive(node);
         this->setPreferedTargetRecursive(node);
@@ -331,7 +332,7 @@ SplitContainer::Position SplitContainer::releaseSplit(Split *split)
 {
     assertInGuiThread();
 
-    Node *node = this->baseNode_.findNodeContainingSplit(split);
+    Node *node = this->baseNode_->findNodeContainingSplit(split);
     assert(node != nullptr);
 
     this->splits_.erase(
@@ -372,7 +373,7 @@ void SplitContainer::selectNextSplit(SplitDirection direction)
 {
     assertInGuiThread();
 
-    if (Node *node = this->baseNode_.findNodeContainingSplit(this->selected_))
+    if (Node *node = this->baseNode_->findNodeContainingSplit(this->selected_))
     {
         this->selectSplitRecursive(node, direction);
     }
@@ -489,7 +490,7 @@ void SplitContainer::layout()
     }
 
     // update top right split
-    auto *topRight = this->getTopRightSplit(this->baseNode_);
+    auto *topRight = this->getTopRightSplit(*this->baseNode_);
     if (this->topRight_)
     {
         this->topRight_->setIsTopRightSplit(false);
@@ -501,14 +502,14 @@ void SplitContainer::layout()
     }
 
     // layout
-    this->baseNode_.geometry_ = this->rect().adjusted(-1, -1, 0, 0);
+    this->baseNode_->geometry_ = this->rect().adjusted(-1, -1, 0, 0);
 
     std::vector<DropRect> dropRects;
     std::vector<ResizeRect> resizeRects;
 
     const bool addSpacing =
         Split::modifierStatus == SHOW_ADD_SPLIT_REGIONS || this->isDragging_;
-    this->baseNode_.layout(addSpacing, this->scale(), dropRects, resizeRects);
+    this->baseNode_->layout(addSpacing, this->scale(), dropRects, resizeRects);
 
     this->dropRects_ = dropRects;
 
@@ -516,7 +517,7 @@ void SplitContainer::layout()
     {
         const QRect &g = split->geometry();
 
-        Node *node = this->baseNode_.findNodeContainingSplit(split);
+        Node *node = this->baseNode_->findNodeContainingSplit(split);
 
         // left
         dropRects.emplace_back(
@@ -632,7 +633,7 @@ void SplitContainer::paintSplitBorder(Node *node, QPainter *painter)
         break;
         case Node::Type::VerticalContainer:
         case Node::Type::HorizontalContainer: {
-            for (std::unique_ptr<Node> &child : node->children_)
+            for (const auto &child : node->children_)
             {
                 paintSplitBorder(child.get(), painter);
             }
@@ -766,7 +767,7 @@ void SplitContainer::leaveEvent(QEvent * /*event*/)
 
 void SplitContainer::focusInEvent(QFocusEvent * /*event*/)
 {
-    if (this->baseNode_.findNodeContainingSplit(this->selected_) != nullptr)
+    if (this->baseNode_->findNodeContainingSplit(this->selected_) != nullptr)
     {
         this->selected_->setFocus();
         return;
@@ -791,20 +792,20 @@ std::vector<Split *> SplitContainer::getSplits() const
 
 SplitContainer::Node *SplitContainer::getBaseNode()
 {
-    return &this->baseNode_;
+    return this->baseNode_.get();
 }
 
 NodeDescriptor SplitContainer::buildDescriptor() const
 {
-    return this->buildDescriptorRecursively(&this->baseNode_);
+    return this->buildDescriptorRecursively(this->baseNode_.get());
 }
 
 void SplitContainer::applyFromDescriptor(const NodeDescriptor &rootNode)
 {
-    assert(this->baseNode_.type_ == Node::Type::EmptyRoot);
+    assert(this->baseNode_->type_ == Node::Type::EmptyRoot);
 
     this->disableLayouting_ = true;
-    this->applyFromDescriptorRecursively(rootNode, &this->baseNode_);
+    this->applyFromDescriptorRecursively(rootNode, this->baseNode_.get());
     this->disableLayouting_ = false;
     this->layout();
 }
@@ -943,20 +944,20 @@ void SplitContainer::applyFromDescriptorRecursively(
                 split->setChannel(WindowManager::decodeChannel(splitNode));
                 split->setModerationMode(splitNode.moderationMode_);
 
-                auto *node = new Node();
+                auto node = std::make_shared<Node>();
                 node->parent_ = baseNode;
                 node->split_ = split;
                 node->type_ = Node::Type::Split;
 
                 node->flexH_ = splitNode.flexH_;
                 node->flexV_ = splitNode.flexV_;
-                baseNode->children_.emplace_back(node);
+                baseNode->children_.emplace_back(std::move(node));
 
                 this->addSplit(split);
             }
             else
             {
-                auto *node = new Node();
+                auto node = std::make_shared<Node>();
                 node->parent_ = baseNode;
 
                 if (const auto *inner =
@@ -967,7 +968,7 @@ void SplitContainer::applyFromDescriptorRecursively(
                 }
 
                 baseNode->children_.emplace_back(node);
-                this->applyFromDescriptorRecursively(item, node);
+                this->applyFromDescriptorRecursively(item, node.get());
             }
         }
     }
@@ -1070,7 +1071,7 @@ qreal SplitContainer::Node::getVerticalFlex() const
     return this->flexV_;
 }
 
-const std::vector<std::unique_ptr<SplitContainer::Node>> &
+const std::vector<std::shared_ptr<SplitContainer::Node>> &
     SplitContainer::Node::getChildren()
 {
     return this->children_;
@@ -1098,7 +1099,7 @@ bool SplitContainer::Node::isOrContainsNode(SplitContainer::Node *_node)
     }
 
     return std::any_of(this->children_.begin(), this->children_.end(),
-                       [_node](std::unique_ptr<Node> &n) {
+                       [_node](const auto &n) {
                            return n->isOrContainsNode(_node);
                        });
 }
@@ -1111,7 +1112,7 @@ SplitContainer::Node *SplitContainer::Node::findNodeContainingSplit(
         return this;
     }
 
-    for (std::unique_ptr<Node> &node : this->children_)
+    for (const auto &node : this->children_)
     {
         Node *a = node->findNodeContainingSplit(_split);
 
@@ -1167,24 +1168,24 @@ void SplitContainer::Node::nestSplitIntoCollection(Split *_split,
 {
     if (toContainerType(_direction) == this->type_)
     {
-        this->children_.emplace_back(new Node(_split, this));
+        this->children_.emplace_back(std::make_shared<Node>(_split, this));
     }
     else
     {
         // we'll need to nest outselves
         // move all our data into a new node
-        Node *clone = new Node();
+        auto clone = std::make_shared<Node>();
         clone->type_ = this->type_;
         clone->children_ = std::move(this->children_);
-        for (std::unique_ptr<Node> &node : clone->children_)
+        for (const auto &node : clone->children_)
         {
-            node->parent_ = clone;
+            node->parent_ = clone.get();
         }
         clone->split_ = this->split_;
         clone->parent_ = this;
 
         // add the node to our children and change our type
-        this->children_.push_back(std::unique_ptr<Node>(clone));
+        this->children_.emplace_back(clone);
         this->type_ = toContainerType(_direction);
         this->split_ = nullptr;
 
@@ -1219,9 +1220,9 @@ void SplitContainer::Node::insertNextToThis(Split *_split,
         it++;
     }
 
-    Node *node = new Node(_split, this->parent_);
+    auto node = std::make_shared<Node>(_split, this->parent_);
     node->geometry_ = QRectF(0, 0, width, height);
-    siblings.insert(it, std::unique_ptr<Node>(node));
+    siblings.insert(it, node);
 }
 
 void SplitContainer::Node::setSplit(Split *_split)
@@ -1275,10 +1276,10 @@ SplitContainer::Position SplitContainer::Node::releaseSplit()
 
         auto *parent = this->parent_;
         siblings.erase(it);
-        std::unique_ptr<Node> &sibling = siblings.front();
+        const auto &sibling = siblings.front();
         parent->type_ = sibling->type_;
         parent->split_ = sibling->split_;
-        std::vector<std::unique_ptr<Node>> nodes =
+        std::vector<std::shared_ptr<Node>> nodes =
             std::move(sibling->children_);
         for (auto &node : nodes)
         {
@@ -1324,8 +1325,7 @@ qreal SplitContainer::Node::getSize(bool isVertical)
 qreal SplitContainer::Node::getChildrensTotalFlex(bool isVertical)
 {
     return std::accumulate(this->children_.begin(), this->children_.end(),
-                           qreal(0),
+                           qreal(0), [=](qreal val, const auto &node) {
-                           [=](qreal val, std::unique_ptr<Node> &node) {
                                return val + node->getFlex(isVertical);
                            });
 }
@@ -1334,7 +1334,7 @@ void SplitContainer::Node::layout(bool addSpacing, float _scale,
                                   std::vector<DropRect> &dropRects,
                                   std::vector<ResizeRect> &resizeRects)
 {
-    for (std::unique_ptr<Node> &node : this->children_)
+    for (const auto &node : this->children_)
     {
         node->clamp();
     }
@@ -1358,7 +1358,7 @@ void SplitContainer::Node::layout(bool addSpacing, float _scale,
                 0.0001, this->getChildrensTotalFlex(isVertical));
             qreal totalSize = std::accumulate(
                 this->children_.begin(), this->children_.end(), qreal(0),
-                [=, this](int val, std::unique_ptr<Node> &node) {
+                [=, this](int val, const auto &node) {
                     return val + std::max<qreal>(
                                      this->getSize(isVertical) /
                                          std::max<qreal>(0.0001, totalFlex) *
@@ -1421,7 +1421,7 @@ void SplitContainer::Node::layout(bool addSpacing, float _scale,
 
             // iterate children
             auto pos = int(isVertical ? childRect.top() : childRect.left());
-            for (std::unique_ptr<Node> &child : this->children_)
+            for (const auto &child : this->children_)
             {
                 // set rect
                 QRect rect = childRect.toRect();
@@ -1668,10 +1668,9 @@ void SplitContainer::ResizeHandle::mouseMoveEvent(QMouseEvent *event)
     assert(node->parent_ != nullptr);
 
     const auto &siblings = node->parent_->getChildren();
-    auto it = std::find_if(siblings.begin(), siblings.end(),
+    auto it = std::ranges::find_if(siblings, [this](const auto &n) {
-                           [this](const std::unique_ptr<Node> &n) {
+        return n.get() == this->node;
-                               return n.get() == this->node;
+    });
-                           });
 
     assert(it != siblings.end());
     Node *before = siblings[it - siblings.begin() - 1].get();

src/widgets/splits/SplitContainer.hpp

@@ -78,7 +78,10 @@ private:
     };
 
 public:
-    struct Node final {
+    struct Node final : public std::enable_shared_from_this<Node> {
+        Node();
+        Node(Split *_split, Node *_parent);
+
         enum class Type {
             EmptyRoot,
             Split,
@@ -91,12 +94,9 @@ public:
         Node *getParent() const;
         qreal getHorizontalFlex() const;
         qreal getVerticalFlex() const;
-        const std::vector<std::unique_ptr<Node>> &getChildren();
+        const std::vector<std::shared_ptr<Node>> &getChildren();
 
     private:
-        Node();
-        Node(Split *_split, Node *_parent);
-
         bool isOrContainsNode(Node *_node);
         Node *findNodeContainingSplit(Split *_split);
         void insertSplitRelative(Split *_split, SplitDirection _direction);
@@ -123,7 +123,7 @@ public:
         QRectF geometry_;
         qreal flexH_ = 1;
         qreal flexV_ = 1;
-        std::vector<std::unique_ptr<Node>> children_;
+        std::vector<std::shared_ptr<Node>> children_;
 
         friend class SplitContainer;
     };
@@ -260,7 +260,7 @@ private:
     std::vector<std::unique_ptr<ResizeHandle>> resizeHandles_;
     QPoint mouseOverPoint_;
 
-    Node baseNode_;
+    std::shared_ptr<Node> baseNode_;
     Split *selected_{};
     Split *topRight_{};
     bool disableLayouting_{};