core/popupwindow: clean up popup lifecycle and window init

- Makes popup lifecycle less complex
- Creates all QWindows lazily
- May break live reloading of open popups to some degree

src/core/popupanchor.cpp

@@ -28,7 +28,7 @@ void PopupAnchor::markClean() { this->lastState = this->state; }
 void PopupAnchor::markDirty() { this->lastState.reset(); }
 
 QWindow* PopupAnchor::backingWindow() const {
-	return this->mProxyWindow ? this->mProxyWindow->backingWindow() : nullptr;
+	return this->bProxyWindow ? this->bProxyWindow->backingWindow() : nullptr;
 }
 
 void PopupAnchor::setWindowInternal(QObject* window) {
@@ -36,14 +36,14 @@ void PopupAnchor::setWindowInternal(QObject* window) {
 
 	if (this->mWindow) {
 		QObject::disconnect(this->mWindow, nullptr, this, nullptr);
-		QObject::disconnect(this->mProxyWindow, nullptr, this, nullptr);
+		QObject::disconnect(this->bProxyWindow, nullptr, this, nullptr);
 	}
 
 	if (window) {
 		if (auto* proxy = qobject_cast<ProxyWindowBase*>(window)) {
-			this->mProxyWindow = proxy;
+			this->bProxyWindow = proxy;
 		} else if (auto* interface = qobject_cast<WindowInterface*>(window)) {
-			this->mProxyWindow = interface->proxyWindow();
+			this->bProxyWindow = interface->proxyWindow();
 		} else {
 			qWarning() << "Tried to set popup anchor window to" << window
 			           << "which is not a quickshell window.";
@@ -55,7 +55,7 @@ void PopupAnchor::setWindowInternal(QObject* window) {
 		QObject::connect(this->mWindow, &QObject::destroyed, this, &PopupAnchor::onWindowDestroyed);
 
 		QObject::connect(
-		    this->mProxyWindow,
+		    this->bProxyWindow,
 		    &ProxyWindowBase::backerVisibilityChanged,
 		    this,
 		    &PopupAnchor::backingWindowVisibilityChanged
@@ -70,7 +70,7 @@ void PopupAnchor::setWindowInternal(QObject* window) {
 setnull:
 	if (this->mWindow) {
 		this->mWindow = nullptr;
-		this->mProxyWindow = nullptr;
+		this->bProxyWindow = nullptr;
 
 		emit this->windowChanged();
 		emit this->backingWindowVisibilityChanged();
@@ -100,7 +100,7 @@ void PopupAnchor::setItem(QQuickItem* item) {
 
 void PopupAnchor::onWindowDestroyed() {
 	this->mWindow = nullptr;
-	this->mProxyWindow = nullptr;
+	this->bProxyWindow = nullptr;
 	emit this->windowChanged();
 	emit this->backingWindowVisibilityChanged();
 }
@@ -186,11 +186,11 @@ void PopupAnchor::updatePlacement(const QPoint& anchorpoint, const QSize& size)
 }
 
 void PopupAnchor::updateAnchor() {
-	if (this->mItem && this->mProxyWindow) {
+	if (this->mItem && this->bProxyWindow) {
 		auto baseRect =
 		    this->mUserRect.isEmpty() ? this->mItem->boundingRect() : this->mUserRect.qrect();
 
-		auto rect = this->mProxyWindow->contentItem()->mapFromItem(
+		auto rect = this->bProxyWindow->contentItem()->mapFromItem(
 		    this->mItem,
 		    baseRect.marginsRemoved(this->mMargins.qmargins())
 		);

src/core/popupanchor.hpp

@@ -6,6 +6,7 @@
 #include <qnamespace.h>
 #include <qobject.h>
 #include <qpoint.h>
+#include <qproperty.h>
 #include <qqmlintegration.h>
 #include <qquickitem.h>
 #include <qsize.h>
@@ -139,7 +140,9 @@ public:
 	void markDirty();
 
 	[[nodiscard]] QObject* window() const { return this->mWindow; }
-	[[nodiscard]] ProxyWindowBase* proxyWindow() const { return this->mProxyWindow; }
+	[[nodiscard]] QBindable<ProxyWindowBase*> bindableProxyWindow() const {
+		return &this->bProxyWindow;
+	}
 	[[nodiscard]] QWindow* backingWindow() const;
 	void setWindowInternal(QObject* window);
 	void setWindow(QObject* window);
@@ -193,11 +196,12 @@ private slots:
 private:
 	QObject* mWindow = nullptr;
 	QQuickItem* mItem = nullptr;
-	ProxyWindowBase* mProxyWindow = nullptr;
 	PopupAnchorState state;
 	Box mUserRect;
 	Margins mMargins;
 	std::optional<PopupAnchorState> lastState;
+
+	Q_OBJECT_BINDABLE_PROPERTY(PopupAnchor, ProxyWindowBase*, bProxyWindow);
 };
 
 class PopupPositioner {

src/wayland/popupanchor.cpp

@@ -16,7 +16,6 @@ using XdgPositioner = QtWayland::xdg_positioner;
 using qs::wayland::xdg_shell::XdgWmBase;
 
 void WaylandPopupPositioner::reposition(PopupAnchor* anchor, QWindow* window, bool onlyIfDirty) {
-
 	auto* waylandWindow = dynamic_cast<QWaylandWindow*>(window->handle());
 	auto* popupRole = waylandWindow ? waylandWindow->surfaceRole<::xdg_popup>() : nullptr;
 

src/window/popupwindow.cpp

@@ -12,29 +12,74 @@
 
 ProxyPopupWindow::ProxyPopupWindow(QObject* parent): ProxyWindowBase(parent) {
 	this->mVisible = false;
+
 	// clang-format off
-	QObject::connect(&this->mAnchor, &PopupAnchor::windowChanged, this, &ProxyPopupWindow::parentWindowChanged);
+	QObject::connect(&this->mAnchor, &PopupAnchor::windowChanged, this, &ProxyPopupWindow::onParentWindowChanged);
 	QObject::connect(&this->mAnchor, &PopupAnchor::windowRectChanged, this, &ProxyPopupWindow::reposition);
 	QObject::connect(&this->mAnchor, &PopupAnchor::edgesChanged, this, &ProxyPopupWindow::reposition);
 	QObject::connect(&this->mAnchor, &PopupAnchor::gravityChanged, this, &ProxyPopupWindow::reposition);
 	QObject::connect(&this->mAnchor, &PopupAnchor::adjustmentChanged, this, &ProxyPopupWindow::reposition);
-	QObject::connect(&this->mAnchor, &PopupAnchor::backingWindowVisibilityChanged, this, &ProxyPopupWindow::onParentUpdated);
 	// clang-format on
+
+	this->bTargetVisible.setBinding([this] {
+		auto* window = this->mAnchor.bindableProxyWindow().value();
+
+		if (window == this) {
+			qmlWarning(this) << "Anchor assigned to current window";
+			return false;
+		}
+
+		if (!window) return false;
+
+		if (!this->bWantsVisible) return false;
+		return window->bindableBackerVisibility().value();
+	});
+}
+
+void ProxyPopupWindow::targetVisibleChanged() {
+	this->ProxyWindowBase::setVisible(this->bTargetVisible);
 }
 
 void ProxyPopupWindow::completeWindow() {
 	this->ProxyWindowBase::completeWindow();
 
 	// clang-format off
-	QObject::connect(this->window, &QWindow::visibleChanged, this, &ProxyPopupWindow::onVisibleChanged);
+	QObject::connect(this, &ProxyWindowBase::closed, this, &ProxyPopupWindow::onClosed);
 	QObject::connect(this->window, &QWindow::widthChanged, this, &ProxyPopupWindow::reposition);
 	QObject::connect(this->window, &QWindow::heightChanged, this, &ProxyPopupWindow::reposition);
 	// clang-format on
 
+	auto* bw = this->mAnchor.backingWindow();
+
+	if (bw && PopupPositioner::instance()->shouldRepositionOnMove()) {
+		QObject::connect(bw, &QWindow::xChanged, this, &ProxyPopupWindow::reposition);
+		QObject::connect(bw, &QWindow::yChanged, this, &ProxyPopupWindow::reposition);
+		QObject::connect(bw, &QWindow::widthChanged, this, &ProxyPopupWindow::reposition);
+		QObject::connect(bw, &QWindow::heightChanged, this, &ProxyPopupWindow::reposition);
+	}
+
+	this->window->setTransientParent(bw);
 	this->window->setFlag(Qt::ToolTip);
+
+	this->mAnchor.markDirty();
+	PopupPositioner::instance()->reposition(&this->mAnchor, this->window);
 }
 
-void ProxyPopupWindow::postCompleteWindow() { this->updateTransientParent(); }
+void ProxyPopupWindow::postCompleteWindow() {
+	this->ProxyWindowBase::setVisible(this->bTargetVisible);
+}
+
+void ProxyPopupWindow::onClosed() { this->bWantsVisible = false; }
+
+void ProxyPopupWindow::onParentWindowChanged() {
+	// recreate for new parent
+	if (this->bTargetVisible && this->isVisibleDirect()) {
+		this->ProxyWindowBase::setVisibleDirect(false);
+		this->ProxyWindowBase::setVisibleDirect(true);
+	}
+
+	emit this->parentWindowChanged();
+}
 
 void ProxyPopupWindow::setParentWindow(QObject* parent) {
 	qmlWarning(this) << "PopupWindow.parentWindow is deprecated. Use PopupWindow.anchor.window.";
@@ -43,60 +88,13 @@ void ProxyPopupWindow::setParentWindow(QObject* parent) {
 
 QObject* ProxyPopupWindow::parentWindow() const { return this->mAnchor.window(); }
 
-void ProxyPopupWindow::updateTransientParent() {
-	auto* bw = this->mAnchor.backingWindow();
-
-	if (this->window != nullptr && bw != this->window->transientParent()) {
-		if (this->window->transientParent()) {
-			QObject::disconnect(this->window->transientParent(), nullptr, this, nullptr);
-		}
-
-		if (bw && PopupPositioner::instance()->shouldRepositionOnMove()) {
-			QObject::connect(bw, &QWindow::xChanged, this, &ProxyPopupWindow::reposition);
-			QObject::connect(bw, &QWindow::yChanged, this, &ProxyPopupWindow::reposition);
-			QObject::connect(bw, &QWindow::widthChanged, this, &ProxyPopupWindow::reposition);
-			QObject::connect(bw, &QWindow::heightChanged, this, &ProxyPopupWindow::reposition);
-		}
-
-		this->window->setTransientParent(bw);
-	}
-
-	this->updateVisible();
-}
-
-void ProxyPopupWindow::onParentUpdated() { this->updateTransientParent(); }
-
 void ProxyPopupWindow::setScreen(QuickshellScreenInfo* /*unused*/) {
 	qmlWarning(
 	    this
 	) << "Cannot set screen of popup window, as that is controlled by the parent window";
 }
 
-void ProxyPopupWindow::setVisible(bool visible) {
+void ProxyPopupWindow::setVisible(bool visible) { this->bWantsVisible = visible; }
-	if (visible == this->wantsVisible) return;
-	this->wantsVisible = visible;
-	this->updateVisible();
-}
-
-void ProxyPopupWindow::updateVisible() {
-	auto target = this->wantsVisible && this->mAnchor.window() != nullptr
-	           && this->mAnchor.proxyWindow()->isVisibleDirect();
-
-	if (target && this->window != nullptr && !this->window->isVisible()) {
-		PopupPositioner::instance()->reposition(&this->mAnchor, this->window);
-	}
-
-	this->ProxyWindowBase::setVisible(target);
-}
-
-void ProxyPopupWindow::onVisibleChanged() {
-	// If the window was made invisible without its parent becoming invisible
-	// the compositor probably destroyed it. Without this the window won't ever
-	// be able to become visible again.
-	if (this->window->transientParent() && this->window->transientParent()->isVisible()) {
-		this->wantsVisible = this->window->isVisible();
-	}
-}
 
 void ProxyPopupWindow::setRelativeX(qint32 x) {
 	qmlWarning(this) << "PopupWindow.relativeX is deprecated. Use PopupWindow.anchor.rect.x.";
@@ -144,3 +142,5 @@ void ProxyPopupWindow::onPolished() {
 		}
 	}
 }
+
+bool ProxyPopupWindow::deleteOnInvisible() const { return true; }

src/window/popupwindow.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <qobject.h>
+#include <qproperty.h>
 #include <qqmlintegration.h>
 #include <qquickwindow.h>
 #include <qtmetamacros.h>
@@ -88,6 +89,7 @@ public:
 	void completeWindow() override;
 	void postCompleteWindow() override;
 	void onPolished() override;
+	bool deleteOnInvisible() const override;
 
 	void setScreen(QuickshellScreenInfo* screen) override;
 	void setVisible(bool visible) override;
@@ -109,16 +111,24 @@ signals:
 	void relativeYChanged();
 
 private slots:
-	void onVisibleChanged();
+	void onParentWindowChanged();
-	void onParentUpdated();
+	void onClosed();
 	void reposition();
 
 private:
+	void targetVisibleChanged();
+
 	QQuickWindow* parentBackingWindow();
-	void updateTransientParent();
-	void updateVisible();
 
 	PopupAnchor mAnchor {this};
-	bool wantsVisible = false;
 	bool pendingReposition = false;
+
+	Q_OBJECT_BINDABLE_PROPERTY(ProxyPopupWindow, bool, bWantsVisible);
+
+	Q_OBJECT_BINDABLE_PROPERTY(
+	    ProxyPopupWindow,
+	    bool,
+	    bTargetVisible,
+	    &ProxyPopupWindow::targetVisibleChanged
+	);
 };

src/window/proxywindow.cpp

@@ -57,9 +57,10 @@ ProxyWindowBase::ProxyWindowBase(QObject* parent)
 ProxyWindowBase::~ProxyWindowBase() { this->deleteWindow(true); }
 
 void ProxyWindowBase::onReload(QObject* oldInstance) {
-	this->window = this->retrieveWindow(oldInstance);
+	if (this->mVisible) this->window = this->retrieveWindow(oldInstance);
 	auto wasVisible = this->window != nullptr && this->window->isVisible();
-	this->ensureQWindow();
+
+	if (this->mVisible) this->ensureQWindow();
 
 	// The qml engine will leave the WindowInterface as owner of everything
 	// nested in an item, so we have to make sure the interface's children
@@ -76,17 +77,21 @@ void ProxyWindowBase::onReload(QObject* oldInstance) {
 
 	Reloadable::reloadChildrenRecursive(this, oldInstance);
 
-	this->connectWindow();
+	if (this->mVisible) {
-	this->completeWindow();
+		this->connectWindow();
+		this->completeWindow();
+	}
 
 	this->reloadComplete = true;
 
-	emit this->windowConnected();
+	if (this->mVisible) {
-	this->postCompleteWindow();
+		emit this->windowConnected();
+		this->postCompleteWindow();
 
-	if (wasVisible && this->isVisibleDirect()) {
+		if (wasVisible && this->isVisibleDirect()) {
-		emit this->backerVisibilityChanged();
+			this->bBackerVisibility = true;
-		this->onExposed();
+			this->onExposed();
+		}
 	}
 }
 
@@ -272,24 +277,21 @@ void ProxyWindowBase::setVisible(bool visible) {
 
 void ProxyWindowBase::setVisibleDirect(bool visible) {
 	if (this->deleteOnInvisible()) {
-		if (visible == this->isVisibleDirect()) return;
-
 		if (visible) {
+			if (visible == this->isVisibleDirect()) return;
 			this->createWindow();
 			this->polishItems();
 			this->window->setVisible(true);
-			emit this->backerVisibilityChanged();
+			this->bBackerVisibility = true;
 		} else {
-			if (this->window != nullptr) {
+			if (this->window != nullptr) this->window->setVisible(false);
-				this->window->setVisible(false);
+			this->bBackerVisibility = false;
-				emit this->backerVisibilityChanged();
+			this->deleteWindow();
-				this->deleteWindow();
-			}
 		}
 	} else if (this->window != nullptr) {
 		if (visible) this->polishItems();
 		this->window->setVisible(visible);
-		emit this->backerVisibilityChanged();
+		this->bBackerVisibility = visible;
 	}
 }
 

src/window/proxywindow.hpp

@@ -101,6 +101,10 @@ public:
 	virtual void setVisible(bool visible);
 	virtual void setVisibleDirect(bool visible);
 
+	[[nodiscard]] QBindable<bool> bindableBackerVisibility() const {
+		return &this->bBackerVisibility;
+	}
+
 	void schedulePolish();
 
 	[[nodiscard]] virtual qint32 x() const;
@@ -206,6 +210,13 @@ protected:
 	    &ProxyWindowBase::implicitHeightChanged
 	);
 
+	Q_OBJECT_BINDABLE_PROPERTY(
+	    ProxyWindowBase,
+	    bool,
+	    bBackerVisibility,
+	    &ProxyWindowBase::backerVisibilityChanged
+	);
+
 private:
 	void polishItems();
 	void updateMask();

src/window/test/popupwindow.cpp

@@ -13,7 +13,7 @@ void TestPopupWindow::initiallyVisible() { // NOLINT
 	auto parent = ProxyWindowBase();
 	auto popup = ProxyPopupWindow();
 
-	popup.setParentWindow(&parent);
+	popup.anchor()->setWindow(&parent);
 	popup.setVisible(true);
 
 	parent.reload();
@@ -33,7 +33,7 @@ void TestPopupWindow::reloadReparent() { // NOLINT
 	win2->setVisible(true);
 
 	parent.setVisible(true);
-	popup.setParentWindow(&parent);
+	popup.anchor()->setWindow(&parent);
 	popup.setVisible(true);
 
 	parent.reload();
@@ -43,7 +43,7 @@ void TestPopupWindow::reloadReparent() { // NOLINT
 	auto newParent = ProxyWindowBase();
 	auto newPopup = ProxyPopupWindow();
 
-	newPopup.setParentWindow(&newParent);
+	newPopup.anchor()->setWindow(&newParent);
 	newPopup.setVisible(true);
 
 	auto* oldWindow = popup.backingWindow();
@@ -66,7 +66,7 @@ void TestPopupWindow::reloadUnparent() { // NOLINT
 	auto parent = ProxyWindowBase();
 	auto popup = ProxyPopupWindow();
 
-	popup.setParentWindow(&parent);
+	popup.anchor()->setWindow(&parent);
 	popup.setVisible(true);
 
 	parent.reload();
@@ -80,8 +80,7 @@ void TestPopupWindow::reloadUnparent() { // NOLINT
 	newPopup.reload(&popup);
 
 	QVERIFY(!newPopup.isVisible());
-	QVERIFY(!newPopup.backingWindow()->isVisible());
+	QVERIFY(!newPopup.backingWindow() || !newPopup.backingWindow()->isVisible());
-	QCOMPARE(newPopup.backingWindow()->transientParent(), nullptr);
 }
 
 void TestPopupWindow::invisibleWithoutParent() { // NOLINT
@@ -97,9 +96,11 @@ void TestPopupWindow::moveWithParent() { // NOLINT
 	auto parent = ProxyWindowBase();
 	auto popup = ProxyPopupWindow();
 
-	popup.setParentWindow(&parent);
+	popup.anchor()->setWindow(&parent);
-	popup.setRelativeX(10);
+	auto rect = popup.anchor()->rect();
-	popup.setRelativeY(10);
+	rect.x = 10;
+	rect.y = 10;
+	popup.anchor()->setRect(rect);
 	popup.setVisible(true);
 
 	parent.reload();
@@ -126,7 +127,7 @@ void TestPopupWindow::attachParentLate() { // NOLINT
 
 	QVERIFY(!popup.isVisible());
 
-	popup.setParentWindow(&parent);
+	popup.anchor()->setWindow(&parent);
 	QVERIFY(popup.isVisible());
 	QVERIFY(popup.backingWindow()->isVisible());
 	QCOMPARE(popup.backingWindow()->transientParent(), parent.backingWindow());
@@ -136,7 +137,7 @@ void TestPopupWindow::reparentLate() { // NOLINT
 	auto parent = ProxyWindowBase();
 	auto popup = ProxyPopupWindow();
 
-	popup.setParentWindow(&parent);
+	popup.anchor()->setWindow(&parent);
 	popup.setVisible(true);
 
 	parent.reload();
@@ -151,7 +152,7 @@ void TestPopupWindow::reparentLate() { // NOLINT
 	parent2.backingWindow()->setX(10);
 	parent2.backingWindow()->setY(10);
 
-	popup.setParentWindow(&parent2);
+	popup.anchor()->setWindow(&parent2);
 	QVERIFY(popup.isVisible());
 	QVERIFY(popup.backingWindow()->isVisible());
 	QCOMPARE(popup.backingWindow()->transientParent(), parent2.backingWindow());
@@ -163,7 +164,7 @@ void TestPopupWindow::xMigrationFix() { // NOLINT
 	auto parent = ProxyWindowBase();
 	auto popup = ProxyPopupWindow();
 
-	popup.setParentWindow(&parent);
+	popup.anchor()->setWindow(&parent);
 	popup.setVisible(true);
 
 	parent.reload();