Commit 65fe02e5 authored by Dmitry Kazakov's avatar Dmitry Kazakov

Fixed a bug in storing the filter configuration in filter-based layers

The storage of the filter configuration in now implemented in
KisNodeFilterInterface and is done by means of external shared pointers
(QSharedPointer). This makes the use of this configuration thread safe.

This patch also fixes various memory leaks in
KisLayerManager and KisMaskManager.

BUG:309099
parent cecf6e3f
......@@ -102,6 +102,7 @@ set(kritaimage_LIB_SRCS
generator/kis_generator_registry.cpp
kis_adjustment_layer.cc
kis_selection_based_layer.cpp
kis_node_filter_interface.cpp
kis_background.cpp
kis_base_accessor.cpp
kis_base_node.cpp
......
......@@ -24,44 +24,76 @@
#include "kis_types.h"
#include <klocale.h>
#include "filter/kis_filter_configuration.h"
#include "kis_node.h"
#include "kis_node_filter_interface.h"
#include "filter/kis_filter_registry.h"
#include "filter/kis_filter.h"
#include "generator/kis_generator_registry.h"
#include "generator/kis_generator.h"
class KisNode;
template <typename T>
class KisChangeFilterCmd : public KUndo2Command
{
public:
// The QStrings are the _serialized_ configs
KisChangeFilterCmd(T node,
KisFilterConfiguration* config,
const QString& before,
const QString& after)
KisChangeFilterCmd(KisNodeSP node,
const QString &filterNameBefore,
const QString &xmlBefore,
const QString &filterNameAfter,
const QString &xmlAfter,
bool useGeneratorRegistry)
: KUndo2Command(i18nc("(qtundo-format)", "Change Filter")) {
m_node = node;
m_config = config;
m_before = before;
m_after = after;
m_filterInterface = dynamic_cast<KisNodeFilterInterface*>(node.data());
Q_ASSERT(m_filterInterface);
m_useGeneratorRegistry = useGeneratorRegistry;
m_xmlBefore = xmlBefore;
m_xmlAfter = xmlAfter;
m_filterNameBefore = filterNameBefore;
m_filterNameAfter = filterNameAfter;
}
public:
virtual void redo() {
if (m_config)
m_config->fromXML(m_after);
m_node->setFilter(m_config);
m_filterInterface->setFilter(createConfiguration(m_filterNameAfter, m_xmlAfter));
m_node->setDirty();
}
virtual void undo() {
if (m_config)
m_config->fromXML(m_before);
m_node->setFilter(m_config);
m_filterInterface->setFilter(createConfiguration(m_filterNameBefore, m_xmlBefore));
m_node->setDirty();
}
private:
T m_node;
KisFilterConfiguration* m_config;
QString m_before;
QString m_after;
KisFilterConfiguration* createConfiguration(const QString &name, const QString &data)
{
KisFilterConfiguration *config;
if (m_useGeneratorRegistry) {
KisGeneratorSP generator = KisGeneratorRegistry::instance()->value(name);
config = generator->defaultConfiguration(0);
} else {
KisFilterSP filter = KisFilterRegistry::instance()->value(name);
config = filter->defaultConfiguration(0);
}
config->fromXML(data);
return config;
}
private:
KisNodeSP m_node;
KisNodeFilterInterface *m_filterInterface;
bool m_useGeneratorRegistry;
QString m_xmlBefore;
QString m_xmlAfter;
QString m_filterNameBefore;
QString m_filterNameAfter;
};
#endif
......@@ -20,7 +20,6 @@
#define KIS_NODE_COMMANDS_H
#include "kis_change_filter_command.h"
#include "kis_change_generator_command.h"
#include "kis_node_compositeop_command.h"
#include "kis_node_opacity_command.h"
......
......@@ -30,59 +30,42 @@
#include "kis_node_visitor.h"
#include "kis_processing_visitor.h"
struct KisGeneratorLayer::Private
{
public:
KisFilterConfiguration * filterConfig;
};
KisGeneratorLayer::KisGeneratorLayer(KisImageWSP image,
const QString &name,
KisFilterConfiguration *kfc,
KisSelectionSP selection)
: KisSelectionBasedLayer(image, name, selection),
m_d(new Private())
: KisSelectionBasedLayer(image, name, selection, kfc, true)
{
if(kfc)
kfc = KisGeneratorRegistry::instance()->cloneConfiguration(kfc);
m_d->filterConfig = kfc;
update();
}
KisGeneratorLayer::KisGeneratorLayer(const KisGeneratorLayer& rhs)
: KisSelectionBasedLayer(rhs),
m_d(new Private())
: KisSelectionBasedLayer(rhs)
{
m_d->filterConfig = KisGeneratorRegistry::instance()->cloneConfiguration(rhs.m_d->filterConfig);
}
KisGeneratorLayer::~KisGeneratorLayer()
{
delete m_d->filterConfig;
delete m_d;
}
KisFilterConfiguration * KisGeneratorLayer::filter() const
{
Q_ASSERT(m_d->filterConfig);
return m_d->filterConfig;
}
void KisGeneratorLayer::setFilter(KisFilterConfiguration *filterConfig)
{
Q_ASSERT(filterConfig);
delete m_d->filterConfig;
m_d->filterConfig = KisGeneratorRegistry::instance()->cloneConfiguration(filterConfig);
KisSelectionBasedLayer::setFilter(filterConfig);
update();
}
void KisGeneratorLayer::update()
{
KisSafeFilterConfigurationSP filterConfig = filter();
KisGeneratorSP f = KisGeneratorRegistry::instance()->value(m_d->filterConfig->name());
if (!filterConfig) {
warnImage << "BUG: No Filter configuration in KisGeneratorLayer";
return;
}
KisGeneratorSP f = KisGeneratorRegistry::instance()->value(filterConfig->name());
if (!f) return;
QRect processRect = exactBounds();
......@@ -94,8 +77,8 @@ void KisGeneratorLayer::update()
processRect.topLeft(),
selection());
m_d->filterConfig->setChannelFlags(channelFlags());
f->generate(dstCfg, processRect.size(), m_d->filterConfig);
filterConfig->setChannelFlags(channelFlags());
f->generate(dstCfg, processRect.size(), filterConfig.data());
}
bool KisGeneratorLayer::accept(KisNodeVisitor & v)
......@@ -115,9 +98,11 @@ QIcon KisGeneratorLayer::icon() const
KoDocumentSectionModel::PropertyList KisGeneratorLayer::sectionModelProperties() const
{
KisSafeFilterConfigurationSP filterConfig = filter();
KoDocumentSectionModel::PropertyList l = KisLayer::sectionModelProperties();
l << KoDocumentSectionModel::Property(i18n("Generator"),
KisGeneratorRegistry::instance()->value(m_d->filterConfig->name())->name());
KisGeneratorRegistry::instance()->value(filterConfig->name())->name());
return l;
}
......
......@@ -51,19 +51,7 @@ public:
return KisNodeSP(new KisGeneratorLayer(*this));
}
KisFilterConfiguration *filter() const;
void setFilter(KisFilterConfiguration * filterConfig);
/**
* Convenience functions.
* FIXME: should be deprecated? (DK)
*/
KisFilterConfiguration *generator() const {
return filter();
};
void setGenerator(KisFilterConfiguration * filterConfig) {
setFilter(filterConfig);
};
void setFilter(KisFilterConfiguration *filterConfig);
bool accept(KisNodeVisitor &);
void accept(KisProcessingVisitor &visitor, KisUndoAdapter *undoAdapter);
......@@ -76,16 +64,12 @@ public:
* of the associated selection.
*/
void update();
public slots:
// KisIndirectPaintingSupport
KisLayer* layer() {
return this;
}
private:
struct Private;
Private * const m_d;
};
#endif
......
......@@ -33,61 +33,42 @@
#include "kis_processing_visitor.h"
struct KisAdjustmentLayer::Private
{
public:
KisFilterConfiguration *filterConfig;
};
KisAdjustmentLayer::KisAdjustmentLayer(KisImageWSP image,
const QString &name,
KisFilterConfiguration * kfc,
KisFilterConfiguration *kfc,
KisSelectionSP selection)
: KisSelectionBasedLayer(image.data(), name, selection),
m_d(new Private())
: KisSelectionBasedLayer(image.data(), name, selection, kfc)
{
// by default Adjustmen Layers have a copy composition,
// which is more natural for users
setCompositeOp(COMPOSITE_COPY);
if(kfc)
m_d->filterConfig = KisFilterRegistry::instance()->cloneConfiguration(kfc);
}
KisAdjustmentLayer::KisAdjustmentLayer(const KisAdjustmentLayer& rhs)
: KisSelectionBasedLayer(rhs),
m_d(new Private())
: KisSelectionBasedLayer(rhs)
{
m_d->filterConfig = KisFilterRegistry::instance()->cloneConfiguration(rhs.m_d->filterConfig);
}
KisAdjustmentLayer::~KisAdjustmentLayer()
{
delete m_d->filterConfig;
delete m_d;
}
KisFilterConfiguration * KisAdjustmentLayer::filter() const
{
return m_d->filterConfig;
}
void KisAdjustmentLayer::setFilter(KisFilterConfiguration * filterConfig)
void KisAdjustmentLayer::setFilter(KisFilterConfiguration *filterConfig)
{
delete m_d->filterConfig;
m_d->filterConfig = KisFilterRegistry::instance()->cloneConfiguration(filterConfig);
m_d->filterConfig->setChannelFlags(channelFlags());
filterConfig->setChannelFlags(channelFlags());
KisSelectionBasedLayer::setFilter(filterConfig);
}
QRect KisAdjustmentLayer::changeRect(const QRect &rect, PositionToFilthy pos) const
{
KisSafeFilterConfigurationSP filterConfig = filter();
QRect filteredRect = rect;
if (m_d->filterConfig) {
KisFilterSP filter = KisFilterRegistry::instance()->value(m_d->filterConfig->name());
filteredRect = filter->changedRect(rect, m_d->filterConfig);
if (filterConfig) {
KisFilterSP filter = KisFilterRegistry::instance()->value(filterConfig->name());
filteredRect = filter->changedRect(rect, filterConfig.data());
}
/**
......@@ -103,8 +84,10 @@ QRect KisAdjustmentLayer::changeRect(const QRect &rect, PositionToFilthy pos) co
QRect KisAdjustmentLayer::needRect(const QRect& rect, PositionToFilthy pos) const
{
Q_UNUSED(pos);
if (!m_d->filterConfig) return rect;
KisFilterSP filter = KisFilterRegistry::instance()->value(m_d->filterConfig->name());
KisSafeFilterConfigurationSP filterConfig = filter();
if (!filterConfig) return rect;
KisFilterSP filter = KisFilterRegistry::instance()->value(filterConfig->name());
/**
* If we need some additional pixels even outside of a selection
......@@ -114,7 +97,7 @@ QRect KisAdjustmentLayer::needRect(const QRect& rect, PositionToFilthy pos) cons
* That's why simply we do not call
* KisSelectionBasedLayer::needRect here :)
*/
return filter->neededRect(rect, m_d->filterConfig);
return filter->neededRect(rect, filterConfig.data());
}
bool KisAdjustmentLayer::accept(KisNodeVisitor & v)
......@@ -134,16 +117,19 @@ QIcon KisAdjustmentLayer::icon() const
KoDocumentSectionModel::PropertyList KisAdjustmentLayer::sectionModelProperties() const
{
KisSafeFilterConfigurationSP filterConfig = filter();
KoDocumentSectionModel::PropertyList l = KisLayer::sectionModelProperties();
if (filter())
l << KoDocumentSectionModel::Property(i18n("Filter"), KisFilterRegistry::instance()->value(filter()->name())->name());
if (filterConfig)
l << KoDocumentSectionModel::Property(i18n("Filter"), KisFilterRegistry::instance()->value(filterConfig->name())->name());
return l;
}
void KisAdjustmentLayer::setChannelFlags(const QBitArray & channelFlags)
{
if (m_d->filterConfig) {
m_d->filterConfig->setChannelFlags(channelFlags);
KisSafeFilterConfigurationSP filterConfig = filter();
if (filterConfig) {
filterConfig->setChannelFlags(channelFlags);
}
KisLayer::setChannelFlags(channelFlags);
}
......
......@@ -96,15 +96,7 @@ public:
public:
/**
* gets a pointer to the AdjustmentLayer's filter configuration.
* @return a pointer to the AdjustmentLayer's filter configuration
*/
KisFilterConfiguration *filter() const;
/**
* sets the AdjustmentLayer's filter configuration
* @param filterConfig a pointer to the filter configuration to set
* @return void
* \see KisNodeFilterInterface::setFilter()
*/
void setFilter(KisFilterConfiguration *filterConfig);
......@@ -122,10 +114,6 @@ public slots:
KisLayer* layer() {
return this;
}
private:
struct Private;
Private * const m_d;
};
#endif // KIS_ADJUSTMENT_LAYER_H_
......
......@@ -83,7 +83,7 @@ public:
return true;
}
KisFilterConfiguration *filterConfig = layer->filter();
KisSafeFilterConfigurationSP filterConfig = layer->filter();
if (!filterConfig) return true;
KisFilterSP filter = KisFilterRegistry::instance()->value(filterConfig->name());
......@@ -105,7 +105,7 @@ public:
QPointer<KoUpdater> updaterPtr = updater.startSubtask();
// We do not create a transaction here, as srcDevice != dstDevice
filter->process(m_projection, originalDevice, 0, applyRect, filterConfig, updaterPtr);
filter->process(m_projection, originalDevice, 0, applyRect, filterConfig.data(), updaterPtr);
updaterPtr->setProgress(100);
......
......@@ -37,28 +37,17 @@
#include "kis_painter.h"
#include <KoUpdater.h>
#include <QMutex>
struct KisFilterMask::Private
{
public:
KisFilterConfiguration * filterConfig;
QMutex filterConfigMutex;
};
KisFilterMask::KisFilterMask()
: KisEffectMask()
, m_d(new Private())
: KisEffectMask(),
KisNodeFilterInterface(0, false)
{
m_d->filterConfig = 0;
setCompositeOp(COMPOSITE_COPY);
}
KisFilterMask::~KisFilterMask()
{
delete m_d->filterConfig;
delete m_d;
}
bool KisFilterMask::allowAsChild(KisNodeSP node) const
......@@ -70,14 +59,7 @@ bool KisFilterMask::allowAsChild(KisNodeSP node) const
KisFilterMask::KisFilterMask(const KisFilterMask& rhs)
: KisEffectMask(rhs)
, KisNodeFilterInterface(rhs)
, m_d(new Private())
{
m_d->filterConfig = KisFilterRegistry::instance()->cloneConfiguration(rhs.m_d->filterConfig);
}
KisFilterConfiguration * KisFilterMask::filter() const
{
return m_d->filterConfig;
}
QIcon KisFilterMask::icon() const
......@@ -87,39 +69,33 @@ QIcon KisFilterMask::icon() const
void KisFilterMask::setFilter(KisFilterConfiguration * filterConfig)
{
QMutexLocker l(&m_d->filterConfigMutex);
Q_ASSERT(filterConfig);
delete m_d->filterConfig;
m_d->filterConfig = KisFilterRegistry::instance()->cloneConfiguration(filterConfig);
if (parent() && parent()->inherits("KisLayer")) {
m_d->filterConfig->setChannelFlags(qobject_cast<KisLayer*>(parent().data())->channelFlags());
filterConfig->setChannelFlags(qobject_cast<KisLayer*>(parent().data())->channelFlags());
}
KisNodeFilterInterface::setFilter(filterConfig);
}
QRect KisFilterMask::decorateRect(KisPaintDeviceSP &src,
KisPaintDeviceSP &dst,
const QRect & rc) const
{
KisSafeFilterConfigurationSP filterConfig = filter();
Q_ASSERT(nodeProgressProxy());
Q_ASSERT_X(src != dst, "KisFilterMask::decorateRect",
"src must be != dst, because we cant create transactions "
"during merge, as it breaks reentrancy");
if (!m_d->filterConfig) {
if (!filterConfig) {
warnKrita << "No filter configuration present";
return QRect();
}
KisFilterConfiguration* config;
{
QMutexLocker l(&m_d->filterConfigMutex);
config = KisFilterRegistry::instance()->cloneConfiguration(m_d->filterConfig);
}
KisFilterSP filter =
KisFilterRegistry::instance()->value(config->name());
KisFilterRegistry::instance()->value(filterConfig->name());
if (!filter) {
warnKrita << "Could not retrieve filter \"" << config->name() << "\"";
warnKrita << "Could not retrieve filter \"" << filterConfig->name() << "\"";
return QRect();
}
......@@ -128,12 +104,11 @@ QRect KisFilterMask::decorateRect(KisPaintDeviceSP &src,
QPointer<KoUpdater> updaterPtr = updater.startSubtask();
filter->process(src, dst, 0, rc, config, updaterPtr);
filter->process(src, dst, 0, rc, filterConfig.data(), updaterPtr);
updaterPtr->setProgress(100);
QRect r = filter->changedRect(rc, config);
delete config;
QRect r = filter->changedRect(rc, filterConfig.data());
return r;
}
......@@ -160,9 +135,10 @@ QRect KisFilterMask::changeRect(const QRect &rect, PositionToFilthy pos) const
QRect filteredRect = rect;
if (m_d->filterConfig) {
KisFilterSP filter = KisFilterRegistry::instance()->value(m_d->filterConfig->name());
filteredRect = filter->changedRect(rect, m_d->filterConfig);
KisSafeFilterConfigurationSP filterConfig = filter();
if (filterConfig) {
KisFilterSP filter = KisFilterRegistry::instance()->value(filterConfig->name());
filteredRect = filter->changedRect(rect, filterConfig.data());
}
/**
......@@ -188,9 +164,13 @@ QRect KisFilterMask::needRect(const QRect& rect, PositionToFilthy pos) const
* FIXME: This check of the emptiness should be done
* on the higher/lower level
*/
if(rect.isEmpty()) return rect;
if (!m_d->filterConfig) return rect;
KisFilterSP filter = KisFilterRegistry::instance()->value(m_d->filterConfig->name());
KisSafeFilterConfigurationSP filterConfig = filter();
if (!filterConfig) return rect;
KisFilterSP filter = KisFilterRegistry::instance()->value(filterConfig->name());
/**
* If we need some additional pixels even outside of a selection
......@@ -198,7 +178,7 @@ QRect KisFilterMask::needRect(const QRect& rect, PositionToFilthy pos) const
* And no KisMask::needRect will prevent us from doing this! ;)
* That's why simply we do not call KisMask::needRect here :)
*/
return filter->neededRect(rect, m_d->filterConfig);
return filter->neededRect(rect, filterConfig.data());
}
#include "kis_filter_mask.moc"
......@@ -58,8 +58,7 @@ public:
bool allowAsChild(KisNodeSP) const;
KisFilterConfiguration * filter() const;
void setFilter(KisFilterConfiguration * filterConfig);
void setFilter(KisFilterConfiguration *filterConfig);
QRect decorateRect(KisPaintDeviceSP &src,
KisPaintDeviceSP &dst,
......@@ -67,11 +66,6 @@ public:
QRect changeRect(const QRect &rect, PositionToFilthy pos = N_FILTHY) const;
QRect needRect(const QRect &rect, PositionToFilthy pos = N_FILTHY) const;
private:
struct Private;
Private * const m_d;
};
#endif //_KIS_FILTER_MASK_
/*
* Copyright (c) 2007 Boudewijn Rempt <boud@kde.org>
* Copyright (c) 2008 Cyrille Berger <cberger@cberger.net>
* Copyright (c) 2012 Dmitry Kazakov <dimula73@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
......@@ -15,52 +16,39 @@
* 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_CHANGE_GENERATOR_COMMAND_H_
#define KIS_CHANGE_GENERATOR_COMMAND_H_
#include <krita_export.h>
#include <kundo2command.h>
#include <QRect>
#include "kis_types.h"
#include <klocale.h>
#include "filter/kis_filter_configuration.h"
#include "kis_node_filter_interface.h"
class KisNode;
#include "kis_filter_registry.h"
#include "kis_generator_registry.h"
template <typename T>
class KisChangeGeneratorCmd : public KUndo2Command
KisNodeFilterInterface::KisNodeFilterInterface(KisFilterConfiguration *filterConfig, bool useGeneratorRegistry)
: m_filter(filterConfig),
m_useGeneratorRegistry(useGeneratorRegistry)
{
}
public:
// The QStrings are the _serialized_ configs
KisChangeGeneratorCmd(T node,
KisFilterConfiguration* config,
const QString& before,
const QString& after)
: KUndo2Command(i18nc("(qtundo-format)", "Change Generator")) {
m_node = node;
m_config = config;
m_before = before;
m_after = after;
}
public:
virtual void redo() {
m_config->fromXML(m_after);
m_node->setGenerator(m_config);
m_node->setDirty();
KisNodeFilterInterface::KisNodeFilterInterface(const KisNodeFilterInterface &rhs)
: m_useGeneratorRegistry(rhs.m_useGeneratorRegistry)
{
if (m_useGeneratorRegistry) {
m_filter = KisSafeFilterConfigurationSP(KisGeneratorRegistry::instance()->cloneConfiguration(rhs.m_filter.data()));
} else {
m_filter = KisSafeFilterConfigurationSP(KisFilterRegistry::instance()->cloneConfiguration(rhs.m_filter.data()));
}
}
virtual void undo() {
m_config->fromXML(m_before);
m_node->setGenerator(m_config);
m_node->setDirty();
}
KisNodeFilterInterface::~KisNodeFilterInterface()
{
}
private:
T m_node;
KisFilterConfiguration* m_config;
QString m_before;
QString m_after;
};
KisSafeFilterConfigurationSP KisNodeFilterInterface::filter() const
{
return m_filter;
}
#endif
void KisNodeFilterInterface::setFilter(KisFilterConfiguration *filterConfig)
{
Q_ASSERT(filterConfig);
m_filter = KisSafeFilterConfigurationSP(filterConfig);
}
......@@ -19,23 +19,43 @@
#ifndef _KIS_NODE_FILTER_INTERFACE_H_
#define _KIS_NODE_FILTER_INTERFACE_H_
#include <krita_export.h>
#include <QSharedPointer>
class KisFilterConfiguration;
typedef QSharedPointer<KisFilterConfiguration> KisSafeFilterConfigurationSP;
/**
* Define an interface for nodes that are associated with a filter.
*/