[calligra/krita-testing-kazakov] krita: Fix updates of the color selector when color space of the image or node is changed
Dmitry Kazakov
dimula73 at gmail.com
Thu Apr 24 12:06:27 UTC 2014
Git commit 20753a5f170645c17306f24ad0b22eb855f98ca3 by Dmitry Kazakov.
Committed on 24/04/2014 at 12:05.
Pushed by dkazakov into branch 'krita-testing-kazakov'.
Fix updates of the color selector when color space of the image or node is changed
IMPORTANT:
This commit introduces a KisSignalsBlocker class which does scope-defined
blocking of signals of a QObject-based. Please use it instead of direct
calls to object->blockSignals(true) when writing new code. The latter way
is purely unsafe (from 'return' and exceptions point of view).
CCMAIL:kimageshop at kde.org
The rest of the patch fixes updates of the color selectors in the following
cases:
1) Image color space is changed
2) Current node color space is changed
3) Image projection color space is changed
4-6) Undo of the cases 1-3 [PLEASE TEST IT]
A +58 -0 krita/image/kis_signals_blocker.h [License: GPL (v2+)]
M +2 -3 krita/plugins/extensions/dockers/advancedcolorselector/kis_color_selector_base.cpp
M +8 -19 krita/plugins/extensions/dockers/advancedcolorselector/kis_color_selector_ng_docker_widget.cpp
M +25 -39 krita/plugins/extensions/dockers/lut/lutdocker_dock.cpp
M +41 -7 krita/ui/canvas/kis_display_color_converter.cpp
M +3 -1 krita/ui/canvas/kis_display_color_converter.h
M +3 -0 krita/ui/kis_config.cc
http://commits.kde.org/calligra/20753a5f170645c17306f24ad0b22eb855f98ca3
diff --git a/krita/image/kis_signals_blocker.h b/krita/image/kis_signals_blocker.h
new file mode 100644
index 0000000..c98b19e
--- /dev/null
+++ b/krita/image/kis_signals_blocker.h
@@ -0,0 +1,58 @@
+/*
+ * Copyright (c) 2014 Dmitry Kazakov <dimula73 at gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+
+#ifndef __KIS_SIGNALS_BLOCKER_H
+#define __KIS_SIGNALS_BLOCKER_H
+
+/**
+ * Block QObject's signals in a safe and sane way.
+ *
+ * Avoid using direct calls to QObject::blockSignals(bool),
+ * because:
+ *
+ * 1) They are not safe. One beautifully sunny day someone (it might
+ * easily be you youself) will forget about these call and will put
+ * a 'return' statement somewhere among the lines. Surely this is
+ * not what you expect to happen.
+ *
+ * 2) Two lines of blocking for every line of access can easily make
+ * the code unreadable.
+ */
+
+class KisSignalsBlocker
+{
+public:
+ KisSignalsBlocker(QObject *object)
+ : m_object(object)
+ {
+ m_object->blockSignals(true);
+ }
+
+ ~KisSignalsBlocker()
+ {
+ m_object->blockSignals(false);
+ }
+
+private:
+ Q_DISABLE_COPY(KisSignalsBlocker);
+
+private:
+ QObject *m_object;
+};
+
+#endif /* __KIS_SIGNALS_BLOCKER_H */
diff --git a/krita/plugins/extensions/dockers/advancedcolorselector/kis_color_selector_base.cpp b/krita/plugins/extensions/dockers/advancedcolorselector/kis_color_selector_base.cpp
index f76ec41..4baf19a 100644
--- a/krita/plugins/extensions/dockers/advancedcolorselector/kis_color_selector_base.cpp
+++ b/krita/plugins/extensions/dockers/advancedcolorselector/kis_color_selector_base.cpp
@@ -144,9 +144,6 @@ void KisColorSelectorBase::setCanvas(KisCanvas2 *canvas)
connect(m_canvas->displayColorConverter(), SIGNAL(displayConfigurationChanged()),
SLOT(update()));
-
- connect(m_canvas->view()->image(), SIGNAL(sigColorSpaceChanged(const KoColorSpace*)),
- SLOT(update()));
}
if (m_popup) {
m_popup->setCanvas(canvas);
@@ -436,6 +433,8 @@ void KisColorSelectorBase::updateSettings()
if(m_isPopup) {
resize(cfg.readEntry("zoomSize", 280), cfg.readEntry("zoomSize", 280));
}
+
+ update();
}
KisDisplayColorConverter* KisColorSelectorBase::converter() const
diff --git a/krita/plugins/extensions/dockers/advancedcolorselector/kis_color_selector_ng_docker_widget.cpp b/krita/plugins/extensions/dockers/advancedcolorselector/kis_color_selector_ng_docker_widget.cpp
index e9dd239..d780279 100644
--- a/krita/plugins/extensions/dockers/advancedcolorselector/kis_color_selector_ng_docker_widget.cpp
+++ b/krita/plugins/extensions/dockers/advancedcolorselector/kis_color_selector_ng_docker_widget.cpp
@@ -209,24 +209,13 @@ void KisColorSelectorNgDockerWidget::updateLayout()
void KisColorSelectorNgDockerWidget::reactOnLayerChange()
{
- // this will trigger settings update and therefore an update of the color space setting and therefore it will change
- // the color space to the current layer
+ /**
+ * Trigger the update for the case if some legacy code needs it.
+ * Now the node's color space is managed by the
+ * KisDisplayColorConverter and KisColorSelectorBase objects, so
+ * technically this call is not needed anymore. Please remove it
+ * when you are totally sure this will not break something.
+ */
+
emit settingsChanged();
- if (m_canvas) {
- KisNodeSP node = m_canvas->view()->resourceProvider()->currentNode();
- if (node && node->paintDevice()) {
- KisPaintDeviceSP device = node->paintDevice();
- connect(device.data(), SIGNAL(profileChanged(const KoColorProfile*)), this, SIGNAL(settingsChanged()), Qt::UniqueConnection);
- connect(device.data(), SIGNAL(colorSpaceChanged(const KoColorSpace*)), this, SIGNAL(settingsChanged()), Qt::UniqueConnection);
-
- if (device) {
- m_colorHistoryAction->setEnabled(true);
- m_commonColorsAction->setEnabled(true);
- }
- else {
- m_colorHistoryAction->setEnabled(false);
- m_commonColorsAction->setEnabled(false);
- }
- }
- }
}
diff --git a/krita/plugins/extensions/dockers/lut/lutdocker_dock.cpp b/krita/plugins/extensions/dockers/lut/lutdocker_dock.cpp
index 49702f7..0330464 100644
--- a/krita/plugins/extensions/dockers/lut/lutdocker_dock.cpp
+++ b/krita/plugins/extensions/dockers/lut/lutdocker_dock.cpp
@@ -48,6 +48,7 @@
#include <widgets/kis_double_widget.h>
#include <kis_image.h>
#include "widgets/squeezedcombobox.h"
+#include "kis_signals_blocker.h"
#include "ocio_display_filter.h"
@@ -151,8 +152,6 @@ void LutDockerDock::setCanvas(KoCanvasBase* _canvas)
void LutDockerDock::slotImageColorSpaceChanged()
{
- //qDebug() << "slotImageColorSpaceChanged();";
-
if (m_canvas && m_canvas->view() && m_canvas->view()->image()) {
const KoColorSpace *cs = m_canvas->view()->image()->colorSpace();
@@ -160,6 +159,10 @@ void LutDockerDock::slotImageColorSpaceChanged()
refillComboboxes();
+ KisSignalsBlocker exposureBlocker(m_exposureDoubleWidget);
+ KisSignalsBlocker gammaBlocker(m_gammaDoubleWidget);
+ KisSignalsBlocker componentsBlocker(m_cmbComponents);
+
m_exposureDoubleWidget->setValue(m_canvas->view()->resourceProvider()->HDRExposure());
m_gammaDoubleWidget->setValue(m_canvas->view()->resourceProvider()->HDRGamma());
@@ -170,6 +173,7 @@ void LutDockerDock::slotImageColorSpaceChanged()
m_cmbComponents->addSqueezedItem(channel->name());
}
m_cmbComponents->setCurrentIndex(1); // All Channels...
+
}
updateDisplaySettings();
}
@@ -319,49 +323,35 @@ void LutDockerDock::resetOcioConfiguration()
void LutDockerDock::refillComboboxes()
{
- //qDebug() << "refillComboboxes();";
- m_cmbInputColorSpace->blockSignals(true);
+ {
+ KisSignalsBlocker inputCSBlocker(m_cmbInputColorSpace);
+ m_cmbInputColorSpace->clear();
- m_cmbInputColorSpace->clear();
+ if (!m_ocioConfig) return;
- if (!m_ocioConfig) return;
-
- int numOcioColorSpaces = m_ocioConfig->getNumColorSpaces();
- for(int i = 0; i < numOcioColorSpaces; ++i) {
- const char *cs = m_ocioConfig->getColorSpaceNameByIndex(i);
- OCIO::ConstColorSpaceRcPtr colorSpace = m_ocioConfig->getColorSpace(cs);
- m_cmbInputColorSpace->addSqueezedItem(QString::fromUtf8(colorSpace->getName()));
+ int numOcioColorSpaces = m_ocioConfig->getNumColorSpaces();
+ for(int i = 0; i < numOcioColorSpaces; ++i) {
+ const char *cs = m_ocioConfig->getColorSpaceNameByIndex(i);
+ OCIO::ConstColorSpaceRcPtr colorSpace = m_ocioConfig->getColorSpace(cs);
+ m_cmbInputColorSpace->addSqueezedItem(QString::fromUtf8(colorSpace->getName()));
+ }
}
- m_cmbInputColorSpace->blockSignals(false);
-
- // int numRoles = m_ocioConfig->getNumRoles();
- // for (int i = 0; i < numRoles; ++i) {
- // //qDebug() << "role" << m_ocioConfig->getRoleName(i);
- // }
-
- m_cmbDisplayDevice->blockSignals(true);
- m_cmbDisplayDevice->clear();
- int numDisplays = m_ocioConfig->getNumDisplays();
- for (int i = 0; i < numDisplays; ++i) {
- m_cmbDisplayDevice->addSqueezedItem(QString::fromUtf8(m_ocioConfig->getDisplay(i)));
+ {
+ KisSignalsBlocker displayDeviceLocker(m_cmbDisplayDevice);
+ m_cmbDisplayDevice->clear();
+ int numDisplays = m_ocioConfig->getNumDisplays();
+ for (int i = 0; i < numDisplays; ++i) {
+ m_cmbDisplayDevice->addSqueezedItem(QString::fromUtf8(m_ocioConfig->getDisplay(i)));
+ }
}
- m_cmbDisplayDevice->blockSignals(false);
- refillViewCombobox();
-
- // int numLooks = m_ocioConfig->getNumLooks();
- // //qDebug() << "number of looks" << numLooks;
- // for (int i = 0; i < numLooks; ++i) {
- // //qDebug() << "look" << m_ocioConfig->getLookNameByIndex(i);
- // }
-
+ refillViewCombobox();
}
void LutDockerDock::refillViewCombobox()
{
- // qDebug() << "refillViewCombobox();";
- m_cmbView->blockSignals(true);
+ KisSignalsBlocker viewComboLocker(m_cmbView);
m_cmbView->clear();
if (!m_ocioConfig) return;
@@ -369,15 +359,12 @@ void LutDockerDock::refillViewCombobox()
int numViews = m_ocioConfig->getNumViews(display);
for (int j = 0; j < numViews; ++j) {
- // qDebug() << "\tview" << m_ocioConfig->getView(display, j);
m_cmbView->addSqueezedItem(QString::fromUtf8(m_ocioConfig->getView(display, j)));
}
- m_cmbView->blockSignals(false);
}
void LutDockerDock::selectLut()
{
- //qDebug() << "selectLut();";
QString filename = m_txtLut->text();
filename = KFileDialog::getOpenFileName(QDir::cleanPath(filename), "*.*", this);
@@ -392,7 +379,6 @@ void LutDockerDock::selectLut()
void LutDockerDock::clearLut()
{
- //qDebug() << "clearLut();";
m_txtLut->clear();
updateDisplaySettings();
}
diff --git a/krita/ui/canvas/kis_display_color_converter.cpp b/krita/ui/canvas/kis_display_color_converter.cpp
index 135f97f..b87eaf7 100644
--- a/krita/ui/canvas/kis_display_color_converter.cpp
+++ b/krita/ui/canvas/kis_display_color_converter.cpp
@@ -24,6 +24,7 @@
#include <KoColorModelStandardIds.h>
#include <KoCanvasResourceManager.h>
+#include "kis_config_notifier.h"
#include "kis_canvas_resource_provider.h"
#include "kis_canvas2.h"
#include "kis_view2.h"
@@ -66,14 +67,16 @@ struct KisDisplayColorConverter::Private
const KoColorSpace *intermediateColorSpace;
KoColor intermediateFgColor;
+ KisNodeSP connectedNode;
inline KoColor approximateFromQColor(const QColor &qcolor);
inline QColor approximateToQColor(const KoColor &color);
void slotCanvasResourceChanged(int key, const QVariant &v);
- void setCurrentNode(KisNodeSP node);
-
+ void slotUpdateCurrentNodeColorSpace();
void selectPaintingColorSpace();
+
+ void setCurrentNode(KisNodeSP node);
bool useOcio() const;
};
@@ -84,7 +87,10 @@ KisDisplayColorConverter::KisDisplayColorConverter(KisCanvas2 *parentCanvas)
m_d->parentCanvas = parentCanvas;
connect(m_d->parentCanvas->resourceManager(), SIGNAL(canvasResourceChanged(int, const QVariant&)),
- this, SLOT(slotCanvasResourceChanged(int, const QVariant&)), Qt::UniqueConnection);
+ SLOT(slotCanvasResourceChanged(int, const QVariant&)));
+ connect(KisConfigNotifier::instance(), SIGNAL(configChanged()),
+ SLOT(selectPaintingColorSpace()));
+
m_d->setCurrentNode(0);
setMonitorProfile(0);
@@ -129,17 +135,46 @@ void KisDisplayColorConverter::Private::slotCanvasResourceChanged(int key, const
}
}
+void KisDisplayColorConverter::Private::slotUpdateCurrentNodeColorSpace()
+{
+ setCurrentNode(connectedNode);
+}
+
+inline KisPaintDeviceSP findValidDevice(KisNodeSP node) {
+ return node->paintDevice() ? node->paintDevice() : node->original();
+}
+
void KisDisplayColorConverter::Private::setCurrentNode(KisNodeSP node)
{
+ if (connectedNode) {
+ KisPaintDeviceSP device = findValidDevice(connectedNode);
+
+ if (device) {
+ q->disconnect(device, 0);
+ }
+ }
+
if (node) {
- nodeColorSpace =
- node->paintDevice() ?
- node->paintDevice()->compositionSourceColorSpace() :
+ KisPaintDeviceSP device = findValidDevice(node);
+
+ nodeColorSpace = device ?
+ device->compositionSourceColorSpace() :
node->colorSpace();
+
+ KIS_ASSERT_RECOVER_NOOP(nodeColorSpace);
+
+ if (device) {
+ q->connect(device, SIGNAL(profileChanged(const KoColorProfile*)),
+ SLOT(slotUpdateCurrentNodeColorSpace()), Qt::UniqueConnection);
+ q->connect(device, SIGNAL(colorSpaceChanged(const KoColorSpace*)),
+ SLOT(slotUpdateCurrentNodeColorSpace()), Qt::UniqueConnection);
+ }
+
} else {
nodeColorSpace = KoColorSpaceRegistry::instance()->rgb8();
}
+ connectedNode = node;
selectPaintingColorSpace();
}
@@ -203,7 +238,6 @@ void KisDisplayColorConverter::setDisplayFilter(KisDisplayFilterSP displayFilter
//KIS_ASSERT_RECOVER_NOOP(cfg.useOcio() == (bool) m_d->displayFilter);
}
- emit displayConfigurationChanged();
m_d->selectPaintingColorSpace();
}
diff --git a/krita/ui/canvas/kis_display_color_converter.h b/krita/ui/canvas/kis_display_color_converter.h
index 5dad424..5e1798d 100644
--- a/krita/ui/canvas/kis_display_color_converter.h
+++ b/krita/ui/canvas/kis_display_color_converter.h
@@ -81,7 +81,9 @@ private:
KisDisplayColorConverter();
private:
- Q_PRIVATE_SLOT(m_d, void slotCanvasResourceChanged(int key, const QVariant &v))
+ Q_PRIVATE_SLOT(m_d, void slotCanvasResourceChanged(int key, const QVariant &v));
+ Q_PRIVATE_SLOT(m_d, void selectPaintingColorSpace());
+ Q_PRIVATE_SLOT(m_d, void slotUpdateCurrentNodeColorSpace());
private:
struct Private;
diff --git a/krita/ui/kis_config.cc b/krita/ui/kis_config.cc
index 4b38927..95e7a94 100644
--- a/krita/ui/kis_config.cc
+++ b/krita/ui/kis_config.cc
@@ -47,6 +47,7 @@
#include "kis_canvas_resource_provider.h"
#include "kis_global.h"
+#include "kis_config_notifier.h"
#include <config-ocio.h>
@@ -1146,4 +1147,6 @@ void KisConfig::setCustomColorSelectorColorSpace(const KoColorSpace *cs)
cfg.writeEntry("customColorSpaceDepthID", cs->colorDepthId().id());
cfg.writeEntry("customColorSpaceProfile", cs->profile()->name());
}
+
+ KisConfigNotifier::instance()->notifyConfigChanged();
}
More information about the kimageshop
mailing list