From 18803af47e3211c3c496751ae9b8581578455790 Mon Sep 17 00:00:00 2001 From: ck Date: Thu, 15 Aug 2024 15:33:35 +0800 Subject: [PATCH 1/2] fix: crashed on floatingmessage closed cherry-pick from waylib#436 --- src/private/dbackdropnode.cpp | 99 ++++++++++++++++----------- src/private/dquickbackdropblitter.cpp | 35 ++++------ 2 files changed, 73 insertions(+), 61 deletions(-) diff --git a/src/private/dbackdropnode.cpp b/src/private/dbackdropnode.cpp index 37183bbee..91c398ab8 100644 --- a/src/private/dbackdropnode.cpp +++ b/src/private/dbackdropnode.cpp @@ -161,34 +161,51 @@ class Q_DECL_HIDDEN DataManager : public DataManagerBase return static_cast(parent()); } - Data *resolve(Data *data, DataKeys&&... keys) { - if (data && get()->check(data->data, std::forward(keys)...) - && dataList.contains(data)) { - return data; + std::weak_ptr resolve(std::weak_ptr data, DataKeys&&... keys) { + struct TryClean { + TryClean(DataManager *m) + : manager(m) {} + ~TryClean() { + manager->tryClean(); + } + DataManager *manager; + }; + + TryClean cleanJob(this); + Q_UNUSED(cleanJob) + + { + auto d = data.lock(); + if (d && dataList.contains(d)) { + if (get()->check(d->data, std::forward(keys)...)) { + d->released = 0; + return data; + } + release(data); + } } - if (data) - release(data); - for (auto data : dataList) { + for (auto data : std::as_const(dataList)) { if (get()->check(data->data, std::forward(keys)...)) { data->released = 0; return data; } } - data = new Data(); - if ((data->data = get()->create(std::forward(keys)...))) { - dataList.append(data); - return data; + auto newData = std::shared_ptr(new Data()); + if ((newData->data = get()->create(std::forward(keys)...))) { + dataList.append(newData); + return newData; } - delete data; - return nullptr; + return {}; } - inline void release(Data *data) { - data->released++; - ensureCleanJob(); + inline void release(std::weak_ptr data) { + auto d = data.lock(); + if (!d) + return; + d->released++; } protected: @@ -202,21 +219,18 @@ class Q_DECL_HIDDEN DataManager : public DataManagerBase manager->cleanJob = nullptr; - QList tmp; + QList> tmp; std::swap(manager->dataList, tmp); manager->dataList.reserve(tmp.size()); - for (Data *data : tmp) { + for (const auto &data : std::as_const(tmp)) { if (data->released > 2) { manager->get()->destroy(data->data); - delete data; } else { manager->dataList << data; - if (data->released > 0) { - data->released++; - manager->ensureCleanJob(); - } + if (data->released > 0) + ++data->released; } } } @@ -224,8 +238,8 @@ class Q_DECL_HIDDEN DataManager : public DataManagerBase QPointer manager; }; - inline void ensureCleanJob() { - if (!cleanJob) { + inline void tryClean() { + if (Q_LIKELY(!cleanJob)) { cleanJob = new CleanJob(this); owner()->scheduleRenderJob(cleanJob, QQuickWindow::AfterRenderingStage); } @@ -246,13 +260,12 @@ class Q_DECL_HIDDEN DataManager : public DataManagerBase using QObject::deleteLater; ~DataManager() { - for (auto data : dataList) { + for (auto data : std::as_const(dataList)) { Derive::destroy(data->data); - delete data; } } - QList dataList; + QList> dataList; QRunnable *cleanJob = nullptr; }; @@ -706,8 +719,9 @@ class Q_DECL_HIDDEN RhiNode : public DBackdropNode { if (oldManager != manager) { sgTexture()->setTexture(nullptr); - if (oldManager && texture) + if (oldManager) oldManager->release(texture); + texture.reset(); } rhi = rhi->resolve(rhi, window); @@ -747,10 +761,11 @@ class Q_DECL_HIDDEN RhiNode : public DBackdropNode { } texture = manager->resolve(texture, ct ? ct->format() : glfbManager->format(), itemPixelSize); - if (Q_UNLIKELY(!texture)) { + if (Q_UNLIKELY(texture.expired())) { reset(); return; } + auto texture = this->texture.lock(); Q_ASSERT(texture->data); if (renderData) { @@ -800,6 +815,7 @@ class Q_DECL_HIDDEN RhiNode : public DBackdropNode { texture->data->pixelSize().height() / float(m_rect.height() * devicePixelRatio)}); rhi->render(renderData->rt.get()); } else { + auto rhi = this->rhi->rhi(); QPointF sourcePos = renderMatrix.map(m_rect.topLeft()) * devicePixelRatio; if (ct) { @@ -914,9 +930,9 @@ class Q_DECL_HIDDEN RhiNode : public DBackdropNode { if (!sgTexture()->rhiTexture() && notifyTexture) doNotifyTextureChanged(); sgTexture()->setTexture(nullptr); - if (texture && manager) - manager->release(texture); - texture = nullptr; + if (!texture.expired() && manager) + manager->release(texture.lock()); + texture.reset(); #ifndef QT_NO_OPENGL if (glfbManager) @@ -929,14 +945,14 @@ class Q_DECL_HIDDEN RhiNode : public DBackdropNode { renderData.reset(); node.reset(); manager = nullptr; - texture = nullptr; + texture.reset(); #ifndef QT_NO_OPENGL glfbManager = nullptr; #endif } DataManagerPointer manager; - RhiTextureManager::Data *texture = nullptr; + std::weak_ptr texture; DataManagerPointer rhi; #ifndef QT_NO_OPENGL @@ -1078,7 +1094,7 @@ class Q_DECL_HIDDEN SoftwareNode : public DBackdropNode { QImage toImage() const override { - return image ? *image->data : QImage(); + return image.expired() ? QImage() : *image.lock()->data; } void render(const RenderState *state) override { @@ -1097,8 +1113,9 @@ class Q_DECL_HIDDEN SoftwareNode : public DBackdropNode { if (oldManager != manager) { texture()->setTexture(nullptr); - if (oldManager && image) + if (oldManager) oldManager->release(image); + image.reset(); } const bool hasRotation = matrix.flags().testAnyFlags(QMatrix4x4::Rotation2D | QMatrix4x4::Rotation); @@ -1137,6 +1154,7 @@ class Q_DECL_HIDDEN SoftwareNode : public DBackdropNode { image = manager->resolve(image, sourceImage.format(), pixelSize); } + auto image = this->image.lock(); painter.begin(image->data); painter.setRenderHint(QPainter::SmoothPixmapTransform); painter.setCompositionMode(QPainter::CompositionMode_Source); @@ -1170,20 +1188,19 @@ class Q_DECL_HIDDEN SoftwareNode : public DBackdropNode { if (!texture()->image().isNull() && notifyTexture) doNotifyTextureChanged(); texture()->setTexture(nullptr); - if (image && manager) + if (manager) manager->release(image); - image = nullptr; + image.reset(); } void destroy() { reset(false); manager = nullptr; - image = nullptr; } friend class DBackdropNode; DataManagerPointer manager; - QImageManager::Data *image = nullptr; + std::weak_ptr image; QPainter painter; }; diff --git a/src/private/dquickbackdropblitter.cpp b/src/private/dquickbackdropblitter.cpp index 59ba15652..8fb6929e0 100644 --- a/src/private/dquickbackdropblitter.cpp +++ b/src/private/dquickbackdropblitter.cpp @@ -44,6 +44,10 @@ class Q_DECL_HIDDEN DQuickBackdropBlitterPrivate : public DCORE_NAMESPACE::DObje } + ~DQuickBackdropBlitterPrivate() { + cleanTextureProvider(); + } + static inline DQuickBackdropBlitterPrivate *get(DQuickBackdropBlitter *qq) { return qq->d_func(); } @@ -59,6 +63,7 @@ class Q_DECL_HIDDEN DQuickBackdropBlitterPrivate : public DCORE_NAMESPACE::DObje } BlitTextureProvider *ensureTextureProvider() const; + void cleanTextureProvider(); D_DECLARE_PUBLIC(DQuickBackdropBlitter) Content *content; @@ -178,6 +183,14 @@ BlitTextureProvider *DQuickBackdropBlitterPrivate::ensureTextureProvider() const return tp; } +void DQuickBackdropBlitterPrivate::cleanTextureProvider() +{ + if (tp) { + QQuickWindowQObjectCleanupJob::schedule(q_func()->window(), tp); + tp = nullptr; + } +} + DQuickBackdropBlitter::DQuickBackdropBlitter(QQuickItem *parent) : QQuickItem(parent) , DObject(*new DQuickBackdropBlitterPrivate(this)) @@ -240,23 +253,8 @@ static void onTextureChanged(DBackdropNode *node, void *data) { if (!d->tp) return; - const bool textureChanged = node->texture() != d->tp->texture(); d->tp->setTexture(node->texture()); - - struct Notifer : public QRunnable { - void run() override { - if (tp) - Q_EMIT tp->textureChanged(); - } - - QPointer tp; - }; - - auto notifer = new Notifer(); - notifer->tp = d->tp; - - if (textureChanged) - d->content->window()->scheduleRenderJob(notifer, QQuickWindow::BeforeSynchronizingStage); + Q_EMIT d->tp->textureChanged(); } QSGNode *DQuickBackdropBlitter::updatePaintNode(QSGNode *oldNode, QQuickItem::UpdatePaintNodeData *oldData) @@ -307,10 +305,7 @@ void DQuickBackdropBlitter::geometryChange(const QRectF &newGeometry, const QRec void DQuickBackdropBlitter::releaseResources() { D_D(DQuickBackdropBlitter); - if (d->tp) { - QQuickWindowQObjectCleanupJob::schedule(window(), d->tp); - d->tp = nullptr; - } + d->cleanTextureProvider(); } DQUICK_END_NAMESPACE From 113ec6949fa01933276ba6e4c32fa08a4a450d94 Mon Sep 17 00:00:00 2001 From: ck Date: Thu, 15 Aug 2024 15:35:56 +0800 Subject: [PATCH 2/2] fix: cashed on onTextureChanged invalid BlitTextureProvider pointer --- src/private/dquickbackdropblitter.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/private/dquickbackdropblitter.cpp b/src/private/dquickbackdropblitter.cpp index 8fb6929e0..646c12e5a 100644 --- a/src/private/dquickbackdropblitter.cpp +++ b/src/private/dquickbackdropblitter.cpp @@ -276,6 +276,10 @@ QSGNode *DQuickBackdropBlitter::updatePaintNode(QSGNode *oldNode, QQuickItem::Up node->setContentItem(d->container); node->setTextureChangedCallback(onTextureChanged, d); + connect(this, &QObject::destroyed, this, [node](){ + // fix callback crashed... + node->setTextureChangedCallback(nullptr, nullptr); + }); node->resize(size()); onTextureChanged(node, d);