QmlDesigner: Fix crash with ColorEditorSingleton

Fix a crash that was caused by ColorPaletteSingleton being shared among
multiple QQnlEngines. Each engine requires a unique instance of
ColorEditorSingleton.

* Rewrite ColorEditorSingleton to not be a singleton anymore
* Rename ColorEditorSingleton to ColorEditorBackend
* Add meaningful warnings

Task-number: QDS-4728
Change-Id: I1ed3315add33754b41870ad6f43c1365a899102b
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Thomas Hartmann <thomas.hartmann@qt.io>
This commit is contained in:
Henning Gruendl
2021-08-13 17:04:58 +02:00
committed by Henning Gründl
parent 341234f3cf
commit 14d1726c95
8 changed files with 72 additions and 78 deletions

View File

@@ -478,7 +478,7 @@ SecondColumnLayout {
icon: StudioTheme.Constants.eyeDropper icon: StudioTheme.Constants.eyeDropper
pixelSize: StudioTheme.Values.myIconFontSize * 1.4 pixelSize: StudioTheme.Values.myIconFontSize * 1.4
tooltip: qsTr("Eye Dropper") tooltip: qsTr("Eye Dropper")
onClicked: ColorPaletteSingleton.eyeDropper() onClicked: ColorPaletteBackend.eyeDropper()
} }
IconIndicator { IconIndicator {
@@ -700,7 +700,7 @@ SecondColumnLayout {
StudioControls.MenuItem { StudioControls.MenuItem {
text: qsTr("Add to Favorites") text: qsTr("Add to Favorites")
onTriggered: ColorPaletteSingleton.addFavoriteColor( onTriggered: ColorPaletteBackend.addFavoriteColor(
contextMenuFavorite.currentColor) contextMenuFavorite.currentColor)
} }
} }

View File

@@ -41,12 +41,12 @@ Column {
spacing: 10 spacing: 10
function addColorToPalette(colorStr) { function addColorToPalette(colorStr) {
ColorPaletteSingleton.addRecentColor(colorStr) ColorPaletteBackend.addRecentColor(colorStr)
} }
function showColorDialog(color) { function showColorDialog(color) {
root.oldColor = color root.oldColor = color
ColorPaletteSingleton.showDialog(color) ColorPaletteBackend.showDialog(color)
} }
signal dialogColorChanged signal dialogColorChanged
@@ -92,14 +92,14 @@ Column {
StudioControls.MenuItem { StudioControls.MenuItem {
visible: colorMode.currentText === "Favorite" visible: colorMode.currentText === "Favorite"
text: qsTr("Remove from Favorites") text: qsTr("Remove from Favorites")
onTriggered: ColorPaletteSingleton.removeFavoriteColor(index) onTriggered: ColorPaletteBackend.removeFavoriteColor(index)
height: visible ? implicitHeight : 0 height: visible ? implicitHeight : 0
} }
StudioControls.MenuItem { StudioControls.MenuItem {
visible: colorMode.currentText !== "Favorite" visible: colorMode.currentText !== "Favorite"
text: qsTr("Add to Favorites") text: qsTr("Add to Favorites")
onTriggered: ColorPaletteSingleton.addFavoriteColor(modelData) onTriggered: ColorPaletteBackend.addFavoriteColor(modelData)
height: visible ? implicitHeight : 0 height: visible ? implicitHeight : 0
} }
} }
@@ -108,7 +108,7 @@ Column {
Connections { Connections {
id: singletonConnection id: singletonConnection
target: ColorPaletteSingleton target: ColorPaletteBackend
function onCurrentColorChanged(color) { function onCurrentColorChanged(color) {
root.selectedColor = color root.selectedColor = color
@@ -132,18 +132,18 @@ Column {
+ 4 * StudioTheme.Values.colorEditorPopupSpinBoxWidth + 4 * StudioTheme.Values.colorEditorPopupSpinBoxWidth
width: implicitWidth width: implicitWidth
actionIndicatorVisible: false actionIndicatorVisible: false
model: ColorPaletteSingleton.palettes model: ColorPaletteBackend.palettes
currentIndex: colorMode.find(ColorPaletteSingleton.currentPalette) currentIndex: colorMode.find(ColorPaletteBackend.currentPalette)
onActivated: ColorPaletteSingleton.currentPalette = colorMode.currentText onActivated: ColorPaletteBackend.currentPalette = colorMode.currentText
Component.onCompleted: colorMode.currentIndex = colorMode.find(ColorPaletteSingleton.currentPalette) Component.onCompleted: colorMode.currentIndex = colorMode.find(ColorPaletteBackend.currentPalette)
} }
} }
GridView { GridView {
id: colorPaletteView id: colorPaletteView
model: ColorPaletteSingleton.currentPaletteColors model: ColorPaletteBackend.currentPaletteColors
delegate: colorItemDelegate delegate: colorItemDelegate
cellWidth: StudioTheme.Values.colorEditorPopupSpinBoxWidth cellWidth: StudioTheme.Values.colorEditorPopupSpinBoxWidth
+ StudioTheme.Values.controlGap + StudioTheme.Values.controlGap

View File

@@ -353,7 +353,7 @@ extend_qtc_plugin(QmlDesigner
SOURCES_PREFIX components/propertyeditor SOURCES_PREFIX components/propertyeditor
SOURCES SOURCES
aligndistribute.cpp aligndistribute.h aligndistribute.cpp aligndistribute.h
colorpalettesingleton.cpp colorpalettesingleton.h colorpalettebackend.cpp colorpalettebackend.h
designerpropertymap.cpp designerpropertymap.h designerpropertymap.cpp designerpropertymap.h
fileresourcesmodel.cpp fileresourcesmodel.h fileresourcesmodel.cpp fileresourcesmodel.h
itemfiltermodel.cpp itemfiltermodel.h itemfiltermodel.cpp itemfiltermodel.h

View File

@@ -23,7 +23,7 @@
** **
****************************************************************************/ ****************************************************************************/
#include "colorpalettesingleton.h" #include "colorpalettebackend.h"
#include <QDebug> #include <QDebug>
#include <QSettings> #include <QSettings>
@@ -36,9 +36,9 @@
namespace QmlDesigner { namespace QmlDesigner {
QPointer<ColorPaletteSingleton> ColorPaletteSingleton::m_instance = nullptr; QPointer<ColorPaletteBackend> ColorPaletteBackend::m_instance = nullptr;
ColorPaletteSingleton::ColorPaletteSingleton() ColorPaletteBackend::ColorPaletteBackend()
: m_currentPalette() : m_currentPalette()
, m_data() , m_data()
, m_colorPickingEventFilter(nullptr) , m_colorPickingEventFilter(nullptr)
@@ -58,24 +58,16 @@ ColorPaletteSingleton::ColorPaletteSingleton()
dummyTransparentWindow.resize(1, 1); dummyTransparentWindow.resize(1, 1);
dummyTransparentWindow.setFlags(Qt::Tool | Qt::FramelessWindowHint); dummyTransparentWindow.setFlags(Qt::Tool | Qt::FramelessWindowHint);
updateTimer = new QTimer(this); updateTimer = new QTimer(this);
connect(updateTimer, &QTimer::timeout, this, &ColorPaletteSingleton::updateEyeDropper); connect(updateTimer, &QTimer::timeout, this, &ColorPaletteBackend::updateEyeDropper);
#endif #endif
} }
ColorPaletteSingleton::~ColorPaletteSingleton() ColorPaletteBackend::~ColorPaletteBackend()
{ {
//writePalettes(); // TODO crash on QtDS close //writePalettes(); // TODO crash on QtDS close
} }
ColorPaletteSingleton *ColorPaletteSingleton::instance() void ColorPaletteBackend::readPalettes()
{
if (m_instance == nullptr)
m_instance = new ColorPaletteSingleton();
return m_instance;
}
void ColorPaletteSingleton::readPalettes()
{ {
QHash<QString, Palette>::iterator i = m_data.begin(); QHash<QString, Palette>::iterator i = m_data.begin();
while (i != m_data.end()) { while (i != m_data.end()) {
@@ -84,7 +76,7 @@ void ColorPaletteSingleton::readPalettes()
} }
} }
void ColorPaletteSingleton::writePalettes() void ColorPaletteBackend::writePalettes()
{ {
QHash<QString, Palette>::iterator i = m_data.begin(); QHash<QString, Palette>::iterator i = m_data.begin();
while (i != m_data.end()) { while (i != m_data.end()) {
@@ -93,10 +85,10 @@ void ColorPaletteSingleton::writePalettes()
} }
} }
void ColorPaletteSingleton::addColor(const QString &color, const QString &palette) void ColorPaletteBackend::addColor(const QString &color, const QString &palette)
{ {
if (!m_data.contains(palette)) { if (!m_data.contains(palette)) {
qWarning() << "TODO"; qWarning() << Q_FUNC_INFO << "Unknown palette: " << palette;
return; return;
} }
@@ -116,15 +108,15 @@ void ColorPaletteSingleton::addColor(const QString &color, const QString &palett
m_data[palette].write(); m_data[palette].write();
} }
void ColorPaletteSingleton::removeColor(int id, const QString &palette) void ColorPaletteBackend::removeColor(int id, const QString &palette)
{ {
if (!m_data.contains(palette)) { if (!m_data.contains(palette)) {
qWarning() << "TODO"; qWarning() << Q_FUNC_INFO << "Unknown palette: " << palette;
return; return;
} }
if (id >= m_data[palette].m_colors.size()) { if (id >= m_data[palette].m_colors.size()) {
qWarning() << "TODO"; qWarning() << Q_FUNC_INFO << "Id(" << id << ") is out of bounds for palette " << palette;
return; return;
} }
@@ -143,7 +135,7 @@ void ColorPaletteSingleton::removeColor(int id, const QString &palette)
m_data[palette].write(); m_data[palette].write();
} }
void ColorPaletteSingleton::addRecentColor(const QString &item) void ColorPaletteBackend::addRecentColor(const QString &item)
{ {
if (m_data[g_recent].m_colors.isEmpty()) { if (m_data[g_recent].m_colors.isEmpty()) {
addColor(item, g_recent); addColor(item, g_recent);
@@ -155,30 +147,30 @@ void ColorPaletteSingleton::addRecentColor(const QString &item)
addColor(item, g_recent); addColor(item, g_recent);
} }
void ColorPaletteSingleton::addFavoriteColor(const QString &item) void ColorPaletteBackend::addFavoriteColor(const QString &item)
{ {
addColor(item, g_favorite); addColor(item, g_favorite);
} }
void ColorPaletteSingleton::removeFavoriteColor(int id) void ColorPaletteBackend::removeFavoriteColor(int id)
{ {
removeColor(id, g_favorite); removeColor(id, g_favorite);
} }
QStringList ColorPaletteSingleton::palettes() const QStringList ColorPaletteBackend::palettes() const
{ {
return m_data.keys(); return m_data.keys();
} }
const QString &ColorPaletteSingleton::currentPalette() const const QString &ColorPaletteBackend::currentPalette() const
{ {
return m_currentPalette; return m_currentPalette;
} }
void ColorPaletteSingleton::setCurrentPalette(const QString &palette) void ColorPaletteBackend::setCurrentPalette(const QString &palette)
{ {
if (!m_data.contains(palette)) { if (!m_data.contains(palette)) {
qWarning() << "TODO"; qWarning() << Q_FUNC_INFO << "Unknown palette: " << palette;
return; return;
} }
@@ -207,35 +199,35 @@ void ColorPaletteSingleton::setCurrentPalette(const QString &palette)
emit currentPaletteColorsChanged(); emit currentPaletteColorsChanged();
} }
const QStringList &ColorPaletteSingleton::currentPaletteColors() const const QStringList &ColorPaletteBackend::currentPaletteColors() const
{ {
return m_currentPaletteColors; return m_currentPaletteColors;
} }
void ColorPaletteSingleton::registerDeclarativeType() void ColorPaletteBackend::registerDeclarativeType()
{ {
static const int typeIndex = qmlRegisterSingletonType<ColorPaletteSingleton>( static const int typeIndex = qmlRegisterSingletonType<ColorPaletteBackend>(
"QtQuickDesignerColorPalette", 1, 0, "ColorPaletteSingleton", [](QQmlEngine *, QJSEngine *) { "QtQuickDesignerColorPalette", 1, 0, "ColorPaletteBackend", [](QQmlEngine *, QJSEngine *) {
return ColorPaletteSingleton::instance(); return new ColorPaletteBackend();
}); });
Q_UNUSED(typeIndex) Q_UNUSED(typeIndex)
} }
void ColorPaletteSingleton::showDialog(QColor color) void ColorPaletteBackend::showDialog(QColor color)
{ {
auto colorDialog = new QColorDialog(Core::ICore::dialogParent()); auto colorDialog = new QColorDialog(Core::ICore::dialogParent());
colorDialog->setCurrentColor(color); colorDialog->setCurrentColor(color);
colorDialog->setAttribute(Qt::WA_DeleteOnClose); colorDialog->setAttribute(Qt::WA_DeleteOnClose);
connect(colorDialog, &QDialog::rejected, connect(colorDialog, &QDialog::rejected,
this, &ColorPaletteSingleton::colorDialogRejected); this, &ColorPaletteBackend::colorDialogRejected);
connect(colorDialog, &QColorDialog::currentColorChanged, connect(colorDialog, &QColorDialog::currentColorChanged,
this, &ColorPaletteSingleton::currentColorChanged); this, &ColorPaletteBackend::currentColorChanged);
QTimer::singleShot(0, [colorDialog](){ colorDialog->exec(); }); QTimer::singleShot(0, [colorDialog](){ colorDialog->exec(); });
} }
void ColorPaletteSingleton::eyeDropper() void ColorPaletteBackend::eyeDropper()
{ {
QWidget *widget = QApplication::activeWindow(); QWidget *widget = QApplication::activeWindow();
if (!widget) if (!widget)
@@ -274,12 +266,12 @@ const int g_screenGrabHeight = 7;
const int g_pixelX = 3; const int g_pixelX = 3;
const int g_pixelY = 3; const int g_pixelY = 3;
QColor ColorPaletteSingleton::grabScreenColor(const QPoint &p) QColor ColorPaletteBackend::grabScreenColor(const QPoint &p)
{ {
return grabScreenRect(p).pixel(g_pixelX, g_pixelY); return grabScreenRect(p).pixel(g_pixelX, g_pixelY);
} }
QImage ColorPaletteSingleton::grabScreenRect(const QPoint &p) QImage ColorPaletteBackend::grabScreenRect(const QPoint &p)
{ {
QScreen *screen = QGuiApplication::screenAt(p); QScreen *screen = QGuiApplication::screenAt(p);
if (!screen) if (!screen)
@@ -289,7 +281,7 @@ QImage ColorPaletteSingleton::grabScreenRect(const QPoint &p)
return pixmap.toImage(); return pixmap.toImage();
} }
void ColorPaletteSingleton::updateEyeDropper() void ColorPaletteBackend::updateEyeDropper()
{ {
#ifndef QT_NO_CURSOR #ifndef QT_NO_CURSOR
static QPoint lastGlobalPos; static QPoint lastGlobalPos;
@@ -306,12 +298,12 @@ void ColorPaletteSingleton::updateEyeDropper()
#endif // ! QT_NO_CURSOR #endif // ! QT_NO_CURSOR
} }
void ColorPaletteSingleton::updateEyeDropperPosition(const QPoint &globalPos) void ColorPaletteBackend::updateEyeDropperPosition(const QPoint &globalPos)
{ {
updateCursor(grabScreenRect(globalPos)); updateCursor(grabScreenRect(globalPos));
} }
void ColorPaletteSingleton::updateCursor(const QImage &image) void ColorPaletteBackend::updateCursor(const QImage &image)
{ {
QWidget *widget = QApplication::activeWindow(); QWidget *widget = QApplication::activeWindow();
if (!widget) if (!widget)
@@ -351,7 +343,7 @@ void ColorPaletteSingleton::updateCursor(const QImage &image)
widget->setCursor(cursor); widget->setCursor(cursor);
} }
void ColorPaletteSingleton::releaseEyeDropper() void ColorPaletteBackend::releaseEyeDropper()
{ {
QWidget *widget = QApplication::activeWindow(); QWidget *widget = QApplication::activeWindow();
if (!widget) if (!widget)
@@ -369,13 +361,13 @@ void ColorPaletteSingleton::releaseEyeDropper()
widget->unsetCursor(); widget->unsetCursor();
} }
bool ColorPaletteSingleton::handleEyeDropperMouseMove(QMouseEvent *e) bool ColorPaletteBackend::handleEyeDropperMouseMove(QMouseEvent *e)
{ {
updateEyeDropperPosition(e->globalPos()); updateEyeDropperPosition(e->globalPos());
return true; return true;
} }
bool ColorPaletteSingleton::handleEyeDropperMouseButtonRelease(QMouseEvent *e) bool ColorPaletteBackend::handleEyeDropperMouseButtonRelease(QMouseEvent *e)
{ {
if (e->button() == Qt::LeftButton) if (e->button() == Qt::LeftButton)
emit currentColorChanged(grabScreenColor(e->globalPos())); emit currentColorChanged(grabScreenColor(e->globalPos()));
@@ -386,7 +378,7 @@ bool ColorPaletteSingleton::handleEyeDropperMouseButtonRelease(QMouseEvent *e)
return true; return true;
} }
bool ColorPaletteSingleton::handleEyeDropperKeyPress(QKeyEvent *e) bool ColorPaletteBackend::handleEyeDropperKeyPress(QKeyEvent *e)
{ {
#if QT_CONFIG(shortcut) #if QT_CONFIG(shortcut)
if (e->matches(QKeySequence::Cancel)) { if (e->matches(QKeySequence::Cancel)) {

View File

@@ -78,7 +78,7 @@ struct Palette
QStringList m_colors; QStringList m_colors;
}; };
class ColorPaletteSingleton : public QObject class ColorPaletteBackend : public QObject
{ {
Q_OBJECT Q_OBJECT
@@ -94,9 +94,7 @@ class ColorPaletteSingleton : public QObject
NOTIFY palettesChanged) NOTIFY palettesChanged)
public: public:
static ColorPaletteSingleton *instance(); ~ColorPaletteBackend();
~ColorPaletteSingleton();
void readPalettes(); void readPalettes();
void writePalettes(); void writePalettes();
@@ -140,8 +138,8 @@ public:
bool handleEyeDropperKeyPress(QKeyEvent *e); bool handleEyeDropperKeyPress(QKeyEvent *e);
ColorPaletteSingleton(const ColorPaletteSingleton &) = delete; ColorPaletteBackend(const ColorPaletteBackend &) = delete;
void operator=(const ColorPaletteSingleton &) = delete; void operator=(const ColorPaletteBackend &) = delete;
signals: signals:
void currentPaletteChanged(const QString &palette); void currentPaletteChanged(const QString &palette);
@@ -154,10 +152,10 @@ signals:
void eyeDropperRejected(); void eyeDropperRejected();
private: private:
ColorPaletteSingleton(); ColorPaletteBackend();
private: private:
static QPointer<ColorPaletteSingleton> m_instance; static QPointer<ColorPaletteBackend> m_instance;
QString m_currentPalette; QString m_currentPalette;
QStringList m_currentPaletteColors; QStringList m_currentPaletteColors;
QHash<QString, Palette> m_data; QHash<QString, Palette> m_data;
@@ -171,29 +169,33 @@ private:
class QColorPickingEventFilter : public QObject { class QColorPickingEventFilter : public QObject {
public: public:
explicit QColorPickingEventFilter(QObject *parent = 0) explicit QColorPickingEventFilter(ColorPaletteBackend *colorPalette)
: QObject(parent) : QObject(colorPalette)
, m_colorPalette(colorPalette)
{} {}
bool eventFilter(QObject *, QEvent *event) override bool eventFilter(QObject *, QEvent *event) override
{ {
switch (event->type()) { switch (event->type()) {
case QEvent::MouseMove: case QEvent::MouseMove:
return ColorPaletteSingleton::instance()->handleEyeDropperMouseMove( return m_colorPalette->handleEyeDropperMouseMove(
static_cast<QMouseEvent *>(event)); static_cast<QMouseEvent *>(event));
case QEvent::MouseButtonRelease: case QEvent::MouseButtonRelease:
return ColorPaletteSingleton::instance()->handleEyeDropperMouseButtonRelease( return m_colorPalette->handleEyeDropperMouseButtonRelease(
static_cast<QMouseEvent *>(event)); static_cast<QMouseEvent *>(event));
case QEvent::KeyPress: case QEvent::KeyPress:
return ColorPaletteSingleton::instance()->handleEyeDropperKeyPress( return m_colorPalette->handleEyeDropperKeyPress(
static_cast<QKeyEvent *>(event)); static_cast<QKeyEvent *>(event));
default: default:
break; break;
} }
return false; return false;
} }
private:
ColorPaletteBackend *m_colorPalette;
}; };
} // namespace QmlDesigner } // namespace QmlDesigner
QML_DECLARE_TYPE(QmlDesigner::ColorPaletteSingleton) QML_DECLARE_TYPE(QmlDesigner::ColorPaletteBackend)

View File

@@ -16,7 +16,7 @@ SOURCES += propertyeditorview.cpp \
gradientpresetlistmodel.cpp \ gradientpresetlistmodel.cpp \
gradientpresetdefaultlistmodel.cpp \ gradientpresetdefaultlistmodel.cpp \
gradientpresetcustomlistmodel.cpp \ gradientpresetcustomlistmodel.cpp \
colorpalettesingleton.cpp \ colorpalettebackend.cpp \
itemfiltermodel.cpp \ itemfiltermodel.cpp \
aligndistribute.cpp \ aligndistribute.cpp \
tooltip.cpp tooltip.cpp
@@ -37,7 +37,7 @@ HEADERS += propertyeditorview.h \
gradientpresetlistmodel.h \ gradientpresetlistmodel.h \
gradientpresetdefaultlistmodel.h \ gradientpresetdefaultlistmodel.h \
gradientpresetcustomlistmodel.h \ gradientpresetcustomlistmodel.h \
colorpalettesingleton.h \ colorpalettebackend.h \
itemfiltermodel.h \ itemfiltermodel.h \
aligndistribute.h \ aligndistribute.h \
tooltip.h tooltip.h

View File

@@ -29,7 +29,7 @@
#include "annotationeditor/annotationeditor.h" #include "annotationeditor/annotationeditor.h"
#include "bindingeditor/actioneditor.h" #include "bindingeditor/actioneditor.h"
#include "bindingeditor/bindingeditor.h" #include "bindingeditor/bindingeditor.h"
#include "colorpalettesingleton.h" #include "colorpalettebackend.h"
#include "fileresourcesmodel.h" #include "fileresourcesmodel.h"
#include "gradientmodel.h" #include "gradientmodel.h"
#include "gradientpresetcustomlistmodel.h" #include "gradientpresetcustomlistmodel.h"
@@ -63,7 +63,7 @@ void Quick2PropertyEditorView::registerQmlTypes()
GradientPresetDefaultListModel::registerDeclarativeType(); GradientPresetDefaultListModel::registerDeclarativeType();
GradientPresetCustomListModel::registerDeclarativeType(); GradientPresetCustomListModel::registerDeclarativeType();
ItemFilterModel::registerDeclarativeType(); ItemFilterModel::registerDeclarativeType();
ColorPaletteSingleton::registerDeclarativeType(); ColorPaletteBackend::registerDeclarativeType();
Internal::QmlAnchorBindingProxy::registerDeclarativeType(); Internal::QmlAnchorBindingProxy::registerDeclarativeType();
BindingEditor::registerDeclarativeType(); BindingEditor::registerDeclarativeType();
ActionEditor::registerDeclarativeType(); ActionEditor::registerDeclarativeType();

View File

@@ -677,8 +677,8 @@ Project {
"navigator/previewtooltip.ui", "navigator/previewtooltip.ui",
"propertyeditor/aligndistribute.cpp", "propertyeditor/aligndistribute.cpp",
"propertyeditor/aligndistribute.h", "propertyeditor/aligndistribute.h",
"propertyeditor/colorpalettesingleton.cpp", "propertyeditor/colorpalettebackend.cpp",
"propertyeditor/colorpalettesingleton.h", "propertyeditor/colorpalettebackend.h",
"propertyeditor/designerpropertymap.cpp", "propertyeditor/designerpropertymap.cpp",
"propertyeditor/designerpropertymap.h", "propertyeditor/designerpropertymap.h",
"propertyeditor/fileresourcesmodel.cpp", "propertyeditor/fileresourcesmodel.cpp",