core/window: backing windows can now be destroyed and recreated

This fixes a crash in layershells and the setVisible crash on nvidia.
This commit is contained in:
outfoxxed 2024-03-27 00:44:13 -07:00
parent b6dc6967a1
commit 3a0381dcbe
Signed by: outfoxxed
GPG key ID: 4C88A185FB89301E
16 changed files with 257 additions and 112 deletions

View file

@ -122,6 +122,32 @@ void EngineGeneration::registerIncubationController(QQmlIncubationController* co
}
}
void EngineGeneration::deregisterIncubationController(QQmlIncubationController* controller) {
QObject* obj = nullptr;
this->incubationControllers.removeIf([&](QPair<QQmlIncubationController*, QObject*> other) {
if (controller == other.first) {
obj = other.second;
return true;
} else return false;
});
if (obj == nullptr) {
qCWarning(logIncubator) << "Failed to deregister incubation controller" << controller
<< "as it was not registered to begin with";
qCWarning(logIncubator) << "Current registered incuabation controllers"
<< this->incubationControllers;
} else {
QObject::disconnect(obj, nullptr, this, nullptr);
qCDebug(logIncubator) << "Deregistered incubation controller" << controller;
}
if (this->engine.incubationController() == controller) {
qCDebug(logIncubator
) << "Destroyed incubation controller was currently active, reassigning from pool";
this->assignIncubationController();
}
}
void EngineGeneration::incubationControllerDestroyed() {
auto* sender = this->sender();
QQmlIncubationController* controller = nullptr;
@ -150,8 +176,9 @@ void EngineGeneration::incubationControllerDestroyed() {
}
void EngineGeneration::assignIncubationController() {
auto* controller = this->incubationControllers.first().first;
if (controller == nullptr) controller = &this->delayedIncubationController;
QQmlIncubationController* controller = nullptr;
if (this->incubationControllers.isEmpty()) controller = &this->delayedIncubationController;
else controller = this->incubationControllers.first().first;
qCDebug(logIncubator) << "Assigning incubation controller to engine:" << controller
<< "fallback:" << (controller == &this->delayedIncubationController);
@ -162,9 +189,14 @@ void EngineGeneration::assignIncubationController() {
EngineGeneration* EngineGeneration::findObjectGeneration(QObject* object) {
while (object != nullptr) {
auto* context = QQmlEngine::contextForObject(object);
if (auto* generation = g_generations.value(context->engine())) {
return generation;
if (context != nullptr) {
if (auto* generation = g_generations.value(context->engine())) {
return generation;
}
}
object = object->parent();
}
return nullptr;

View file

@ -28,6 +28,7 @@ public:
void setWatchingFiles(bool watching);
void registerIncubationController(QQmlIncubationController* controller);
void deregisterIncubationController(QQmlIncubationController* controller);
static EngineGeneration* findObjectGeneration(QObject* object);

View file

@ -15,15 +15,19 @@ ProxyPopupWindow::ProxyPopupWindow(QObject* parent): ProxyWindowBase(parent) {
this->mVisible = false;
}
void ProxyPopupWindow::setupWindow() {
this->ProxyWindowBase::setupWindow();
void ProxyPopupWindow::completeWindow() {
this->ProxyWindowBase::completeWindow();
this->window->setFlag(Qt::ToolTip);
this->updateTransientParent();
}
void ProxyPopupWindow::postCompleteWindow() {}
qint32 ProxyPopupWindow::x() const {
return this->ProxyWindowBase::x() + 1; // QTBUG-121550
// QTBUG-121550
auto basepos = this->mParentProxyWindow == nullptr ? 0 : this->mParentProxyWindow->x();
return basepos + this->mRelativeX;
}
void ProxyPopupWindow::setParentWindow(QObject* parent) {
@ -58,7 +62,7 @@ void ProxyPopupWindow::setParentWindow(QObject* parent) {
QObject::connect(this->mParentProxyWindow, &ProxyWindowBase::xChanged, this, &ProxyPopupWindow::updateX);
QObject::connect(this->mParentProxyWindow, &ProxyWindowBase::yChanged, this, &ProxyPopupWindow::updateY);
QObject::connect(this->mParentProxyWindow, &ProxyWindowBase::windowConnected, this, &ProxyPopupWindow::onParentConnected);
QObject::connect(this->mParentProxyWindow, &ProxyWindowBase::backerVisibilityChanged, this, &ProxyPopupWindow::onParentUpdated);
// clang-format on
}
@ -68,18 +72,19 @@ void ProxyPopupWindow::setParentWindow(QObject* parent) {
QObject* ProxyPopupWindow::parentWindow() const { return this->mParentWindow; }
void ProxyPopupWindow::updateTransientParent() {
if (this->window == nullptr) return;
this->updateX();
this->updateY();
this->window->setTransientParent(
this->mParentProxyWindow == nullptr ? nullptr : this->mParentProxyWindow->backingWindow()
);
if (this->window != nullptr) {
this->window->setTransientParent(
this->mParentProxyWindow == nullptr ? nullptr : this->mParentProxyWindow->backingWindow()
);
}
this->updateVisible();
}
void ProxyPopupWindow::onParentConnected() { this->updateTransientParent(); }
void ProxyPopupWindow::onParentUpdated() { this->updateTransientParent(); }
void ProxyPopupWindow::onParentDestroyed() {
this->mParentWindow = nullptr;
@ -99,7 +104,8 @@ void ProxyPopupWindow::setVisible(bool visible) {
}
void ProxyPopupWindow::updateVisible() {
auto target = this->wantsVisible && this->mParentWindow != nullptr;
auto target = this->wantsVisible && this->mParentWindow != nullptr
&& this->mParentProxyWindow->isVisibleDirect();
if (target && this->window != nullptr && !this->window->isVisible()) {
this->updateX(); // QTBUG-121550
@ -127,13 +133,12 @@ qint32 ProxyPopupWindow::relativeY() const { return this->mRelativeY; }
void ProxyPopupWindow::updateX() {
if (this->mParentWindow == nullptr || this->window == nullptr) return;
// use the backing window's x to account for popups in popups with overridden x positions
auto target = this->mParentProxyWindow->backingWindow()->x() + this->relativeX();
auto target = this->x() - 1; // QTBUG-121550
auto reshow = this->window->isVisible() && (this->window->x() != target && this->x() != target);
if (reshow) this->window->setVisible(false);
this->window->setX(target - 1); // -1 due to QTBUG-121550
if (reshow && this->wantsVisible) this->window->setVisible(true);
auto reshow = this->isVisibleDirect() && (this->window->x() != target && this->x() != target);
if (reshow) this->setVisibleDirect(false);
if (this->window != nullptr) this->window->setX(target);
if (reshow && this->wantsVisible) this->setVisibleDirect(true);
}
void ProxyPopupWindow::updateY() {
@ -141,11 +146,11 @@ void ProxyPopupWindow::updateY() {
auto target = this->mParentProxyWindow->y() + this->relativeY();
auto reshow = this->window->isVisible() && this->window->y() != target;
auto reshow = this->isVisibleDirect() && this->window->y() != target;
if (reshow) {
this->window->setVisible(false);
this->setVisibleDirect(false);
this->updateX(); // QTBUG-121550
}
this->window->setY(target);
if (reshow && this->wantsVisible) this->window->setVisible(true);
if (this->window != nullptr) this->window->setY(target);
if (reshow && this->wantsVisible) this->setVisibleDirect(true);
}

View file

@ -62,7 +62,8 @@ class ProxyPopupWindow: public ProxyWindowBase {
public:
explicit ProxyPopupWindow(QObject* parent = nullptr);
void setupWindow() override;
void completeWindow() override;
void postCompleteWindow() override;
void setScreen(QuickshellScreenInfo* screen) override;
void setVisible(bool visible) override;
@ -84,7 +85,7 @@ signals:
void relativeYChanged();
private slots:
void onParentConnected();
void onParentUpdated();
void onParentDestroyed();
void updateX();
void updateY();

View file

@ -23,51 +23,81 @@ ProxyWindowBase::ProxyWindowBase(QObject* parent)
QQmlEngine::setObjectOwnership(this->mContentItem, QQmlEngine::CppOwnership);
this->mContentItem->setParent(this);
// clang-format off
QObject::connect(this, &ProxyWindowBase::widthChanged, this, &ProxyWindowBase::onWidthChanged);
QObject::connect(this, &ProxyWindowBase::heightChanged, this, &ProxyWindowBase::onHeightChanged);
QObject::connect(this, &ProxyWindowBase::maskChanged, this, &ProxyWindowBase::onMaskChanged);
QObject::connect(this, &ProxyWindowBase::widthChanged, this, &ProxyWindowBase::onMaskChanged);
QObject::connect(this, &ProxyWindowBase::heightChanged, this, &ProxyWindowBase::onMaskChanged);
// clang-format on
}
ProxyWindowBase::~ProxyWindowBase() {
if (this->window != nullptr) {
this->window->deleteLater();
ProxyWindowBase::~ProxyWindowBase() { this->deleteWindow(); }
void ProxyWindowBase::onReload(QObject* oldInstance) {
this->window = this->retrieveWindow(oldInstance);
auto wasVisible = this->window != nullptr && this->window->isVisible();
if (this->window == nullptr) this->window = new QQuickWindow();
Reloadable::reloadRecursive(this->mContentItem, oldInstance);
this->connectWindow();
this->completeWindow();
emit this->windowConnected();
this->postCompleteWindow();
if (wasVisible && this->isVisibleDirect()) emit this->backerVisibilityChanged();
}
void ProxyWindowBase::postCompleteWindow() { this->setVisible(this->mVisible); }
QQuickWindow* ProxyWindowBase::createQQuickWindow() { return new QQuickWindow(); }
void ProxyWindowBase::createWindow() {
if (this->window != nullptr) return;
this->window = this->createQQuickWindow();
this->connectWindow();
this->completeWindow();
emit this->windowConnected();
}
void ProxyWindowBase::deleteWindow() {
if (auto* window = this->disownWindow()) {
if (auto* generation = EngineGeneration::findObjectGeneration(this)) {
generation->deregisterIncubationController(window->incubationController());
}
window->deleteLater();
}
}
void ProxyWindowBase::onReload(QObject* oldInstance) {
this->window = this->createWindow(oldInstance);
QQuickWindow* ProxyWindowBase::disownWindow() {
if (this->window == nullptr) return nullptr;
QObject::disconnect(this->window, nullptr, this, nullptr);
this->mContentItem->setParentItem(nullptr);
auto* window = this->window;
this->window = nullptr;
return window;
}
QQuickWindow* ProxyWindowBase::retrieveWindow(QObject* oldInstance) {
auto* old = qobject_cast<ProxyWindowBase*>(oldInstance);
return old == nullptr ? nullptr : old->disownWindow();
}
void ProxyWindowBase::connectWindow() {
if (auto* generation = EngineGeneration::findObjectGeneration(this)) {
// All windows have effectively the same incubation controller so it dosen't matter
// which window it belongs to. We do want to replace the delay one though.
generation->registerIncubationController(this->window->incubationController());
}
this->setupWindow();
Reloadable::reloadRecursive(this->mContentItem, oldInstance);
this->mContentItem->setParentItem(this->window->contentItem());
this->mContentItem->setWidth(this->width());
this->mContentItem->setHeight(this->height());
// without this the dangling screen pointer wont be updated to a real screen
emit this->screenChanged();
emit this->windowConnected();
this->window->setVisible(this->mVisible);
}
QQuickWindow* ProxyWindowBase::createWindow(QObject* oldInstance) {
auto* old = qobject_cast<ProxyWindowBase*>(oldInstance);
if (old == nullptr || old->window == nullptr) {
return new QQuickWindow();
} else {
return old->disownWindow();
}
}
void ProxyWindowBase::setupWindow() {
// clang-format off
QObject::connect(this->window, &QWindow::visibilityChanged, this, &ProxyWindowBase::visibleChanged);
QObject::connect(this->window, &QWindow::xChanged, this, &ProxyWindowBase::xChanged);
@ -76,12 +106,10 @@ void ProxyWindowBase::setupWindow() {
QObject::connect(this->window, &QWindow::heightChanged, this, &ProxyWindowBase::heightChanged);
QObject::connect(this->window, &QWindow::screenChanged, this, &ProxyWindowBase::screenChanged);
QObject::connect(this->window, &QQuickWindow::colorChanged, this, &ProxyWindowBase::colorChanged);
QObject::connect(this, &ProxyWindowBase::maskChanged, this, &ProxyWindowBase::onMaskChanged);
QObject::connect(this, &ProxyWindowBase::widthChanged, this, &ProxyWindowBase::onMaskChanged);
QObject::connect(this, &ProxyWindowBase::heightChanged, this, &ProxyWindowBase::onMaskChanged);
// clang-format on
}
void ProxyWindowBase::completeWindow() {
if (this->mScreen != nullptr && this->window->screen() != this->mScreen) {
if (this->window->isVisible()) this->window->setVisible(false);
this->window->setScreen(this->mScreen);
@ -95,16 +123,24 @@ void ProxyWindowBase::setupWindow() {
// notify initial x and y positions
emit this->xChanged();
emit this->yChanged();
this->mContentItem->setParentItem(this->window->contentItem());
this->mContentItem->setWidth(this->width());
this->mContentItem->setHeight(this->height());
// without this the dangling screen pointer wont be updated to a real screen
emit this->screenChanged();
}
QQuickWindow* ProxyWindowBase::disownWindow() {
QObject::disconnect(this->window, nullptr, this, nullptr);
this->mContentItem->setParentItem(nullptr);
auto* window = this->window;
this->window = nullptr;
return window;
bool ProxyWindowBase::deleteOnInvisible() const {
#ifdef NVIDIA_COMPAT
// Nvidia drivers and Qt do not play nice when hiding and showing a window
// so for nvidia compatibility we can never reuse windows if they have been
// hidden.
return true;
#else
return false;
#endif
}
QQuickWindow* ProxyWindowBase::backingWindow() const { return this->window; }
@ -112,14 +148,38 @@ QQuickItem* ProxyWindowBase::contentItem() const { return this->mContentItem; }
bool ProxyWindowBase::isVisible() const {
if (this->window == nullptr) return this->mVisible;
else return this->isVisibleDirect();
}
bool ProxyWindowBase::isVisibleDirect() const {
if (this->window == nullptr) return false;
else return this->window->isVisible();
}
void ProxyWindowBase::setVisible(bool visible) {
if (this->window == nullptr) {
this->mVisible = visible;
emit this->visibleChanged();
} else this->window->setVisible(visible);
this->mVisible = visible;
this->setVisibleDirect(visible);
}
void ProxyWindowBase::setVisibleDirect(bool visible) {
if (this->deleteOnInvisible()) {
if (visible == this->isVisibleDirect()) return;
if (visible) {
this->createWindow();
this->window->setVisible(true);
emit this->backerVisibilityChanged();
} else {
if (this->window != nullptr) {
this->window->setVisible(false);
emit this->backerVisibilityChanged();
this->deleteWindow();
}
}
} else if (this->window != nullptr) {
this->window->setVisible(visible);
emit this->backerVisibilityChanged();
}
}
qint32 ProxyWindowBase::x() const {
@ -138,8 +198,8 @@ qint32 ProxyWindowBase::width() const {
}
void ProxyWindowBase::setWidth(qint32 width) {
this->mWidth = width;
if (this->window == nullptr) {
this->mWidth = width;
emit this->widthChanged();
} else this->window->setWidth(width);
}
@ -150,8 +210,8 @@ qint32 ProxyWindowBase::height() const {
}
void ProxyWindowBase::setHeight(qint32 height) {
this->mHeight = height;
if (this->window == nullptr) {
this->mHeight = height;
emit this->heightChanged();
} else this->window->setHeight(height);
}
@ -170,10 +230,10 @@ void ProxyWindowBase::setScreen(QuickshellScreenInfo* screen) {
this->mScreen = qscreen;
emit this->screenChanged();
} else {
auto reshow = this->window->isVisible();
if (reshow) this->window->setVisible(false);
this->window->setScreen(qscreen);
if (reshow) this->window->setVisible(true);
auto reshow = this->isVisibleDirect();
if (reshow) this->setVisibleDirect(false);
if (this->window != nullptr) this->window->setScreen(qscreen);
if (reshow) this->setVisibleDirect(true);
}
}

View file

@ -12,6 +12,7 @@
#include <qtmetamacros.h>
#include <qtypes.h>
#include "qmlglobal.hpp"
#include "qmlscreen.hpp"
#include "region.hpp"
#include "reload.hpp"
@ -54,18 +55,26 @@ public:
void operator=(ProxyWindowBase&&) = delete;
void onReload(QObject* oldInstance) override;
virtual QQuickWindow* createWindow(QObject* oldInstance);
virtual void setupWindow();
void createWindow();
void deleteWindow();
// Disown the backing window and delete all its children.
virtual QQuickWindow* disownWindow();
virtual QQuickWindow* retrieveWindow(QObject* oldInstance);
virtual QQuickWindow* createQQuickWindow();
virtual void connectWindow();
virtual void completeWindow();
virtual void postCompleteWindow();
[[nodiscard]] virtual bool deleteOnInvisible() const;
[[nodiscard]] QQuickWindow* backingWindow() const;
[[nodiscard]] QQuickItem* contentItem() const;
[[nodiscard]] virtual bool isVisible() const;
[[nodiscard]] virtual bool isVisibleDirect() const;
virtual void setVisible(bool visible);
virtual void setVisibleDirect(bool visible);
[[nodiscard]] virtual qint32 x() const;
[[nodiscard]] virtual qint32 y() const;
@ -90,10 +99,12 @@ public:
signals:
void windowConnected();
void visibleChanged();
void backerVisibilityChanged();
void xChanged();
void yChanged();
void widthChanged();
void heightChanged();
void windowTransformChanged();
void screenChanged();
void colorChanged();
void maskChanged();

View file

@ -1,15 +1,7 @@
function (qs_test name)
add_executable(${name} ${ARGN})
target_link_libraries(${name} PRIVATE ${QT_DEPS} Qt6::Test)
target_link_libraries(${name} PRIVATE ${QT_DEPS} Qt6::Test quickshell-core)
add_test(NAME ${name} WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}" COMMAND $<TARGET_FILE:${name}>)
endfunction()
qs_test(popupwindow
popupwindow.cpp
../popupwindow.cpp
../proxywindow.cpp
../qmlscreen.cpp
../region.cpp
../reload.cpp
../windowinterface.cpp
)
qs_test(popupwindow popupwindow.cpp)

View file

@ -1,6 +1,5 @@
#include "popupwindow.hpp"
#include <qlogging.h>
#include <qquickwindow.h>
#include <qsignalspy.h>
#include <qtest.h>
@ -52,7 +51,6 @@ void TestPopupWindow::reloadReparent() { // NOLINT
auto spy = QSignalSpy(oldWindow, &QWindow::visibleChanged);
qDebug() << "reload";
newParent.onReload(&parent);
newPopup.onReload(&popup);