Don't track brush shortcuts in brush tool settings

Because the shortcut isn't actually used there, it's only used in the preferences and the brush settings dialog, so we don't need to load them in other places.
drawpile · Oct 14, 2024 · 34b0e89 · 34b0e89
1 parent acd23c2
commit 34b0e89
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 58 deletions.
diff --git a/src/desktop/mainwindow.cpp b/src/desktop/mainwindow.cpp
@@ -2350,7 +2350,9 @@ void MainWindow::showBrushSettingsDialog(bool openOnPresetPage)
 
 		tools::BrushSettings *brushSettings =
 			m_dockToolSettings->brushSettings();
-		std::function<void(int)> updatePreset = [brushSettings,
+		brushes::BrushPresetModel *presetModel =
+			dpApp().brushPresets()->presetModel();
+		std::function<void(int)> updatePreset = [brushSettings, presetModel,
 												 dlg](int presetId) {
 			bool attached = presetId > 0;
 			dlg->setPresetAttached(attached, presetId);
@@ -2361,7 +2363,8 @@ void MainWindow::showBrushSettingsDialog(bool openOnPresetPage)
 					brushSettings->currentPresetDescription());
 				dlg->setPresetThumbnail(
 					brushSettings->currentPresetThumbnail());
-				dlg->setPresetShortcut(brushSettings->currentPresetShortcut());
+				dlg->setPresetShortcut(
+					presetModel->getShortcutForPresetId(presetId));
 			}
 		};
 		connect(
@@ -2412,8 +2415,7 @@ void MainWindow::showBrushSettingsDialog(bool openOnPresetPage)
 				&docks::BrushPalette::overwriteCurrentPreset,
 				m_dockBrushPalette, dlg));
 		connect(
-			dpApp().brushPresets()->presetModel(),
-			&brushes::BrushPresetModel::presetShortcutChanged, dlg,
+			presetModel, &brushes::BrushPresetModel::presetShortcutChanged, dlg,
 			[dlg](int presetId, const QKeySequence &shortcut) {
 				if(dlg->isPresetAttached() && dlg->presetId() == presetId) {
 					dlg->setPresetShortcut(shortcut);

diff --git a/src/desktop/toolwidgets/brushsettings.cpp b/src/desktop/toolwidgets/brushsettings.cpp
@@ -276,7 +276,6 @@ void BrushSettings::connectBrushPresets(brushes::BrushPresetModel *brushPresets)
 				preset.originalDescription = opt->originalDescription;
 				preset.originalThumbnail = opt->originalThumbnail;
 				preset.originalBrush = opt->originalBrush;
-				preset.shortcut = opt->shortcut;
 				preset.changedName = opt->changedName;
 				preset.changedDescription = opt->changedDescription;
 				preset.changedThumbnail = opt->changedThumbnail;
@@ -295,9 +294,6 @@ void BrushSettings::connectBrushPresets(brushes::BrushPresetModel *brushPresets)
 	connect(
 		brushPresets, &brushes::BrushPresetModel::presetChanged, this,
 		&BrushSettings::handlePresetChanged);
-	connect(
-		brushPresets, &brushes::BrushPresetModel::presetShortcutChanged, this,
-		&BrushSettings::handlePresetShortcutChanged);
 	connect(
 		brushPresets, &brushes::BrushPresetModel::presetRemoved, this,
 		&BrushSettings::handlePresetRemoved);
@@ -838,11 +834,6 @@ const QPixmap &BrushSettings::currentPresetThumbnail() const
 	return d->currentPreset().effectiveThumbnail();
 }
 
-const QKeySequence &BrushSettings::currentPresetShortcut() const
-{
-	return d->currentPreset().shortcut;
-}
-
 int BrushSettings::currentBrushSlot() const
 {
 	return d->current;
@@ -1729,17 +1720,6 @@ void BrushSettings::handlePresetChanged(
 	}
 }
 
-void BrushSettings::handlePresetShortcutChanged(
-	int presetId, const QKeySequence &shortcut)
-{
-	for(int i = 0; i < TOTAL_SLOT_COUNT; ++i) {
-		Preset &preset = d->presetAt(i);
-		if(preset.isAttached() && preset.id == presetId) {
-			preset.shortcut = shortcut;
-		}
-	}
-}
-
 void BrushSettings::handlePresetRemoved(int presetId)
 {
 	for(int i = 0; i < TOTAL_SLOT_COUNT; ++i) {

diff --git a/src/desktop/toolwidgets/brushsettings.h b/src/desktop/toolwidgets/brushsettings.h
@@ -65,7 +65,6 @@ class BrushSettings final : public ToolSettings {
 	const QString &currentPresetName() const;
 	const QString &currentPresetDescription() const;
 	const QPixmap &currentPresetThumbnail() const;
-	const QKeySequence &currentPresetShortcut() const;
 
 	int currentBrushSlot() const;
 	bool isCurrentEraserSlot() const;
@@ -127,8 +126,6 @@ private slots:
 	void handlePresetChanged(
 		int presetId, const QString &name, const QString &description,
 		const QPixmap &thumbnail, const brushes::ActiveBrush &brush);
-	void
-	handlePresetShortcutChanged(int presetId, const QKeySequence &shortcut);
 	void handlePresetRemoved(int presetId);
 	void detachCurrentSlot();
 

diff --git a/src/libclient/brushes/brushpresetmodel.cpp b/src/libclient/brushes/brushpresetmodel.cpp
@@ -79,6 +79,16 @@ struct PresetShortcutEntry {
 struct PresetShortcut {
 	QVector<PresetShortcutEntry> entries;
 
+	bool containsId(int id) const
+	{
+		for(const PresetShortcutEntry &entry : entries) {
+			if(entry.id == id) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 	QVector<int> ids() const
 	{
 		QVector<int> v;
@@ -355,9 +365,9 @@ class BrushPresetTagModel::Private {
 		QMutexLocker locker(&m_mutex);
 		QSqlQuery query(m_db);
 		QString sql = QStringLiteral(
-			"select id, name, description, thumbnail, data, shortcut, "
-			"changed_name, changed_description, changed_thumbnail, "
-			"changed_data from preset where id = ?");
+			"select id, name, description, thumbnail, data, changed_name, "
+			"changed_description, changed_thumbnail, changed_data from preset "
+			"where id = ?");
 		if(exec(query, sql, {id}) && query.next()) {
 			Preset preset;
 			readPreset(preset, query);
@@ -672,6 +682,19 @@ class BrushPresetTagModel::Private {
 		}
 	}
 
+	QKeySequence getShortcutForPresetId(int presetId) const
+	{
+		for(PresetShortcutMap::const_iterator
+				it = m_presetShortcuts.constBegin(),
+				end = m_presetShortcuts.constEnd();
+			it != end; ++it) {
+			if(it->containsId(presetId)) {
+				return it.key();
+			}
+		}
+		return QKeySequence();
+	}
+
 	QVector<int> getPresetIdsForShortcut(const QKeySequence &shortcut) const
 	{
 		PresetShortcutMap::const_iterator found =
@@ -798,9 +821,9 @@ class BrushPresetTagModel::Private {
 
 		QString sql = QStringLiteral(
 			"select p.id, p.name, p.description, p.thumbnail, p.data, "
-			"p.shortcut, p.changed_name, p.changed_description, "
-			"p.changed_thumbnail, p.changed_data, group_concat(t.tag_id) tags "
-			"from preset p left join preset_tag t on t.preset_id = p.id");
+			"p.changed_name, p.changed_description, p.changed_thumbnail, "
+			"p.changed_data, group_concat(t.tag_id) tags from preset p left "
+			"join preset_tag t on t.preset_id = p.id");
 		QVariantList params;
 		switch(m_tagIdToFilter) {
 		case ALL_ID:
@@ -822,7 +845,7 @@ class BrushPresetTagModel::Private {
 			while(query.next()) {
 				CachedPreset cp;
 				readPreset(cp, query);
-				parseGroupedTagIds(query.value(10).toString(), cp.tagIds);
+				parseGroupedTagIds(query.value(9).toString(), cp.tagIds);
 				m_presetCache.append(cp);
 			}
 		}
@@ -843,20 +866,18 @@ class BrushPresetTagModel::Private {
 
 		preset.originalBrush =
 			loadBrush(preset.id, query.value(4).toByteArray());
-		preset.shortcut = QKeySequence::fromString(
-			query.value(5).toString(), QKeySequence::PortableText);
 
-		QVariant changedName = query.value(6);
+		QVariant changedName = query.value(5);
 		if(!changedName.isNull()) {
 			preset.changedName = changedName.toString();
 		}
 
-		QVariant changedDescription = query.value(7);
+		QVariant changedDescription = query.value(6);
 		if(!changedDescription.isNull()) {
 			preset.changedDescription = changedDescription.toString();
 		}
 
-		QVariant changedThumbnail = query.value(8);
+		QVariant changedThumbnail = query.value(7);
 		if(!changedThumbnail.isNull()) {
 			if(pixmap.loadFromData(changedThumbnail.toByteArray())) {
 				preset.changedThumbnail = pixmap;
@@ -866,7 +887,7 @@ class BrushPresetTagModel::Private {
 			}
 		}
 
-		QVariant changedData = query.value(9);
+		QVariant changedData = query.value(8);
 		if(!changedData.isNull()) {
 			preset.changedBrush =
 				loadBrush(preset.id, changedData.toByteArray());
@@ -2280,8 +2301,7 @@ std::optional<Preset> BrushPresetModel::newPreset(
 	d->refreshShortcuts();
 	if(presetId > 0) {
 		return Preset{
-			presetId,		name, description, thumbnail, brush,
-			QKeySequence(), {},	  {},		   {},		  {},
+			presetId, name, description, thumbnail, brush, {}, {}, {}, {},
 		};
 	} else {
 		return {};
@@ -2309,20 +2329,6 @@ bool BrushPresetModel::updatePresetShortcut(
 {
 	bool ok = d->updatePresetShortcut(
 		presetId, shortcut.toString(QKeySequence::PortableText));
-
-	int count = d->presetCacheSize();
-	for(int i = 0; i < count; ++i) {
-		CachedPreset &cp = d->getCachedPreset(i);
-		if(cp.id == presetId) {
-			if(cp.shortcut != shortcut) {
-				cp.shortcut = shortcut;
-				QModelIndex idx = createIndex(i, 0);
-				emit dataChanged(idx, idx);
-				break;
-			}
-		}
-	}
-
 	d->refreshShortcuts();
 	emit presetShortcutChanged(presetId, shortcut);
 	return ok;
@@ -2391,6 +2397,11 @@ void BrushPresetModel::getShortcutActions(
 	d->getShortcutActions(fn);
 }
 
+QKeySequence BrushPresetModel::getShortcutForPresetId(int presetId)
+{
+	return d->getShortcutForPresetId(presetId);
+}
+
 QVector<int>
 BrushPresetModel::getPresetIdsForShortcut(const QKeySequence &shortcut) const
 {

diff --git a/src/libclient/brushes/brushpresetmodel.h b/src/libclient/brushes/brushpresetmodel.h
@@ -44,7 +44,6 @@ struct Preset {
 	QString originalDescription;
 	QPixmap originalThumbnail;
 	ActiveBrush originalBrush;
-	QKeySequence shortcut;
 	std::optional<QString> changedName;
 	std::optional<QString> changedDescription;
 	std::optional<QPixmap> changedThumbnail;
@@ -290,8 +289,8 @@ class BrushPresetModel final : public QAbstractItemModel {
 		const std::function<void(
 			const QString &, const QString &, const QKeySequence &)> &fn) const;
 
+	QKeySequence getShortcutForPresetId(int presetId);
 	QVector<int> getPresetIdsForShortcut(const QKeySequence &shortcut) const;
-
 	QVector<ShortcutPreset> getShortcutPresets() const;
 
 	QSize iconSize() const;