Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cedcfb7
refactor(splits): Store containers as shared pointers
Nerixyz Aug 31, 2025
e123e52
changelog
Nerixyz Aug 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Unversioned

- Dev: Refactored split container nodes to use shared pointers. (#6435)

## 2.5.4-beta.1

- Minor: Added `Open in custom player` to `twitch.tv/<channel>` link context menus. (#6403)
Expand Down
2 changes: 1 addition & 1 deletion src/singletons/WindowManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -687,7 +687,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);
Expand Down
89 changes: 44 additions & 45 deletions src/widgets/splits/SplitContainer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ SplitContainer::SplitContainer(Notebook *parent)
: BaseWidget(parent)
, overlay_(this)
, mouseOverPoint_(-10000, -10000)
, baseNode_(std::make_shared<Node>())
, tab_(nullptr)
{
this->refreshTabTitle();
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
Expand All @@ -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(
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand All @@ -501,22 +502,22 @@ 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;

for (Split *split : this->splits_)
{
const QRect &g = split->geometry();

Node *node = this->baseNode_.findNodeContainingSplit(split);
Node *node = this->baseNode_->findNodeContainingSplit(split);

// left
dropRects.emplace_back(
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -764,7 +765,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;
Expand All @@ -789,20 +790,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();
}
Expand Down Expand Up @@ -941,20 +942,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 =
Expand All @@ -965,7 +966,7 @@ void SplitContainer::applyFromDescriptorRecursively(
}

baseNode->children_.emplace_back(node);
this->applyFromDescriptorRecursively(item, node);
this->applyFromDescriptorRecursively(item, node.get());
}
}
}
Expand Down Expand Up @@ -1068,7 +1069,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_;
Expand Down Expand Up @@ -1096,7 +1097,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);
});
}
Expand All @@ -1109,7 +1110,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);

Expand Down Expand Up @@ -1165,24 +1166,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;

Expand Down Expand Up @@ -1217,9 +1218,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)
Expand Down Expand Up @@ -1273,10 +1274,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)
{
Expand Down Expand Up @@ -1322,8 +1323,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 val, std::unique_ptr<Node> &node) {
qreal(0), [=](qreal val, const auto &node) {
return val + node->getFlex(isVertical);
});
}
Expand All @@ -1332,7 +1332,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();
}
Expand All @@ -1356,7 +1356,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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: lambdas that capture 'this' should not specify a by-value capture default [cppcoreguidelines-misleading-capture-default-by-value]

Suggested change
[=, this](int val, const auto &node) {
[isVertical, totalFlex, minSize, this](int val, const auto &node) {

return val + std::max<qreal>(
this->getSize(isVertical) /
std::max<qreal>(0.0001, totalFlex) *
Expand Down Expand Up @@ -1419,7 +1419,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();
Expand Down Expand Up @@ -1664,10 +1664,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(),
[this](const std::unique_ptr<Node> &n) {
return n.get() == this->node;
});
auto it = std::ranges::find_if(siblings, [this](const auto &n) {
return n.get() == this->node;
});

assert(it != siblings.end());
Node *before = siblings[it - siblings.begin() - 1].get();
Expand Down
14 changes: 7 additions & 7 deletions src/widgets/splits/SplitContainer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,10 @@ class SplitContainer final : public BaseWidget
};

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,
Expand All @@ -91,12 +94,9 @@ class SplitContainer final : public BaseWidget
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);
Expand All @@ -123,7 +123,7 @@ class SplitContainer final : public BaseWidget
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;
};
Expand Down Expand Up @@ -260,7 +260,7 @@ class SplitContainer final : public BaseWidget
std::vector<std::unique_ptr<ResizeHandle>> resizeHandles_;
QPoint mouseOverPoint_;

Node baseNode_;
std::shared_ptr<Node> baseNode_;
Split *selected_{};
Split *topRight_{};
bool disableLayouting_{};
Expand Down
Loading