From 06e1c238a09165208fed384ad94c457ffbee08a2 Mon Sep 17 00:00:00 2001 From: MinyazevR <89993880+MinyazevR@users.noreply.github.com> Date: Mon, 22 Jul 2024 17:58:02 +0300 Subject: [PATCH] Fix macos crash (#1746) Fixed #1707 Problem description: dragging an element using QDrag->exec() crashed the program, because by the time DraggableElement was called, it automatically managed to free itself. Even though the palette has been updated with Qt::QueuedConnection, which made it possible to call mousePressEvent using drag->exec before cleaning the elements, the drag->exec call is not blocked in macOS. As a solution, it was possible to block the event loop for drag->exec, but instead it was decided to stupidly emit to update the palette after the corresponding operations (since cleaning is put after mousePressEvent in the event loop). P.S. No problems with memory leaks of mPaletteElements and mPaletteItems were found, they are cleaned when QTreeWidget::clear() is called. However, they have problems with hashing. --- qrgui/mainWindow/palette/draggableElement.cpp | 9 +++++++++ qrgui/mainWindow/palette/draggableElement.h | 6 ++++++ qrgui/mainWindow/palette/paletteTreeWidget.cpp | 14 ++++++++++++++ qrgui/mainWindow/palette/paletteTreeWidget.h | 6 ++++++ qrgui/mainWindow/palette/paletteTreeWidgets.cpp | 13 ++++++++++++- qrgui/mainWindow/palette/paletteTreeWidgets.h | 2 ++ qrtranslations/fr/qrgui_mainWindow_fr.ts | 6 +++--- qrtranslations/ru/qrgui_mainWindow_ru.ts | 6 +++--- 8 files changed, 55 insertions(+), 7 deletions(-) diff --git a/qrgui/mainWindow/palette/draggableElement.cpp b/qrgui/mainWindow/palette/draggableElement.cpp index 34de75bbd2..84cc026b3a 100644 --- a/qrgui/mainWindow/palette/draggableElement.cpp +++ b/qrgui/mainWindow/palette/draggableElement.cpp @@ -56,6 +56,7 @@ DraggableElement::DraggableElement(MainWindow &mainWindow , mEditorManagerProxy(editorManagerProxy) , mMainWindow(mainWindow) { + readyForDelete = true; QHBoxLayout *layout = new QHBoxLayout(this); layout->setContentsMargins(0, 4, 0, 4); @@ -97,6 +98,11 @@ DraggableElement::DraggableElement(MainWindow &mainWindow setObjectName(mData.name()); } +bool DraggableElement::getReadyForDelete() const +{ + return readyForDelete; +} + QIcon DraggableElement::icon() const { return mData.icon(); @@ -375,6 +381,7 @@ void DraggableElement::mousePressEvent(QMouseEvent *event) menu->exec(QCursor::pos()); } } else { + readyForDelete = false; QDrag *drag = new QDrag(this); drag->setMimeData(mimeData(elementId)); @@ -385,6 +392,8 @@ void DraggableElement::mousePressEvent(QMouseEvent *event) } drag->exec(Qt::CopyAction); + readyForDelete = true; + emit signalReadyForDelete(); } } diff --git a/qrgui/mainWindow/palette/draggableElement.h b/qrgui/mainWindow/palette/draggableElement.h index 88a4d3c6bd..acedb43ec6 100644 --- a/qrgui/mainWindow/palette/draggableElement.h +++ b/qrgui/mainWindow/palette/draggableElement.h @@ -66,6 +66,11 @@ class DraggableElement : public QWidget /// Returns a mime data instance binded with object during drag-and-drop. QMimeData *mimeData(const Id &elementId) const; + bool getReadyForDelete() const; + +signals: + void signalReadyForDelete(); + private slots: void changePropertiesPaletteActionTriggered(); void changeDynamicPropertiesPaletteActionTriggered(); @@ -104,6 +109,7 @@ private slots: MainWindow &mMainWindow; Id mDeletedElementId; bool mIsRootDiagramNode {}; + bool readyForDelete; }; } diff --git a/qrgui/mainWindow/palette/paletteTreeWidget.cpp b/qrgui/mainWindow/palette/paletteTreeWidget.cpp index 208697a3e2..90f4ae8ca3 100644 --- a/qrgui/mainWindow/palette/paletteTreeWidget.cpp +++ b/qrgui/mainWindow/palette/paletteTreeWidget.cpp @@ -35,6 +35,18 @@ PaletteTreeWidget::PaletteTreeWidget(PaletteTree &palette, MainWindow &mainWindo mEditorManager = &editorManagerProxy; } +bool PaletteTreeWidget::readyToRefresh() +{ + for (auto& draggableElement: mDraggableElements) { + if (!draggableElement->getReadyForDelete()) { + connect(draggableElement, &DraggableElement::signalReadyForDelete, + this, &PaletteTreeWidget::signalReadyToRefresh); + return false; + } + } + return true; +} + void PaletteTreeWidget::addGroups(QList>> &groups , QMap const &descriptions , bool hideIfEmpty @@ -45,6 +57,7 @@ void PaletteTreeWidget::addGroups(QList>> & mPaletteElements.clear(); mElementsSet.clear(); mItemsVisible.clear(); + mDraggableElements.clear(); if (groups.isEmpty() && hideIfEmpty) { hide(); @@ -96,6 +109,7 @@ void PaletteTreeWidget::addItemType(const PaletteElement &data, QTreeWidgetItem mElementsSet.insert(data); mPaletteElements.insert(data.id(), element); mPaletteItems.insert(data.id(), leaf); + mDraggableElements.append(element); parent->addChild(leaf); setItemWidget(leaf, 0, element); diff --git a/qrgui/mainWindow/palette/paletteTreeWidget.h b/qrgui/mainWindow/palette/paletteTreeWidget.h index b00aaf25e6..d7352bdb6f 100644 --- a/qrgui/mainWindow/palette/paletteTreeWidget.h +++ b/qrgui/mainWindow/palette/paletteTreeWidget.h @@ -74,6 +74,11 @@ class PaletteTreeWidget : public QTreeWidget /// Returns set that may let quickly determine should we update palette or no. const QSet &elementsSet() const; + bool readyToRefresh(); + +signals: + void signalReadyToRefresh(); + protected: void mousePressEvent(QMouseEvent *event); @@ -122,6 +127,7 @@ class PaletteTreeWidget : public QTreeWidget QHash mPaletteElements; // Takes ownership. QHash mPaletteItems; // Takes ownership. QHash mItemsVisible; + QList mDraggableElements; // Takes ownership }; } diff --git a/qrgui/mainWindow/palette/paletteTreeWidgets.cpp b/qrgui/mainWindow/palette/paletteTreeWidgets.cpp index aaacf9dfc5..e916988ddd 100644 --- a/qrgui/mainWindow/palette/paletteTreeWidgets.cpp +++ b/qrgui/mainWindow/palette/paletteTreeWidgets.cpp @@ -112,7 +112,7 @@ void PaletteTreeWidgets::initUserTree() { refreshUserPalette(); connect(&mMainWindow->models().exploser(), &models::Exploser::explosionsSetCouldChange - , this, [&](){refreshUserPalette(true);}); + , this, [&](){refreshUserPaletteHandler(true);}); } void PaletteTreeWidgets::addTopItemType(const PaletteElement &data, QTreeWidget *tree) @@ -241,6 +241,17 @@ void PaletteTreeWidgets::customizeExplosionTitles(const QString &userGroupTitle, mUserGroupDescription = userGroupDescription; } +void PaletteTreeWidgets::refreshUserPaletteHandler(bool force) +{ + if (mUserTree->readyToRefresh()) { + refreshUserPalette(force); + return; + } + connect(mUserTree, &PaletteTreeWidget::signalReadyToRefresh, this, [=]() { + refreshUserPalette(force); + }); +} + void PaletteTreeWidgets::refreshUserPalette(bool force) { QList>> groups; diff --git a/qrgui/mainWindow/palette/paletteTreeWidgets.h b/qrgui/mainWindow/palette/paletteTreeWidgets.h index 4fdf9ee3bd..e173ec5901 100644 --- a/qrgui/mainWindow/palette/paletteTreeWidgets.h +++ b/qrgui/mainWindow/palette/paletteTreeWidgets.h @@ -68,6 +68,8 @@ class PaletteTreeWidgets : public QSplitter /// Rereads user blocks information. void refreshUserPalette(bool force = false); + void refreshUserPaletteHandler(bool force = false); + /// Sets user palette header and description. void customizeExplosionTitles(const QString &userGroupTitle , const QString &userGroupDescription); diff --git a/qrtranslations/fr/qrgui_mainWindow_fr.ts b/qrtranslations/fr/qrgui_mainWindow_fr.ts index 5d4aa0b17a..99ff9c30a2 100644 --- a/qrtranslations/fr/qrgui_mainWindow_fr.ts +++ b/qrtranslations/fr/qrgui_mainWindow_fr.ts @@ -1006,12 +1006,12 @@ WARNING: The settings will be restored after application restart qReal::gui::DraggableElement - + Mouse gesture Geste souris - + Deleting an element: Supression d'un élément : @@ -1121,7 +1121,7 @@ WARNING: The settings will be restored after application restart Ajouter un élément - + Add Entity diff --git a/qrtranslations/ru/qrgui_mainWindow_ru.ts b/qrtranslations/ru/qrgui_mainWindow_ru.ts index adeffdfb9a..62ee5f5e24 100644 --- a/qrtranslations/ru/qrgui_mainWindow_ru.ts +++ b/qrtranslations/ru/qrgui_mainWindow_ru.ts @@ -1120,12 +1120,12 @@ WARNING: The settings will be restored after application restart qReal::gui::DraggableElement - + Mouse gesture Жест мышью - + Deleting an element: Удаление элемента: @@ -1250,7 +1250,7 @@ WARNING: The settings will be restored after application restart Добавить элемент - + Add Entity Добавить сущность