From kde-kimageshop Thu Apr 24 12:06:27 2014 From: Dmitry Kazakov Date: Thu, 24 Apr 2014 12:06:27 +0000 To: kde-kimageshop Subject: [calligra/krita-testing-kazakov] krita: Fix updates of the color selector when color space of the im Message-Id: X-MARC-Message: https://marc.info/?l=kde-kimageshop&m=139834120900742 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@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 + * + * 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 #include #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 #include +#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 @@ -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(); } _______________________________________________ Krita mailing list kimageshop@kde.org https://mail.kde.org/mailman/listinfo/kimageshop