From 6eb68d2cd7e83b43866f9cc3df8701a1a1207447 Mon Sep 17 00:00:00 2001 From: outfoxxed Date: Fri, 19 Apr 2024 15:43:26 -0700 Subject: [PATCH] core/reloader: fix late creation of Reloadable types --- src/core/floatingwindow.cpp | 2 +- src/core/generation.cpp | 5 ++++- src/core/generation.hpp | 2 ++ src/core/lazyloader.cpp | 14 ++---------- src/core/lazyloader.hpp | 1 - src/core/main.cpp | 1 - src/core/reload.cpp | 39 +++++++++++++++++++++++++++++++++- src/core/reload.hpp | 23 ++++++++++++++------ src/core/singleton.cpp | 2 +- src/core/test/popupwindow.cpp | 38 ++++++++++++++++----------------- src/core/variants.cpp | 4 ++-- src/wayland/session_lock.cpp | 2 +- src/wayland/wlr_layershell.cpp | 2 +- 13 files changed, 87 insertions(+), 48 deletions(-) diff --git a/src/core/floatingwindow.cpp b/src/core/floatingwindow.cpp index 1a6eb6b6..a1d23f54 100644 --- a/src/core/floatingwindow.cpp +++ b/src/core/floatingwindow.cpp @@ -43,7 +43,7 @@ void FloatingWindowInterface::onReload(QObject* oldInstance) { QQmlEngine::setContextForObject(this->window, QQmlEngine::contextForObject(this)); auto* old = qobject_cast(oldInstance); - this->window->onReload(old != nullptr ? old->window : nullptr); + this->window->reload(old != nullptr ? old->window : nullptr); } QQmlListProperty FloatingWindowInterface::data() { return this->window->data(); } diff --git a/src/core/generation.cpp b/src/core/generation.cpp index 38464ed5..62dd42f0 100644 --- a/src/core/generation.cpp +++ b/src/core/generation.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include "iconimageprovider.hpp" #include "incubator.hpp" @@ -54,8 +55,10 @@ void EngineGeneration::onReload(EngineGeneration* old) { QObject::connect(&this->engine, &QQmlEngine::quit, app, &QCoreApplication::quit); QObject::connect(&this->engine, &QQmlEngine::exit, app, &QCoreApplication::exit); - this->root->onReload(old == nullptr ? nullptr : old->root); + this->root->reload(old == nullptr ? nullptr : old->root); this->singletonRegistry.onReload(old == nullptr ? nullptr : &old->singletonRegistry); + this->reloadComplete = true; + emit this->reloadFinished(); if (old != nullptr) { QTimer::singleShot(0, [this, old]() { diff --git a/src/core/generation.hpp b/src/core/generation.hpp index d80911d9..8db48137 100644 --- a/src/core/generation.hpp +++ b/src/core/generation.hpp @@ -41,9 +41,11 @@ public: SingletonRegistry singletonRegistry; QFileSystemWatcher* watcher = nullptr; DelayedQmlIncubationController delayedIncubationController; + bool reloadComplete = false; signals: void filesChanged(); + void reloadFinished(); private slots: void incubationControllerDestroyed(); diff --git a/src/core/lazyloader.cpp b/src/core/lazyloader.cpp index 0779c0d8..8831670b 100644 --- a/src/core/lazyloader.cpp +++ b/src/core/lazyloader.cpp @@ -23,13 +23,11 @@ void LazyLoader::onReload(QObject* oldInstance) { if (this->mItem != nullptr) { if (auto* reloadable = qobject_cast(this->mItem)) { - reloadable->onReload(old == nullptr ? nullptr : old->mItem); + reloadable->reload(old == nullptr ? nullptr : old->mItem); } else { Reloadable::reloadRecursive(this->mItem, old); } } - - this->postReload = true; } QObject* LazyLoader::item() { @@ -48,14 +46,6 @@ void LazyLoader::setItem(QObject* item) { if (item != nullptr) { item->setParent(this); - - if (this->postReload) { - if (auto* reloadable = qobject_cast(this->mItem)) { - reloadable->onReload(nullptr); - } else { - Reloadable::reloadRecursive(this->mItem, nullptr); - } - } } this->targetActive = this->isActive(); @@ -160,7 +150,7 @@ void LazyLoader::setSource(QString source) { } void LazyLoader::incubateIfReady(bool overrideReloadCheck) { - if (!(this->postReload || overrideReloadCheck) || !(this->targetLoading || this->targetActive) + if (!(this->reloadComplete || overrideReloadCheck) || !(this->targetLoading || this->targetActive) || this->mComponent == nullptr || this->incubator != nullptr) { return; diff --git a/src/core/lazyloader.hpp b/src/core/lazyloader.hpp index 4364783b..2e470135 100644 --- a/src/core/lazyloader.hpp +++ b/src/core/lazyloader.hpp @@ -152,7 +152,6 @@ private: void incubateIfReady(bool overrideReloadCheck = false); void waitForObjectCreation(); - bool postReload = false; bool targetLoading = false; bool targetActive = false; QObject* mItem = nullptr; diff --git a/src/core/main.cpp b/src/core/main.cpp index 36a8e6b6..2cfd4d9c 100644 --- a/src/core/main.cpp +++ b/src/core/main.cpp @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include diff --git a/src/core/reload.cpp b/src/core/reload.cpp index 8768dc74..4940ddc8 100644 --- a/src/core/reload.cpp +++ b/src/core/reload.cpp @@ -4,6 +4,43 @@ #include #include +#include "generation.hpp" + +void Reloadable::componentComplete() { + this->engineGeneration = EngineGeneration::findObjectGeneration(this); + + if (this->engineGeneration != nullptr) { + // When called this way there is no chance a reload will have old data, + // but this will at least help prevent weird behaviors due to never getting a reload. + if (this->engineGeneration->reloadComplete) this->reload(); + else { + QObject::connect( + this->engineGeneration, + &EngineGeneration::reloadFinished, + this, + &Reloadable::onReloadFinished + ); + } + } +} + +void Reloadable::reload(QObject* oldInstance) { + if (this->reloadComplete) return; + this->onReload(oldInstance); + this->reloadComplete = true; + + if (this->engineGeneration != nullptr) { + QObject::disconnect( + this->engineGeneration, + &EngineGeneration::reloadFinished, + this, + &Reloadable::onReloadFinished + ); + } +} + +void Reloadable::onReloadFinished() { this->reload(nullptr); } + void ReloadPropagator::onReload(QObject* oldInstance) { auto* old = qobject_cast(oldInstance); @@ -13,7 +50,7 @@ void ReloadPropagator::onReload(QObject* oldInstance) { auto* oldChild = old == nullptr || old->mChildren.length() <= i ? nullptr : qobject_cast(old->mChildren.at(i)); - newChild->onReload(oldChild); + newChild->reload(oldChild); } else { Reloadable::reloadRecursive(newChild, oldInstance); } diff --git a/src/core/reload.hpp b/src/core/reload.hpp index 2ae459a3..0d33e2b6 100644 --- a/src/core/reload.hpp +++ b/src/core/reload.hpp @@ -7,6 +7,8 @@ #include #include +class EngineGeneration; + ///! The base class of all types that can be reloaded. /// Reloadables will attempt to take specific state from previous config revisions if possible. /// Some examples are [ProxyWindowBase] and [PersistentProperties] @@ -56,14 +58,10 @@ class Reloadable public: explicit Reloadable(QObject* parent = nullptr): QObject(parent) {} - // Called unconditionally in the reload phase, with nullptr if no source could be determined. - // If non null the old instance may or may not be of the same type, and should be checked - // by `onReload`. - virtual void onReload(QObject* oldInstance) = 0; + void reload(QObject* oldInstance = nullptr); - // TODO: onReload runs after initialization for reloadable objects created late - void classBegin() override {} - void componentComplete() override {} + void classBegin() override {}; + void componentComplete() override; // Reload objects in the parent->child graph recursively. static void reloadRecursive(QObject* newObj, QObject* oldRoot); @@ -71,6 +69,17 @@ public: static void reloadChildrenRecursive(QObject* newRoot, QObject* oldRoot); QString mReloadableId; + bool reloadComplete = false; + EngineGeneration* engineGeneration = nullptr; + +private slots: + void onReloadFinished(); + +protected: + // Called unconditionally in the reload phase, with nullptr if no source could be determined. + // If non null the old instance may or may not be of the same type, and should be checked + // by `onReload`. + virtual void onReload(QObject* oldInstance) = 0; private: static QObject* getChildByReloadId(QObject* parent, const QString& reloadId); diff --git a/src/core/singleton.cpp b/src/core/singleton.cpp index 2e5445a3..61ac9927 100644 --- a/src/core/singleton.cpp +++ b/src/core/singleton.cpp @@ -48,7 +48,7 @@ void SingletonRegistry::registerSingleton(const QUrl& url, Singleton* singleton) void SingletonRegistry::onReload(SingletonRegistry* old) { for (auto [url, singleton]: this->registry.asKeyValueRange()) { - singleton->onReload(old == nullptr ? nullptr : old->registry.value(url)); + singleton->reload(old == nullptr ? nullptr : old->registry.value(url)); } } diff --git a/src/core/test/popupwindow.cpp b/src/core/test/popupwindow.cpp index 91dcfa5f..1262044c 100644 --- a/src/core/test/popupwindow.cpp +++ b/src/core/test/popupwindow.cpp @@ -16,8 +16,8 @@ void TestPopupWindow::initiallyVisible() { // NOLINT popup.setParentWindow(&parent); popup.setVisible(true); - parent.onReload(nullptr); - popup.onReload(nullptr); + parent.reload(); + popup.reload(); QVERIFY(popup.isVisible()); QVERIFY(popup.backingWindow()->isVisible()); @@ -36,8 +36,8 @@ void TestPopupWindow::reloadReparent() { // NOLINT popup.setParentWindow(&parent); popup.setVisible(true); - parent.onReload(nullptr); - popup.onReload(nullptr); + parent.reload(); + popup.reload(); // second generation auto newParent = ProxyWindowBase(); @@ -51,8 +51,8 @@ void TestPopupWindow::reloadReparent() { // NOLINT auto spy = QSignalSpy(oldWindow, &QWindow::visibleChanged); - newParent.onReload(&parent); - newPopup.onReload(&popup); + newParent.reload(&parent); + newPopup.reload(&popup); QVERIFY(newPopup.isVisible()); QVERIFY(newPopup.backingWindow()->isVisible()); @@ -69,15 +69,15 @@ void TestPopupWindow::reloadUnparent() { // NOLINT popup.setParentWindow(&parent); popup.setVisible(true); - parent.onReload(nullptr); - popup.onReload(nullptr); + parent.reload(); + popup.reload(); // second generation auto newPopup = ProxyPopupWindow(); // parent not set newPopup.setVisible(true); - newPopup.onReload(&popup); + newPopup.reload(&popup); QVERIFY(!newPopup.isVisible()); QVERIFY(!newPopup.backingWindow()->isVisible()); @@ -88,7 +88,7 @@ void TestPopupWindow::invisibleWithoutParent() { // NOLINT auto popup = ProxyPopupWindow(); popup.setVisible(true); - popup.onReload(nullptr); + popup.reload(); QVERIFY(!popup.isVisible()); } @@ -102,8 +102,8 @@ void TestPopupWindow::moveWithParent() { // NOLINT popup.setRelativeY(10); popup.setVisible(true); - parent.onReload(nullptr); - popup.onReload(nullptr); + parent.reload(); + popup.reload(); QCOMPARE(popup.x(), parent.x() + 10); QCOMPARE(popup.y(), parent.y() + 10); @@ -121,8 +121,8 @@ void TestPopupWindow::attachParentLate() { // NOLINT popup.setVisible(true); - parent.onReload(nullptr); - popup.onReload(nullptr); + parent.reload(); + popup.reload(); QVERIFY(!popup.isVisible()); @@ -139,14 +139,14 @@ void TestPopupWindow::reparentLate() { // NOLINT popup.setParentWindow(&parent); popup.setVisible(true); - parent.onReload(nullptr); - popup.onReload(nullptr); + parent.reload(); + popup.reload(); QCOMPARE(popup.x(), parent.x()); QCOMPARE(popup.y(), parent.y()); auto parent2 = ProxyWindowBase(); - parent2.onReload(nullptr); + parent2.reload(); parent2.backingWindow()->setX(10); parent2.backingWindow()->setY(10); @@ -166,8 +166,8 @@ void TestPopupWindow::xMigrationFix() { // NOLINT popup.setParentWindow(&parent); popup.setVisible(true); - parent.onReload(nullptr); - popup.onReload(nullptr); + parent.reload(); + popup.reload(); QCOMPARE(popup.x(), parent.x()); diff --git a/src/core/variants.cpp b/src/core/variants.cpp index aa3bddef..6c8713b6 100644 --- a/src/core/variants.cpp +++ b/src/core/variants.cpp @@ -64,7 +64,7 @@ void Variants::onReload(QObject* oldInstance) { auto* instance = qobject_cast(instanceObj); - if (instance != nullptr) instance->onReload(oldInstance); + if (instance != nullptr) instance->reload(oldInstance); else Reloadable::reloadChildrenRecursive(instanceObj, oldInstance); } @@ -168,7 +168,7 @@ void Variants::updateVariants() { this->mInstances.insert(variant, instance); if (this->loaded) { - if (auto* reloadable = qobject_cast(instance)) reloadable->onReload(nullptr); + if (auto* reloadable = qobject_cast(instance)) reloadable->reload(nullptr); else Reloadable::reloadChildrenRecursive(instance, nullptr); } } diff --git a/src/wayland/session_lock.cpp b/src/wayland/session_lock.cpp index a99ed252..0bbfe056 100644 --- a/src/wayland/session_lock.cpp +++ b/src/wayland/session_lock.cpp @@ -90,7 +90,7 @@ void WlSessionLock::updateSurfaces(WlSessionLock* old) { instance->setScreen(screen); auto* oldInstance = old == nullptr ? nullptr : old->surfaces.value(screen, nullptr); - instance->onReload(oldInstance); + instance->reload(oldInstance); this->surfaces[screen] = instance; } diff --git a/src/wayland/wlr_layershell.cpp b/src/wayland/wlr_layershell.cpp index 945706f6..dd1fee93 100644 --- a/src/wayland/wlr_layershell.cpp +++ b/src/wayland/wlr_layershell.cpp @@ -197,7 +197,7 @@ void WaylandPanelInterface::onReload(QObject* oldInstance) { QQmlEngine::setContextForObject(this->layer, QQmlEngine::contextForObject(this)); auto* old = qobject_cast(oldInstance); - this->layer->onReload(old != nullptr ? old->layer : nullptr); + this->layer->reload(old != nullptr ? old->layer : nullptr); } QQmlListProperty WaylandPanelInterface::data() { return this->layer->data(); }