From fdb0a1582c37cc2f28851bdc7438a045baf605f1 Mon Sep 17 00:00:00 2001 From: pajlada Date: Sun, 25 Dec 2022 12:09:25 +0100 Subject: [PATCH] SplitContainer refactor (#4261) * Remove unused include util/Helpers.hpp * SplitContainer::setTag fix parameter naming * autofy/constify where possible * More const auto ptr magicifying * Make SplitNode::Type an enum class * Move QuickSwitcherPopup includes from header to source file * Remove unused DropRegion code * use empty() instead of size() == 0 * Add curly braces everywhere * Remove useless reinterpret_cast It was casting Node* to Node* * Clarify that the connect is QObject::connect * SplitContainer::setSelected fix parameter naming * Rename function variables to remove unneccesary underscore Also move addSpacing parameter out of the layout function * emplace_back where possible * Name parameters * Remove ineffective const from return type * Make node getters const * Flatten Node::releaseSplit * Rename in-function variable to match code style * [ACTUAL CODE CHANGE/MOVE] Move clamp logic to its own function * name params * applyFromDescriptorRecursively: rename node param to baseNode * [ACTUAL CODE CHANGE/MOVE] Remove the many overloads for append/insertSplit This utilizes the C++20 designed initializers aggregate initialization feature * Remove unused includes * [ACTUAL CODE CHANGE] Clean up dragging logic There's no need to keep a pointer around to which split is being dragged, it's already stored in the QDropEvent source() * UNRELATED .clang-tidy: Only suggest UPPER_CASE for constant global variables * Remove unused SplitContainer::getSplitCount function * Use std::max in Node's clamp function * Remove test code * DraggedSplit.hpp: remove unused include * Split `setDraggingSplit` into two functions, `startDraggingSplit` and `stopDraggingSplit` --- .clang-tidy | 2 +- src/CMakeLists.txt | 2 + src/singletons/WindowManager.cpp | 13 +- src/widgets/Window.cpp | 2 +- src/widgets/dialogs/UserInfoPopup.cpp | 2 +- src/widgets/dialogs/switcher/NewTabItem.cpp | 2 +- .../dialogs/switcher/QuickSwitcherPopup.cpp | 3 + .../dialogs/switcher/QuickSwitcherPopup.hpp | 3 - src/widgets/helper/NotebookButton.cpp | 40 +- src/widgets/helper/NotebookTab.cpp | 8 +- src/widgets/settingspages/GeneralPageView.hpp | 1 + src/widgets/splits/DraggedSplit.cpp | 28 ++ src/widgets/splits/DraggedSplit.hpp | 17 + src/widgets/splits/Split.cpp | 45 +- src/widgets/splits/SplitContainer.cpp | 434 ++++++++++-------- src/widgets/splits/SplitContainer.hpp | 84 ++-- 16 files changed, 397 insertions(+), 289 deletions(-) create mode 100644 src/widgets/splits/DraggedSplit.cpp create mode 100644 src/widgets/splits/DraggedSplit.hpp diff --git a/.clang-tidy b/.clang-tidy index 0b130cbb5..ba619439a 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -48,7 +48,7 @@ CheckOptions: value: _ - key: readability-identifier-naming.UnionCase value: CamelCase - - key: readability-identifier-naming.GlobalVariableCase + - key: readability-identifier-naming.GlobalConstantCase value: UPPER_CASE - key: readability-identifier-naming.VariableCase value: camelBack diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 3f282b898..0367bb190 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -525,6 +525,8 @@ set(SOURCE_FILES widgets/splits/ClosedSplits.cpp widgets/splits/ClosedSplits.hpp + widgets/splits/DraggedSplit.cpp + widgets/splits/DraggedSplit.hpp widgets/splits/InputCompletionItem.cpp widgets/splits/InputCompletionItem.hpp widgets/splits/InputCompletionPopup.cpp diff --git a/src/singletons/WindowManager.cpp b/src/singletons/WindowManager.cpp index c358bdf6f..cc074dd0f 100644 --- a/src/singletons/WindowManager.cpp +++ b/src/singletons/WindowManager.cpp @@ -555,7 +555,7 @@ void WindowManager::encodeNodeRecursively(SplitNode *node, QJsonObject &obj) { switch (node->getType()) { - case SplitNode::_Split: { + case SplitNode::Type::Split: { obj.insert("type", "split"); obj.insert("moderationMode", node->getSplit()->getModerationMode()); @@ -569,11 +569,12 @@ void WindowManager::encodeNodeRecursively(SplitNode *node, QJsonObject &obj) obj.insert("filters", filters); } break; - case SplitNode::HorizontalContainer: - case SplitNode::VerticalContainer: { - obj.insert("type", node->getType() == SplitNode::HorizontalContainer - ? "horizontal" - : "vertical"); + case SplitNode::Type::HorizontalContainer: + case SplitNode::Type::VerticalContainer: { + obj.insert("type", + node->getType() == SplitNode::Type::HorizontalContainer + ? "horizontal" + : "vertical"); QJsonArray itemsArr; for (const std::unique_ptr &n : node->getChildren()) diff --git a/src/widgets/Window.cpp b/src/widgets/Window.cpp index e1a0c44c9..dfac8ba6e 100644 --- a/src/widgets/Window.cpp +++ b/src/widgets/Window.cpp @@ -424,7 +424,7 @@ void Window::addShortcuts() split->setChannel( getApp()->twitch->getOrAddChannel(si.channelName)); split->setFilters(si.filters); - splitContainer->appendSplit(split); + splitContainer->insertSplit(split); splitContainer->setSelected(split); this->notebook_->select(splitContainer); return ""; diff --git a/src/widgets/dialogs/UserInfoPopup.cpp b/src/widgets/dialogs/UserInfoPopup.cpp index 808c0503a..ed7e33033 100644 --- a/src/widgets/dialogs/UserInfoPopup.cpp +++ b/src/widgets/dialogs/UserInfoPopup.cpp @@ -313,7 +313,7 @@ UserInfoPopup::UserInfoPopup(bool closeAutomatically, QWidget *parent, SplitContainer *container = nb.addPage(true); Split *split = new Split(container); split->setChannel(channel); - container->appendSplit(split); + container->insertSplit(split); }); menu->popup(QCursor::pos()); menu->raise(); diff --git a/src/widgets/dialogs/switcher/NewTabItem.cpp b/src/widgets/dialogs/switcher/NewTabItem.cpp index 69203f68a..2bd8867ec 100644 --- a/src/widgets/dialogs/switcher/NewTabItem.cpp +++ b/src/widgets/dialogs/switcher/NewTabItem.cpp @@ -26,7 +26,7 @@ void NewTabItem::action() Split *split = new Split(container); split->setChannel(getApp()->twitch->getOrAddChannel(this->channelName_)); - container->appendSplit(split); + container->insertSplit(split); } void NewTabItem::paint(QPainter *painter, const QRect &rect) const diff --git a/src/widgets/dialogs/switcher/QuickSwitcherPopup.cpp b/src/widgets/dialogs/switcher/QuickSwitcherPopup.cpp index 63ba96759..f6e2836d7 100644 --- a/src/widgets/dialogs/switcher/QuickSwitcherPopup.cpp +++ b/src/widgets/dialogs/switcher/QuickSwitcherPopup.cpp @@ -1,6 +1,7 @@ #include "widgets/dialogs/switcher/QuickSwitcherPopup.hpp" #include "Application.hpp" +#include "common/Channel.hpp" #include "singletons/Theme.hpp" #include "singletons/WindowManager.hpp" #include "util/LayoutCreator.hpp" @@ -10,6 +11,8 @@ #include "widgets/helper/NotebookTab.hpp" #include "widgets/listview/GenericListView.hpp" #include "widgets/Notebook.hpp" +#include "widgets/splits/Split.hpp" +#include "widgets/splits/SplitContainer.hpp" #include "widgets/Window.hpp" namespace chatterino { diff --git a/src/widgets/dialogs/switcher/QuickSwitcherPopup.hpp b/src/widgets/dialogs/switcher/QuickSwitcherPopup.hpp index 955f3f2dc..ffc707aa8 100644 --- a/src/widgets/dialogs/switcher/QuickSwitcherPopup.hpp +++ b/src/widgets/dialogs/switcher/QuickSwitcherPopup.hpp @@ -1,10 +1,7 @@ #pragma once -#include "common/Channel.hpp" #include "widgets/BasePopup.hpp" #include "widgets/dialogs/switcher/QuickSwitcherModel.hpp" -#include "widgets/splits/Split.hpp" -#include "widgets/splits/SplitContainer.hpp" #include diff --git a/src/widgets/helper/NotebookButton.cpp b/src/widgets/helper/NotebookButton.cpp index 64bf03917..99e655a1f 100644 --- a/src/widgets/helper/NotebookButton.cpp +++ b/src/widgets/helper/NotebookButton.cpp @@ -1,8 +1,10 @@ #include "widgets/helper/NotebookButton.hpp" +#include "common/QLogging.hpp" #include "singletons/Theme.hpp" #include "widgets/helper/Button.hpp" #include "widgets/Notebook.hpp" +#include "widgets/splits/DraggedSplit.hpp" #include "widgets/splits/Split.hpp" #include "widgets/splits/SplitContainer.hpp" @@ -67,7 +69,7 @@ void NotebookButton::paintEvent(QPaintEvent *event) case Plus: { painter.setPen([&] { QColor tmp = foreground; - if (SplitContainer::isDraggingSplit) + if (isDraggingSplit()) { tmp = this->theme->tabs.selected.line.regular; } @@ -181,22 +183,30 @@ void NotebookButton::dragLeaveEvent(QDragLeaveEvent *) void NotebookButton::dropEvent(QDropEvent *event) { - if (SplitContainer::isDraggingSplit) + auto *draggedSplit = dynamic_cast(event->source()); + if (!draggedSplit) { - event->acceptProposedAction(); - - Notebook *notebook = dynamic_cast(this->parentWidget()); - - if (notebook != nuuls) - { - SplitContainer *page = new SplitContainer(notebook); - auto *tab = notebook->addPage(page); - page->setTab(tab); - - SplitContainer::draggingSplit->setParent(page); - page->appendSplit(SplitContainer::draggingSplit); - } + qCDebug(chatterinoWidget) + << "Dropped something that wasn't a split onto a notebook button"; + return; } + + auto *notebook = dynamic_cast(this->parentWidget()); + if (!notebook) + { + qCDebug(chatterinoWidget) << "Dropped a split onto a notebook button " + "without a parent notebook"; + return; + } + + event->acceptProposedAction(); + + auto *page = new SplitContainer(notebook); + auto *tab = notebook->addPage(page); + page->setTab(tab); + + draggedSplit->setParent(page); + page->insertSplit(draggedSplit); } void NotebookButton::hideEvent(QHideEvent *) diff --git a/src/widgets/helper/NotebookTab.cpp b/src/widgets/helper/NotebookTab.cpp index 2eef532a6..7d8bd0003 100644 --- a/src/widgets/helper/NotebookTab.cpp +++ b/src/widgets/helper/NotebookTab.cpp @@ -12,6 +12,7 @@ #include "util/Helpers.hpp" #include "widgets/dialogs/SettingsDialog.hpp" #include "widgets/Notebook.hpp" +#include "widgets/splits/DraggedSplit.hpp" #include "widgets/splits/SplitContainer.hpp" #include @@ -695,10 +696,15 @@ void NotebookTab::leaveEvent(QEvent *event) void NotebookTab::dragEnterEvent(QDragEnterEvent *event) { if (!event->mimeData()->hasFormat("chatterino/split")) + { return; + } - if (!SplitContainer::isDraggingSplit) + if (!isDraggingSplit()) + { + // Ensure dragging a split from a different Chatterino instance doesn't switch tabs around return; + } if (this->notebook_->getAllowUserTabManagement()) { diff --git a/src/widgets/settingspages/GeneralPageView.hpp b/src/widgets/settingspages/GeneralPageView.hpp index fcc51eb6e..38a043fe8 100644 --- a/src/widgets/settingspages/GeneralPageView.hpp +++ b/src/widgets/settingspages/GeneralPageView.hpp @@ -12,6 +12,7 @@ #include #include #include +#include class QScrollArea; diff --git a/src/widgets/splits/DraggedSplit.cpp b/src/widgets/splits/DraggedSplit.cpp new file mode 100644 index 000000000..308f03226 --- /dev/null +++ b/src/widgets/splits/DraggedSplit.cpp @@ -0,0 +1,28 @@ +#include "widgets/splits/DraggedSplit.hpp" + +#include + +namespace chatterino { + +static bool currentlyDraggingSplit = false; + +bool isDraggingSplit() +{ + return currentlyDraggingSplit; +} + +void startDraggingSplit() +{ + assert(currentlyDraggingSplit == false); + + currentlyDraggingSplit = true; +} + +void stopDraggingSplit() +{ + assert(currentlyDraggingSplit == true); + + currentlyDraggingSplit = false; +} + +} // namespace chatterino diff --git a/src/widgets/splits/DraggedSplit.hpp b/src/widgets/splits/DraggedSplit.hpp new file mode 100644 index 000000000..80d5e637e --- /dev/null +++ b/src/widgets/splits/DraggedSplit.hpp @@ -0,0 +1,17 @@ +#pragma once + +namespace chatterino { + +// Returns true if the user is currently dragging a split in this Chatterino instance +// We need to keep track of this to ensure splits from other Chatterino instances aren't treated as memory we own +[[nodiscard]] bool isDraggingSplit(); + +// Set that a split is currently being dragged +// Used by the Split::drag function when a drag is initiated +void startDraggingSplit(); + +// Set that a split is no longer being dragged +// Used by the Split::drag function when a drag is finished +void stopDraggingSplit(); + +} // namespace chatterino diff --git a/src/widgets/splits/Split.cpp b/src/widgets/splits/Split.cpp index 4f38943e2..1261e13f7 100644 --- a/src/widgets/splits/Split.cpp +++ b/src/widgets/splits/Split.cpp @@ -34,6 +34,7 @@ #include "widgets/helper/SearchPopup.hpp" #include "widgets/Notebook.hpp" #include "widgets/Scrollbar.hpp" +#include "widgets/splits/DraggedSplit.hpp" #include "widgets/splits/SplitContainer.hpp" #include "widgets/splits/SplitHeader.hpp" #include "widgets/splits/SplitInput.hpp" @@ -628,7 +629,7 @@ void Split::joinChannelInNewTab(ChannelPtr channel) Split *split = new Split(container); split->setChannel(channel); - container->appendSplit(split); + container->insertSplit(split); } void Split::openChannelInBrowserPlayer(ChannelPtr channel) @@ -899,7 +900,7 @@ void Split::popup() split->setModerationMode(this->getModerationMode()); split->setFilters(this->getFilters()); - window.getNotebook().getOrAddSelectedPage()->appendSplit(split); + window.getNotebook().getOrAddSelectedPage()->insertSplit(split); window.show(); } @@ -1256,25 +1257,31 @@ static Iter select_randomly(Iter start, Iter end) void Split::drag() { - if (auto container = dynamic_cast(this->parentWidget())) + auto *container = dynamic_cast(this->parentWidget()); + if (!container) { - SplitContainer::isDraggingSplit = true; - SplitContainer::draggingSplit = this; - - auto originalLocation = container->releaseSplit(this); - auto drag = new QDrag(this); - auto mimeData = new QMimeData; - - mimeData->setData("chatterino/split", "xD"); - drag->setMimeData(mimeData); - - if (drag->exec(Qt::MoveAction) == Qt::IgnoreAction) - { - container->insertSplit(this, originalLocation); - } - - SplitContainer::isDraggingSplit = false; + qCWarning(chatterinoWidget) + << "Attempted to initiate split drag without a container parent"; + return; } + + startDraggingSplit(); + + auto originalLocation = container->releaseSplit(this); + auto drag = new QDrag(this); + auto mimeData = new QMimeData; + + mimeData->setData("chatterino/split", "xD"); + drag->setMimeData(mimeData); + + // drag->exec is a blocking action + if (drag->exec(Qt::MoveAction) == Qt::IgnoreAction) + { + // The split wasn't dropped in a valid spot, return it to its original position + container->insertSplit(this, {.position = originalLocation}); + } + + stopDraggingSplit(); } void Split::setInputReply(const std::shared_ptr &reply) diff --git a/src/widgets/splits/SplitContainer.cpp b/src/widgets/splits/SplitContainer.cpp index 8b33b33bc..65dde68a3 100644 --- a/src/widgets/splits/SplitContainer.cpp +++ b/src/widgets/splits/SplitContainer.cpp @@ -2,38 +2,30 @@ #include "Application.hpp" #include "common/Common.hpp" +#include "common/QLogging.hpp" #include "debug/AssertInGuiThread.hpp" #include "singletons/Fonts.hpp" #include "singletons/Theme.hpp" #include "singletons/WindowManager.hpp" -#include "util/Helpers.hpp" -#include "util/LayoutCreator.hpp" #include "widgets/helper/ChannelView.hpp" #include "widgets/helper/NotebookTab.hpp" #include "widgets/Notebook.hpp" #include "widgets/splits/ClosedSplits.hpp" +#include "widgets/splits/DraggedSplit.hpp" #include "widgets/splits/Split.hpp" #include "widgets/Window.hpp" #include #include -#include -#include #include #include #include -#include #include -#include -#include #include namespace chatterino { -bool SplitContainer::isDraggingSplit = false; -Split *SplitContainer::draggingSplit = nullptr; - SplitContainer::SplitContainer(Notebook *parent) : BaseWidget(parent) , overlay_(this) @@ -91,9 +83,9 @@ NotebookTab *SplitContainer::getTab() const return this->tab_; } -void SplitContainer::setTab(NotebookTab *_tab) +void SplitContainer::setTab(NotebookTab *tab) { - this->tab_ = _tab; + this->tab_ = tab; this->tab_->page = this; @@ -120,8 +112,8 @@ Split *SplitContainer::appendNewSplit(bool openChannelNameDialog) { assertInGuiThread(); - Split *split = new Split(this); - this->appendSplit(split); + auto *split = new Split(this); + this->insertSplit(split); if (openChannelNameDialog) { @@ -136,41 +128,46 @@ Split *SplitContainer::appendNewSplit(bool openChannelNameDialog) return split; } -void SplitContainer::appendSplit(Split *split) -{ - this->insertSplit(split, Direction::Right); -} - -void SplitContainer::insertSplit(Split *split, const Position &position) -{ - this->insertSplit(split, position.direction_, - reinterpret_cast(position.relativeNode_)); -} - -void SplitContainer::insertSplit(Split *split, Direction direction, - Split *relativeTo) -{ - Node *node = this->baseNode_.findNodeContainingSplit(relativeTo); - assert(node != nullptr); - - this->insertSplit(split, direction, node); -} - -void SplitContainer::insertSplit(Split *split, Direction direction, - Node *relativeTo) +void SplitContainer::insertSplit(Split *split, InsertOptions &&options) { // Queue up save because: Split added getApp()->windows->queueSave(); assertInGuiThread(); + if (options.position) + { + // options.position must not be set together with any other options + assert(!options.relativeSplit); + assert(!options.relativeNode); + assert(!options.direction.has_value()); + + options.relativeNode = options.position->relativeNode_; + options.direction = options.position->direction_; + } + + if (options.relativeSplit) + { + // options.relativeNode must not be set together with relativeSplit + assert(!options.relativeNode); + + Node *node = + this->baseNode_.findNodeContainingSplit(options.relativeSplit); + assert(node != nullptr); + + options.relativeNode = node; + } + + auto *relativeTo = options.relativeNode; + const auto direction = options.direction.value_or(Direction::Right); + if (relativeTo == nullptr) { - if (this->baseNode_.type_ == Node::EmptyRoot) + if (this->baseNode_.type_ == Node::Type::EmptyRoot) { this->baseNode_.setSplit(split); } - else if (this->baseNode_.type_ == Node::_Split) + else if (this->baseNode_.type_ == Node::Type::Split) { this->baseNode_.nestSplitIntoCollection(split, direction); } @@ -254,7 +251,7 @@ void SplitContainer::addSplit(Split *split) case Split::Action::Delete: { this->deleteSplit(split); auto *tab = this->getTab(); - tab->connect(tab, &QWidget::destroyed, [tab]() mutable { + QObject::connect(tab, &QWidget::destroyed, [tab]() mutable { ClosedSplits::invalidateTab(tab); }); ClosedSplits::push({split->getChannel()->getName(), @@ -277,10 +274,14 @@ void SplitContainer::addSplit(Split *split) } }); - conns.managedConnect(split->insertSplitRequested, [this](int dir, - Split *parent) { - this->insertSplit(new Split(this), static_cast(dir), parent); - }); + conns.managedConnect( + split->insertSplitRequested, [this](int dir, Split *parent) { + this->insertSplit(new Split(this), + { + .relativeSplit = parent, + .direction = static_cast(dir), + }); + }); this->layout(); } @@ -325,7 +326,7 @@ SplitContainer::Position SplitContainer::releaseSplit(Split *split) split->setParent(nullptr); Position position = node->releaseSplit(); this->layout(); - if (splits_.size() == 0) + if (splits_.empty()) { this->setSelected(nullptr); this->setCursor(Qt::PointingHandCursor); @@ -414,13 +415,13 @@ void SplitContainer::focusSplitRecursive(Node *node) { switch (node->type_) { - case Node::_Split: { + case Node::Type::Split: { node->split_->setFocus(Qt::FocusReason::OtherFocusReason); } break; - case Node::HorizontalContainer: - case Node::VerticalContainer: { + case Node::Type::HorizontalContainer: + case Node::Type::VerticalContainer: { auto &children = node->children_; auto it = std::find_if( @@ -447,15 +448,19 @@ Split *SplitContainer::getTopRightSplit(Node &node) { switch (node.getType()) { - case Node::_Split: + case Node::Type::Split: return node.getSplit(); - case Node::VerticalContainer: + case Node::Type::VerticalContainer: if (!node.getChildren().empty()) + { return getTopRightSplit(*node.getChildren().front()); + } break; - case Node::HorizontalContainer: + case Node::Type::HorizontalContainer: if (!node.getChildren().empty()) + { return getTopRightSplit(*node.getChildren().back()); + } break; default:; } @@ -470,23 +475,28 @@ 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); + } this->topRight_ = topRight; if (topRight) + { this->topRight_->setIsTopRightSplit(true); + } // layout this->baseNode_.geometry_ = this->rect().adjusted(-1, -1, 0, 0); - std::vector _dropRects; - std::vector _resizeRects; - this->baseNode_.layout( - Split::modifierStatus == showAddSplitRegions || this->isDragging_, - this->scale(), _dropRects, _resizeRects); + std::vector dropRects; + std::vector resizeRects; - this->dropRects_ = _dropRects; + const bool addSpacing = + Split::modifierStatus == showAddSplitRegions || this->isDragging_; + this->baseNode_.layout(addSpacing, this->scale(), dropRects, resizeRects); + + this->dropRects_ = dropRects; for (Split *split : this->splits_) { @@ -495,52 +505,51 @@ void SplitContainer::layout() Node *node = this->baseNode_.findNodeContainingSplit(split); // left - _dropRects.push_back( - DropRect(QRect(g.left(), g.top(), g.width() / 3, g.height()), - Position(node, Direction::Left))); + dropRects.emplace_back( + QRect(g.left(), g.top(), g.width() / 3, g.height()), + Position(node, Direction::Left)); // right - _dropRects.push_back(DropRect(QRect(g.right() - g.width() / 3, g.top(), - g.width() / 3, g.height()), - Position(node, Direction::Right))); + dropRects.emplace_back(QRect(g.right() - g.width() / 3, g.top(), + g.width() / 3, g.height()), + Position(node, Direction::Right)); // top - _dropRects.push_back( - DropRect(QRect(g.left(), g.top(), g.width(), g.height() / 2), - Position(node, Direction::Above))); + dropRects.emplace_back( + QRect(g.left(), g.top(), g.width(), g.height() / 2), + Position(node, Direction::Above)); // bottom - _dropRects.push_back( - DropRect(QRect(g.left(), g.bottom() - g.height() / 2, g.width(), - g.height() / 2), - Position(node, Direction::Below))); + dropRects.emplace_back(QRect(g.left(), g.bottom() - g.height() / 2, + g.width(), g.height() / 2), + Position(node, Direction::Below)); } if (this->splits_.empty()) { QRect g = this->rect(); - _dropRects.push_back( - DropRect(QRect(g.left(), g.top(), g.width() - 1, g.height() - 1), - Position(nullptr, Direction::Below))); + dropRects.emplace_back( + QRect(g.left(), g.top(), g.width() - 1, g.height() - 1), + Position(nullptr, Direction::Below)); } - this->overlay_.setRects(std::move(_dropRects)); + this->overlay_.setRects(std::move(dropRects)); // handle resizeHandles - if (this->resizeHandles_.size() < _resizeRects.size()) + if (this->resizeHandles_.size() < resizeRects.size()) { - while (this->resizeHandles_.size() < _resizeRects.size()) + while (this->resizeHandles_.size() < resizeRects.size()) { this->resizeHandles_.push_back( std::make_unique(this)); } } - else if (this->resizeHandles_.size() > _resizeRects.size()) + else if (this->resizeHandles_.size() > resizeRects.size()) { - this->resizeHandles_.resize(_resizeRects.size()); + this->resizeHandles_.resize(resizeRects.size()); } { size_t i = 0; - for (ResizeRect &resizeRect : _resizeRects) + for (ResizeRect &resizeRect : resizeRects) { ResizeHandle *handle = this->resizeHandles_[i].get(); handle->setGeometry(resizeRect.rect); @@ -572,7 +581,7 @@ void SplitContainer::mouseReleaseEvent(QMouseEvent *event) { if (event->button() == Qt::LeftButton) { - if (this->splits_.size() == 0) + if (this->splits_.empty()) { // "Add Chat" was clicked this->appendNewSplit(true); @@ -589,17 +598,17 @@ void SplitContainer::mouseReleaseEvent(QMouseEvent *event) }); if (it != this->dropRects_.end()) { - this->insertSplit(new Split(this), it->position); + this->insertSplit(new Split(this), {.position = it->position}); } } } } -void SplitContainer::paintEvent(QPaintEvent *) +void SplitContainer::paintEvent(QPaintEvent * /*event*/) { QPainter painter(this); - if (this->splits_.size() == 0) + if (this->splits_.empty()) { painter.fillRect(rect(), this->theme->splits.background); @@ -611,7 +620,7 @@ void SplitContainer::paintEvent(QPaintEvent *) QString text = "Click to add a split"; - Notebook *notebook = dynamic_cast(this->parentWidget()); + auto *notebook = dynamic_cast(this->parentWidget()); if (notebook != nullptr) { @@ -690,10 +699,14 @@ void SplitContainer::paintEvent(QPaintEvent *) void SplitContainer::dragEnterEvent(QDragEnterEvent *event) { if (!event->mimeData()->hasFormat("chatterino/split")) + { return; + } - if (!SplitContainer::isDraggingSplit) + if (!isDraggingSplit()) + { return; + } this->isDragging_ = true; this->layout(); @@ -714,13 +727,13 @@ void SplitContainer::mouseMoveEvent(QMouseEvent *event) this->update(); } -void SplitContainer::leaveEvent(QEvent *) +void SplitContainer::leaveEvent(QEvent * /*event*/) { this->mouseOverPoint_ = QPoint(-10000, -10000); this->update(); } -void SplitContainer::focusInEvent(QFocusEvent *) +void SplitContainer::focusInEvent(QFocusEvent * /*event*/) { if (this->baseNode_.findNodeContainingSplit(this->selected_) != nullptr) { @@ -740,12 +753,7 @@ void SplitContainer::refreshTab() this->refreshTabLiveStatus(); } -int SplitContainer::getSplitCount() -{ - return this->splits_.size(); -} - -const std::vector SplitContainer::getSplits() const +std::vector SplitContainer::getSplits() const { return this->splits_; } @@ -757,7 +765,7 @@ SplitContainer::Node *SplitContainer::getBaseNode() void SplitContainer::applyFromDescriptor(const NodeDescriptor &rootNode) { - assert(this->baseNode_.type_ == Node::EmptyRoot); + assert(this->baseNode_.type_ == Node::Type::EmptyRoot); this->disableLayouting_ = true; this->applyFromDescriptorRecursively(rootNode, &this->baseNode_); @@ -768,7 +776,7 @@ void SplitContainer::applyFromDescriptor(const NodeDescriptor &rootNode) void SplitContainer::popup() { Window &window = getApp()->windows->createWindow(WindowType::Popup); - auto popupContainer = window.getNotebook().getOrAddSelectedPage(); + auto *popupContainer = window.getNotebook().getOrAddSelectedPage(); QJsonObject encodedTab; WindowManager::encodeTab(this, true, encodedTab); @@ -793,26 +801,33 @@ void SplitContainer::popup() } void SplitContainer::applyFromDescriptorRecursively( - const NodeDescriptor &rootNode, Node *node) + const NodeDescriptor &rootNode, Node *baseNode) { if (std::holds_alternative(rootNode)) { - auto *n = std::get_if(&rootNode); + // This is a leaf, no further recursion happens from here + + const auto *n = std::get_if(&rootNode); if (!n) { return; } const auto &splitNode = *n; + auto *split = new Split(this); split->setChannel(WindowManager::decodeChannel(splitNode)); split->setModerationMode(splitNode.moderationMode_); split->setFilters(splitNode.filters_); - this->appendSplit(split); + this->insertSplit(split); + + return; } - else if (std::holds_alternative(rootNode)) + + if (std::holds_alternative(rootNode)) { - auto *n = std::get_if(&rootNode); + // This is a branch, it will contain one or more splits/containers + const auto *n = std::get_if(&rootNode); if (!n) { return; @@ -821,14 +836,14 @@ void SplitContainer::applyFromDescriptorRecursively( bool vertical = containerNode.vertical_; - node->type_ = - vertical ? Node::VerticalContainer : Node::HorizontalContainer; + baseNode->type_ = vertical ? Node::Type::VerticalContainer + : Node::Type::HorizontalContainer; for (const auto &item : containerNode.items_) { if (std::holds_alternative(item)) { - auto *n = std::get_if(&item); + const auto *n = std::get_if(&item); if (!n) { return; @@ -839,30 +854,30 @@ void SplitContainer::applyFromDescriptorRecursively( split->setModerationMode(splitNode.moderationMode_); split->setFilters(splitNode.filters_); - Node *_node = new Node(); - _node->parent_ = node; - _node->split_ = split; - _node->type_ = Node::_Split; + auto *node = new Node(); + node->parent_ = baseNode; + node->split_ = split; + node->type_ = Node::Type::Split; - _node->flexH_ = splitNode.flexH_; - _node->flexV_ = splitNode.flexV_; - node->children_.emplace_back(_node); + node->flexH_ = splitNode.flexH_; + node->flexV_ = splitNode.flexV_; + baseNode->children_.emplace_back(node); this->addSplit(split); } else { - Node *_node = new Node(); - _node->parent_ = node; + auto *node = new Node(); + node->parent_ = baseNode; - if (auto *n = std::get_if(&item)) + if (const auto *n = std::get_if(&item)) { - _node->flexH_ = n->flexH_; - _node->flexV_ = n->flexV_; + node->flexH_ = n->flexH_; + node->flexV_ = n->flexV_; } - node->children_.emplace_back(_node); - this->applyFromDescriptorRecursively(item, _node); + baseNode->children_.emplace_back(node); + this->applyFromDescriptorRecursively(item, node); } } } @@ -928,26 +943,26 @@ void SplitContainer::refreshTabLiveStatus() // Node // -SplitContainer::Node::Type SplitContainer::Node::getType() +SplitContainer::Node::Type SplitContainer::Node::getType() const { return this->type_; } -Split *SplitContainer::Node::getSplit() +Split *SplitContainer::Node::getSplit() const { return this->split_; } -SplitContainer::Node *SplitContainer::Node::getParent() +SplitContainer::Node *SplitContainer::Node::getParent() const { return this->parent_; } -qreal SplitContainer::Node::getHorizontalFlex() +qreal SplitContainer::Node::getHorizontalFlex() const { return this->flexH_; } -qreal SplitContainer::Node::getVerticalFlex() +qreal SplitContainer::Node::getVerticalFlex() const { return this->flexV_; } @@ -966,7 +981,7 @@ SplitContainer::Node::Node() } SplitContainer::Node::Node(Split *_split, Node *_parent) - : type_(Type::_Split) + : type_(Type::Split) , split_(_split) , parent_(_parent) { @@ -988,7 +1003,7 @@ bool SplitContainer::Node::isOrContainsNode(SplitContainer::Node *_node) SplitContainer::Node *SplitContainer::Node::findNodeContainingSplit( Split *_split) { - if (this->type_ == Type::_Split && this->split_ == _split) + if (this->type_ == Type::Split && this->split_ == _split) { return this; } @@ -1012,19 +1027,19 @@ void SplitContainer::Node::insertSplitRelative(Split *_split, { switch (this->type_) { - case Node::EmptyRoot: { + case Node::Type::EmptyRoot: { this->setSplit(_split); } break; - case Node::_Split: { + case Node::Type::Split: { this->nestSplitIntoCollection(_split, _direction); } break; - case Node::HorizontalContainer: { + case Node::Type::HorizontalContainer: { this->nestSplitIntoCollection(_split, _direction); } break; - case Node::VerticalContainer: { + case Node::Type::VerticalContainer: { this->nestSplitIntoCollection(_split, _direction); } break; @@ -1107,15 +1122,15 @@ void SplitContainer::Node::insertNextToThis(Split *_split, Direction _direction) void SplitContainer::Node::setSplit(Split *_split) { assert(this->split_ == nullptr); - assert(this->children_.size() == 0); + assert(this->children_.empty()); this->split_ = _split; - this->type_ = Type::_Split; + this->type_ = Type::Split; } SplitContainer::Position SplitContainer::Node::releaseSplit() { - assert(this->type_ == Type::_Split); + assert(this->type_ == Type::Split); if (parent_ == nullptr) { @@ -1127,69 +1142,66 @@ SplitContainer::Position SplitContainer::Node::releaseSplit() pos.direction_ = Direction::Right; return pos; } - else + + auto &siblings = this->parent_->children_; + + auto it = std::find_if(begin(siblings), end(siblings), [this](auto &node) { + return this == node.get(); + }); + assert(it != siblings.end()); + + Position position; + if (siblings.size() == 2) { - auto &siblings = this->parent_->children_; - - auto it = - std::find_if(begin(siblings), end(siblings), [this](auto &node) { - return this == node.get(); - }); - assert(it != siblings.end()); - - Position position; - if (siblings.size() == 2) + // delete this and move split to parent + position.relativeNode_ = this->parent_; + if (this->parent_->type_ == Type::VerticalContainer) { - // delete this and move split to parent - position.relativeNode_ = this->parent_; - if (this->parent_->type_ == Type::VerticalContainer) - { - position.direction_ = siblings.begin() == it ? Direction::Above - : Direction::Below; - } - else - { - position.direction_ = - siblings.begin() == it ? Direction::Left : Direction::Right; - } - - Node *_parent = this->parent_; - siblings.erase(it); - std::unique_ptr &sibling = siblings.front(); - _parent->type_ = sibling->type_; - _parent->split_ = sibling->split_; - std::vector> nodes = - std::move(sibling->children_); - for (auto &node : nodes) - { - node->parent_ = _parent; - } - _parent->children_ = std::move(nodes); + position.direction_ = + siblings.begin() == it ? Direction::Above : Direction::Below; } else { - if (this == siblings.back().get()) - { - position.direction_ = - this->parent_->type_ == Type::VerticalContainer - ? Direction::Below - : Direction::Right; - siblings.erase(it); - position.relativeNode_ = siblings.back().get(); - } - else - { - position.relativeNode_ = (it + 1)->get(); - position.direction_ = - this->parent_->type_ == Type::VerticalContainer - ? Direction::Above - : Direction::Left; - siblings.erase(it); - } + position.direction_ = + siblings.begin() == it ? Direction::Left : Direction::Right; } - return position; + auto *parent = this->parent_; + siblings.erase(it); + std::unique_ptr &sibling = siblings.front(); + parent->type_ = sibling->type_; + parent->split_ = sibling->split_; + std::vector> nodes = + std::move(sibling->children_); + for (auto &node : nodes) + { + node->parent_ = parent; + } + parent->children_ = std::move(nodes); } + else + { + if (this == siblings.back().get()) + { + position.direction_ = + this->parent_->type_ == Type::VerticalContainer + ? Direction::Below + : Direction::Right; + siblings.erase(it); + position.relativeNode_ = siblings.back().get(); + } + else + { + position.relativeNode_ = (it + 1)->get(); + position.direction_ = + this->parent_->type_ == Type::VerticalContainer + ? Direction::Above + : Direction::Left; + siblings.erase(it); + } + } + + return position; } qreal SplitContainer::Node::getFlex(bool isVertical) @@ -1217,26 +1229,23 @@ void SplitContainer::Node::layout(bool addSpacing, float _scale, { for (std::unique_ptr &node : this->children_) { - if (node->flexH_ <= 0) - node->flexH_ = 0; - if (node->flexV_ <= 0) - node->flexV_ = 0; + node->clamp(); } switch (this->type_) { - case Node::_Split: { + case Node::Type::Split: { QRect rect = this->geometry_.toRect(); this->split_->setGeometry( rect.marginsRemoved(QMargins(1, 1, 0, 0))); } break; - case Node::VerticalContainer: - case Node::HorizontalContainer: { - bool isVertical = this->type_ == Node::VerticalContainer; + case Node::Type::VerticalContainer: + case Node::Type::HorizontalContainer: { + bool isVertical = this->type_ == Node::Type::VerticalContainer; // vars - qreal minSize = qreal(48 * _scale); + qreal minSize(48 * _scale); qreal totalFlex = std::max( 0.0001, this->getChildrensTotalFlex(isVertical)); @@ -1370,6 +1379,12 @@ void SplitContainer::Node::layout(bool addSpacing, float _scale, } } +void SplitContainer::Node::clamp() +{ + this->flexH_ = std::max(0.0, this->flexH_); + this->flexV_ = std::max(0.0, this->flexV_); +} + SplitContainer::Node::Type SplitContainer::Node::toContainerType(Direction _dir) { return _dir == Direction::Left || _dir == Direction::Right @@ -1398,7 +1413,7 @@ void SplitContainer::DropOverlay::setRects( // pajlada::Signals::NoArgSignal dragEnded; -void SplitContainer::DropOverlay::paintEvent(QPaintEvent *) +void SplitContainer::DropOverlay::paintEvent(QPaintEvent * /*event*/) { QPainter painter(this); @@ -1438,7 +1453,7 @@ void SplitContainer::DropOverlay::dragMoveEvent(QDragMoveEvent *event) this->update(); } -void SplitContainer::DropOverlay::dragLeaveEvent(QDragLeaveEvent *) +void SplitContainer::DropOverlay::dragLeaveEvent(QDragLeaveEvent * /*event*/) { this->mouseOverPoint_ = QPoint(-10000, -10000); this->close(); @@ -1457,12 +1472,23 @@ void SplitContainer::DropOverlay::dropEvent(QDropEvent *event) } } - if (position != nullptr) + if (!position) { - this->parent_->insertSplit(SplitContainer::draggingSplit, *position); - event->acceptProposedAction(); + qCDebug(chatterinoWidget) << "No valid drop rectangle under cursor"; + return; } + auto *draggedSplit = dynamic_cast(event->source()); + if (!draggedSplit) + { + qCDebug(chatterinoWidget) + << "Dropped something that wasn't a split onto a split container"; + return; + } + + this->parent_->insertSplit(draggedSplit, {.position = *position}); + event->acceptProposedAction(); + this->mouseOverPoint_ = QPoint(-10000, -10000); this->close(); this->dragEnded.invoke(); @@ -1486,7 +1512,7 @@ SplitContainer::ResizeHandle::ResizeHandle(SplitContainer *_parent) this->hide(); } -void SplitContainer::ResizeHandle::paintEvent(QPaintEvent *) +void SplitContainer::ResizeHandle::paintEvent(QPaintEvent * /*event*/) { QPainter painter(this); painter.setPen(QPen(getApp()->themes->splits.resizeHandle, 2)); @@ -1516,7 +1542,7 @@ void SplitContainer::ResizeHandle::mousePressEvent(QMouseEvent *event) } } -void SplitContainer::ResizeHandle::mouseReleaseEvent(QMouseEvent *) +void SplitContainer::ResizeHandle::mouseReleaseEvent(QMouseEvent * /*event*/) { this->isMouseDown_ = false; } @@ -1531,7 +1557,7 @@ void SplitContainer::ResizeHandle::mouseMoveEvent(QMouseEvent *event) assert(node != nullptr); assert(node->parent_ != nullptr); - auto &siblings = node->parent_->getChildren(); + const auto &siblings = node->parent_->getChildren(); auto it = std::find_if(siblings.begin(), siblings.end(), [this](const std::unique_ptr &n) { return n.get() == this->node; @@ -1591,7 +1617,7 @@ void SplitContainer::ResizeHandle::mouseDoubleClickEvent(QMouseEvent *event) void SplitContainer::ResizeHandle::resetFlex() { - for (auto &sibling : this->node->getParent()->getChildren()) + for (const auto &sibling : this->node->getParent()->getChildren()) { sibling->flexH_ = 1; sibling->flexV_ = 1; diff --git a/src/widgets/splits/SplitContainer.hpp b/src/widgets/splits/SplitContainer.hpp index bab73e31e..2a37c8907 100644 --- a/src/widgets/splits/SplitContainer.hpp +++ b/src/widgets/splits/SplitContainer.hpp @@ -6,14 +6,11 @@ #include #include #include -#include #include -#include -#include #include #include -#include +#include #include #include @@ -38,6 +35,7 @@ public: struct Node; // fourtf: !!! preserve the order of left, up, right and down + // It's important to preserve since we cast from an int to this enum, so 0 = left, 1 = above etc enum Direction { Left, Above, Right, Below }; struct Position final { @@ -49,8 +47,8 @@ public: { } - Node *relativeNode_; - Direction direction_; + Node *relativeNode_{nullptr}; + Direction direction_{Direction::Right}; friend struct Node; friend class SplitContainer; @@ -83,13 +81,18 @@ private: public: struct Node final { - enum Type { EmptyRoot, _Split, VerticalContainer, HorizontalContainer }; + enum class Type { + EmptyRoot, + Split, + VerticalContainer, + HorizontalContainer, + }; - Type getType(); - Split *getSplit(); - Node *getParent(); - qreal getHorizontalFlex(); - qreal getVerticalFlex(); + Type getType() const; + Split *getSplit() const; + Node *getParent() const; + qreal getHorizontalFlex() const; + qreal getVerticalFlex() const; const std::vector> &getChildren(); private: @@ -110,6 +113,9 @@ public: std::vector &dropRects_, std::vector &resizeRects); + // Clamps the flex values ensuring they're never below 0 + void clamp(); + static Type toContainerType(Direction _dir); Type type_; @@ -174,33 +180,48 @@ public: SplitContainer(Notebook *parent); Split *appendNewSplit(bool openChannelNameDialog); - void appendSplit(Split *split); - void insertSplit(Split *split, const Position &position); - void insertSplit(Split *split, Direction direction, Split *relativeTo); - void insertSplit(Split *split, Direction direction, - Node *relativeTo = nullptr); + + struct InsertOptions { + /// Position must be set alone, as if it's set it will override direction & relativeNode with its underlying values + std::optional position{}; + + /// Will be used to figure out the relative node, so relative node or position must not be set if using this + Split *relativeSplit{nullptr}; + + Node *relativeNode{nullptr}; + std::optional direction{}; + }; + + // Insert split into the base node of this container + // Default values for each field must be specified due to these bugs: + // - https://bugs.llvm.org/show_bug.cgi?id=36684 + // - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96645 + void insertSplit(Split *split, InsertOptions &&options = InsertOptions{ + .position = std::nullopt, + .relativeSplit = nullptr, + .relativeNode = nullptr, + .direction = std::nullopt, + }); + + // Returns a pointer to the selected split Split *getSelectedSplit() const; Position releaseSplit(Split *split); Position deleteSplit(Split *split); void selectNextSplit(Direction direction); - void setSelected(Split *selected_); + void setSelected(Split *split); - int getSplitCount(); - const std::vector getSplits() const; + std::vector getSplits() const; void refreshTab(); NotebookTab *getTab() const; Node *getBaseNode(); - void setTab(NotebookTab *tab_); + void setTab(NotebookTab *tab); void hideResizeHandles(); void resetMouseStatus(); - static bool isDraggingSplit; - static Split *draggingSplit; - void applyFromDescriptor(const NodeDescriptor &rootNode); void popup(); @@ -219,7 +240,7 @@ protected: private: void applyFromDescriptorRecursively(const NodeDescriptor &rootNode, - Node *node); + Node *baseNode); void layout(); void selectSplitRecursive(Node *node, Direction direction); @@ -233,19 +254,7 @@ private: void refreshTabTitle(); void refreshTabLiveStatus(); - struct DropRegion { - QRect rect; - std::pair position; - - DropRegion(QRect rect, std::pair position) - { - this->rect = rect; - this->position = position; - } - }; - std::vector dropRects_; - std::vector dropRegions_; DropOverlay overlay_; std::vector> resizeHandles_; QPoint mouseOverPoint_; @@ -263,6 +272,7 @@ private: pajlada::Signals::SignalHolder signalHolder_; + // Specifies whether the user is currently dragging something over this container bool isDragging_ = false; };